[PATCH] virtio-blk: clamp max_segments when indirect descriptors are disabled

[PATCH] virtio-blk: clamp max_segments when indirect descriptors are disabled

[Sergii Ushakov]

When VIRTIO_RING_F_INDIRECT_DESC is not negotiated by the host, every
scatter-gather segment in a request must consume a physical slot in
the virtqueue ring.

If the host does not advertise VIRTIO_BLK_F_SEG_MAX and provides a small
virtqueue (e.g. 128 descriptors on QNX Hypervisor), the block layer
defaults max_segments to BLK_MAX_SEGMENTS (1024). When a multi-page
compound bio arrives from the page cache, virtqueue_add_split() rejects
the request with -ENOSPC and triggers:

  WARNING: at drivers/virtio/virtio_ring.c:1493 virtqueue_add+...
  WARN_ON_ONCE(total_sg > vq->split.vring.num && !vq->indirect);

This permanently wedges the blk-mq queue and blocks all subsequent disk
I/O in uninterruptible sleep (D state).

Add a virtio_blk.max_segments module parameter to allow runtime cmdline
overrides, and automatically clamp sg_elems to
(virtqueue_get_vring_size - 2) when indirect descriptors are disabled.

Signed-off-by: Sergii Ushakov <sergiiushakov@google.com>
---
 drivers/block/virtio_blk.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 32bf3ba07a9d..082acd90a02d 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -41,6 +41,10 @@ static unsigned int poll_queues;
 module_param(poll_queues, uint, 0644);
 MODULE_PARM_DESC(poll_queues, "The number of dedicated virtqueues for polling I/O");
 
+static unsigned int max_segments;
+module_param(max_segments, uint, 0644);
+MODULE_PARM_DESC(max_segments, "Override maximum number of segments per request");
+
 static int major;
 static DEFINE_IDA(vd_index_ida);
 
@@ -1267,6 +1271,12 @@ static int virtblk_read_limits(struct virtio_blk *vblk,
 	/* Prevent integer overflows and honor max vq size */
 	sg_elems = min_t(u32, sg_elems, VIRTIO_BLK_MAX_SG_ELEMS - 2);
 
+	if (max_segments)
+		sg_elems = min_t(u32, sg_elems, max_segments);
+	else if (!virtio_has_feature(vdev, VIRTIO_RING_F_INDIRECT_DESC))
+		sg_elems = min_t(u32, sg_elems,
+				 virtqueue_get_vring_size(vblk->vqs[0].vq) - 2);
+
 	/* We can handle whatever the host told us to handle. */
 	lim->max_segments = sg_elems;
 
-- 
2.55.0.691.gc56d675ccc-goog

Re: [PATCH] virtio-blk: clamp max_segments when indirect descriptors are disabled

[Christoph Hellwig]

On Fri, Aug 14, 2026 at 12:59:54PM +0200, Sergii Ushakov wrote:
> When VIRTIO_RING_F_INDIRECT_DESC is not negotiated by the host, every
> scatter-gather segment in a request must consume a physical slot in
> the virtqueue ring.
> 
> If the host does not advertise VIRTIO_BLK_F_SEG_MAX and provides a small
> virtqueue (e.g. 128 descriptors on QNX Hypervisor), the block layer
> defaults max_segments to BLK_MAX_SEGMENTS (1024). When a multi-page
> compound bio arrives from the page cache, virtqueue_add_split() rejects
> the request with -ENOSPC and triggers:
> 
>   WARNING: at drivers/virtio/virtio_ring.c:1493 virtqueue_add+...
>   WARN_ON_ONCE(total_sg > vq->split.vring.num && !vq->indirect);
> 
> This permanently wedges the blk-mq queue and blocks all subsequent disk
> I/O in uninterruptible sleep (D state).
> 
> Add a virtio_blk.max_segments module parameter to allow runtime cmdline
> overrides, and automatically clamp sg_elems to
> (virtqueue_get_vring_size - 2) when indirect descriptors are disabled.

What is the reason for the override?

Re: [PATCH] virtio-blk: clamp max_segments when indirect descriptors are disabled

[Sergii Ushakov]

On Mon, 17 Aug 2026 at 09:47, Christoph Hellwig <hch@infradead.org> wrote:
>
> On Fri, Aug 14, 2026 at 12:59:54PM +0200, Sergii Ushakov wrote:
> > When VIRTIO_RING_F_INDIRECT_DESC is not negotiated by the host, every
> > scatter-gather segment in a request must consume a physical slot in
> > the virtqueue ring.
> >
> > If the host does not advertise VIRTIO_BLK_F_SEG_MAX and provides a small
> > virtqueue (e.g. 128 descriptors on QNX Hypervisor), the block layer
> > defaults max_segments to BLK_MAX_SEGMENTS (1024). When a multi-page
> > compound bio arrives from the page cache, virtqueue_add_split() rejects
> > the request with -ENOSPC and triggers:
> >
> >   WARNING: at drivers/virtio/virtio_ring.c:1493 virtqueue_add+...
> >   WARN_ON_ONCE(total_sg > vq->split.vring.num && !vq->indirect);
> >
> > This permanently wedges the blk-mq queue and blocks all subsequent disk
> > I/O in uninterruptible sleep (D state).
> >
> > Add a virtio_blk.max_segments module parameter to allow runtime cmdline
> > overrides, and automatically clamp sg_elems to
> > (virtqueue_get_vring_size - 2) when indirect descriptors are disabled.
>
> What is the reason for the override?

The module parameter was intended for two main reasons:
1. A safety fallback for non-compliant/buggy hypervisors that may have
   internal segment limits lower than the advertised ring size without
   advertising VIRTIO_BLK_F_SEG_MAX.
2. Debugging and performance benchmarking of smaller scatter-gather lists
   without needing kernel rebuilds.
That said, the automatic clamping to (vring_size - 2) resolves the
hang and panic out-of-the-box. If the preference is to avoid adding a new
module parameter, we may drop it and keep only the automatic
clamping.

Re: [PATCH] virtio-blk: clamp max_segments when indirect descriptors are disabled

[Michael S. Tsirkin]

On Mon, Aug 17, 2026 at 10:08:21AM +0200, Sergii Ushakov wrote:
> On Mon, 17 Aug 2026 at 09:47, Christoph Hellwig <hch@infradead.org> wrote:
> >
> > On Fri, Aug 14, 2026 at 12:59:54PM +0200, Sergii Ushakov wrote:
> > > When VIRTIO_RING_F_INDIRECT_DESC is not negotiated by the host, every
> > > scatter-gather segment in a request must consume a physical slot in
> > > the virtqueue ring.
> > >
> > > If the host does not advertise VIRTIO_BLK_F_SEG_MAX and provides a small
> > > virtqueue (e.g. 128 descriptors on QNX Hypervisor), the block layer
> > > defaults max_segments to BLK_MAX_SEGMENTS (1024). When a multi-page
> > > compound bio arrives from the page cache, virtqueue_add_split() rejects
> > > the request with -ENOSPC and triggers:
> > >
> > >   WARNING: at drivers/virtio/virtio_ring.c:1493 virtqueue_add+...
> > >   WARN_ON_ONCE(total_sg > vq->split.vring.num && !vq->indirect);
> > >
> > > This permanently wedges the blk-mq queue and blocks all subsequent disk
> > > I/O in uninterruptible sleep (D state).
> > >
> > > Add a virtio_blk.max_segments module parameter to allow runtime cmdline
> > > overrides, and automatically clamp sg_elems to
> > > (virtqueue_get_vring_size - 2) when indirect descriptors are disabled.
> >
> > What is the reason for the override?
> 
> The module parameter was intended for two main reasons:
> 1. A safety fallback for non-compliant/buggy hypervisors that may have
>    internal segment limits lower than the advertised ring size without
>    advertising VIRTIO_BLK_F_SEG_MAX.
> 2. Debugging and performance benchmarking of smaller scatter-gather lists
>    without needing kernel rebuilds.
> That said, the automatic clamping to (vring_size - 2) resolves the
> hang and panic out-of-the-box. If the preference is to avoid adding a new
> module parameter, we may drop it and keep only the automatic
> clamping.

sounds better to me.

[PATCH v2] virtio-blk: clamp max_segments when indirect descriptors are disabled

[Sergii Ushakov]

When VIRTIO_RING_F_INDIRECT_DESC is not negotiated by the host, every
scatter-gather segment in a request must consume a physical slot in
the virtqueue ring.

If the host does not advertise VIRTIO_BLK_F_SEG_MAX and provides a small
virtqueue (e.g. 128 descriptors on QNX Hypervisor), the block layer
defaults max_segments to BLK_MAX_SEGMENTS (1024). When a multi-page
compound bio arrives from the page cache, virtqueue_add_split() rejects
the request with -ENOSPC and triggers:

  WARNING: at drivers/virtio/virtio_ring.c:1493 virtqueue_add+...
  WARN_ON_ONCE(total_sg > vq->split.vring.num && !vq->indirect);

This permanently wedges the blk-mq queue and blocks all subsequent disk
I/O in uninterruptible sleep (D state).

Automatically clamp sg_elems to (ring_size - 2) when indirect
descriptors are disabled.

Signed-off-by: Sergii Ushakov <sergiiushakov@google.com>
---
v1 -> v2:
- Drop max_segments module parameter and rely solely on automatic clamping
  when indirect descriptors are disabled (suggested by Christoph Hellwig).
- Guard (ring_size - 2) calculation with ring_size > 2 to prevent underflow.
- Update commit description accordingly.

 drivers/block/virtio_blk.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 32bf3ba07a9d..8f5a2d5323a6 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -1267,6 +1267,13 @@ static int virtblk_read_limits(struct virtio_blk *vblk,
 	/* Prevent integer overflows and honor max vq size */
 	sg_elems = min_t(u32, sg_elems, VIRTIO_BLK_MAX_SG_ELEMS - 2);
 
+	if (!virtio_has_feature(vdev, VIRTIO_RING_F_INDIRECT_DESC)) {
+		u32 ring_size = virtqueue_get_vring_size(vblk->vqs[0].vq);
+
+		if (ring_size > 2)
+			sg_elems = min(sg_elems, ring_size - 2);
+	}
+
 	/* We can handle whatever the host told us to handle. */
 	lim->max_segments = sg_elems;
 
-- 
2.55.0.691.gc56d675ccc-goog

Re: [PATCH v2] virtio-blk: clamp max_segments when indirect descriptors are disabled

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1433 bytes --]

On Mon, Aug 17, 2026 at 03:42:02PM +0200, Sergii Ushakov wrote:
> When VIRTIO_RING_F_INDIRECT_DESC is not negotiated by the host, every
> scatter-gather segment in a request must consume a physical slot in
> the virtqueue ring.
> 
> If the host does not advertise VIRTIO_BLK_F_SEG_MAX and provides a small
> virtqueue (e.g. 128 descriptors on QNX Hypervisor), the block layer
> defaults max_segments to BLK_MAX_SEGMENTS (1024). When a multi-page
> compound bio arrives from the page cache, virtqueue_add_split() rejects
> the request with -ENOSPC and triggers:
> 
>   WARNING: at drivers/virtio/virtio_ring.c:1493 virtqueue_add+...
>   WARN_ON_ONCE(total_sg > vq->split.vring.num && !vq->indirect);
> 
> This permanently wedges the blk-mq queue and blocks all subsequent disk
> I/O in uninterruptible sleep (D state).
> 
> Automatically clamp sg_elems to (ring_size - 2) when indirect
> descriptors are disabled.
> 
> Signed-off-by: Sergii Ushakov <sergiiushakov@google.com>
> ---
> v1 -> v2:
> - Drop max_segments module parameter and rely solely on automatic clamping
>   when indirect descriptors are disabled (suggested by Christoph Hellwig).
> - Guard (ring_size - 2) calculation with ring_size > 2 to prevent underflow.
> - Update commit description accordingly.
> 
>  drivers/block/virtio_blk.c | 7 +++++++
>  1 file changed, 7 insertions(+)

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2] virtio-blk: clamp max_segments when indirect descriptors are disabled

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH v2] virtio-blk: clamp max_segments when indirect descriptors are disabled

[Sergii Ushakov]

Hi Michael, Jens,

Gentle ping on this v2 patch. It has Reviewed-by tags from Stefan and
Christoph:

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>

Could this be picked up for the next cycle?

Thanks,
Sergii

Re: [PATCH v2] virtio-blk: clamp max_segments when indirect descriptors are disabled

[Michael S. Tsirkin]

On Mon, Aug 17, 2026 at 03:42:02PM +0200, Sergii Ushakov wrote:
> When VIRTIO_RING_F_INDIRECT_DESC is not negotiated by the host, every
> scatter-gather segment in a request must consume a physical slot in
> the virtqueue ring.
> 
> If the host does not advertise VIRTIO_BLK_F_SEG_MAX and provides a small
> virtqueue (e.g. 128 descriptors on QNX Hypervisor), the block layer
> defaults max_segments to BLK_MAX_SEGMENTS (1024). When a multi-page
> compound bio arrives from the page cache, virtqueue_add_split() rejects
> the request with -ENOSPC and triggers:
> 
>   WARNING: at drivers/virtio/virtio_ring.c:1493 virtqueue_add+...
>   WARN_ON_ONCE(total_sg > vq->split.vring.num && !vq->indirect);
> 
> This permanently wedges the blk-mq queue and blocks all subsequent disk
> I/O in uninterruptible sleep (D state).
> 
> Automatically clamp sg_elems to (ring_size - 2) when indirect
> descriptors are disabled.
> 
> Signed-off-by: Sergii Ushakov <sergiiushakov@google.com>
> ---
> v1 -> v2:
> - Drop max_segments module parameter and rely solely on automatic clamping
>   when indirect descriptors are disabled (suggested by Christoph Hellwig).
> - Guard (ring_size - 2) calculation with ring_size > 2 to prevent underflow.
> - Update commit description accordingly.
> 
>  drivers/block/virtio_blk.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index 32bf3ba07a9d..8f5a2d5323a6 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -1267,6 +1267,13 @@ static int virtblk_read_limits(struct virtio_blk *vblk,
>  	/* Prevent integer overflows and honor max vq size */
>  	sg_elems = min_t(u32, sg_elems, VIRTIO_BLK_MAX_SG_ELEMS - 2);
>  
> +	if (!virtio_has_feature(vdev, VIRTIO_RING_F_INDIRECT_DESC)) {
> +		u32 ring_size = virtqueue_get_vring_size(vblk->vqs[0].vq);
> +
> +		if (ring_size > 2)
> +			sg_elems = min(sg_elems, ring_size - 2);
> +	}
> +
>  	/* We can handle whatever the host told us to handle. */
>  	lim->max_segments = sg_elems;


I do not get why it does not clamp with VIRTIO_RING_F_INDIRECT_DESC.
Spec says:

 A driver MUST NOT create a descriptor chain longer than the Queue Size of the device.

Also, pls add a comment explaining where does this 2 come from.

What about ring size 2? I guess



> When VIRTIO_RING_F_INDIRECT_DESC is not negotiated by the host, every
> scatter-gather segment in a request must consume a physical slot in
> the virtqueue ring.
> 
> If the host does not advertise VIRTIO_BLK_F_SEG_MAX and provides a small
> virtqueue (e.g. 128 descriptors on QNX Hypervisor), the block layer
> defaults max_segments to BLK_MAX_SEGMENTS

Does it?

        err = virtio_cread_feature(vdev, VIRTIO_BLK_F_SEG_MAX,
                                   struct virtio_blk_config, seg_max,
                                   &sg_elems);
                 
        /* We need at least one SG element, whatever they say. */
        if (err || !sg_elems)
                sg_elems = 1;

so set to 1 without VIRTIO_BLK_F_SEG_MAX


        /* Prevent integer overflows and honor max vq size */
        sg_elems = min_t(u32, sg_elems, VIRTIO_BLK_MAX_SG_ELEMS - 2);

unchanged here

        /* We can handle whatever the host told us to handle. */
        lim->max_segments = sg_elems;

assigned to max_segments here


> BLK_MAX_SEGMENTS (1024).

In which tree does BLK_MAX_SEGMENTS equal 1024?

git show next-20260915:include/linux/blkdev.h | grep -n 'BLK_MAX_SEGMENTS'
1234:      BLK_MAX_SEGMENTS        = 128,




> When a multi-page
> compound bio arrives from the page cache, virtqueue_add_split() rejects
> the request with -ENOSPC and triggers:
> 
>   WARNING: at drivers/virtio/virtio_ring.c:1493 virtqueue_add+...
>   WARN_ON_ONCE(total_sg > vq->split.vring.num && !vq->indirect);
> 
> This permanently wedges the blk-mq queue and blocks all subsequent disk
> I/O in uninterruptible sleep (D state).

Please clarify the reproducer, including the negotiated features,
max_segments, actual ring sizes, and total_sg at the failure. As
described, it should not trigger and I do not see how
the patch is supposed to change the failing configuration.

> 
> Automatically clamp sg_elems to (ring_size - 2) when indirect
> descriptors are disabled.
> 
> Signed-off-by: Sergii Ushakov <sergiiushakov@google.com>
> ---
> v1 -> v2:
> - Drop max_segments module parameter and rely solely on automatic clamping
>   when indirect descriptors are disabled (suggested by Christoph Hellwig).
> - Guard (ring_size - 2) calculation with ring_size > 2 to prevent underflow.
> - Update commit description accordingly.
> 
>  drivers/block/virtio_blk.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index 32bf3ba07a9d..8f5a2d5323a6 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -1267,6 +1267,13 @@ static int virtblk_read_limits(struct virtio_blk *vblk,

Well we only set the limit at probe. virtblk_restore_priv() recreates
the queues on resume and reset recovery, then resumes dispatch without
checking their sizes against the existing limit. ring allocation
can reduce the size under memory pressure, including during recovery.

Since you are now (correctly) tying request size to vq size,
this needs to be resolved.



>  	/* Prevent integer overflows and honor max vq size */
>  	sg_elems = min_t(u32, sg_elems, VIRTIO_BLK_MAX_SG_ELEMS - 2);
>  
> +	if (!virtio_has_feature(vdev, VIRTIO_RING_F_INDIRECT_DESC)) {

Why exempt indirect descriptors? The virtio spec says:

	A driver MUST NOT create a descriptor chain longer than the Queue Size
	of the device.

Even on devices ignoring that -
there is a failure path even when indirect descriptors are
negotiated: virtqueue_add_split() falls back to direct descriptors if
the indirect-table allocation fails. So if the request exceeds the ring
size, it returns -ENOSPC even on an empty ring. virtio_queue_rq() then
stops the hardware queue, with no outstanding completion to restart it.


> +		u32 ring_size = virtqueue_get_vring_size(vblk->vqs[0].vq);

Why 0? VQ 0 is not necessarily the smallest queue. The transport specifies
sizes per queue, and Linux can reduce individual split-ring sizes
during allocation.

> +
> +		if (ring_size > 2)
> +			sg_elems = min(sg_elems, ring_size - 2);

The subtraction is correct: virtblk_add_req() uses separate outgoing
and incoming header descriptors, including for zone append.
However, we really should have a comment explaining that, here.

And, ring_size <= 2 must be rejected rather than bypassing the clamp:
A two-entry direct ring can pass probe, yet even one data segment
needs three descriptors and hits the same permanent -ENOSPC condition.

> +	}
> +
>  	/* We can handle whatever the host told us to handle. */
>  	lim->max_segments = sg_elems;
>  
> -- 
> 2.55.0.691.gc56d675ccc-goog
> 

>  
> -- 
> 2.55.0.691.gc56d675ccc-goog