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

[Stefan Hajnoczi]

On Mon, Feb 06, 2023 at 03:11:21PM +0100, Michal Prívozník wrote:
> 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?

QEMU will have the ability to assign an individual virtqueue to a
specific IOThread.

At the moment I believe no one will need that much control. Users
probably just want to round-robin threads for virtio-blk and virtio-scsi
devices.

I think it's fine for libvirt domain XML to only support round-robin for
virtio-blk and virtio-scsi.

Stefan

signature.asc
Description: PGP signature