* Re: [PATCH v2 6/8] inotify: simplify subdirectory registration with register_sysctl() [not found] ` <20211123202422.819032-7-mcgrof@kernel.org> @ 2021-11-24 9:44 ` Jan Kara 2021-11-24 13:40 ` Luis Chamberlain 0 siblings, 1 reply; 5+ messages in thread From: Jan Kara @ 2021-11-24 9:44 UTC (permalink / raw) To: Luis Chamberlain Cc: akpm, keescook, yzaikin, nixiaoming, ebiederm, clemens, arnd, gregkh, jani.nikula, joonas.lahtinen, rodrigo.vivi, tvrtko.ursulin, airlied, benh, mark, jlbec, joseph.qi, jack, amir73il, phil, viro, julia.lawall, ocfs2-devel, linuxppc-dev, intel-gfx, dri-devel, linux-fsdevel, linux-kernel On Tue 23-11-21 12:24:20, Luis Chamberlain wrote: > From: Xiaoming Ni <nixiaoming@huawei.com> > > There is no need to user boiler plate code to specify a set of base > directories we're going to stuff sysctls under. Simplify this by using > register_sysctl() and specifying the directory path directly. > > Move inotify_user sysctl to inotify_user.c while at it to remove clutter > from kernel/sysctl.c. > > Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com> > [mcgrof: update commit log to reflect new path we decided to take] > Signed-off-by: Luis Chamberlain <mcgrof@kernel.org> This looks fishy. You register inotify_table but not fanotify_table and remove both... Honza > --- > fs/notify/inotify/inotify_user.c | 11 ++++++++++- > include/linux/inotify.h | 3 --- > kernel/sysctl.c | 21 --------------------- > 3 files changed, 10 insertions(+), 25 deletions(-) > > diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c > index 29fca3284bb5..54583f62dc44 100644 > --- a/fs/notify/inotify/inotify_user.c > +++ b/fs/notify/inotify/inotify_user.c > @@ -58,7 +58,7 @@ struct kmem_cache *inotify_inode_mark_cachep __read_mostly; > static long it_zero = 0; > static long it_int_max = INT_MAX; > > -struct ctl_table inotify_table[] = { > +static struct ctl_table inotify_table[] = { > { > .procname = "max_user_instances", > .data = &init_user_ns.ucount_max[UCOUNT_INOTIFY_INSTANCES], > @@ -87,6 +87,14 @@ struct ctl_table inotify_table[] = { > }, > { } > }; > + > +static void __init inotify_sysctls_init(void) > +{ > + register_sysctl("fs/inotify", inotify_table); > +} > + > +#else > +#define inotify_sysctls_init() do { } while (0) > #endif /* CONFIG_SYSCTL */ > > static inline __u32 inotify_arg_to_mask(struct inode *inode, u32 arg) > @@ -849,6 +857,7 @@ static int __init inotify_user_setup(void) > inotify_max_queued_events = 16384; > init_user_ns.ucount_max[UCOUNT_INOTIFY_INSTANCES] = 128; > init_user_ns.ucount_max[UCOUNT_INOTIFY_WATCHES] = watches_max; > + inotify_sysctls_init(); > > return 0; > } > diff --git a/include/linux/inotify.h b/include/linux/inotify.h > index 6a24905f6e1e..8d20caa1b268 100644 > --- a/include/linux/inotify.h > +++ b/include/linux/inotify.h > @@ -7,11 +7,8 @@ > #ifndef _LINUX_INOTIFY_H > #define _LINUX_INOTIFY_H > > -#include <linux/sysctl.h> > #include <uapi/linux/inotify.h> > > -extern struct ctl_table inotify_table[]; /* for sysctl */ > - > #define ALL_INOTIFY_BITS (IN_ACCESS | IN_MODIFY | IN_ATTRIB | IN_CLOSE_WRITE | \ > IN_CLOSE_NOWRITE | IN_OPEN | IN_MOVED_FROM | \ > IN_MOVED_TO | IN_CREATE | IN_DELETE | \ > diff --git a/kernel/sysctl.c b/kernel/sysctl.c > index 7a90a12b9ea4..6aa67c737e4e 100644 > --- a/kernel/sysctl.c > +++ b/kernel/sysctl.c > @@ -125,13 +125,6 @@ static const int maxolduid = 65535; > static const int ngroups_max = NGROUPS_MAX; > static const int cap_last_cap = CAP_LAST_CAP; > > -#ifdef CONFIG_INOTIFY_USER > -#include <linux/inotify.h> > -#endif > -#ifdef CONFIG_FANOTIFY > -#include <linux/fanotify.h> > -#endif > - > #ifdef CONFIG_PROC_SYSCTL > > /** > @@ -3099,20 +3092,6 @@ static struct ctl_table fs_table[] = { > .proc_handler = proc_dointvec, > }, > #endif > -#ifdef CONFIG_INOTIFY_USER > - { > - .procname = "inotify", > - .mode = 0555, > - .child = inotify_table, > - }, > -#endif > -#ifdef CONFIG_FANOTIFY > - { > - .procname = "fanotify", > - .mode = 0555, > - .child = fanotify_table, > - }, > -#endif > #ifdef CONFIG_EPOLL > { > .procname = "epoll", > -- > 2.33.0 > -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 6/8] inotify: simplify subdirectory registration with register_sysctl() 2021-11-24 9:44 ` [PATCH v2 6/8] inotify: simplify subdirectory registration with register_sysctl() Jan Kara @ 2021-11-24 13:40 ` Luis Chamberlain 0 siblings, 0 replies; 5+ messages in thread From: Luis Chamberlain @ 2021-11-24 13:40 UTC (permalink / raw) To: Jan Kara Cc: akpm, keescook, yzaikin, nixiaoming, ebiederm, clemens, arnd, gregkh, jani.nikula, joonas.lahtinen, rodrigo.vivi, tvrtko.ursulin, airlied, benh, mark, jlbec, joseph.qi, amir73il, phil, viro, julia.lawall, ocfs2-devel, linuxppc-dev, intel-gfx, dri-devel, linux-fsdevel, linux-kernel On Wed, Nov 24, 2021 at 10:44:09AM +0100, Jan Kara wrote: > On Tue 23-11-21 12:24:20, Luis Chamberlain wrote: > > From: Xiaoming Ni <nixiaoming@huawei.com> > > > > There is no need to user boiler plate code to specify a set of base > > directories we're going to stuff sysctls under. Simplify this by using > > register_sysctl() and specifying the directory path directly. > > > > Move inotify_user sysctl to inotify_user.c while at it to remove clutter > > from kernel/sysctl.c. > > > > Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com> > > [mcgrof: update commit log to reflect new path we decided to take] > > Signed-off-by: Luis Chamberlain <mcgrof@kernel.org> > > This looks fishy. You register inotify_table but not fanotify_table and > remove both... Indeed, the following was missing, I'll roll it in: diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c index 559bc1e9926d..a35693eb1f36 100644 --- a/fs/notify/fanotify/fanotify_user.c +++ b/fs/notify/fanotify/fanotify_user.c @@ -59,7 +59,7 @@ static int fanotify_max_queued_events __read_mostly; static long ft_zero = 0; static long ft_int_max = INT_MAX; -struct ctl_table fanotify_table[] = { +static struct ctl_table fanotify_table[] = { { .procname = "max_user_groups", .data = &init_user_ns.ucount_max[UCOUNT_FANOTIFY_GROUPS], @@ -88,6 +88,13 @@ struct ctl_table fanotify_table[] = { }, { } }; + +static void __init fanotify_sysctls_init(void) +{ + register_sysctl("fs/fanotify", fanotify_table); +} +#else +#define fanotify_sysctls_init() do { } while (0) #endif /* CONFIG_SYSCTL */ /* @@ -1685,6 +1692,7 @@ static int __init fanotify_user_setup(void) init_user_ns.ucount_max[UCOUNT_FANOTIFY_GROUPS] = FANOTIFY_DEFAULT_MAX_GROUPS; init_user_ns.ucount_max[UCOUNT_FANOTIFY_MARKS] = max_marks; + fanotify_sysctls_init(); return 0; } diff --git a/include/linux/fanotify.h b/include/linux/fanotify.h index 616af2ea20f3..556cc63c88ee 100644 --- a/include/linux/fanotify.h +++ b/include/linux/fanotify.h @@ -5,8 +5,6 @@ #include <linux/sysctl.h> #include <uapi/linux/fanotify.h> -extern struct ctl_table fanotify_table[]; /* for sysctl */ - #define FAN_GROUP_FLAG(group, flag) \ ((group)->fanotify_data.flags & (flag)) ^ permalink raw reply related [flat|nested] 5+ messages in thread
[parent not found: <20211123202422.819032-5-mcgrof@kernel.org>]
* Re: [PATCH v2 4/8] ocfs2: simplify subdirectory registration with register_sysctl() [not found] ` <20211123202422.819032-5-mcgrof@kernel.org> @ 2021-11-24 9:49 ` Jan Kara 0 siblings, 0 replies; 5+ messages in thread From: Jan Kara @ 2021-11-24 9:49 UTC (permalink / raw) To: Luis Chamberlain Cc: akpm, keescook, yzaikin, nixiaoming, ebiederm, clemens, arnd, gregkh, jani.nikula, joonas.lahtinen, rodrigo.vivi, tvrtko.ursulin, airlied, benh, mark, jlbec, joseph.qi, jack, amir73il, phil, viro, julia.lawall, ocfs2-devel, linuxppc-dev, intel-gfx, dri-devel, linux-fsdevel, linux-kernel On Tue 23-11-21 12:24:18, Luis Chamberlain wrote: > There is no need to user boiler plate code to specify a set of base > directories we're going to stuff sysctls under. Simplify this by using > register_sysctl() and specifying the directory path directly. > > // pycocci sysctl-subdir-register-sysctl-simplify.cocci PATH Heh, nice example of using Coccinelle. The result looks good. Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > > @c1@ > expression E1; > identifier subdir, sysctls; > @@ > > static struct ctl_table subdir[] = { > { > .procname = E1, > .maxlen = 0, > .mode = 0555, > .child = sysctls, > }, > { } > }; > > @c2@ > identifier c1.subdir; > > expression E2; > identifier base; > @@ > > static struct ctl_table base[] = { > { > .procname = E2, > .maxlen = 0, > .mode = 0555, > .child = subdir, > }, > { } > }; > > @c3@ > identifier c2.base; > identifier header; > @@ > > header = register_sysctl_table(base); > > @r1 depends on c1 && c2 && c3@ > expression c1.E1; > identifier c1.subdir, c1.sysctls; > @@ > > -static struct ctl_table subdir[] = { > - { > - .procname = E1, > - .maxlen = 0, > - .mode = 0555, > - .child = sysctls, > - }, > - { } > -}; > > @r2 depends on c1 && c2 && c3@ > identifier c1.subdir; > > expression c2.E2; > identifier c2.base; > @@ > -static struct ctl_table base[] = { > - { > - .procname = E2, > - .maxlen = 0, > - .mode = 0555, > - .child = subdir, > - }, > - { } > -}; > > @initialize:python@ > @@ > > def make_my_fresh_expression(s1, s2): > return '"' + s1.strip('"') + "/" + s2.strip('"') + '"' > > @r3 depends on c1 && c2 && c3@ > expression c1.E1; > identifier c1.sysctls; > expression c2.E2; > identifier c2.base; > identifier c3.header; > fresh identifier E3 = script:python(E2, E1) { make_my_fresh_expression(E2, E1) }; > @@ > > header = > -register_sysctl_table(base); > +register_sysctl(E3, sysctls); > > Generated-by: Coccinelle SmPL > Signed-off-by: Luis Chamberlain <mcgrof@kernel.org> > --- > fs/ocfs2/stackglue.c | 25 +------------------------ > 1 file changed, 1 insertion(+), 24 deletions(-) > > diff --git a/fs/ocfs2/stackglue.c b/fs/ocfs2/stackglue.c > index 16f1bfc407f2..731558a6f27d 100644 > --- a/fs/ocfs2/stackglue.c > +++ b/fs/ocfs2/stackglue.c > @@ -672,31 +672,8 @@ static struct ctl_table ocfs2_mod_table[] = { > { } > }; > > -static struct ctl_table ocfs2_kern_table[] = { > - { > - .procname = "ocfs2", > - .data = NULL, > - .maxlen = 0, > - .mode = 0555, > - .child = ocfs2_mod_table > - }, > - { } > -}; > - > -static struct ctl_table ocfs2_root_table[] = { > - { > - .procname = "fs", > - .data = NULL, > - .maxlen = 0, > - .mode = 0555, > - .child = ocfs2_kern_table > - }, > - { } > -}; > - > static struct ctl_table_header *ocfs2_table_header; > > - > /* > * Initialization > */ > @@ -705,7 +682,7 @@ static int __init ocfs2_stack_glue_init(void) > { > strcpy(cluster_stack_name, OCFS2_STACK_PLUGIN_O2CB); > > - ocfs2_table_header = register_sysctl_table(ocfs2_root_table); > + ocfs2_table_header = register_sysctl("fs/ocfs2", ocfs2_mod_table); > if (!ocfs2_table_header) { > printk(KERN_ERR > "ocfs2 stack glue: unable to register sysctl\n"); > -- > 2.33.0 > -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 5+ messages in thread
[parent not found: <20211123202422.819032-8-mcgrof@kernel.org>]
* Re: [PATCH v2 7/8] cdrom: simplify subdirectory registration with register_sysctl() [not found] ` <20211123202422.819032-8-mcgrof@kernel.org> @ 2021-11-25 9:23 ` Phillip Potter 0 siblings, 0 replies; 5+ messages in thread From: Phillip Potter @ 2021-11-25 9:23 UTC (permalink / raw) To: Luis Chamberlain, axboe Cc: akpm, keescook, yzaikin, nixiaoming, ebiederm, clemens, arnd, gregkh, jani.nikula, joonas.lahtinen, rodrigo.vivi, tvrtko.ursulin, airlied, benh, mark, jlbec, joseph.qi, jack, amir73il, phil, viro, julia.lawall, ocfs2-devel, linuxppc-dev, intel-gfx, dri-devel, linux-fsdevel, linux-kernel, linux-block On Tue, Nov 23, 2021 at 12:24:21PM -0800, Luis Chamberlain wrote: > There is no need to user boiler plate code to specify a set of base > directories we're going to stuff sysctls under. Simplify this by using > register_sysctl() and specifying the directory path directly. > > // pycocci sysctl-subdir-register-sysctl-simplify.cocci PATH > > @c1@ > expression E1; > identifier subdir, sysctls; > @@ > > static struct ctl_table subdir[] = { > { > .procname = E1, > .maxlen = 0, > .mode = 0555, > .child = sysctls, > }, > { } > }; > > @c2@ > identifier c1.subdir; > > expression E2; > identifier base; > @@ > > static struct ctl_table base[] = { > { > .procname = E2, > .maxlen = 0, > .mode = 0555, > .child = subdir, > }, > { } > }; > > @c3@ > identifier c2.base; > identifier header; > @@ > > header = register_sysctl_table(base); > > @r1 depends on c1 && c2 && c3@ > expression c1.E1; > identifier c1.subdir, c1.sysctls; > @@ > > -static struct ctl_table subdir[] = { > - { > - .procname = E1, > - .maxlen = 0, > - .mode = 0555, > - .child = sysctls, > - }, > - { } > -}; > > @r2 depends on c1 && c2 && c3@ > identifier c1.subdir; > > expression c2.E2; > identifier c2.base; > @@ > -static struct ctl_table base[] = { > - { > - .procname = E2, > - .maxlen = 0, > - .mode = 0555, > - .child = subdir, > - }, > - { } > -}; > > @initialize:python@ > @@ > > def make_my_fresh_expression(s1, s2): > return '"' + s1.strip('"') + "/" + s2.strip('"') + '"' > > @r3 depends on c1 && c2 && c3@ > expression c1.E1; > identifier c1.sysctls; > expression c2.E2; > identifier c2.base; > identifier c3.header; > fresh identifier E3 = script:python(E2, E1) { make_my_fresh_expression(E2, E1) }; > @@ > > header = > -register_sysctl_table(base); > +register_sysctl(E3, sysctls); > > Generated-by: Coccinelle SmPL > Signed-off-by: Luis Chamberlain <mcgrof@kernel.org> > --- > drivers/cdrom/cdrom.c | 23 +---------------------- > 1 file changed, 1 insertion(+), 22 deletions(-) > > diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c > index 9877e413fce3..1b57d4666e43 100644 > --- a/drivers/cdrom/cdrom.c > +++ b/drivers/cdrom/cdrom.c > @@ -3691,27 +3691,6 @@ static struct ctl_table cdrom_table[] = { > }, > { } > }; > - > -static struct ctl_table cdrom_cdrom_table[] = { > - { > - .procname = "cdrom", > - .maxlen = 0, > - .mode = 0555, > - .child = cdrom_table, > - }, > - { } > -}; > - > -/* Make sure that /proc/sys/dev is there */ > -static struct ctl_table cdrom_root_table[] = { > - { > - .procname = "dev", > - .maxlen = 0, > - .mode = 0555, > - .child = cdrom_cdrom_table, > - }, > - { } > -}; > static struct ctl_table_header *cdrom_sysctl_header; > > static void cdrom_sysctl_register(void) > @@ -3721,7 +3700,7 @@ static void cdrom_sysctl_register(void) > if (!atomic_add_unless(&initialized, 1, 1)) > return; > > - cdrom_sysctl_header = register_sysctl_table(cdrom_root_table); > + cdrom_sysctl_header = register_sysctl("dev/cdrom", cdrom_table); > > /* set the defaults */ > cdrom_sysctl_settings.autoclose = autoclose; > -- > 2.33.0 > Dear Luis, Thank you for the patch. Tested and working, looks good to me. As this has already been pulled into Andrew Morton's tree, I have added in Jens and the linux-block list so there is awareness that the patch will go via -mm then linux-next tree. For what it's worth (although guess it won't be in the commit now): Reviewed-by: Phillip Potter <phil@philpotter.co.uk> Regards, Phil ^ permalink raw reply [flat|nested] 5+ messages in thread
[parent not found: <20211123202422.819032-3-mcgrof@kernel.org>]
* Re: [PATCH v2 2/8] i915: simplify subdirectory registration with register_sysctl() [not found] ` <20211123202422.819032-3-mcgrof@kernel.org> @ 2021-11-25 9:41 ` Jani Nikula 0 siblings, 0 replies; 5+ messages in thread From: Jani Nikula @ 2021-11-25 9:41 UTC (permalink / raw) To: Luis Chamberlain, akpm, keescook, yzaikin, nixiaoming, ebiederm, clemens, arnd, gregkh, joonas.lahtinen, rodrigo.vivi, tvrtko.ursulin, airlied, daniel, benh, mark, jlbec, joseph.qi, jack, amir73il, phil, viro, julia.lawall Cc: ocfs2-devel, linuxppc-dev, intel-gfx, dri-devel, linux-fsdevel, linux-kernel, Luis Chamberlain On Tue, 23 Nov 2021, Luis Chamberlain <mcgrof@kernel.org> wrote: > There is no need to user boiler plate code to specify a set of base > directories we're going to stuff sysctls under. Simplify this by using > register_sysctl() and specifying the directory path directly. \o/ Acked-by: Jani Nikula <jani.nikula@intel.com> > > // pycocci sysctl-subdir-register-sysctl-simplify.cocci PATH > > @c1@ > expression E1; > identifier subdir, sysctls; > @@ > > static struct ctl_table subdir[] = { > { > .procname = E1, > .maxlen = 0, > .mode = 0555, > .child = sysctls, > }, > { } > }; > > @c2@ > identifier c1.subdir; > > expression E2; > identifier base; > @@ > > static struct ctl_table base[] = { > { > .procname = E2, > .maxlen = 0, > .mode = 0555, > .child = subdir, > }, > { } > }; > > @c3@ > identifier c2.base; > identifier header; > @@ > > header = register_sysctl_table(base); > > @r1 depends on c1 && c2 && c3@ > expression c1.E1; > identifier c1.subdir, c1.sysctls; > @@ > > -static struct ctl_table subdir[] = { > - { > - .procname = E1, > - .maxlen = 0, > - .mode = 0555, > - .child = sysctls, > - }, > - { } > -}; > > @r2 depends on c1 && c2 && c3@ > identifier c1.subdir; > > expression c2.E2; > identifier c2.base; > @@ > -static struct ctl_table base[] = { > - { > - .procname = E2, > - .maxlen = 0, > - .mode = 0555, > - .child = subdir, > - }, > - { } > -}; > > @initialize:python@ > @@ > > def make_my_fresh_expression(s1, s2): > return '"' + s1.strip('"') + "/" + s2.strip('"') + '"' > > @r3 depends on c1 && c2 && c3@ > expression c1.E1; > identifier c1.sysctls; > expression c2.E2; > identifier c2.base; > identifier c3.header; > fresh identifier E3 = script:python(E2, E1) { make_my_fresh_expression(E2, E1) }; > @@ > > header = > -register_sysctl_table(base); > +register_sysctl(E3, sysctls); > > Generated-by: Coccinelle SmPL > Signed-off-by: Luis Chamberlain <mcgrof@kernel.org> > --- > drivers/gpu/drm/i915/i915_perf.c | 22 +--------------------- > 1 file changed, 1 insertion(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c > index 2f01b8c0284c..5979e3258647 100644 > --- a/drivers/gpu/drm/i915/i915_perf.c > +++ b/drivers/gpu/drm/i915/i915_perf.c > @@ -4273,26 +4273,6 @@ static struct ctl_table oa_table[] = { > {} > }; > > -static struct ctl_table i915_root[] = { > - { > - .procname = "i915", > - .maxlen = 0, > - .mode = 0555, > - .child = oa_table, > - }, > - {} > -}; > - > -static struct ctl_table dev_root[] = { > - { > - .procname = "dev", > - .maxlen = 0, > - .mode = 0555, > - .child = i915_root, > - }, > - {} > -}; > - > static void oa_init_supported_formats(struct i915_perf *perf) > { > struct drm_i915_private *i915 = perf->i915; > @@ -4488,7 +4468,7 @@ static int destroy_config(int id, void *p, void *data) > > int i915_perf_sysctl_register(void) > { > - sysctl_header = register_sysctl_table(dev_root); > + sysctl_header = register_sysctl("dev/i915", oa_table); > return 0; > } -- Jani Nikula, Intel Open Source Graphics Center ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-11-25 9:43 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20211123202422.819032-1-mcgrof@kernel.org>
[not found] ` <20211123202422.819032-7-mcgrof@kernel.org>
2021-11-24 9:44 ` [PATCH v2 6/8] inotify: simplify subdirectory registration with register_sysctl() Jan Kara
2021-11-24 13:40 ` Luis Chamberlain
[not found] ` <20211123202422.819032-5-mcgrof@kernel.org>
2021-11-24 9:49 ` [PATCH v2 4/8] ocfs2: " Jan Kara
[not found] ` <20211123202422.819032-8-mcgrof@kernel.org>
2021-11-25 9:23 ` [PATCH v2 7/8] cdrom: " Phillip Potter
[not found] ` <20211123202422.819032-3-mcgrof@kernel.org>
2021-11-25 9:41 ` [PATCH v2 2/8] i915: " Jani Nikula
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).