Re: [RFC 0/3] virtio-blk: add iothread-vq-mapping parameter
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: > > > > > > > > > > > > > > > > ... > > > > Just for completeness, this how disk XML looks now: > > > > > > function='0x0'/> > > > 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 > 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.: > > > > > > > > > > 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
Re: [RFC 0/3] virtio-blk: add iothread-vq-mapping parameter
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: > > > > > > > > ... > Just for completeness, this how disk XML looks now: 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 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.: 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
[RFC 0/3] virtio-blk: add iothread-vq-mapping parameter
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: ... and that would generate this QEMU command-line snippet: "iothread-vq-mapping":[{"iothread":"iothread1"},{"iothread":"iothread2"},{"iothread":"iothread3"}] Note that JSON --device syntax is required for the iothread-vq-mapping parameter because it's non-scalar. What do you think? Stefan Hajnoczi (3): qdev-properties: alias all object class properties qdev: add IOThreadVirtQueueMappingList property type virtio-blk: add iothread-vq-mapping parameter qapi/virtio.json| 30 +++ include/hw/qdev-properties-system.h | 4 ++ include/hw/virtio/virtio-blk.h | 2 + hw/block/virtio-blk.c | 78 + hw/core/qdev-properties-system.c| 47 + hw/core/qdev-properties.c | 18 --- 6 files changed, 171 insertions(+), 8 deletions(-) -- 2.39.0