15.04.26 18:24, Raphael Norwitz пишет:
Looks in keeping with how virtio-blk does things with seg_max_adjust, so I’m
happy in principle.
Just a nit and a question.
On 4/2/26 1:05 AM, Sergei Heifetz wrote:
[You don't often get email from [email protected]. Learn why this is
important at https://aka.ms/LearnAboutSenderIdentification ]
The virtio specification is not completely clear about seg_max and its
relationship with queue_size. Some drivers (for example, the modern
Linux kernel driver) rely on seg_max to set the maximum number of
segments used in a request. If seg_max is set larger than queue_size,
such a driver might overwhelm a virtqueue by trying to send more
segments than it can handle. As a result, it either hangs or faults.
One might argue that it is the vhost-user server's responsibility to set
a valid seg_max value. However, due to the issue described in the
previous paragraph, this value should generally depend on queue_size.
That's why it may be necessary to control it on QEMU's side.
This patch adds the seg-max-adjust flag. A flag with the same name
already exists for virtio-blk (and it exists to solve the same problem,
except that here we have a vhost-user server to consult).
If seg-max-adjust is set, the final seg_max is the minimum of the value
provided by the vhost-user server and (queue_size - 2). It is not
enabled by default.
Signed-off-by: Sergei Heifetz <[email protected]>
---
hw/block/vhost-user-blk.c | 11 +++++++++++
include/hw/virtio/vhost-user-blk.h | 1 +
2 files changed, 12 insertions(+)
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index c151e836770..23f910d9fe3 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -65,6 +65,11 @@ static void vhost_user_blk_update_config(VirtIODevice *vdev,
uint8_t *config)
/* Our num_queues overrides the device backend */
virtio_stw_p(vdev, &s->blkcfg.num_queues, s->num_queues);
+ if (s->seg_max_adjust) {
NIT: declaration at the top of the function
+ uint32_t seg_max = MIN(s->blkcfg.seg_max, s->queue_size - 2);
+ virtio_stl_p(vdev, &s->blkcfg.seg_max, seg_max);
+ }
+
memcpy(config, &s->blkcfg, vdev->config_len);
}
@@ -474,6 +479,10 @@ static void vhost_user_blk_device_realize(DeviceState
*dev, Error **errp)
error_setg(errp, "queue size must be non-zero");
return;
}
Can you explain why queue size must be greater than 4 if seg_max_adjust is set?
Sorry if I'm missing something obvious.
Because we want seg_max = MIN(s->blkcfg.seg_max, s->queue_size - 2),
and queue size must be power of two. So, for seg_max be > 0, we need queue_size
at least 4.
+ if (s->queue_size < 4 && s->seg_max_adjust) {
+ error_setg(errp, "queue size must be >= 4 when seg-max-adjust is set");
+ return;
+ }
if (s->queue_size > VIRTQUEUE_MAX_SIZE) {
error_setg(errp, "queue size must not exceed %d",
VIRTQUEUE_MAX_SIZE);
@@ -608,6 +617,8 @@ static const Property vhost_user_blk_properties[] = {
DEFINE_PROP_UINT16("num-queues", VHostUserBlk, num_queues,
VHOST_USER_BLK_AUTO_NUM_QUEUES),
DEFINE_PROP_UINT32("queue-size", VHostUserBlk, queue_size, 128),
+ DEFINE_PROP_BOOL("seg-max-adjust", VHostUserBlk, seg_max_adjust,
+ false),
DEFINE_PROP_BIT64("config-wce", VHostUserBlk, parent_obj.host_features,
VIRTIO_BLK_F_CONFIG_WCE, true),
DEFINE_PROP_BIT64("discard", VHostUserBlk, parent_obj.host_features,
diff --git a/include/hw/virtio/vhost-user-blk.h
b/include/hw/virtio/vhost-user-blk.h
index 1e41a2bcdf6..dee848cfd81 100644
--- a/include/hw/virtio/vhost-user-blk.h
+++ b/include/hw/virtio/vhost-user-blk.h
@@ -34,6 +34,7 @@ struct VHostUserBlk {
struct virtio_blk_config blkcfg;
uint16_t num_queues;
uint32_t queue_size;
+ bool seg_max_adjust;
struct vhost_dev dev;
struct vhost_inflight *inflight;
VhostUserState vhost_user;
--
2.53.0
--
Best regards,
Vladimir
