Re: [PATCH] virtiofs: remove max_pages_limit in indirect descriptor mode

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, Oct 14, 2025 at 02:53:56PM -0400, Stefan Hajnoczi wrote:
> On Sat, Oct 11, 2025 at 11:30:18AM +0800, Wei Gong wrote:
> > From: Wei Gong <gongwei09@baidu.com>
> > 
> > Currently, indirect descriptor mode unnecessarily restricts the maximum
> > IO size based on virtqueue vringsize. However, the indirect descriptor
> > mechanism inherently supports larger IO operations by chaining descriptors.
> > 
> > This patch removes the artificial constraint, allowing indirect descriptor
> > mode to utilize its full potential without being limited by vringsize.
> > The maximum supported descriptors per IO is now determined by the indirect
> > descriptor capability rather than the virtqueue size.
> > 
> > Signed-off-by: Wei Gong <gongwei09@baidu.com>
> > ---
> >  fs/fuse/virtio_fs.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
> > index 76c8fd0bfc75..c0d5db7d7504 100644
> > --- a/fs/fuse/virtio_fs.c
> > +++ b/fs/fuse/virtio_fs.c
> > @@ -12,6 +12,7 @@
> >  #include <linux/memremap.h>
> >  #include <linux/module.h>
> >  #include <linux/virtio.h>
> > +#include <linux/virtio_ring.h>
> >  #include <linux/virtio_fs.h>
> >  #include <linux/delay.h>
> >  #include <linux/fs_context.h>
> > @@ -1701,9 +1702,11 @@ static int virtio_fs_get_tree(struct fs_context *fsc)
> >  	fc->sync_fs = true;
> >  	fc->use_pages_for_kvec_io = true;
> >  
> > -	/* Tell FUSE to split requests that exceed the virtqueue's size */
> > -	fc->max_pages_limit = min_t(unsigned int, fc->max_pages_limit,
> > -				    virtqueue_size - FUSE_HEADER_OVERHEAD);
> > +	if (!virtio_has_feature(fs->vqs[VQ_REQUEST].vq->vdev, VIRTIO_RING_F_INDIRECT_DESC)) {
> > +		/* Tell FUSE to split requests that exceed the virtqueue's size */
> > +		fc->max_pages_limit = min_t(unsigned int, fc->max_pages_limit,
> > +						virtqueue_size - FUSE_HEADER_OVERHEAD);
> > +	}
> 
> The VIRTIO 1.3 specification defines the maximum descriptor chain length
> as follows
> (https://docs.oasis-open.org/virtio/virtio/v1.3/csd01/virtio-v1.3-csd01.html#x1-9200019):
> 
>   The number of descriptors in the table is defined by the queue size for this virtqueue: this is the maximum possible descriptor chain length.
> 
> The driver requirements for indirect descriptors say
> (https://docs.oasis-open.org/virtio/virtio/v1.3/csd01/virtio-v1.3-csd01.html#x1-9200019):
> 
>   A driver MUST NOT create a descriptor chain longer than allowed by the device.
> 
> My interpretation is that this patch violates the specification because
> it allows descriptor chains that exceed the maximum possible descriptor
> chain length.
> 
> Device implementations are not required to enforce this limit, so you
> may not see issues when testing. Nevertheless, this patch has the
> potential to break other device implementations though that work fine
> today, so it doesn't seem safe to merge this patch in its current form.
> 
> I have CCed Michael Tsirkin in case he has thoughts. It would be nice to
> boost performance by allowing longer I/O requests, but the driver must
> comply with the VIRTIO specification.
> 
> Thanks,
> Stefan

This request is not uncommon. We wanted a field for max s/g supported as
separate from the VQ depth for a while now, and for various device
types. No one yet bothered implementing this or even adding the
description to the spec yet.


-- 
MST