This is the 2nd set of kernel/sysctl.c cleanups. The diff stat should reflect how this is a much better way to deal with theses. Fortunately coccinelle can be used to ensure correctness for most of these and/or future merge conflicts.
Note that since this is part of a larger effort to cleanup kernel/sysctl.c I think we have no other option but to go with merging these patches in either Andrew's tree or keep them staged in a separate tree and send a merge request later. Otherwise kernel/sysctl.c will end up becoming a sore spot for the next merge window.
Changes in this v2:
* As suggested by Eric W. Biederman I dropped the subdir new call and just used the register_sysctl() by specifying the parent directory. * 0-day cleanups, commit log enhancements * Updated the coccinelle patch with register_sysctl()
Luis Chamberlain (6): hpet: simplify subdirectory registration with register_sysctl() i915: simplify subdirectory registration with register_sysctl() macintosh/mac_hid.c: simplify subdirectory registration with register_sysctl() ocfs2: simplify subdirectory registration with register_sysctl() test_sysctl: simplify subdirectory registration with register_sysctl() cdrom: simplify subdirectory registration with register_sysctl()
Xiaoming Ni (2): inotify: simplify subdirectory registration with register_sysctl() eventpoll: simplify sysctl declaration with register_sysctl()
drivers/cdrom/cdrom.c | 23 +---------------------- drivers/char/hpet.c | 22 +--------------------- drivers/gpu/drm/i915/i915_perf.c | 22 +--------------------- drivers/macintosh/mac_hid.c | 24 +----------------------- fs/eventpoll.c | 10 +++++++++- fs/notify/inotify/inotify_user.c | 11 ++++++++++- fs/ocfs2/stackglue.c | 25 +------------------------ include/linux/inotify.h | 3 --- include/linux/poll.h | 2 -- include/linux/sysctl.h | 1 - kernel/sysctl.c | 28 ---------------------------- lib/test_sysctl.c | 22 +--------------------- 12 files changed, 25 insertions(+), 168 deletions(-)
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 drivers/char/hpet.c
@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/char/hpet.c | 22 +--------------------- 1 file changed, 1 insertion(+), 21 deletions(-)
diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c index 4e5431f01450..563dfae3b8da 100644 --- a/drivers/char/hpet.c +++ b/drivers/char/hpet.c @@ -746,26 +746,6 @@ static struct ctl_table hpet_table[] = { {} };
-static struct ctl_table hpet_root[] = { - { - .procname = "hpet", - .maxlen = 0, - .mode = 0555, - .child = hpet_table, - }, - {} -}; - -static struct ctl_table dev_root[] = { - { - .procname = "dev", - .maxlen = 0, - .mode = 0555, - .child = hpet_root, - }, - {} -}; - static struct ctl_table_header *sysctl_header;
/* @@ -1061,7 +1041,7 @@ static int __init hpet_init(void) if (result < 0) return -ENODEV;
- sysctl_header = register_sysctl_table(dev_root); + sysctl_header = register_sysctl("dev/hpet", hpet_table);
result = acpi_bus_register_driver(&hpet_acpi_driver); if (result < 0) {
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/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; }
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;
}
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/macintosh/mac_hid.c | 24 +----------------------- 1 file changed, 1 insertion(+), 23 deletions(-)
diff --git a/drivers/macintosh/mac_hid.c b/drivers/macintosh/mac_hid.c index 28b8581b44dd..d8c4d5664145 100644 --- a/drivers/macintosh/mac_hid.c +++ b/drivers/macintosh/mac_hid.c @@ -239,33 +239,11 @@ static struct ctl_table mac_hid_files[] = { { } };
-/* dir in /proc/sys/dev */ -static struct ctl_table mac_hid_dir[] = { - { - .procname = "mac_hid", - .maxlen = 0, - .mode = 0555, - .child = mac_hid_files, - }, - { } -}; - -/* /proc/sys/dev itself, in case that is not there yet */ -static struct ctl_table mac_hid_root_dir[] = { - { - .procname = "dev", - .maxlen = 0, - .mode = 0555, - .child = mac_hid_dir, - }, - { } -}; - static struct ctl_table_header *mac_hid_sysctl_header;
static int __init mac_hid_init(void) { - mac_hid_sysctl_header = register_sysctl_table(mac_hid_root_dir); + mac_hid_sysctl_header = register_sysctl("dev/mac_hid", mac_hid_files); if (!mac_hid_sysctl_header) return -ENOMEM;
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 --- 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");
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
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 lib/test_sysctl.c
@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 --- lib/test_sysctl.c | 22 +--------------------- 1 file changed, 1 insertion(+), 21 deletions(-)
diff --git a/lib/test_sysctl.c b/lib/test_sysctl.c index 3750323973f4..a5a3d6c27e1f 100644 --- a/lib/test_sysctl.c +++ b/lib/test_sysctl.c @@ -128,26 +128,6 @@ static struct ctl_table test_table[] = { { } };
-static struct ctl_table test_sysctl_table[] = { - { - .procname = "test_sysctl", - .maxlen = 0, - .mode = 0555, - .child = test_table, - }, - { } -}; - -static struct ctl_table test_sysctl_root_table[] = { - { - .procname = "debug", - .maxlen = 0, - .mode = 0555, - .child = test_sysctl_table, - }, - { } -}; - static struct ctl_table_header *test_sysctl_header;
static int __init test_sysctl_init(void) @@ -155,7 +135,7 @@ static int __init test_sysctl_init(void) test_data.bitmap_0001 = kzalloc(SYSCTL_TEST_BITMAP_SIZE/8, GFP_KERNEL); if (!test_data.bitmap_0001) return -ENOMEM; - test_sysctl_header = register_sysctl_table(test_sysctl_root_table); + test_sysctl_header = register_sysctl("debug/test_sysctl", test_table); if (!test_sysctl_header) { kfree(test_data.bitmap_0001); return -ENOMEM;
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 --- 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",
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
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))
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;
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
From: Xiaoming Ni nixiaoming@huawei.com
The kernel/sysctl.c is a kitchen sink where everyone leaves their dirty dishes, this makes it very difficult to maintain.
To help with this maintenance let's start by moving sysctls to places where they actually belong. The proc sysctl maintainers do not want to know what sysctl knobs you wish to add for your own piece of code, we just care about the core logic.
So move the epoll_table sysctl to fs/eventpoll.c and use use register_sysctl().
Signed-off-by: Xiaoming Ni nixiaoming@huawei.com Signed-off-by: Luis Chamberlain mcgrof@kernel.org --- fs/eventpoll.c | 10 +++++++++- include/linux/poll.h | 2 -- include/linux/sysctl.h | 1 - kernel/sysctl.c | 7 ------- 4 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/fs/eventpoll.c b/fs/eventpoll.c index 06f4c5ae1451..e2daa940ebce 100644 --- a/fs/eventpoll.c +++ b/fs/eventpoll.c @@ -307,7 +307,7 @@ static void unlist_file(struct epitems_head *head) static long long_zero; static long long_max = LONG_MAX;
-struct ctl_table epoll_table[] = { +static struct ctl_table epoll_table[] = { { .procname = "max_user_watches", .data = &max_user_watches, @@ -319,6 +319,13 @@ struct ctl_table epoll_table[] = { }, { } }; + +static void __init epoll_sysctls_init(void) +{ + register_sysctl("fs/epoll", epoll_table); +} +#else +#define epoll_sysctls_init() do { } while (0) #endif /* CONFIG_SYSCTL */
static const struct file_operations eventpoll_fops; @@ -2378,6 +2385,7 @@ static int __init eventpoll_init(void) /* Allocates slab cache used to allocate "struct eppoll_entry" */ pwq_cache = kmem_cache_create("eventpoll_pwq", sizeof(struct eppoll_entry), 0, SLAB_PANIC|SLAB_ACCOUNT, NULL); + epoll_sysctls_init();
ephead_cache = kmem_cache_create("ep_head", sizeof(struct epitems_head), 0, SLAB_PANIC|SLAB_ACCOUNT, NULL); diff --git a/include/linux/poll.h b/include/linux/poll.h index 1cdc32b1f1b0..a9e0e1c2d1f2 100644 --- a/include/linux/poll.h +++ b/include/linux/poll.h @@ -8,12 +8,10 @@ #include <linux/wait.h> #include <linux/string.h> #include <linux/fs.h> -#include <linux/sysctl.h> #include <linux/uaccess.h> #include <uapi/linux/poll.h> #include <uapi/linux/eventpoll.h>
-extern struct ctl_table epoll_table[]; /* for sysctl */ /* ~832 bytes of stack space used max in sys_select/sys_poll before allocating additional memory. */ #ifdef __clang__ diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h index 718492057c70..5e0428a71899 100644 --- a/include/linux/sysctl.h +++ b/include/linux/sysctl.h @@ -218,7 +218,6 @@ extern int no_unaligned_warning; extern struct ctl_table sysctl_mount_point[]; extern struct ctl_table random_table[]; extern struct ctl_table firmware_config_table[]; -extern struct ctl_table epoll_table[];
#else /* CONFIG_SYSCTL */ static inline struct ctl_table_header *register_sysctl_table(struct ctl_table * table) diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 6aa67c737e4e..b09ff41720e3 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -3092,13 +3092,6 @@ static struct ctl_table fs_table[] = { .proc_handler = proc_dointvec, }, #endif -#ifdef CONFIG_EPOLL - { - .procname = "epoll", - .mode = 0555, - .child = epoll_table, - }, -#endif #endif { .procname = "protected_symlinks",
dri-devel@lists.freedesktop.org
-
Jan Kara -
Jani Nikula -
Luis Chamberlain -
Phillip Potter