* Re: [Ocfs2-devel] [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 via Ocfs2-devel
2021-11-24 13:40 ` Luis Chamberlain via Ocfs2-devel
0 siblings, 1 reply; 3+ messages in thread
From: Jan Kara via Ocfs2-devel @ 2021-11-24 9:44 UTC (permalink / raw)
To: Luis Chamberlain
Cc: jack, airlied, benh, amir73il, clemens, dri-devel, phil, yzaikin,
joonas.lahtinen, keescook, arnd, intel-gfx, jani.nikula, viro,
rodrigo.vivi, nixiaoming, tvrtko.ursulin, gregkh, linux-kernel,
julia.lawall, ebiederm, linux-fsdevel, linuxppc-dev, ocfs2-devel
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
_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Ocfs2-devel] [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 via Ocfs2-devel
0 siblings, 0 replies; 3+ messages in thread
From: Jan Kara via Ocfs2-devel @ 2021-11-24 9:49 UTC (permalink / raw)
To: Luis Chamberlain
Cc: jack, airlied, benh, amir73il, clemens, dri-devel, phil, yzaikin,
joonas.lahtinen, keescook, arnd, intel-gfx, jani.nikula, viro,
rodrigo.vivi, nixiaoming, tvrtko.ursulin, gregkh, linux-kernel,
julia.lawall, ebiederm, linux-fsdevel, linuxppc-dev, ocfs2-devel
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
_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Ocfs2-devel] [PATCH v2 6/8] inotify: simplify subdirectory registration with register_sysctl()
2021-11-24 9:44 ` [Ocfs2-devel] [PATCH v2 6/8] inotify: simplify subdirectory registration with register_sysctl() Jan Kara via Ocfs2-devel
@ 2021-11-24 13:40 ` Luis Chamberlain via Ocfs2-devel
0 siblings, 0 replies; 3+ messages in thread
From: Luis Chamberlain via Ocfs2-devel @ 2021-11-24 13:40 UTC (permalink / raw)
To: Jan Kara
Cc: airlied, benh, amir73il, clemens, dri-devel, phil, yzaikin,
joonas.lahtinen, keescook, arnd, intel-gfx, jani.nikula, viro,
rodrigo.vivi, nixiaoming, tvrtko.ursulin, gregkh, linux-kernel,
julia.lawall, ebiederm, linux-fsdevel, linuxppc-dev, ocfs2-devel
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))
_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-11-24 13:41 UTC | newest]
Thread overview: 3+ 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 ` [Ocfs2-devel] [PATCH v2 6/8] inotify: simplify subdirectory registration with register_sysctl() Jan Kara via Ocfs2-devel
2021-11-24 13:40 ` Luis Chamberlain via Ocfs2-devel
[not found] ` <20211123202422.819032-5-mcgrof@kernel.org>
2021-11-24 9:49 ` [Ocfs2-devel] [PATCH v2 4/8] ocfs2: " Jan Kara via Ocfs2-devel
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).