All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] virtio-blk: clamp max_segments when indirect descriptors are disabled
@ 2026-08-14 10:59 Sergii Ushakov
  2026-08-17  7:47 ` Christoph Hellwig
  2026-08-17 13:42 ` [PATCH v2] " Sergii Ushakov
  0 siblings, 2 replies; 6+ messages in thread
From: Sergii Ushakov @ 2026-08-14 10:59 UTC (permalink / raw)
  To: virtualization, linux-block
  Cc: linux-kernel, Michael S . Tsirkin, Jason Wang, Jens Axboe,
	Xuan Zhuo, Eugenio Pérez, Paolo Bonzini, Stefan Hajnoczi,
	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


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

* Re: [PATCH] virtio-blk: clamp max_segments when indirect descriptors are disabled
  2026-08-14 10:59 [PATCH] virtio-blk: clamp max_segments when indirect descriptors are disabled Sergii Ushakov
@ 2026-08-17  7:47 ` Christoph Hellwig
  2026-08-17  8:08   ` Sergii Ushakov
  2026-08-17 13:42 ` [PATCH v2] " Sergii Ushakov
  1 sibling, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2026-08-17  7:47 UTC (permalink / raw)
  To: Sergii Ushakov
  Cc: virtualization, linux-block, linux-kernel, Michael S . Tsirkin,
	Jason Wang, Jens Axboe, Xuan Zhuo, Eugenio Pérez,
	Paolo Bonzini, Stefan Hajnoczi

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?


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

* Re: [PATCH] virtio-blk: clamp max_segments when indirect descriptors are disabled
  2026-08-17  7:47 ` Christoph Hellwig
@ 2026-08-17  8:08   ` Sergii Ushakov
  2026-08-17 12:41     ` Michael S. Tsirkin
  0 siblings, 1 reply; 6+ messages in thread
From: Sergii Ushakov @ 2026-08-17  8:08 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: virtualization, linux-block, linux-kernel, Michael S . Tsirkin,
	Jason Wang, Jens Axboe, Xuan Zhuo, Eugenio Pérez,
	Paolo Bonzini, Stefan Hajnoczi

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.

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

* Re: [PATCH] virtio-blk: clamp max_segments when indirect descriptors are disabled
  2026-08-17  8:08   ` Sergii Ushakov
@ 2026-08-17 12:41     ` Michael S. Tsirkin
  0 siblings, 0 replies; 6+ messages in thread
From: Michael S. Tsirkin @ 2026-08-17 12:41 UTC (permalink / raw)
  To: Sergii Ushakov
  Cc: Christoph Hellwig, virtualization, linux-block, linux-kernel,
	Jason Wang, Jens Axboe, Xuan Zhuo, Eugenio Pérez,
	Paolo Bonzini, Stefan Hajnoczi

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.


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

* [PATCH v2] virtio-blk: clamp max_segments when indirect descriptors are disabled
  2026-08-14 10:59 [PATCH] virtio-blk: clamp max_segments when indirect descriptors are disabled Sergii Ushakov
  2026-08-17  7:47 ` Christoph Hellwig
@ 2026-08-17 13:42 ` Sergii Ushakov
  2026-08-17 17:28   ` Stefan Hajnoczi
  1 sibling, 1 reply; 6+ messages in thread
From: Sergii Ushakov @ 2026-08-17 13:42 UTC (permalink / raw)
  To: virtualization, linux-block
  Cc: linux-kernel, Christoph Hellwig, Michael S . Tsirkin, Jason Wang,
	Jens Axboe, Xuan Zhuo, Eugenio Pérez, Paolo Bonzini,
	Stefan Hajnoczi, 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


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

* Re: [PATCH v2] virtio-blk: clamp max_segments when indirect descriptors are disabled
  2026-08-17 13:42 ` [PATCH v2] " Sergii Ushakov
@ 2026-08-17 17:28   ` Stefan Hajnoczi
  0 siblings, 0 replies; 6+ messages in thread
From: Stefan Hajnoczi @ 2026-08-17 17:28 UTC (permalink / raw)
  To: Sergii Ushakov
  Cc: virtualization, linux-block, linux-kernel, Christoph Hellwig,
	Michael S . Tsirkin, Jason Wang, Jens Axboe, Xuan Zhuo,
	Eugenio Pérez, Paolo Bonzini

[-- 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 --]

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

end of thread, other threads:[~2026-08-17 17:28 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-14 10:59 [PATCH] virtio-blk: clamp max_segments when indirect descriptors are disabled Sergii Ushakov
2026-08-17  7:47 ` Christoph Hellwig
2026-08-17  8:08   ` Sergii Ushakov
2026-08-17 12:41     ` Michael S. Tsirkin
2026-08-17 13:42 ` [PATCH v2] " Sergii Ushakov
2026-08-17 17:28   ` Stefan Hajnoczi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.