Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230726105953.843-1-jaco@uls.co.za> <b5255112-922f-b965-398e-38b9f5fb4892@fastmail.fm>
 <7d762c95-e4ca-d612-f70f-64789d4624cf@uls.co.za> <0731f4b9-cd4e-2cb3-43ba-c74d238b824f@fastmail.fm>
 <831e5a03-7126-3d45-2137-49c1a25769df@spawn.link> <27875beb-bd1c-0087-ac4c-420a9d92a5a9@uls.co.za>
 <CAJfpegtaxHu2RCqStSFyGzEUrQx-cpuQaCCxiB-F6YmBEvNiJw@mail.gmail.com> <21fff874-d4ed-1781-32a6-06f154a4bc99@fastmail.fm>
In-Reply-To: <21fff874-d4ed-1781-32a6-06f154a4bc99@fastmail.fm>
From:   Miklos Szeredi <miklos@szeredi.hu>
Date:   Fri, 28 Jul 2023 10:42:59 +0200
Message-ID: <CAJfpegvAZ-A09VKMcKJF1NMcgMf7Jq6yoCQR8ixh2eme6LkOjw@mail.gmail.com>
Subject: Re: [PATCH] fuse: enable larger read buffers for readdir.
To:     Bernd Schubert <bernd.schubert@fastmail.fm>
Cc:     Jaco Kroon <jaco@uls.co.za>,
        Antonio SJ Musumeci <trapexit@spawn.link>,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, 27 Jul 2023 at 21:43, Bernd Schubert <bernd.schubert@fastmail.fm> wrote:
>
>
>
> On 7/27/23 21:21, Miklos Szeredi wrote:
> > On Wed, 26 Jul 2023 at 20:40, Jaco Kroon <jaco@uls.co.za> wrote:
> >
> >> Will look into FUSE_INIT.  The FUSE_INIT as I understand from what I've
> >> read has some expansion constraints or the structure is somehow
> >> negotiated.  Older clients in other words that's not aware of the option
> >> will follow some default.  For backwards compatibility that default
> >> should probably be 1 page.  For performance reasons it makes sense that
> >> this limit be larger.
> >
> > Yes, might need this for backward compatibility.  But perhaps a
> > feature flag is enough and the readdir buf can be limited to
> > fc->max_read.
>
> fc->max_read is set by default to ~0 and only set to something else when
> the max_read mount option is given? So typically that is a large value
> (UINT_MAX)?

That's fine.  It probably still makes sense to limit it to 128k, but
with the ctx->count patch it would be naturally limited by the size of
the userspace buffer.  There's really no downside to enabling a large
buffer, other than an unlikely regression in userspace.    If server
wants to return less entries, it still can.  Unlike plain reads,
filling the buffer to the fullest extent isn't required for readdir.

So the buffer size calculation can be somthing like:

init:
#define FUSE_READDIR_MAX (128*1024)
fc->max_readdir = PAGE_SIZE;
if (flags & FUSE_LARGE_READDIR)
   fc->max_readdir = min(fc->max_read, FUSE_READDIR_MAX);

[...]
readdir:
   bufsize = min(fc->max_readdir, max(ctx->count, PAGE_SIZE));

Thanks,
Miklos