Hi Joanne, On 9/5/24 19:45, Joanne Koong wrote: > Introduce the capability to dynamically configure the fuse max pages > limit (formerly #defined as FUSE_MAX_MAX_PAGES) 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]. the change itself looks good to me, but have you seen this discussion here? https://lore.kernel.org/lkml/CAJfpegs10SdtzNXJfj3=vxoAZMhksT5A1u5W5L6nKL-P2UOuLQ@xxxxxxxxxxxxxx/T/ Miklos is basically worried about page pinning and accounting for that for unprivileged user processes. Thanks, Bernd > > $ sysctl -a | grep max_pages_limit > fs.fuse.max_pages_limit = 256 > > $ sysctl -n fs.fuse.max_pages_limit > 256 > > $ echo 1024 | sudo tee /proc/sys/fs/fuse/max_pages_limit > 1024 > > $ sysctl -n fs.fuse.max_pages_limit > 1024 > > $ echo 65536 | sudo tee /proc/sys/fs/fuse/max_pages_limit > tee: /proc/sys/fs/fuse/max_pages_limit: Invalid argument > > $ echo 0 | sudo tee /proc/sys/fs/fuse/max_pages_limit > tee: /proc/sys/fs/fuse/max_pages_limit: Invalid argument > > $ echo 65535 | sudo tee /proc/sys/fs/fuse/max_pages_limit > 65535 > > $ sysctl -n fs.fuse.max_pages_limit > 65535 > > v2 (original): > https://lore.kernel.org/linux-fsdevel/20240702014627.4068146-1-joannelkoong@xxxxxxxxx/ > > v1: > https://lore.kernel.org/linux-fsdevel/20240628001355.243805-1-joannelkoong@xxxxxxxxx/ > > Changes from v1: > - Rename fuse_max_max_pages to fuse_max_pages_limit internally > - Rename /proc/sys/fs/fuse/fuse_max_max_pages to > /proc/sys/fs/fuse/max_pages_limit > - Restrict fuse max_pages_limit sysctl values to between 1 and 65535 > (inclusive) > > [1] https://lore.kernel.org/linux-fsdevel/20240124070512.52207-1-jefflexu@xxxxxxxxxxxxxxxxx/T/#u > > Signed-off-by: Joanne Koong <joannelkoong@xxxxxxxxx> > Reviewed-by: Josef Bacik <josef@xxxxxxxxxxxxxx> > --- > Documentation/admin-guide/sysctl/fs.rst | 10 +++++++ > fs/fuse/Makefile | 2 +- > fs/fuse/fuse_i.h | 14 +++++++-- > fs/fuse/inode.c | 11 ++++++- > fs/fuse/ioctl.c | 4 ++- > fs/fuse/sysctl.c | 40 +++++++++++++++++++++++++ > 6 files changed, 75 insertions(+), 6 deletions(-) > create mode 100644 fs/fuse/sysctl.c > > diff --git a/Documentation/admin-guide/sysctl/fs.rst b/Documentation/admin-guide/sysctl/fs.rst > index 47499a1742bd..fa25d7e718b3 100644 > --- a/Documentation/admin-guide/sysctl/fs.rst > +++ b/Documentation/admin-guide/sysctl/fs.rst > @@ -332,3 +332,13 @@ Each "watch" costs roughly 90 bytes on a 32-bit kernel, and roughly 160 bytes > on a 64-bit one. > The current default value for ``max_user_watches`` is 4% of the > available low memory, divided by the "watch" cost in bytes. > + > +5. /proc/sys/fs/fuse - Configuration options for FUSE filesystems > +===================================================================== > + > +This directory contains the following configuration options for FUSE > +filesystems: > + > +``/proc/sys/fs/fuse/max_pages_limit`` is a read/write file for > +setting/getting the maximum number of pages that can be used for servicing > +requests in FUSE. > diff --git a/fs/fuse/Makefile b/fs/fuse/Makefile > index 6e0228c6d0cb..cd4ef3e08ebf 100644 > --- a/fs/fuse/Makefile > +++ b/fs/fuse/Makefile > @@ -7,7 +7,7 @@ obj-$(CONFIG_FUSE_FS) += fuse.o > obj-$(CONFIG_CUSE) += cuse.o > obj-$(CONFIG_VIRTIO_FS) += virtiofs.o > > -fuse-y := dev.o dir.o file.o inode.o control.o xattr.o acl.o readdir.o ioctl.o > +fuse-y := dev.o dir.o file.o inode.o control.o xattr.o acl.o readdir.o ioctl.o sysctl.o > fuse-y += iomode.o > fuse-$(CONFIG_FUSE_DAX) += dax.o > fuse-$(CONFIG_FUSE_PASSTHROUGH) += passthrough.o > diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h > index f23919610313..bb252a3ea37b 100644 > --- a/fs/fuse/fuse_i.h > +++ b/fs/fuse/fuse_i.h > @@ -35,9 +35,6 @@ > /** Default max number of pages that can be used in a single read request */ > #define FUSE_DEFAULT_MAX_PAGES_PER_REQ 32 > > -/** Maximum of max_pages received in init_out */ > -#define FUSE_MAX_MAX_PAGES 256 > - > /** Bias for fi->writectr, meaning new writepages must not be sent */ > #define FUSE_NOWRITE INT_MIN > > @@ -47,6 +44,9 @@ > /** Number of dentries for each connection in the control filesystem */ > #define FUSE_CTL_NUM_DENTRIES 5 > > +/** Maximum of max_pages received in init_out */ > +extern unsigned int fuse_max_pages_limit; > + > /** List of active connections */ > extern struct list_head fuse_conn_list; > > @@ -1472,4 +1472,12 @@ ssize_t fuse_passthrough_splice_write(struct pipe_inode_info *pipe, > size_t len, unsigned int flags); > ssize_t fuse_passthrough_mmap(struct file *file, struct vm_area_struct *vma); > > +#ifdef CONFIG_SYSCTL > +extern int fuse_sysctl_register(void); > +extern void fuse_sysctl_unregister(void); > +#else > +#define fuse_sysctl_register() (0) > +#define fuse_sysctl_unregister() do { } while (0) > +#endif /* CONFIG_SYSCTL */ > + > #endif /* _FS_FUSE_I_H */ > diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c > index 99e44ea7d875..973e58df816a 100644 > --- a/fs/fuse/inode.c > +++ b/fs/fuse/inode.c > @@ -35,6 +35,8 @@ DEFINE_MUTEX(fuse_mutex); > > static int set_global_limit(const char *val, const struct kernel_param *kp); > > +unsigned int fuse_max_pages_limit = 256; > + > unsigned max_user_bgreq; > module_param_call(max_user_bgreq, set_global_limit, param_get_uint, > &max_user_bgreq, 0644); > @@ -932,7 +934,7 @@ void fuse_conn_init(struct fuse_conn *fc, struct fuse_mount *fm, > fc->pid_ns = get_pid_ns(task_active_pid_ns(current)); > fc->user_ns = get_user_ns(user_ns); > fc->max_pages = FUSE_DEFAULT_MAX_PAGES_PER_REQ; > - fc->max_pages_limit = FUSE_MAX_MAX_PAGES; > + fc->max_pages_limit = fuse_max_pages_limit; > > if (IS_ENABLED(CONFIG_FUSE_PASSTHROUGH)) > fuse_backing_files_init(fc); > @@ -2039,8 +2041,14 @@ static int __init fuse_fs_init(void) > if (err) > goto out3; > > + err = fuse_sysctl_register(); > + if (err) > + goto out4; > + > return 0; > > + out4: > + unregister_filesystem(&fuse_fs_type); > out3: > unregister_fuseblk(); > out2: > @@ -2053,6 +2061,7 @@ static void fuse_fs_cleanup(void) > { > unregister_filesystem(&fuse_fs_type); > unregister_fuseblk(); > + fuse_sysctl_unregister(); > > /* > * Make sure all delayed rcu free inodes are flushed before we > diff --git a/fs/fuse/ioctl.c b/fs/fuse/ioctl.c > index 572ce8a82ceb..a6c8ee551635 100644 > --- a/fs/fuse/ioctl.c > +++ b/fs/fuse/ioctl.c > @@ -10,6 +10,8 @@ > #include <linux/fileattr.h> > #include <linux/fsverity.h> > > +#define FUSE_VERITY_ENABLE_ARG_MAX_PAGES 256 > + > static ssize_t fuse_send_ioctl(struct fuse_mount *fm, struct fuse_args *args, > struct fuse_ioctl_out *outarg) > { > @@ -140,7 +142,7 @@ static int fuse_setup_enable_verity(unsigned long arg, struct iovec *iov, > { > struct fsverity_enable_arg enable; > struct fsverity_enable_arg __user *uarg = (void __user *)arg; > - const __u32 max_buffer_len = FUSE_MAX_MAX_PAGES * PAGE_SIZE; > + const __u32 max_buffer_len = FUSE_VERITY_ENABLE_ARG_MAX_PAGES * PAGE_SIZE; > > if (copy_from_user(&enable, uarg, sizeof(enable))) > return -EFAULT; > diff --git a/fs/fuse/sysctl.c b/fs/fuse/sysctl.c > new file mode 100644 > index 000000000000..b272bb333005 > --- /dev/null > +++ b/fs/fuse/sysctl.c > @@ -0,0 +1,40 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * linux/fs/fuse/fuse_sysctl.c > + * > + * Sysctl interface to fuse parameters > + */ > +#include <linux/sysctl.h> > + > +#include "fuse_i.h" > + > +static struct ctl_table_header *fuse_table_header; > + > +/* Bound by fuse_init_out max_pages, which is a u16 */ > +static unsigned int sysctl_fuse_max_pages_limit = 65535; > + > +static struct ctl_table fuse_sysctl_table[] = { > + { > + .procname = "max_pages_limit", > + .data = &fuse_max_pages_limit, > + .maxlen = sizeof(fuse_max_pages_limit), > + .mode = 0644, > + .proc_handler = proc_douintvec_minmax, > + .extra1 = SYSCTL_ONE, > + .extra2 = &sysctl_fuse_max_pages_limit, > + }, > +}; > + > +int fuse_sysctl_register(void) > +{ > + fuse_table_header = register_sysctl("fs/fuse", fuse_sysctl_table); > + if (!fuse_table_header) > + return -ENOMEM; > + return 0; > +} > + > +void fuse_sysctl_unregister(void) > +{ > + unregister_sysctl_table(fuse_table_header); > + fuse_table_header = NULL; > +}
