On 7/1/24 12:48 PM, Joanne Koong wrote:
On Fri, Jun 28, 2024 at 11:03 AM Josef Bacik <josef@xxxxxxxxxxxxxx> wrote:
On Thu, Jun 27, 2024 at 05:13:55PM -0700, Joanne Koong wrote:
Introduce the capability to dynamically configure the FUSE_MAX_MAX_PAGES
limit through a sysctl. This enhancement allows system administrators to
adjust the value based on system-specific requirements.
This removes the previous static limit of 256 max pages, which limits
the max write size of a request to 1 MiB (on 4096 pagesize systems).
Having the ability to up the max write size beyond 1 MiB allows for the
perf improvements detailed in this thread [1].
$ sysctl -a | grep fuse_max_max_pages
fs.fuse.fuse_max_max_pages = 256
$ sysctl -n fs.fuse.fuse_max_max_pages
256
$ echo 1024 | sudo tee /proc/sys/fs/fuse/fuse_max_max_pages
1024
$ sysctl -n fs.fuse.fuse_max_max_pages
1024
[1] https://lore.kernel.org/linux-fsdevel/20240124070512.52207-1-jefflexu@xxxxxxxxxxxxxxxxx/T/#u
Signed-off-by: Joanne Koong <joannelkoong@xxxxxxxxx>
Overall the change is great, and I see why you named it the way you did, but I
think may be 'fuse_max_pages_limit' may be a better name? The original constant
name wasn't great, but it was fine in its context. I think having it as an
interface we should name it something less silly.
'fuse_max_pages_limit' sounds great to me. I'll submit v2 with this rename.
Whatever-it's-called might make sense to be in bytes, since max_read,
max_write are the values visible to a userspace fuse server and are in
bytes.
Additionally, the sysctl should probably disallow values at or above
1<<16 pages -- fuse_init_out expects that max_pages fits in a u16, and
there are a couple of places where fc->max_pages << PAGE_SHIFT is
expected to fit in a u32. fc->max_pages is constrained by max_max_pages
at present so this is true but would not be true if max_max_pages is
only constrained by being a uint.