| Message ID | 20240730002348.3431931-3-joannelkoong@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | fuse: add timeout option for requests | expand |
Hi Joanne, kernel test robot noticed the following build errors: [auto build test ERROR on mszeredi-fuse/for-next] [also build test ERROR on linus/master v6.11-rc1 next-20240730] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Joanne-Koong/fuse-add-optional-kernel-enforced-timeout-for-requests/20240730-085106 base: https://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git for-next patch link: https://lore.kernel.org/r/20240730002348.3431931-3-joannelkoong%40gmail.com patch subject: [PATCH v2 2/2] fuse: add default_request_timeout and max_request_timeout sysctls config: i386-buildonly-randconfig-001-20240730 (https://download.01.org/0day-ci/archive/20240730/202407301513.fphdIYEE-lkp@intel.com/config) compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240730/202407301513.fphdIYEE-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202407301513.fphdIYEE-lkp@intel.com/ All error/warnings (new ones prefixed by >>): >> fs/fuse/sysctl.c:30:30: error: macro "fuse_sysctl_register" passed 1 arguments, but takes just 0 30 | int fuse_sysctl_register(void) | ^ In file included from fs/fuse/sysctl.c:9: fs/fuse/fuse_i.h:1501: note: macro "fuse_sysctl_register" defined here 1501 | #define fuse_sysctl_register() (0) | >> fs/fuse/sysctl.c:31:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token 31 | { | ^ >> fs/fuse/sysctl.c:38:33: error: macro "fuse_sysctl_unregister" passed 1 arguments, but takes just 0 38 | void fuse_sysctl_unregister(void) | ^ fs/fuse/fuse_i.h:1502: note: macro "fuse_sysctl_unregister" defined here 1502 | #define fuse_sysctl_unregister() do { } while (0) | fs/fuse/sysctl.c:39:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token 39 | { | ^ >> fs/fuse/sysctl.c:13:25: warning: 'fuse_sysctl_table' defined but not used [-Wunused-variable] 13 | static struct ctl_table fuse_sysctl_table[] = { | ^~~~~~~~~~~~~~~~~ >> fs/fuse/sysctl.c:11:33: warning: 'fuse_table_header' defined but not used [-Wunused-variable] 11 | static struct ctl_table_header *fuse_table_header; | ^~~~~~~~~~~~~~~~~ vim +/fuse_sysctl_register +30 fs/fuse/sysctl.c 10 > 11 static struct ctl_table_header *fuse_table_header; 12 > 13 static struct ctl_table fuse_sysctl_table[] = { 14 { 15 .procname = "default_request_timeout", 16 .data = &fuse_default_req_timeout, 17 .maxlen = sizeof(fuse_default_req_timeout), 18 .mode = 0644, 19 .proc_handler = proc_douintvec, 20 }, 21 { 22 .procname = "max_request_timeout", 23 .data = &fuse_max_req_timeout, 24 .maxlen = sizeof(fuse_max_req_timeout), 25 .mode = 0644, 26 .proc_handler = proc_douintvec, 27 }, 28 }; 29 > 30 int fuse_sysctl_register(void) > 31 { 32 fuse_table_header = register_sysctl("fs/fuse", fuse_sysctl_table); 33 if (!fuse_table_header) 34 return -ENOMEM; 35 return 0; 36 } 37 > 38 void fuse_sysctl_unregister(void)
Hi Joanne, kernel test robot noticed the following build errors: [auto build test ERROR on mszeredi-fuse/for-next] [also build test ERROR on linus/master v6.11-rc1 next-20240730] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Joanne-Koong/fuse-add-optional-kernel-enforced-timeout-for-requests/20240730-085106 base: https://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git for-next patch link: https://lore.kernel.org/r/20240730002348.3431931-3-joannelkoong%40gmail.com patch subject: [PATCH v2 2/2] fuse: add default_request_timeout and max_request_timeout sysctls config: i386-buildonly-randconfig-006-20240730 (https://download.01.org/0day-ci/archive/20240730/202407301619.Mja5LwBe-lkp@intel.com/config) compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240730/202407301619.Mja5LwBe-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202407301619.Mja5LwBe-lkp@intel.com/ All errors (new ones prefixed by >>): >> fs/fuse/sysctl.c:30:26: error: too many arguments provided to function-like macro invocation 30 | int fuse_sysctl_register(void) | ^ fs/fuse/fuse_i.h:1501:9: note: macro 'fuse_sysctl_register' defined here 1501 | #define fuse_sysctl_register() (0) | ^ >> fs/fuse/sysctl.c:30:25: error: expected ';' after top level declarator 30 | int fuse_sysctl_register(void) | ^ | ; fs/fuse/sysctl.c:38:29: error: too many arguments provided to function-like macro invocation 38 | void fuse_sysctl_unregister(void) | ^ fs/fuse/fuse_i.h:1502:9: note: macro 'fuse_sysctl_unregister' defined here 1502 | #define fuse_sysctl_unregister() do { } while (0) | ^ >> fs/fuse/sysctl.c:38:6: error: variable has incomplete type 'void' 38 | void fuse_sysctl_unregister(void) | ^ fs/fuse/sysctl.c:38:28: error: expected ';' after top level declarator 38 | void fuse_sysctl_unregister(void) | ^ | ; 5 errors generated. vim +30 fs/fuse/sysctl.c 29 > 30 int fuse_sysctl_register(void) 31 { 32 fuse_table_header = register_sysctl("fs/fuse", fuse_sysctl_table); 33 if (!fuse_table_header) 34 return -ENOMEM; 35 return 0; 36 } 37 > 38 void fuse_sysctl_unregister(void)
On 7/30/24 8:23 AM, Joanne Koong wrote: > Introduce two new sysctls, "default_request_timeout" and > "max_request_timeout". These control timeouts on replies by the > server to kernel-issued fuse requests. > > "default_request_timeout" sets a timeout if no timeout is specified by > the fuse server on mount. 0 (default) indicates no timeout should be enforced. > > "max_request_timeout" sets a maximum timeout for fuse requests. If the > fuse server attempts to set a timeout greater than max_request_timeout, > the system will default to max_request_timeout. Similarly, if the max > default timeout is greater than the max request timeout, the system will > default to the max request timeout. 0 (default) indicates no timeout should > be enforced. > > $ sysctl -a | grep fuse > fs.fuse.default_request_timeout = 0 > fs.fuse.max_request_timeout = 0 > > $ echo 0x100000000 | sudo tee /proc/sys/fs/fuse/default_request_timeout > tee: /proc/sys/fs/fuse/default_request_timeout: Invalid argument > > $ echo 0xFFFFFFFF | sudo tee /proc/sys/fs/fuse/default_request_timeout > 0xFFFFFFFF > > $ sysctl -a | grep fuse > fs.fuse.default_request_timeout = 4294967295 > fs.fuse.max_request_timeout = 0 > > Signed-off-by: Joanne Koong <joannelkoong@gmail.com> Why do we introduce a new sysfs knob for fuse instead of putting it under fusectl? Though we also encounter some issues with fusectl since fusectl doesn't work well in container scenarios as it doesn't support FS_USERNS_MOUNT.
On Mon, Aug 5, 2024 at 12:38 AM Jingbo Xu <jefflexu@linux.alibaba.com> wrote: > > > > On 7/30/24 8:23 AM, Joanne Koong wrote: > > Introduce two new sysctls, "default_request_timeout" and > > "max_request_timeout". These control timeouts on replies by the > > server to kernel-issued fuse requests. > > > > "default_request_timeout" sets a timeout if no timeout is specified by > > the fuse server on mount. 0 (default) indicates no timeout should be enforced. > > > > "max_request_timeout" sets a maximum timeout for fuse requests. If the > > fuse server attempts to set a timeout greater than max_request_timeout, > > the system will default to max_request_timeout. Similarly, if the max > > default timeout is greater than the max request timeout, the system will > > default to the max request timeout. 0 (default) indicates no timeout should > > be enforced. > > > > $ sysctl -a | grep fuse > > fs.fuse.default_request_timeout = 0 > > fs.fuse.max_request_timeout = 0 > > > > $ echo 0x100000000 | sudo tee /proc/sys/fs/fuse/default_request_timeout > > tee: /proc/sys/fs/fuse/default_request_timeout: Invalid argument > > > > $ echo 0xFFFFFFFF | sudo tee /proc/sys/fs/fuse/default_request_timeout > > 0xFFFFFFFF > > > > $ sysctl -a | grep fuse > > fs.fuse.default_request_timeout = 4294967295 > > fs.fuse.max_request_timeout = 0 > > > > Signed-off-by: Joanne Koong <joannelkoong@gmail.com> > > Why do we introduce a new sysfs knob for fuse instead of putting it > under fusectl? Though we also encounter some issues with fusectl since > fusectl doesn't work well in container scenarios as it doesn't support > FS_USERNS_MOUNT. > I think having these constraints configured via sysctl offers more flexibility (eg if we want to enforce the max or default request timeout across the board for all incoming connections on the system), whereas I believe fusectl is on a per-connection basis. > > -- > Thanks, > Jingbo
diff --git a/Documentation/admin-guide/sysctl/fs.rst b/Documentation/admin-guide/sysctl/fs.rst index 47499a1742bd..44fd495f69b4 100644 --- a/Documentation/admin-guide/sysctl/fs.rst +++ b/Documentation/admin-guide/sysctl/fs.rst @@ -332,3 +332,20 @@ 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/default_request_timeout`` is a read/write file for +setting/getting the default timeout (in seconds) for a fuse server to +reply to a kernel-issued request in the event where the server did not +specify a timeout at mount. 0 indicates no timeout. + +``/proc/sys/fs/fuse/max_request_timeout`` is a read/write file for +setting/getting the maximum timeout (in seconds) for a fuse server to +reply to a kernel-issued request. If the server attempts to set a +timeout greater than max_request_timeout, the system will use +max_request_timeout as the timeout. 0 indicates no timeout. 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 2b616c5977b4..1a3a3b8a83ae 100644 --- a/fs/fuse/fuse_i.h +++ b/fs/fuse/fuse_i.h @@ -47,6 +47,14 @@ /** Number of dentries for each connection in the control filesystem */ #define FUSE_CTL_NUM_DENTRIES 5 +/* + * Default timeout (in seconds) for the server to reply to a request + * if no timeout was specified on mount + */ +extern u32 fuse_default_req_timeout; +/** Max timeout (in seconds) for the server to reply to a request */ +extern u32 fuse_max_req_timeout; + /** List of active connections */ extern struct list_head fuse_conn_list; @@ -1486,4 +1494,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 9e69006fc026..cf333448f2d3 100644 --- a/fs/fuse/inode.c +++ b/fs/fuse/inode.c @@ -35,6 +35,10 @@ DEFINE_MUTEX(fuse_mutex); static int set_global_limit(const char *val, const struct kernel_param *kp); +/* default is no timeout */ +u32 fuse_default_req_timeout = 0; +u32 fuse_max_req_timeout = 0; + unsigned max_user_bgreq; module_param_call(max_user_bgreq, set_global_limit, param_get_uint, &max_user_bgreq, 0644); @@ -1678,6 +1682,7 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx) struct fuse_conn *fc = fm->fc; struct inode *root; struct dentry *root_dentry; + u32 req_timeout; int err; err = -EINVAL; @@ -1730,10 +1735,16 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx) fc->group_id = ctx->group_id; fc->legacy_opts_show = ctx->legacy_opts_show; fc->max_read = max_t(unsigned int, 4096, ctx->max_read); - fc->req_timeout = ctx->req_timeout * HZ; fc->destroy = ctx->destroy; fc->no_control = ctx->no_control; fc->no_force_umount = ctx->no_force_umount; + req_timeout = ctx->req_timeout ?: fuse_default_req_timeout; + if (!fuse_max_req_timeout) + fc->req_timeout = req_timeout * HZ; + else if (!req_timeout) + fc->req_timeout = fuse_max_req_timeout * HZ; + else + fc->req_timeout = min(req_timeout, fuse_max_req_timeout) * HZ; err = -ENOMEM; root = fuse_get_root_inode(sb, ctx->rootmode); @@ -2046,8 +2057,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: diff --git a/fs/fuse/sysctl.c b/fs/fuse/sysctl.c new file mode 100644 index 000000000000..c87bb0ecbfa9 --- /dev/null +++ b/fs/fuse/sysctl.c @@ -0,0 +1,42 @@ +// 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; + +static struct ctl_table fuse_sysctl_table[] = { + { + .procname = "default_request_timeout", + .data = &fuse_default_req_timeout, + .maxlen = sizeof(fuse_default_req_timeout), + .mode = 0644, + .proc_handler = proc_douintvec, + }, + { + .procname = "max_request_timeout", + .data = &fuse_max_req_timeout, + .maxlen = sizeof(fuse_max_req_timeout), + .mode = 0644, + .proc_handler = proc_douintvec, + }, +}; + +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; +}
Introduce two new sysctls, "default_request_timeout" and "max_request_timeout". These control timeouts on replies by the server to kernel-issued fuse requests. "default_request_timeout" sets a timeout if no timeout is specified by the fuse server on mount. 0 (default) indicates no timeout should be enforced. "max_request_timeout" sets a maximum timeout for fuse requests. If the fuse server attempts to set a timeout greater than max_request_timeout, the system will default to max_request_timeout. Similarly, if the max default timeout is greater than the max request timeout, the system will default to the max request timeout. 0 (default) indicates no timeout should be enforced. $ sysctl -a | grep fuse fs.fuse.default_request_timeout = 0 fs.fuse.max_request_timeout = 0 $ echo 0x100000000 | sudo tee /proc/sys/fs/fuse/default_request_timeout tee: /proc/sys/fs/fuse/default_request_timeout: Invalid argument $ echo 0xFFFFFFFF | sudo tee /proc/sys/fs/fuse/default_request_timeout 0xFFFFFFFF $ sysctl -a | grep fuse fs.fuse.default_request_timeout = 4294967295 fs.fuse.max_request_timeout = 0 Signed-off-by: Joanne Koong <joannelkoong@gmail.com> --- Documentation/admin-guide/sysctl/fs.rst | 17 ++++++++++ fs/fuse/Makefile | 2 +- fs/fuse/fuse_i.h | 16 ++++++++++ fs/fuse/inode.c | 19 ++++++++++- fs/fuse/sysctl.c | 42 +++++++++++++++++++++++++ 5 files changed, 94 insertions(+), 2 deletions(-) create mode 100644 fs/fuse/sysctl.c