Re: [RFC 0/3] virtio-blk: add iothread-vq-mapping parameter

[Michal Prívozník]

On 1/18/23 20:47, Stefan Hajnoczi wrote:
> This is a preview of the iothread-vq-mapping parameter that assigns virtqueues
> to IOThreads. The syntax is implemented but multiple IOThreads are not 
> actually
> supported yet. The purpose of this RFC is to reach agreement on the syntax and
> to prepare for libvirt support.
> 
> virtio-blk and virtio-scsi devices will need a way to specify the
> mapping between IOThreads and virtqueues. At the moment all virtqueues
> are assigned to a single IOThread or the main loop. This single thread
> can be a CPU bottleneck, so it is necessary to allow finer-grained
> assignment to spread the load.
> 
> This series introduces command-line syntax for the new iothread-vq-mapping
> property is as follows:
> 
>   --device 
> '{"driver":"virtio-blk-pci","iothread-vq-mapping":[{"iothread":"iothread0","vqs":[0,1,2]},...]},...'
> 
> IOThreads are specified by name and virtqueues are specified by 0-based
> index.
> 
> It will be common to simply assign virtqueues round-robin across a set
> of IOThreads. A convenient syntax that does not require specifying
> individual virtqueue indices is available:
> 
>   --device 
> '{"driver":"virtio-blk-pci","iothread-vq-mapping":[{"iothread":"iothread0"},{"iothread":"iothread1"},...]},...'
> 
> There is no way to reassign virtqueues at runtime and I expect that to be a
> very rare requirement.
> 
> Perhaps libvirt only needs to support round-robin because specifying 
> individual
> virtqueues is very specific and probably only useful for low-level performance
> investigation. The libvirt domain XML syntax for this could be:
> 
>   <driver name='qemu' type='qcow2'>
>     <iothreads>
>       <iothread id="1"/>
>       <iothread id="2"/>
>       <iothread id="3"/>
>     </iothreads>
>     ...
>   </driver>

Just for completeness, this how disk XML looks now:

  <disk type='file' device='disk'>
    <driver name='qemu' type='qcow2' queues='4' iothread='1'/>
    <source file='/tmp/data.qcow'/>
    <target dev='vda' bus='virtio'/>
    <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
  </disk>

It corresponds to the following cmd line:

  -blockdev 
'{"driver":"file","filename":"/tmp/data.qcow","node-name":"libvirt-3-storage","auto-read-only":true,"discard":"unmap"}'
 \
  -blockdev 
'{"node-name":"libvirt-3-format","read-only":false,"driver":"qcow2","file":"libvirt-3-storage"}'
 \
  -device 
'{"driver":"virtio-blk-pci","iothread":"iothread1","num-queues":4,"bus":"pci.0","addr":"0x3","drive":"libvirt-3-format","id":"virtio-disk0","bootindex":1}'
 \

We already have @iothread attribute, so inventing an <iothread/>
subelement is a bit misleading, because if users query which disk uses
iothreads, they need to change their XPATH. Currently they can get away
with:

  //disk[driver/@iothread]/source/@file

but I guess for backwards compatibility, we can put the first iothread
ID into the attribute, e.g.:

  <driver iothread="2">
    <iothread>
     <iothread id="2"/>
     <iothread id="4"/>
    </iothread>
  </driver>


We've done something similar, when introducing multiple listen addresses
for VNC.

Now, an iothread is actually a thread pool. I guess we will never ever
need to assign individual threads from the pool to queues, right?

Michal