Re: [PATCH v2] virtio: increase virtuqueue size for virtio-scsi and virtio-blk
On 18.02.2020 16:59, Denis Plotnikov wrote: On 18.02.2020 16:53, Stefan Hajnoczi wrote: On Thu, Feb 13, 2020 at 05:59:27PM +0300, Denis Plotnikov wrote: v1: * seg_max default value changing removed --- The goal is to reduce the amount of requests issued by a guest on 1M reads/writes. This rises the performance up to 4% on that kind of disk access pattern. The maximum chunk size to be used for the guest disk accessing is limited with seg_max parameter, which represents the max amount of pices in the scatter-geather list in one guest disk request. Since seg_max is virqueue_size dependent, increasing the virtqueue size increases seg_max, which, in turn, increases the maximum size of data to be read/write from a guest disk. More details in the original problem statment: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg03721.html Suggested-by: Denis V. Lunev Signed-off-by: Denis Plotnikov --- hw/block/virtio-blk.c | 2 +- hw/core/machine.c | 2 ++ hw/scsi/virtio-scsi.c | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) I fixed up the "virtuqueue" typo in the commit message and the mis-formatted commit description (git-am(1) stops including lines after the first "---"). Actually, I sent the corrected version v3 of the patch last week. But it seems it got lost among that gigantic patch flow in the mailing list :) Thanks for applying! Denis Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan I'm going to send the test checking the virtqueue-sizes for machine types a little bit later. Denis
Re: [PATCH v2] virtio: increase virtuqueue size for virtio-scsi and virtio-blk
On 18.02.2020 16:53, Stefan Hajnoczi wrote: On Thu, Feb 13, 2020 at 05:59:27PM +0300, Denis Plotnikov wrote: v1: * seg_max default value changing removed --- The goal is to reduce the amount of requests issued by a guest on 1M reads/writes. This rises the performance up to 4% on that kind of disk access pattern. The maximum chunk size to be used for the guest disk accessing is limited with seg_max parameter, which represents the max amount of pices in the scatter-geather list in one guest disk request. Since seg_max is virqueue_size dependent, increasing the virtqueue size increases seg_max, which, in turn, increases the maximum size of data to be read/write from a guest disk. More details in the original problem statment: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg03721.html Suggested-by: Denis V. Lunev Signed-off-by: Denis Plotnikov --- hw/block/virtio-blk.c | 2 +- hw/core/machine.c | 2 ++ hw/scsi/virtio-scsi.c | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) I fixed up the "virtuqueue" typo in the commit message and the mis-formatted commit description (git-am(1) stops including lines after the first "---"). Actually, I sent the corrected version v3 of the patch last week. But it seems it got lost among that gigantic patch flow in the mailing list :) Thanks for applying! Denis Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan
Re: [PATCH v2] virtio: increase virtuqueue size for virtio-scsi and virtio-blk
On Thu, Feb 13, 2020 at 05:59:27PM +0300, Denis Plotnikov wrote: > v1: > * seg_max default value changing removed > > --- > The goal is to reduce the amount of requests issued by a guest on > 1M reads/writes. This rises the performance up to 4% on that kind of > disk access pattern. > > The maximum chunk size to be used for the guest disk accessing is > limited with seg_max parameter, which represents the max amount of > pices in the scatter-geather list in one guest disk request. > > Since seg_max is virqueue_size dependent, increasing the virtqueue > size increases seg_max, which, in turn, increases the maximum size > of data to be read/write from a guest disk. > > More details in the original problem statment: > https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg03721.html > > Suggested-by: Denis V. Lunev > Signed-off-by: Denis Plotnikov > --- > hw/block/virtio-blk.c | 2 +- > hw/core/machine.c | 2 ++ > hw/scsi/virtio-scsi.c | 2 +- > 3 files changed, 4 insertions(+), 2 deletions(-) I fixed up the "virtuqueue" typo in the commit message and the mis-formatted commit description (git-am(1) stops including lines after the first "---"). Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan signature.asc Description: PGP signature
Re: [PATCH v2] virtio: increase virtuqueue size for virtio-scsi and virtio-blk
Typo 'virtuqueue' in subject.
On 2/13/20 3:59 PM, Denis Plotnikov wrote:
v1:
* seg_max default value changing removed
^^^ This part ...
---
The goal is to reduce the amount of requests issued by a guest on
1M reads/writes. This rises the performance up to 4% on that kind of
disk access pattern.
The maximum chunk size to be used for the guest disk accessing is
limited with seg_max parameter, which represents the max amount of
pices in the scatter-geather list in one guest disk request.
Since seg_max is virqueue_size dependent, increasing the virtqueue
size increases seg_max, which, in turn, increases the maximum size
of data to be read/write from a guest disk.
More details in the original problem statment:
https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg03721.html
Suggested-by: Denis V. Lunev
Signed-off-by: Denis Plotnikov
---
... goes here.
hw/block/virtio-blk.c | 2 +-
hw/core/machine.c | 2 ++
hw/scsi/virtio-scsi.c | 2 +-
3 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 09f46ed85f..142863a3b2 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -1272,7 +1272,7 @@ static Property virtio_blk_properties[] = {
DEFINE_PROP_BIT("request-merging", VirtIOBlock, conf.request_merging, 0,
true),
DEFINE_PROP_UINT16("num-queues", VirtIOBlock, conf.num_queues, 1),
-DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 128),
+DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 256),
DEFINE_PROP_BOOL("seg-max-adjust", VirtIOBlock, conf.seg_max_adjust,
true),
DEFINE_PROP_LINK("iothread", VirtIOBlock, conf.iothread, TYPE_IOTHREAD,
IOThread *),
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 2501b540ec..3427d6cf4c 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -28,6 +28,8 @@
#include "hw/mem/nvdimm.h"
GlobalProperty hw_compat_4_2[] = {
+{ "virtio-blk-device", "queue-size", "128"},
+{ "virtio-scsi-device", "virtqueue_size", "128"},
{ "virtio-blk-device", "x-enable-wce-if-config-wce", "off" },
{ "virtio-blk-device", "seg-max-adjust", "off"},
{ "virtio-scsi-device", "seg_max_adjust", "off"},
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 3b61563609..472bbd233b 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -965,7 +965,7 @@ static void virtio_scsi_device_unrealize(DeviceState *dev,
Error **errp)
static Property virtio_scsi_properties[] = {
DEFINE_PROP_UINT32("num_queues", VirtIOSCSI, parent_obj.conf.num_queues,
1),
DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSI,
- parent_obj.conf.virtqueue_size, 128),
+ parent_obj.conf.virtqueue_size, 256),
DEFINE_PROP_BOOL("seg_max_adjust", VirtIOSCSI,
parent_obj.conf.seg_max_adjust, true),
DEFINE_PROP_UINT32("max_sectors", VirtIOSCSI, parent_obj.conf.max_sectors,
[PATCH v2] virtio: increase virtuqueue size for virtio-scsi and virtio-blk
v1:
* seg_max default value changing removed
---
The goal is to reduce the amount of requests issued by a guest on
1M reads/writes. This rises the performance up to 4% on that kind of
disk access pattern.
The maximum chunk size to be used for the guest disk accessing is
limited with seg_max parameter, which represents the max amount of
pices in the scatter-geather list in one guest disk request.
Since seg_max is virqueue_size dependent, increasing the virtqueue
size increases seg_max, which, in turn, increases the maximum size
of data to be read/write from a guest disk.
More details in the original problem statment:
https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg03721.html
Suggested-by: Denis V. Lunev
Signed-off-by: Denis Plotnikov
---
hw/block/virtio-blk.c | 2 +-
hw/core/machine.c | 2 ++
hw/scsi/virtio-scsi.c | 2 +-
3 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 09f46ed85f..142863a3b2 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -1272,7 +1272,7 @@ static Property virtio_blk_properties[] = {
DEFINE_PROP_BIT("request-merging", VirtIOBlock, conf.request_merging, 0,
true),
DEFINE_PROP_UINT16("num-queues", VirtIOBlock, conf.num_queues, 1),
-DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 128),
+DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 256),
DEFINE_PROP_BOOL("seg-max-adjust", VirtIOBlock, conf.seg_max_adjust, true),
DEFINE_PROP_LINK("iothread", VirtIOBlock, conf.iothread, TYPE_IOTHREAD,
IOThread *),
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 2501b540ec..3427d6cf4c 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -28,6 +28,8 @@
#include "hw/mem/nvdimm.h"
GlobalProperty hw_compat_4_2[] = {
+{ "virtio-blk-device", "queue-size", "128"},
+{ "virtio-scsi-device", "virtqueue_size", "128"},
{ "virtio-blk-device", "x-enable-wce-if-config-wce", "off" },
{ "virtio-blk-device", "seg-max-adjust", "off"},
{ "virtio-scsi-device", "seg_max_adjust", "off"},
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 3b61563609..472bbd233b 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -965,7 +965,7 @@ static void virtio_scsi_device_unrealize(DeviceState *dev,
Error **errp)
static Property virtio_scsi_properties[] = {
DEFINE_PROP_UINT32("num_queues", VirtIOSCSI, parent_obj.conf.num_queues,
1),
DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSI,
- parent_obj.conf.virtqueue_size, 128),
+ parent_obj.conf.virtqueue_size, 256),
DEFINE_PROP_BOOL("seg_max_adjust", VirtIOSCSI,
parent_obj.conf.seg_max_adjust, true),
DEFINE_PROP_UINT32("max_sectors", VirtIOSCSI, parent_obj.conf.max_sectors,
--
2.17.0
Re: [PATCH v2] virtio: increase virtuqueue size for virtio-scsi and virtio-blk
On 13.02.2020 14:45, Stefan Hajnoczi wrote: On Thu, Feb 13, 2020 at 12:28:25PM +0300, Denis Plotnikov wrote: On 13.02.2020 12:08, Stefan Hajnoczi wrote: On Thu, Feb 13, 2020 at 11:08:35AM +0300, Denis Plotnikov wrote: On 12.02.2020 18:43, Stefan Hajnoczi wrote: On Tue, Feb 11, 2020 at 05:14:14PM +0300, Denis Plotnikov wrote: The goal is to reduce the amount of requests issued by a guest on 1M reads/writes. This rises the performance up to 4% on that kind of disk access pattern. The maximum chunk size to be used for the guest disk accessing is limited with seg_max parameter, which represents the max amount of pices in the scatter-geather list in one guest disk request. Since seg_max is virqueue_size dependent, increasing the virtqueue size increases seg_max, which, in turn, increases the maximum size of data to be read/write from a guest disk. More details in the original problem statment: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg03721.html Suggested-by: Denis V. Lunev Signed-off-by: Denis Plotnikov --- hw/block/virtio-blk.c | 4 ++-- hw/core/machine.c | 2 ++ hw/scsi/virtio-scsi.c | 4 ++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 09f46ed85f..6df3a7a6df 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -914,7 +914,7 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config) memset(, 0, sizeof(blkcfg)); virtio_stq_p(vdev, , capacity); virtio_stl_p(vdev, _max, - s->conf.seg_max_adjust ? s->conf.queue_size - 2 : 128 - 2); + s->conf.seg_max_adjust ? s->conf.queue_size - 2 : 256 - 2); This value must not change on older machine types. Yes, that's true, but .. So does this patch need to turn seg-max-adjust *on* in hw_compat_4_2 so that old machine types get 126 instead of 254? If we set seg-max-adjust "on" in older machine types, the setups using them and having queue_sizes set , for example, 1024 will also set seg_max to 1024 - 2 which isn't the expected behavior: older mt didn't change seg_max in that case and stuck with 128 - 2. So, should we, instead, leave the default 128 - 2, for seg_max? Argh! Good point :-). How about a seg_max_default property that is initialized to 254 for modern machines and 126 to old machines? Hmm, but we'll achieve the same but with more code changes, don't we? 254 is because the queue-size is 256. We gonna leave 128-2 for older machine types just for not breaking anything. All other seg_max adjustment is provided by seg_max_adjust which is "on" by default in modern machine types. to summarize: modern mt defaults: seg_max_adjust = on queue_size = 256 => default seg_max = 254 => changing queue-size will change seg_max = queue_size - 2 old mt defaults: seg_max_adjust = off queue_size = 128 => default seg_max = 126 => changing queue-size won't change seg_max, it's always = 126 like it was before You're right! The only strange case is a modern machine type with seg_max_adjust=off, where queue_size will be 256 but seg_max will be 126. But no user would want to disable seg_max_adjust, so it's okay. I agree with you that the line of code can remain unchanged: /* * Only old machine types use seg_max_adjust=off and there the default * value of queue_size is 128. */ virtio_stl_p(vdev, _max, s->conf.seg_max_adjust ? s->conf.queue_size - 2 : 128 - 2); Stefan Ok, I'll resend the patch sortly Thanks! Denis
Re: [PATCH v2] virtio: increase virtuqueue size for virtio-scsi and virtio-blk
On Thu, Feb 13, 2020 at 12:28:25PM +0300, Denis Plotnikov wrote: > > > On 13.02.2020 12:08, Stefan Hajnoczi wrote: > > On Thu, Feb 13, 2020 at 11:08:35AM +0300, Denis Plotnikov wrote: > > > On 12.02.2020 18:43, Stefan Hajnoczi wrote: > > > > On Tue, Feb 11, 2020 at 05:14:14PM +0300, Denis Plotnikov wrote: > > > > > The goal is to reduce the amount of requests issued by a guest on > > > > > 1M reads/writes. This rises the performance up to 4% on that kind of > > > > > disk access pattern. > > > > > > > > > > The maximum chunk size to be used for the guest disk accessing is > > > > > limited with seg_max parameter, which represents the max amount of > > > > > pices in the scatter-geather list in one guest disk request. > > > > > > > > > > Since seg_max is virqueue_size dependent, increasing the virtqueue > > > > > size increases seg_max, which, in turn, increases the maximum size > > > > > of data to be read/write from a guest disk. > > > > > > > > > > More details in the original problem statment: > > > > > https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg03721.html > > > > > > > > > > Suggested-by: Denis V. Lunev > > > > > Signed-off-by: Denis Plotnikov > > > > > --- > > > > >hw/block/virtio-blk.c | 4 ++-- > > > > >hw/core/machine.c | 2 ++ > > > > >hw/scsi/virtio-scsi.c | 4 ++-- > > > > >3 files changed, 6 insertions(+), 4 deletions(-) > > > > > > > > > > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c > > > > > index 09f46ed85f..6df3a7a6df 100644 > > > > > --- a/hw/block/virtio-blk.c > > > > > +++ b/hw/block/virtio-blk.c > > > > > @@ -914,7 +914,7 @@ static void virtio_blk_update_config(VirtIODevice > > > > > *vdev, uint8_t *config) > > > > >memset(, 0, sizeof(blkcfg)); > > > > >virtio_stq_p(vdev, , capacity); > > > > >virtio_stl_p(vdev, _max, > > > > > - s->conf.seg_max_adjust ? s->conf.queue_size - 2 : > > > > > 128 - 2); > > > > > + s->conf.seg_max_adjust ? s->conf.queue_size - 2 : > > > > > 256 - 2); > > > > This value must not change on older machine types. > > > Yes, that's true, but .. > > > > So does this patch > > > > need to turn seg-max-adjust *on* in hw_compat_4_2 so that old machine > > > > types get 126 instead of 254? > > > If we set seg-max-adjust "on" in older machine types, the setups using > > > them > > > and having queue_sizes set , for example, 1024 will also set seg_max to > > > 1024 > > > - 2 which isn't the expected behavior: older mt didn't change seg_max in > > > that case and stuck with 128 - 2. > > > So, should we, instead, leave the default 128 - 2, for seg_max? > > Argh! Good point :-). > > > > How about a seg_max_default property that is initialized to 254 for > > modern machines and 126 to old machines? > Hmm, but we'll achieve the same but with more code changes, don't we? > 254 is because the queue-size is 256. We gonna leave 128-2 for older machine > types > just for not breaking anything. All other seg_max adjustment is provided by > seg_max_adjust which is "on" by default in modern machine types. > > to summarize: > > modern mt defaults: > seg_max_adjust = on > queue_size = 256 > > => default seg_max = 254 > => changing queue-size will change seg_max = queue_size - 2 > > old mt defaults: > seg_max_adjust = off > queue_size = 128 > > => default seg_max = 126 > => changing queue-size won't change seg_max, it's always = 126 like it was > before You're right! The only strange case is a modern machine type with seg_max_adjust=off, where queue_size will be 256 but seg_max will be 126. But no user would want to disable seg_max_adjust, so it's okay. I agree with you that the line of code can remain unchanged: /* * Only old machine types use seg_max_adjust=off and there the default * value of queue_size is 128. */ virtio_stl_p(vdev, _max, s->conf.seg_max_adjust ? s->conf.queue_size - 2 : 128 - 2); Stefan signature.asc Description: PGP signature
Re: [PATCH v2] virtio: increase virtuqueue size for virtio-scsi and virtio-blk
On 13.02.2020 12:08, Stefan Hajnoczi wrote: On Thu, Feb 13, 2020 at 11:08:35AM +0300, Denis Plotnikov wrote: On 12.02.2020 18:43, Stefan Hajnoczi wrote: On Tue, Feb 11, 2020 at 05:14:14PM +0300, Denis Plotnikov wrote: The goal is to reduce the amount of requests issued by a guest on 1M reads/writes. This rises the performance up to 4% on that kind of disk access pattern. The maximum chunk size to be used for the guest disk accessing is limited with seg_max parameter, which represents the max amount of pices in the scatter-geather list in one guest disk request. Since seg_max is virqueue_size dependent, increasing the virtqueue size increases seg_max, which, in turn, increases the maximum size of data to be read/write from a guest disk. More details in the original problem statment: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg03721.html Suggested-by: Denis V. Lunev Signed-off-by: Denis Plotnikov --- hw/block/virtio-blk.c | 4 ++-- hw/core/machine.c | 2 ++ hw/scsi/virtio-scsi.c | 4 ++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 09f46ed85f..6df3a7a6df 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -914,7 +914,7 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config) memset(, 0, sizeof(blkcfg)); virtio_stq_p(vdev, , capacity); virtio_stl_p(vdev, _max, - s->conf.seg_max_adjust ? s->conf.queue_size - 2 : 128 - 2); + s->conf.seg_max_adjust ? s->conf.queue_size - 2 : 256 - 2); This value must not change on older machine types. Yes, that's true, but .. So does this patch need to turn seg-max-adjust *on* in hw_compat_4_2 so that old machine types get 126 instead of 254? If we set seg-max-adjust "on" in older machine types, the setups using them and having queue_sizes set , for example, 1024 will also set seg_max to 1024 - 2 which isn't the expected behavior: older mt didn't change seg_max in that case and stuck with 128 - 2. So, should we, instead, leave the default 128 - 2, for seg_max? Argh! Good point :-). How about a seg_max_default property that is initialized to 254 for modern machines and 126 to old machines? Hmm, but we'll achieve the same but with more code changes, don't we? 254 is because the queue-size is 256. We gonna leave 128-2 for older machine types just for not breaking anything. All other seg_max adjustment is provided by seg_max_adjust which is "on" by default in modern machine types. to summarize: modern mt defaults: seg_max_adjust = on queue_size = 256 => default seg_max = 254 => changing queue-size will change seg_max = queue_size - 2 old mt defaults: seg_max_adjust = off queue_size = 128 => default seg_max = 126 => changing queue-size won't change seg_max, it's always = 126 like it was before Denis Stefan
Re: [PATCH v2] virtio: increase virtuqueue size for virtio-scsi and virtio-blk
On Thu, Feb 13, 2020 at 11:08:35AM +0300, Denis Plotnikov wrote: > On 12.02.2020 18:43, Stefan Hajnoczi wrote: > > On Tue, Feb 11, 2020 at 05:14:14PM +0300, Denis Plotnikov wrote: > > > The goal is to reduce the amount of requests issued by a guest on > > > 1M reads/writes. This rises the performance up to 4% on that kind of > > > disk access pattern. > > > > > > The maximum chunk size to be used for the guest disk accessing is > > > limited with seg_max parameter, which represents the max amount of > > > pices in the scatter-geather list in one guest disk request. > > > > > > Since seg_max is virqueue_size dependent, increasing the virtqueue > > > size increases seg_max, which, in turn, increases the maximum size > > > of data to be read/write from a guest disk. > > > > > > More details in the original problem statment: > > > https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg03721.html > > > > > > Suggested-by: Denis V. Lunev > > > Signed-off-by: Denis Plotnikov > > > --- > > > hw/block/virtio-blk.c | 4 ++-- > > > hw/core/machine.c | 2 ++ > > > hw/scsi/virtio-scsi.c | 4 ++-- > > > 3 files changed, 6 insertions(+), 4 deletions(-) > > > > > > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c > > > index 09f46ed85f..6df3a7a6df 100644 > > > --- a/hw/block/virtio-blk.c > > > +++ b/hw/block/virtio-blk.c > > > @@ -914,7 +914,7 @@ static void virtio_blk_update_config(VirtIODevice > > > *vdev, uint8_t *config) > > > memset(, 0, sizeof(blkcfg)); > > > virtio_stq_p(vdev, , capacity); > > > virtio_stl_p(vdev, _max, > > > - s->conf.seg_max_adjust ? s->conf.queue_size - 2 : 128 - > > > 2); > > > + s->conf.seg_max_adjust ? s->conf.queue_size - 2 : 256 - > > > 2); > > This value must not change on older machine types. > Yes, that's true, but .. > > So does this patch > > need to turn seg-max-adjust *on* in hw_compat_4_2 so that old machine > > types get 126 instead of 254? > If we set seg-max-adjust "on" in older machine types, the setups using them > and having queue_sizes set , for example, 1024 will also set seg_max to 1024 > - 2 which isn't the expected behavior: older mt didn't change seg_max in > that case and stuck with 128 - 2. > So, should we, instead, leave the default 128 - 2, for seg_max? Argh! Good point :-). How about a seg_max_default property that is initialized to 254 for modern machines and 126 to old machines? Stefan signature.asc Description: PGP signature
Re: [PATCH v2] virtio: increase virtuqueue size for virtio-scsi and virtio-blk
On 12.02.2020 18:43, Stefan Hajnoczi wrote:
On Tue, Feb 11, 2020 at 05:14:14PM +0300, Denis Plotnikov wrote:
The goal is to reduce the amount of requests issued by a guest on
1M reads/writes. This rises the performance up to 4% on that kind of
disk access pattern.
The maximum chunk size to be used for the guest disk accessing is
limited with seg_max parameter, which represents the max amount of
pices in the scatter-geather list in one guest disk request.
Since seg_max is virqueue_size dependent, increasing the virtqueue
size increases seg_max, which, in turn, increases the maximum size
of data to be read/write from a guest disk.
More details in the original problem statment:
https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg03721.html
Suggested-by: Denis V. Lunev
Signed-off-by: Denis Plotnikov
---
hw/block/virtio-blk.c | 4 ++--
hw/core/machine.c | 2 ++
hw/scsi/virtio-scsi.c | 4 ++--
3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 09f46ed85f..6df3a7a6df 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -914,7 +914,7 @@ static void virtio_blk_update_config(VirtIODevice *vdev,
uint8_t *config)
memset(, 0, sizeof(blkcfg));
virtio_stq_p(vdev, , capacity);
virtio_stl_p(vdev, _max,
- s->conf.seg_max_adjust ? s->conf.queue_size - 2 : 128 - 2);
+ s->conf.seg_max_adjust ? s->conf.queue_size - 2 : 256 - 2);
This value must not change on older machine types.
Yes, that's true, but ..
So does this patch
need to turn seg-max-adjust *on* in hw_compat_4_2 so that old machine
types get 126 instead of 254?
If we set seg-max-adjust "on" in older machine types, the setups using
them and having queue_sizes set , for example, 1024 will also set
seg_max to 1024 - 2 which isn't the expected behavior: older mt didn't
change seg_max in that case and stuck with 128 - 2.
So, should we, instead, leave the default 128 - 2, for seg_max?
Denis
virtio_stw_p(vdev, , conf->cyls);
virtio_stl_p(vdev, _size, blk_size);
virtio_stw_p(vdev, _io_size, conf->min_io_size / blk_size);
@@ -1272,7 +1272,7 @@ static Property virtio_blk_properties[] = {
DEFINE_PROP_BIT("request-merging", VirtIOBlock, conf.request_merging, 0,
true),
DEFINE_PROP_UINT16("num-queues", VirtIOBlock, conf.num_queues, 1),
-DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 128),
+DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 256),
DEFINE_PROP_BOOL("seg-max-adjust", VirtIOBlock, conf.seg_max_adjust,
true),
DEFINE_PROP_LINK("iothread", VirtIOBlock, conf.iothread, TYPE_IOTHREAD,
IOThread *),
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 2501b540ec..3427d6cf4c 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -28,6 +28,8 @@
#include "hw/mem/nvdimm.h"
GlobalProperty hw_compat_4_2[] = {
+{ "virtio-blk-device", "queue-size", "128"},
+{ "virtio-scsi-device", "virtqueue_size", "128"},
{ "virtio-blk-device", "x-enable-wce-if-config-wce", "off" },
{ "virtio-blk-device", "seg-max-adjust", "off"},
{ "virtio-scsi-device", "seg_max_adjust", "off"},
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 3b61563609..b38f50a429 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -660,7 +660,7 @@ static void virtio_scsi_get_config(VirtIODevice *vdev,
virtio_stl_p(vdev, >num_queues, s->conf.num_queues);
virtio_stl_p(vdev, >seg_max,
- s->conf.seg_max_adjust ? s->conf.virtqueue_size - 2 : 128 -
2);
+ s->conf.seg_max_adjust ? s->conf.virtqueue_size - 2 : 256 -
2);
virtio_stl_p(vdev, >max_sectors, s->conf.max_sectors);
virtio_stl_p(vdev, >cmd_per_lun, s->conf.cmd_per_lun);
virtio_stl_p(vdev, >event_info_size, sizeof(VirtIOSCSIEvent));
@@ -965,7 +965,7 @@ static void virtio_scsi_device_unrealize(DeviceState *dev,
Error **errp)
static Property virtio_scsi_properties[] = {
DEFINE_PROP_UINT32("num_queues", VirtIOSCSI, parent_obj.conf.num_queues,
1),
DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSI,
- parent_obj.conf.virtqueue_size, 128),
+ parent_obj.conf.virtqueue_size, 256),
DEFINE_PROP_BOOL("seg_max_adjust", VirtIOSCSI,
parent_obj.conf.seg_max_adjust, true),
DEFINE_PROP_UINT32("max_sectors", VirtIOSCSI, parent_obj.conf.max_sectors,
--
2.17.0
Re: [PATCH v2] virtio: increase virtuqueue size for virtio-scsi and virtio-blk
On Tue, Feb 11, 2020 at 05:14:14PM +0300, Denis Plotnikov wrote:
> The goal is to reduce the amount of requests issued by a guest on
> 1M reads/writes. This rises the performance up to 4% on that kind of
> disk access pattern.
>
> The maximum chunk size to be used for the guest disk accessing is
> limited with seg_max parameter, which represents the max amount of
> pices in the scatter-geather list in one guest disk request.
>
> Since seg_max is virqueue_size dependent, increasing the virtqueue
> size increases seg_max, which, in turn, increases the maximum size
> of data to be read/write from a guest disk.
>
> More details in the original problem statment:
> https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg03721.html
>
> Suggested-by: Denis V. Lunev
> Signed-off-by: Denis Plotnikov
> ---
> hw/block/virtio-blk.c | 4 ++--
> hw/core/machine.c | 2 ++
> hw/scsi/virtio-scsi.c | 4 ++--
> 3 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index 09f46ed85f..6df3a7a6df 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -914,7 +914,7 @@ static void virtio_blk_update_config(VirtIODevice *vdev,
> uint8_t *config)
> memset(, 0, sizeof(blkcfg));
> virtio_stq_p(vdev, , capacity);
> virtio_stl_p(vdev, _max,
> - s->conf.seg_max_adjust ? s->conf.queue_size - 2 : 128 - 2);
> + s->conf.seg_max_adjust ? s->conf.queue_size - 2 : 256 - 2);
This value must not change on older machine types. So does this patch
need to turn seg-max-adjust *on* in hw_compat_4_2 so that old machine
types get 126 instead of 254?
> virtio_stw_p(vdev, , conf->cyls);
> virtio_stl_p(vdev, _size, blk_size);
> virtio_stw_p(vdev, _io_size, conf->min_io_size / blk_size);
> @@ -1272,7 +1272,7 @@ static Property virtio_blk_properties[] = {
> DEFINE_PROP_BIT("request-merging", VirtIOBlock, conf.request_merging, 0,
> true),
> DEFINE_PROP_UINT16("num-queues", VirtIOBlock, conf.num_queues, 1),
> -DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 128),
> +DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 256),
> DEFINE_PROP_BOOL("seg-max-adjust", VirtIOBlock, conf.seg_max_adjust,
> true),
> DEFINE_PROP_LINK("iothread", VirtIOBlock, conf.iothread, TYPE_IOTHREAD,
> IOThread *),
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 2501b540ec..3427d6cf4c 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -28,6 +28,8 @@
> #include "hw/mem/nvdimm.h"
>
> GlobalProperty hw_compat_4_2[] = {
> +{ "virtio-blk-device", "queue-size", "128"},
> +{ "virtio-scsi-device", "virtqueue_size", "128"},
> { "virtio-blk-device", "x-enable-wce-if-config-wce", "off" },
> { "virtio-blk-device", "seg-max-adjust", "off"},
> { "virtio-scsi-device", "seg_max_adjust", "off"},
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index 3b61563609..b38f50a429 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -660,7 +660,7 @@ static void virtio_scsi_get_config(VirtIODevice *vdev,
>
> virtio_stl_p(vdev, >num_queues, s->conf.num_queues);
> virtio_stl_p(vdev, >seg_max,
> - s->conf.seg_max_adjust ? s->conf.virtqueue_size - 2 : 128 -
> 2);
> + s->conf.seg_max_adjust ? s->conf.virtqueue_size - 2 : 256 -
> 2);
> virtio_stl_p(vdev, >max_sectors, s->conf.max_sectors);
> virtio_stl_p(vdev, >cmd_per_lun, s->conf.cmd_per_lun);
> virtio_stl_p(vdev, >event_info_size, sizeof(VirtIOSCSIEvent));
> @@ -965,7 +965,7 @@ static void virtio_scsi_device_unrealize(DeviceState
> *dev, Error **errp)
> static Property virtio_scsi_properties[] = {
> DEFINE_PROP_UINT32("num_queues", VirtIOSCSI, parent_obj.conf.num_queues,
> 1),
> DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSI,
> - parent_obj.conf.virtqueue_size,
> 128),
> + parent_obj.conf.virtqueue_size,
> 256),
> DEFINE_PROP_BOOL("seg_max_adjust", VirtIOSCSI,
>parent_obj.conf.seg_max_adjust, true),
> DEFINE_PROP_UINT32("max_sectors", VirtIOSCSI,
> parent_obj.conf.max_sectors,
> --
> 2.17.0
>
>
signature.asc
Description: PGP signature
[PATCH v2] virtio: increase virtuqueue size for virtio-scsi and virtio-blk
The goal is to reduce the amount of requests issued by a guest on
1M reads/writes. This rises the performance up to 4% on that kind of
disk access pattern.
The maximum chunk size to be used for the guest disk accessing is
limited with seg_max parameter, which represents the max amount of
pices in the scatter-geather list in one guest disk request.
Since seg_max is virqueue_size dependent, increasing the virtqueue
size increases seg_max, which, in turn, increases the maximum size
of data to be read/write from a guest disk.
More details in the original problem statment:
https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg03721.html
Suggested-by: Denis V. Lunev
Signed-off-by: Denis Plotnikov
---
hw/block/virtio-blk.c | 4 ++--
hw/core/machine.c | 2 ++
hw/scsi/virtio-scsi.c | 4 ++--
3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 09f46ed85f..6df3a7a6df 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -914,7 +914,7 @@ static void virtio_blk_update_config(VirtIODevice *vdev,
uint8_t *config)
memset(, 0, sizeof(blkcfg));
virtio_stq_p(vdev, , capacity);
virtio_stl_p(vdev, _max,
- s->conf.seg_max_adjust ? s->conf.queue_size - 2 : 128 - 2);
+ s->conf.seg_max_adjust ? s->conf.queue_size - 2 : 256 - 2);
virtio_stw_p(vdev, , conf->cyls);
virtio_stl_p(vdev, _size, blk_size);
virtio_stw_p(vdev, _io_size, conf->min_io_size / blk_size);
@@ -1272,7 +1272,7 @@ static Property virtio_blk_properties[] = {
DEFINE_PROP_BIT("request-merging", VirtIOBlock, conf.request_merging, 0,
true),
DEFINE_PROP_UINT16("num-queues", VirtIOBlock, conf.num_queues, 1),
-DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 128),
+DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 256),
DEFINE_PROP_BOOL("seg-max-adjust", VirtIOBlock, conf.seg_max_adjust, true),
DEFINE_PROP_LINK("iothread", VirtIOBlock, conf.iothread, TYPE_IOTHREAD,
IOThread *),
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 2501b540ec..3427d6cf4c 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -28,6 +28,8 @@
#include "hw/mem/nvdimm.h"
GlobalProperty hw_compat_4_2[] = {
+{ "virtio-blk-device", "queue-size", "128"},
+{ "virtio-scsi-device", "virtqueue_size", "128"},
{ "virtio-blk-device", "x-enable-wce-if-config-wce", "off" },
{ "virtio-blk-device", "seg-max-adjust", "off"},
{ "virtio-scsi-device", "seg_max_adjust", "off"},
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 3b61563609..b38f50a429 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -660,7 +660,7 @@ static void virtio_scsi_get_config(VirtIODevice *vdev,
virtio_stl_p(vdev, >num_queues, s->conf.num_queues);
virtio_stl_p(vdev, >seg_max,
- s->conf.seg_max_adjust ? s->conf.virtqueue_size - 2 : 128 -
2);
+ s->conf.seg_max_adjust ? s->conf.virtqueue_size - 2 : 256 -
2);
virtio_stl_p(vdev, >max_sectors, s->conf.max_sectors);
virtio_stl_p(vdev, >cmd_per_lun, s->conf.cmd_per_lun);
virtio_stl_p(vdev, >event_info_size, sizeof(VirtIOSCSIEvent));
@@ -965,7 +965,7 @@ static void virtio_scsi_device_unrealize(DeviceState *dev,
Error **errp)
static Property virtio_scsi_properties[] = {
DEFINE_PROP_UINT32("num_queues", VirtIOSCSI, parent_obj.conf.num_queues,
1),
DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSI,
- parent_obj.conf.virtqueue_size, 128),
+ parent_obj.conf.virtqueue_size, 256),
DEFINE_PROP_BOOL("seg_max_adjust", VirtIOSCSI,
parent_obj.conf.seg_max_adjust, true),
DEFINE_PROP_UINT32("max_sectors", VirtIOSCSI, parent_obj.conf.max_sectors,
--
2.17.0
