* [RFC 0/3] virtio-blk: add iothread-vq-mapping parameter
@ 2023-01-18 19:47 Stefan Hajnoczi
2023-01-18 19:47 ` [RFC 1/3] qdev-properties: alias all object class properties Stefan Hajnoczi
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Stefan Hajnoczi @ 2023-01-18 19:47 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Eduardo Habkost, Daniel P. Berrangé, Hanna Reitz,
Markus Armbruster, Paolo Bonzini, Emanuele Giuseppe Esposito,
qemu-block, Eric Blake, Stefan Hajnoczi, Michael S. Tsirkin,
Michal Privoznik
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>
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFC 1/3] qdev-properties: alias all object class properties
2023-01-18 19:47 [RFC 0/3] virtio-blk: add iothread-vq-mapping parameter Stefan Hajnoczi
@ 2023-01-18 19:47 ` Stefan Hajnoczi
2023-01-18 19:47 ` [RFC 2/3] qdev: add IOThreadVirtQueueMappingList property type Stefan Hajnoczi
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Stefan Hajnoczi @ 2023-01-18 19:47 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Eduardo Habkost, Daniel P. Berrangé, Hanna Reitz,
Markus Armbruster, Paolo Bonzini, Emanuele Giuseppe Esposito,
qemu-block, Eric Blake, Stefan Hajnoczi, Michael S. Tsirkin,
Michal Privoznik
qdev_alias_all_properties() aliases a DeviceState's qdev properties onto
an Object. This is used for VirtioPCIProxy types so that --device
virtio-blk-pci has properties of its embedded --device virtio-blk-device
object.
Currently this function is implemented using qdev properties. Change the
function to use QOM object class properties instead. This works because
qdev properties create QOM object class properties, but it also catches
any QOM object class-only properties that have no qdev properties.
This change ensures that properties of devices are shown with --device
foo,\? even if they are QOM object class properties.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/core/qdev-properties.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 357b8761b5..fbf3969d3c 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -959,16 +959,18 @@ void device_class_set_props(DeviceClass *dc, Property *props)
void qdev_alias_all_properties(DeviceState *target, Object *source)
{
ObjectClass *class;
- Property *prop;
+ ObjectPropertyIterator iter;
+ ObjectProperty *prop;
class = object_get_class(OBJECT(target));
- do {
- DeviceClass *dc = DEVICE_CLASS(class);
- for (prop = dc->props_; prop && prop->name; prop++) {
- object_property_add_alias(source, prop->name,
- OBJECT(target), prop->name);
+ object_class_property_iter_init(&iter, class);
+ while ((prop = object_property_iter_next(&iter))) {
+ if (object_property_find(source, prop->name)) {
+ continue; /* skip duplicate properties */
}
- class = object_class_get_parent(class);
- } while (class != object_class_by_name(TYPE_DEVICE));
+
+ object_property_add_alias(source, prop->name,
+ OBJECT(target), prop->name);
+ }
}
--
2.39.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [RFC 2/3] qdev: add IOThreadVirtQueueMappingList property type
2023-01-18 19:47 [RFC 0/3] virtio-blk: add iothread-vq-mapping parameter Stefan Hajnoczi
2023-01-18 19:47 ` [RFC 1/3] qdev-properties: alias all object class properties Stefan Hajnoczi
@ 2023-01-18 19:47 ` Stefan Hajnoczi
2023-01-18 19:47 ` [RFC 3/3] virtio-blk: add iothread-vq-mapping parameter Stefan Hajnoczi
2023-02-06 14:11 ` [RFC 0/3] " Michal Prívozník
3 siblings, 0 replies; 6+ messages in thread
From: Stefan Hajnoczi @ 2023-01-18 19:47 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Eduardo Habkost, Daniel P. Berrangé, Hanna Reitz,
Markus Armbruster, Paolo Bonzini, Emanuele Giuseppe Esposito,
qemu-block, Eric Blake, Stefan Hajnoczi, Michael S. Tsirkin,
Michal Privoznik
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.
Introduce DEFINE_PROP_IOTHREAD_VQ_MAPPING_LIST() so devices can take a
parameter that maps virtqueues to IOThreads. The command-line syntax for
this new property is as follows:
--device '{"driver":"foo","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":"foo","iothread-vq-mapping":[{"iothread":"iothread0"},{"iothread":"iothread1"},...]}'
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
qapi/virtio.json | 30 ++++++++++++++++++
include/hw/qdev-properties-system.h | 4 +++
hw/core/qdev-properties-system.c | 47 +++++++++++++++++++++++++++++
3 files changed, 81 insertions(+)
diff --git a/qapi/virtio.json b/qapi/virtio.json
index 019d2d1987..f8006bdf3e 100644
--- a/qapi/virtio.json
+++ b/qapi/virtio.json
@@ -952,3 +952,33 @@
'data': { 'path': 'str', 'queue': 'uint16', '*index': 'uint16' },
'returns': 'VirtioQueueElement',
'features': [ 'unstable' ] }
+
+##
+# @IOThreadVirtQueueMapping:
+#
+# Describes the subset of virtqueues assigned to an IOThread.
+#
+# @iothread: the id of IOThread object
+# @vqs: an optional array of virtqueue indices that will be handled by this
+# IOThread. When absent, virtqueues are assigned round-robin across all
+# IOThreadVirtQueueMappings provided. Either all
+# IOThreadVirtQueueMappings must have @vqs or none of them must have it.
+#
+# Since: 8.0
+#
+##
+
+{ 'struct': 'IOThreadVirtQueueMapping',
+ 'data': { 'iothread': 'str', '*vqs': ['uint16'] } }
+
+##
+# @IOThreadVirtQueueMappings:
+#
+# IOThreadVirtQueueMapping list. This struct is not actually used but the
+# IOThreadVirtQueueMappingList type it generates is!
+#
+# Since: 8.0
+##
+
+{ 'struct': 'IOThreadVirtQueueMappings',
+ 'data': { 'mappings': ['IOThreadVirtQueueMapping'] } }
diff --git a/include/hw/qdev-properties-system.h b/include/hw/qdev-properties-system.h
index 0ac327ae60..c526e502c8 100644
--- a/include/hw/qdev-properties-system.h
+++ b/include/hw/qdev-properties-system.h
@@ -22,6 +22,7 @@ extern const PropertyInfo qdev_prop_audiodev;
extern const PropertyInfo qdev_prop_off_auto_pcibar;
extern const PropertyInfo qdev_prop_pcie_link_speed;
extern const PropertyInfo qdev_prop_pcie_link_width;
+extern const PropertyInfo qdev_prop_iothread_vq_mapping_list;
#define DEFINE_PROP_PCI_DEVFN(_n, _s, _f, _d) \
DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_pci_devfn, int32_t)
@@ -73,5 +74,8 @@ extern const PropertyInfo qdev_prop_pcie_link_width;
#define DEFINE_PROP_UUID_NODEFAULT(_name, _state, _field) \
DEFINE_PROP(_name, _state, _field, qdev_prop_uuid, QemuUUID)
+#define DEFINE_PROP_IOTHREAD_VQ_MAPPING_LIST(_name, _state, _field) \
+ DEFINE_PROP(_name, _state, _field, qdev_prop_iothread_vq_mapping_list, \
+ IOThreadVirtQueueMappingList *)
#endif
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 54a09fa9ac..9b41628d94 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -18,6 +18,7 @@
#include "qapi/qapi-types-block.h"
#include "qapi/qapi-types-machine.h"
#include "qapi/qapi-types-migration.h"
+#include "qapi/qapi-visit-virtio.h"
#include "qapi/qmp/qerror.h"
#include "qemu/ctype.h"
#include "qemu/cutils.h"
@@ -1117,3 +1118,49 @@ const PropertyInfo qdev_prop_uuid = {
.set = set_uuid,
.set_default_value = set_default_uuid_auto,
};
+
+/* --- IOThreadVirtQueueMappingList --- */
+
+static void get_iothread_vq_mapping_list(Object *obj, Visitor *v,
+ const char *name, void *opaque, Error **errp)
+{
+ IOThreadVirtQueueMappingList **prop_ptr =
+ object_field_prop_ptr(obj, opaque);
+ IOThreadVirtQueueMappingList *list = *prop_ptr;
+
+ visit_type_IOThreadVirtQueueMappingList(v, name, &list, errp);
+}
+
+static void set_iothread_vq_mapping_list(Object *obj, Visitor *v,
+ const char *name, void *opaque, Error **errp)
+{
+ IOThreadVirtQueueMappingList **prop_ptr =
+ object_field_prop_ptr(obj, opaque);
+ IOThreadVirtQueueMappingList *list;
+
+ if (!visit_type_IOThreadVirtQueueMappingList(v, name, &list, errp)) {
+ return;
+ }
+
+ qapi_free_IOThreadVirtQueueMappingList(*prop_ptr);
+ *prop_ptr = list;
+}
+
+static void release_iothread_vq_mapping_list(Object *obj,
+ const char *name, void *opaque)
+{
+ IOThreadVirtQueueMappingList **prop_ptr =
+ object_field_prop_ptr(obj, opaque);
+
+ qapi_free_IOThreadVirtQueueMappingList(*prop_ptr);
+ *prop_ptr = NULL;
+}
+
+const PropertyInfo qdev_prop_iothread_vq_mapping_list = {
+ .name = "IOThreadVirtQueueMappingList",
+ .description = "IOThread virtqueue mapping list [{\"iothread\":\"<id>\", "
+ "\"vqs\":[1,2,3,...]},...]",
+ .get = get_iothread_vq_mapping_list,
+ .set = set_iothread_vq_mapping_list,
+ .release = release_iothread_vq_mapping_list,
+};
--
2.39.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [RFC 3/3] virtio-blk: add iothread-vq-mapping parameter
2023-01-18 19:47 [RFC 0/3] virtio-blk: add iothread-vq-mapping parameter Stefan Hajnoczi
2023-01-18 19:47 ` [RFC 1/3] qdev-properties: alias all object class properties Stefan Hajnoczi
2023-01-18 19:47 ` [RFC 2/3] qdev: add IOThreadVirtQueueMappingList property type Stefan Hajnoczi
@ 2023-01-18 19:47 ` Stefan Hajnoczi
2023-02-06 14:11 ` [RFC 0/3] " Michal Prívozník
3 siblings, 0 replies; 6+ messages in thread
From: Stefan Hajnoczi @ 2023-01-18 19:47 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Eduardo Habkost, Daniel P. Berrangé, Hanna Reitz,
Markus Armbruster, Paolo Bonzini, Emanuele Giuseppe Esposito,
qemu-block, Eric Blake, Stefan Hajnoczi, Michael S. Tsirkin,
Michal Privoznik
Add the iothread-vq-mapping parameter to assign virtqueues to IOThreads.
Note that this commit simply adds the
VirtIOBlock->iothread_vq_mapping_list field but does not use it yet. The
block layer doesn't support multi-queue yet, so we cannot safely process
virtqueues from multiple IOThreads at this time.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
include/hw/virtio/virtio-blk.h | 2 +
hw/block/virtio-blk.c | 78 ++++++++++++++++++++++++++++++++++
2 files changed, 80 insertions(+)
diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
index 7f589b4146..7324f4468f 100644
--- a/include/hw/virtio/virtio-blk.h
+++ b/include/hw/virtio/virtio-blk.h
@@ -21,6 +21,7 @@
#include "sysemu/block-backend.h"
#include "sysemu/block-ram-registrar.h"
#include "qom/object.h"
+#include "qapi/qapi-types-virtio.h"
#define TYPE_VIRTIO_BLK "virtio-blk-device"
OBJECT_DECLARE_SIMPLE_TYPE(VirtIOBlock, VIRTIO_BLK)
@@ -37,6 +38,7 @@ struct VirtIOBlkConf
{
BlockConf conf;
IOThread *iothread;
+ IOThreadVirtQueueMappingList *iothread_vq_mapping_list;
char *serial;
uint32_t request_merging;
uint16_t num_queues;
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index f717550fdc..ed53dc3686 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -1098,6 +1098,68 @@ static int virtio_blk_load_device(VirtIODevice *vdev, QEMUFile *f,
return 0;
}
+static bool
+validate_iothread_vq_mapping_list(IOThreadVirtQueueMappingList *list,
+ uint16_t num_queues, Error **errp)
+{
+ g_autofree unsigned long *vqs = bitmap_new(num_queues);
+ g_autoptr(GHashTable) iothreads =
+ g_hash_table_new(g_str_hash, g_str_equal);
+
+ for (IOThreadVirtQueueMappingList *node = list; node; node = node->next) {
+ const char *name = node->value->iothread;
+ uint16List *vq;
+
+ if (!iothread_by_id(name)) {
+ error_setg(errp, "IOThread \"%s\" object does not exist", name);
+ return false;
+ }
+
+ if (!g_hash_table_add(iothreads, (gpointer)name)) {
+ error_setg(errp,
+ "duplicate IOThread name \"%s\" in iothread-vq-mapping",
+ name);
+ return false;
+ }
+
+ if (node != list) {
+ if (!!node->value->vqs != !!list->value->vqs) {
+ error_setg(errp, "either all items in iothread-vq-mapping "
+ "must have vqs or none of them must have it");
+ return false;
+ }
+ }
+
+ for (vq = node->value->vqs; vq; vq = vq->next) {
+ if (vq->value >= num_queues) {
+ error_setg(errp, "vq index %u for IOThread \"%s\" must be "
+ "less than num_queues %u in iothread-vq-mapping",
+ vq->value, name, num_queues);
+ return false;
+ }
+
+ if (test_and_set_bit(vq->value, vqs)) {
+ error_setg(errp, "cannot assign vq %u to IOThread \"%s\" "
+ "because it is already assigned", vq->value, name);
+ return false;
+ }
+ }
+ }
+
+ if (list->value->vqs) {
+ for (uint16_t i = 0; i < num_queues; i++) {
+ if (!test_bit(i, vqs)) {
+ error_setg(errp,
+ "missing vq %u IOThread assignment in iothread-vq-mapping",
+ i);
+ return false;
+ }
+ }
+ }
+
+ return true;
+}
+
static void virtio_resize_cb(void *opaque)
{
VirtIODevice *vdev = opaque;
@@ -1191,6 +1253,19 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
return;
}
+ if (s->conf.iothread_vq_mapping_list) {
+ if (s->conf.iothread) {
+ error_setg(errp, "iothread and iothread-vq-mapping properties "
+ "cannot be set at the same time");
+ return;
+ }
+
+ if (!validate_iothread_vq_mapping_list(s->conf.iothread_vq_mapping_list,
+ s->conf.num_queues, errp)) {
+ return;
+ }
+ }
+
s->config_size = virtio_get_config_size(&virtio_blk_cfg_size_params,
s->host_features);
virtio_init(vdev, VIRTIO_ID_BLOCK, s->config_size);
@@ -1203,6 +1278,7 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
virtio_add_queue(vdev, conf->queue_size, virtio_blk_handle_output);
}
qemu_coroutine_inc_pool_size(conf->num_queues * conf->queue_size / 2);
+ /* TODO use iothread_vq_mapping_list */
virtio_blk_data_plane_create(vdev, conf, &s->dataplane, &err);
if (err != NULL) {
error_propagate(errp, err);
@@ -1284,6 +1360,8 @@ static Property virtio_blk_properties[] = {
DEFINE_PROP_BOOL("seg-max-adjust", VirtIOBlock, conf.seg_max_adjust, true),
DEFINE_PROP_LINK("iothread", VirtIOBlock, conf.iothread, TYPE_IOTHREAD,
IOThread *),
+ DEFINE_PROP_IOTHREAD_VQ_MAPPING_LIST("iothread-vq-mapping", VirtIOBlock,
+ conf.iothread_vq_mapping_list),
DEFINE_PROP_BIT64("discard", VirtIOBlock, host_features,
VIRTIO_BLK_F_DISCARD, true),
DEFINE_PROP_BOOL("report-discard-granularity", VirtIOBlock,
--
2.39.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC 0/3] virtio-blk: add iothread-vq-mapping parameter
2023-01-18 19:47 [RFC 0/3] virtio-blk: add iothread-vq-mapping parameter Stefan Hajnoczi
` (2 preceding siblings ...)
2023-01-18 19:47 ` [RFC 3/3] virtio-blk: add iothread-vq-mapping parameter Stefan Hajnoczi
@ 2023-02-06 14:11 ` Michal Prívozník
2023-02-06 20:31 ` Stefan Hajnoczi
3 siblings, 1 reply; 6+ messages in thread
From: Michal Prívozník @ 2023-02-06 14:11 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel
Cc: Kevin Wolf, Eduardo Habkost, Daniel P. Berrangé, Hanna Reitz,
Markus Armbruster, Paolo Bonzini, Emanuele Giuseppe Esposito,
qemu-block, Eric Blake, Michael S. Tsirkin
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC 0/3] virtio-blk: add iothread-vq-mapping parameter
2023-02-06 14:11 ` [RFC 0/3] " Michal Prívozník
@ 2023-02-06 20:31 ` Stefan Hajnoczi
0 siblings, 0 replies; 6+ messages in thread
From: Stefan Hajnoczi @ 2023-02-06 20:31 UTC (permalink / raw)
To: Michal Prívozník
Cc: qemu-devel, Kevin Wolf, Eduardo Habkost, Daniel P. Berrangé,
Hanna Reitz, Markus Armbruster, Paolo Bonzini,
Emanuele Giuseppe Esposito, qemu-block, Eric Blake,
Michael S. Tsirkin
[-- Attachment #1: Type: text/plain, Size: 3917 bytes --]
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-02-06 20:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-18 19:47 [RFC 0/3] virtio-blk: add iothread-vq-mapping parameter Stefan Hajnoczi
2023-01-18 19:47 ` [RFC 1/3] qdev-properties: alias all object class properties Stefan Hajnoczi
2023-01-18 19:47 ` [RFC 2/3] qdev: add IOThreadVirtQueueMappingList property type Stefan Hajnoczi
2023-01-18 19:47 ` [RFC 3/3] virtio-blk: add iothread-vq-mapping parameter Stefan Hajnoczi
2023-02-06 14:11 ` [RFC 0/3] " Michal Prívozník
2023-02-06 20:31 ` Stefan Hajnoczi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).