Re: [PATCH] fuse: Enable dynamic configuration of FUSE_MAX_MAX_PAGES

[Sweet Tea Dorminy <sweettea-kernel@dorminy.me>]

On 7/1/24 12:48 PM, Joanne Koong wrote:
> On Fri, Jun 28, 2024 at 11:03 AM Josef Bacik <josef@toxicpanda.com> 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@linux.alibaba.com/T/#u
>>>
>>> Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
>>
>> 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.