Linux virtualization list
 help / color / mirror / Atom feed
* [PATCH] virtio_blk: freeze the virtqueue layout at probe time
@ 2026-11-02 13:37 Yunye Zhao
  2026-11-02 13:41 ` sashiko-bot
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Yunye Zhao @ 2026-11-02 13:37 UTC (permalink / raw)
  To: Michael S. Tsirkin, Jason Wang, Jens Axboe
  Cc: Stefan Hajnoczi, Paolo Bonzini, Eugenio Pérez, Xuan Zhuo,
	Max Gurtovoy, virtualization, linux-block, linux-kernel,
	Joseph Qi, Ferry Meng

init_vq() reads the writable num_request_queues and poll_queues module
parameters both at probe and when recreating virtqueues after suspend
or reset. However, virtio-blk does not update the blk-mq hardware queue
count or queue maps on restore.

Increasing poll_queues can therefore remove the completion callback
from a virtqueue still used for interrupt-driven I/O, leaving requests
uncompleted. Reducing the effective request queue count through
num_request_queues can instead leave existing hardware contexts
indexing beyond the reallocated vblk->vqs array.

Determine the virtqueue count and the split between interrupt-driven
and polling queues once at probe, and reuse this layout when
recreating the virtqueues. Changes to these module parameters then
affect only subsequently probed devices.

Fixes: 0989c41bed96 ("virtio-blk: add num_request_queues module parameter")
Signed-off-by: Yunye Zhao <yunye.zhao@linux.alibaba.com>
---
The functional fix was previously tested with QEMU 8.2.2 on
7be4bc52fd9e (v7.3-2415), covering:

- PCI FLR after changing poll_queues from 0 to 1 or num_request_queues
  from 0 to 1 on a device with four request queues.
- Normal and polled I/O across FLR with poll_queues=1 at probe,
  including a subsequent change of poll_queues to 0.
- Device suspend/resume with pm_test=devices and state=freeze (not a
  full s2idle cycle), plus full S3 and S4 suspend/resume cycles.

 drivers/block/virtio_blk.c | 71 +++++++++++++++++++++++++++-----------------
 1 file changed, 43 insertions(+), 28 deletions(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index c8bfbf8435ac..0df89f381047 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -958,42 +958,54 @@ static void virtblk_config_changed(struct virtio_device *vdev)
 	queue_work(virtblk_wq, &vblk->config_work);
 }
 
+/* Keep the queue layout unchanged for the lifetime of the tag set. */
+static int virtblk_init_queue_layout(struct virtio_blk *vblk)
+{
+	struct virtio_device *vdev = vblk->vdev;
+	unsigned short num_vqs;
+	unsigned short num_poll_vqs;
+	int err;
+
+	err = virtio_cread_feature(vdev, VIRTIO_BLK_F_MQ,
+				   struct virtio_blk_config, num_queues,
+				   &num_vqs);
+	if (err)
+		num_vqs = 1;
+
+	if (!err && !num_vqs) {
+		dev_err(&vdev->dev, "MQ advertised but zero queues reported\n");
+		return -EINVAL;
+	}
+
+	num_vqs = blk_mq_num_possible_queues(
+			min_not_zero(num_request_queues, num_vqs));
+
+	num_poll_vqs = min_t(unsigned int, poll_queues, num_vqs - 1);
+
+	vblk->num_vqs = num_vqs;
+	vblk->io_queues[HCTX_TYPE_DEFAULT] = num_vqs - num_poll_vqs;
+	vblk->io_queues[HCTX_TYPE_READ] = 0;
+	vblk->io_queues[HCTX_TYPE_POLL] = num_poll_vqs;
+
+	dev_info(&vdev->dev, "%d/%d/%d default/read/poll queues\n",
+				vblk->io_queues[HCTX_TYPE_DEFAULT],
+				vblk->io_queues[HCTX_TYPE_READ],
+				vblk->io_queues[HCTX_TYPE_POLL]);
+
+	return 0;
+}
+
 static int init_vq(struct virtio_blk *vblk)
 {
 	int err;
 	unsigned short i;
 	struct virtqueue_info *vqs_info;
 	struct virtqueue **vqs;
-	unsigned short num_vqs;
-	unsigned short num_poll_vqs;
+	unsigned short num_vqs = vblk->num_vqs;
+	unsigned short num_poll_vqs = vblk->io_queues[HCTX_TYPE_POLL];
 	struct virtio_device *vdev = vblk->vdev;
 	struct irq_affinity desc = { 0, };
 
-	err = virtio_cread_feature(vdev, VIRTIO_BLK_F_MQ,
-				   struct virtio_blk_config, num_queues,
-				   &num_vqs);
-	if (err)
-		num_vqs = 1;
-
-	if (!err && !num_vqs) {
-		dev_err(&vdev->dev, "MQ advertised but zero queues reported\n");
-		return -EINVAL;
-	}
-
-	num_vqs = blk_mq_num_possible_queues(
-			min_not_zero(num_request_queues, num_vqs));
-
-	num_poll_vqs = min_t(unsigned int, poll_queues, num_vqs - 1);
-
-	vblk->io_queues[HCTX_TYPE_DEFAULT] = num_vqs - num_poll_vqs;
-	vblk->io_queues[HCTX_TYPE_READ] = 0;
-	vblk->io_queues[HCTX_TYPE_POLL] = num_poll_vqs;
-
-	dev_info(&vdev->dev, "%d/%d/%d default/read/poll queues\n",
-				vblk->io_queues[HCTX_TYPE_DEFAULT],
-				vblk->io_queues[HCTX_TYPE_READ],
-				vblk->io_queues[HCTX_TYPE_POLL]);
-
 	vblk->vqs = kmalloc_objs(*vblk->vqs, num_vqs);
 	if (!vblk->vqs)
 		return -ENOMEM;
@@ -1025,7 +1037,6 @@ static int init_vq(struct virtio_blk *vblk)
 		spin_lock_init(&vblk->vqs[i].lock);
 		vblk->vqs[i].vq = vqs[i];
 	}
-	vblk->num_vqs = num_vqs;
 
 out:
 	kfree(vqs);
@@ -1470,6 +1481,10 @@ static int virtblk_probe(struct virtio_device *vdev)
 
 	INIT_WORK(&vblk->config_work, virtblk_config_changed_work);
 
+	err = virtblk_init_queue_layout(vblk);
+	if (err)
+		goto out_free_vblk;
+
 	err = init_vq(vblk);
 	if (err)
 		goto out_free_vblk;

base-commit: 7be4bc52fd9e0e55dc9a2e0232b89427d2c4b614
-- 
2.19.1.6.gb485710b

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] virtio_blk: freeze the virtqueue layout at probe time
  2026-11-02 13:37 [PATCH] virtio_blk: freeze the virtqueue layout at probe time Yunye Zhao
@ 2026-11-02 13:41 ` sashiko-bot
  2026-11-02 19:15 ` Stefan Hajnoczi
  2026-11-03 14:31 ` Jens Axboe
  2 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-11-02 13:41 UTC (permalink / raw)
  To: Yunye Zhao; +Cc: virtualization, Michael S. Tsirkin, Eugenio Perez

> init_vq() reads the writable num_request_queues and poll_queues module
> parameters both at probe and when recreating virtqueues after suspend
> or reset. However, virtio-blk does not update the blk-mq hardware queue
> count or queue maps on restore.
> 
> Increasing poll_queues can therefore remove the completion callback
> from a virtqueue still used for interrupt-driven I/O, leaving requests
> uncompleted. Reducing the effective request queue count through
> num_request_queues can instead leave existing hardware contexts
> indexing beyond the reallocated vblk->vqs array.
> 
> Determine the virtqueue count and the split between interrupt-driven
> and polling queues once at probe, and reuse this layout when
> recreating the virtqueues. Changes to these module parameters then
> affect only subsequently probed devices.
> 
> Fixes: 0989c41bed96 ("virtio-blk: add num_request_queues module parameter")
> Signed-off-by: Yunye Zhao <yunye.zhao@linux.alibaba.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102133740.88919-1-yunye.zhao@linux.alibaba.com?part=1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] virtio_blk: freeze the virtqueue layout at probe time
  2026-11-02 13:37 [PATCH] virtio_blk: freeze the virtqueue layout at probe time Yunye Zhao
  2026-11-02 13:41 ` sashiko-bot
@ 2026-11-02 19:15 ` Stefan Hajnoczi
  2026-11-03 14:31 ` Jens Axboe
  2 siblings, 0 replies; 4+ messages in thread
From: Stefan Hajnoczi @ 2026-11-02 19:15 UTC (permalink / raw)
  To: Yunye Zhao
  Cc: Michael S. Tsirkin, Jason Wang, Jens Axboe, Paolo Bonzini,
	Eugenio Pérez, Xuan Zhuo, Max Gurtovoy, virtualization,
	linux-block, linux-kernel, Joseph Qi, Ferry Meng

On Mon, Nov 02, 2026 at 09:37:40PM +0800, Yunye Zhao wrote:
> init_vq() reads the writable num_request_queues and poll_queues module
> parameters both at probe and when recreating virtqueues after suspend
> or reset. However, virtio-blk does not update the blk-mq hardware queue
> count or queue maps on restore.
> 
> Increasing poll_queues can therefore remove the completion callback
> from a virtqueue still used for interrupt-driven I/O, leaving requests
> uncompleted. Reducing the effective request queue count through
> num_request_queues can instead leave existing hardware contexts
> indexing beyond the reallocated vblk->vqs array.
> 
> Determine the virtqueue count and the split between interrupt-driven
> and polling queues once at probe, and reuse this layout when
> recreating the virtqueues. Changes to these module parameters then
> affect only subsequently probed devices.
> 
> Fixes: 0989c41bed96 ("virtio-blk: add num_request_queues module parameter")
> Signed-off-by: Yunye Zhao <yunye.zhao@linux.alibaba.com>
> ---
> The functional fix was previously tested with QEMU 8.2.2 on
> 7be4bc52fd9e (v7.3-2415), covering:
> 
> - PCI FLR after changing poll_queues from 0 to 1 or num_request_queues
>   from 0 to 1 on a device with four request queues.
> - Normal and polled I/O across FLR with poll_queues=1 at probe,
>   including a subsequent change of poll_queues to 0.
> - Device suspend/resume with pm_test=devices and state=freeze (not a
>   full s2idle cycle), plus full S3 and S4 suspend/resume cycles.

I noticed that drivers/nvme/host/ allows the queues to change:
1. The hardware queue count can be different and the tagset is updated
   with blk_mq_update_nr_hw_queues().
2. The module parameters are refreshed in nvme_setup_io_queues() and are
   allowed to change across power management operations.

Please consider implementing more flexibile restore behavior in
virtio_blk that can toleration hardware queue count changes and module
parameter changes instead though.

However, this patch looks correct to me and can be merged IMO.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] virtio_blk: freeze the virtqueue layout at probe time
  2026-11-02 13:37 [PATCH] virtio_blk: freeze the virtqueue layout at probe time Yunye Zhao
  2026-11-02 13:41 ` sashiko-bot
  2026-11-02 19:15 ` Stefan Hajnoczi
@ 2026-11-03 14:31 ` Jens Axboe
  2 siblings, 0 replies; 4+ messages in thread
From: Jens Axboe @ 2026-11-03 14:31 UTC (permalink / raw)
  To: Michael S. Tsirkin, Jason Wang, Yunye Zhao
  Cc: Stefan Hajnoczi, Paolo Bonzini, Eugenio Pérez, Xuan Zhuo,
	Max Gurtovoy, virtualization, linux-block, linux-kernel,
	Joseph Qi, Ferry Meng


On Mon, 02 Nov 2026 21:37:40 +0800, Yunye Zhao wrote:
> init_vq() reads the writable num_request_queues and poll_queues module
> parameters both at probe and when recreating virtqueues after suspend
> or reset. However, virtio-blk does not update the blk-mq hardware queue
> count or queue maps on restore.
> 
> Increasing poll_queues can therefore remove the completion callback
> from a virtqueue still used for interrupt-driven I/O, leaving requests
> uncompleted. Reducing the effective request queue count through
> num_request_queues can instead leave existing hardware contexts
> indexing beyond the reallocated vblk->vqs array.
> 
> [...]

Applied, thanks!

[1/1] virtio_blk: freeze the virtqueue layout at probe time
      commit: d53c1a12ff1cfdb12770ddcfdf5603915d0ba0d8

Best regards,
-- 
Jens Axboe




^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-11-03 14:31 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-02 13:37 [PATCH] virtio_blk: freeze the virtqueue layout at probe time Yunye Zhao
2026-11-02 13:41 ` sashiko-bot
2026-11-02 19:15 ` Stefan Hajnoczi
2026-11-03 14:31 ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox