| Message ID | 20230518160705.3888592-1-j.granados@samsung.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | sysctl: Remove register_sysctl_table from sources | expand |
On Thu, May 18, 2023 at 06:07:03PM +0200, Joel Granados wrote: > This is part of the general push to deprecate register_sysctl_paths and > register_sysctl_table. This patchset completely removes register_sysctl_table > and replaces it with register_sysctl effectively transitioning 5 base paths > ("kernel", "vm", "fs", "dev" and "debug") to the new call. Besides removing the > actuall function, I also removed it from the checks done in check-sysctl-docs. > > Testing for this change was done in the same way as with previous sysctl > replacement patches: I made sure that the result of `find /proc/sys/ | sha1sum` > was the same before and after the patchset. > > Have pushed this through 0-day. Waiting on results.. > > Feedback greatly appreciated. Thanks so much! I merged this to sysctl-testing as build tests are ongoing. But I incorporated these minor changes to your first patch as register_sysctl_init() is more obvious about when we cannot care about the return value. If the build tests come through I'll push to sysctl-next. diff --git a/fs/sysctls.c b/fs/sysctls.c index 228420f5fe1b..76a0aee8c229 100644 --- a/fs/sysctls.c +++ b/fs/sysctls.c @@ -31,11 +31,7 @@ static struct ctl_table fs_shared_sysctls[] = { static int __init init_fs_sysctls(void) { - /* - * We do not check the return code for register_sysctl because the - * original call to register_sysctl_base always returned 0. - */ - register_sysctl("fs", fs_shared_sysctls); + register_sysctl_init("fs", fs_shared_sysctls); return 0; } diff --git a/kernel/sysctl.c b/kernel/sysctl.c index f784b0fe5689..fa2aa8bd32b6 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -2350,10 +2350,10 @@ static struct ctl_table dev_table[] = { int __init sysctl_init_bases(void) { - register_sysctl("kernel", kern_table); - register_sysctl("vm", vm_table); - register_sysctl("debug", debug_table); - register_sysctl("dev", dev_table); + register_sysctl_init("kernel", kern_table); + register_sysctl_init("vm", vm_table); + register_sysctl_init("debug", debug_table); + register_sysctl_init("dev", dev_table); return 0; }
On Thu, May 18, 2023 at 01:46:44PM -0700, Luis Chamberlain wrote: > On Thu, May 18, 2023 at 06:07:03PM +0200, Joel Granados wrote: > > This is part of the general push to deprecate register_sysctl_paths and > > register_sysctl_table. This patchset completely removes register_sysctl_table > > and replaces it with register_sysctl effectively transitioning 5 base paths > > ("kernel", "vm", "fs", "dev" and "debug") to the new call. Besides removing the > > actuall function, I also removed it from the checks done in check-sysctl-docs. > > > > Testing for this change was done in the same way as with previous sysctl > > replacement patches: I made sure that the result of `find /proc/sys/ | sha1sum` > > was the same before and after the patchset. > > > > Have pushed this through 0-day. Waiting on results.. > > > > Feedback greatly appreciated. > > Thanks so much! I merged this to sysctl-testing as build tests are ongoing. But > I incorporated these minor changes to your first patch as register_sysctl_init() > is more obvious about when we cannot care about the return value. > > If the build tests come through I'll push to sysctl-next. > I also had to apply this (yay more nuking): diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c index 7bc7d3c3a215..8873812d22f3 100644 --- a/fs/proc/proc_sysctl.c +++ b/fs/proc/proc_sysctl.c @@ -1466,19 +1466,6 @@ void __init __register_sysctl_init(const char *path, struct ctl_table *table, kmemleak_not_leak(hdr); } -static char *append_path(const char *path, char *pos, const char *name) -{ - int namelen; - namelen = strlen(name); - if (((pos - path) + namelen + 2) >= PATH_MAX) - return NULL; - memcpy(pos, name, namelen); - pos[namelen] = '/'; - pos[namelen + 1] = '\0'; - pos += namelen + 1; - return pos; -} - static int count_subheaders(struct ctl_table *table) { int has_files = 0; @@ -1498,82 +1485,6 @@ static int count_subheaders(struct ctl_table *table) return nr_subheaders + has_files; } -static int register_leaf_sysctl_tables(const char *path, char *pos, - struct ctl_table_header ***subheader, struct ctl_table_set *set, - struct ctl_table *table) -{ - struct ctl_table *ctl_table_arg = NULL; - struct ctl_table *entry, *files; - int nr_files = 0; - int nr_dirs = 0; - int err = -ENOMEM; - - list_for_each_table_entry(entry, table) { - if (entry->child) - nr_dirs++; - else - nr_files++; - } - - files = table; - /* If there are mixed files and directories we need a new table */ - if (nr_dirs && nr_files) { - struct ctl_table *new; - files = kcalloc(nr_files + 1, sizeof(struct ctl_table), - GFP_KERNEL); - if (!files) - goto out; - - ctl_table_arg = files; - new = files; - - list_for_each_table_entry(entry, table) { - if (entry->child) - continue; - *new = *entry; - new++; - } - } - - /* Register everything except a directory full of subdirectories */ - if (nr_files || !nr_dirs) { - struct ctl_table_header *header; - header = __register_sysctl_table(set, path, files); - if (!header) { - kfree(ctl_table_arg); - goto out; - } - - /* Remember if we need to free the file table */ - header->ctl_table_arg = ctl_table_arg; - **subheader = header; - (*subheader)++; - } - - /* Recurse into the subdirectories. */ - list_for_each_table_entry(entry, table) { - char *child_pos; - - if (!entry->child) - continue; - - err = -ENAMETOOLONG; - child_pos = append_path(path, pos, entry->procname); - if (!child_pos) - goto out; - - err = register_leaf_sysctl_tables(path, child_pos, subheader, - set, entry->child); - pos[0] = '\0'; - if (err) - goto out; - } - err = 0; -out: - /* On failure our caller will unregister all registered subheaders */ - return err; -} - static void put_links(struct ctl_table_header *header) { struct ctl_table_set *root_set = &sysctl_table_root.default_set;
On Thu, May 18, 2023 at 05:26:34PM -0700, Luis Chamberlain wrote: > On Thu, May 18, 2023 at 01:46:44PM -0700, Luis Chamberlain wrote: > > On Thu, May 18, 2023 at 06:07:03PM +0200, Joel Granados wrote: > > > This is part of the general push to deprecate register_sysctl_paths and > > > register_sysctl_table. This patchset completely removes register_sysctl_table > > > and replaces it with register_sysctl effectively transitioning 5 base paths > > > ("kernel", "vm", "fs", "dev" and "debug") to the new call. Besides removing the > > > actuall function, I also removed it from the checks done in check-sysctl-docs. > > > > > > Testing for this change was done in the same way as with previous sysctl > > > replacement patches: I made sure that the result of `find /proc/sys/ | sha1sum` > > > was the same before and after the patchset. > > > > > > Have pushed this through 0-day. Waiting on results.. > > > > > > Feedback greatly appreciated. > > > > Thanks so much! I merged this to sysctl-testing as build tests are ongoing. But > > I incorporated these minor changes to your first patch as register_sysctl_init() > > is more obvious about when we cannot care about the return value. nice! thx. > > > > If the build tests come through I'll push to sysctl-next. > > > > I also had to apply this (yay more nuking): Indeed. I just saw the results of 0-day and there was a warning regarding these functions. Thx again. best joel > > diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c > index 7bc7d3c3a215..8873812d22f3 100644 > --- a/fs/proc/proc_sysctl.c > +++ b/fs/proc/proc_sysctl.c > @@ -1466,19 +1466,6 @@ void __init __register_sysctl_init(const char *path, struct ctl_table *table, > kmemleak_not_leak(hdr); > } > > -static char *append_path(const char *path, char *pos, const char *name) > -{ > - int namelen; > - namelen = strlen(name); > - if (((pos - path) + namelen + 2) >= PATH_MAX) > - return NULL; > - memcpy(pos, name, namelen); > - pos[namelen] = '/'; > - pos[namelen + 1] = '\0'; > - pos += namelen + 1; > - return pos; > -} > - > static int count_subheaders(struct ctl_table *table) > { > int has_files = 0; > @@ -1498,82 +1485,6 @@ static int count_subheaders(struct ctl_table *table) > return nr_subheaders + has_files; > } > > -static int register_leaf_sysctl_tables(const char *path, char *pos, > - struct ctl_table_header ***subheader, struct ctl_table_set *set, > - struct ctl_table *table) > -{ > - struct ctl_table *ctl_table_arg = NULL; > - struct ctl_table *entry, *files; > - int nr_files = 0; > - int nr_dirs = 0; > - int err = -ENOMEM; > - > - list_for_each_table_entry(entry, table) { > - if (entry->child) > - nr_dirs++; > - else > - nr_files++; > - } > - > - files = table; > - /* If there are mixed files and directories we need a new table */ > - if (nr_dirs && nr_files) { > - struct ctl_table *new; > - files = kcalloc(nr_files + 1, sizeof(struct ctl_table), > - GFP_KERNEL); > - if (!files) > - goto out; > - > - ctl_table_arg = files; > - new = files; > - > - list_for_each_table_entry(entry, table) { > - if (entry->child) > - continue; > - *new = *entry; > - new++; > - } > - } > - > - /* Register everything except a directory full of subdirectories */ > - if (nr_files || !nr_dirs) { > - struct ctl_table_header *header; > - header = __register_sysctl_table(set, path, files); > - if (!header) { > - kfree(ctl_table_arg); > - goto out; > - } > - > - /* Remember if we need to free the file table */ > - header->ctl_table_arg = ctl_table_arg; > - **subheader = header; > - (*subheader)++; > - } > - > - /* Recurse into the subdirectories. */ > - list_for_each_table_entry(entry, table) { > - char *child_pos; > - > - if (!entry->child) > - continue; > - > - err = -ENAMETOOLONG; > - child_pos = append_path(path, pos, entry->procname); > - if (!child_pos) > - goto out; > - > - err = register_leaf_sysctl_tables(path, child_pos, subheader, > - set, entry->child); > - pos[0] = '\0'; > - if (err) > - goto out; > - } > - err = 0; > -out: > - /* On failure our caller will unregister all registered subheaders */ > - return err; > -} > - > static void put_links(struct ctl_table_header *header) > { > struct ctl_table_set *root_set = &sysctl_table_root.default_set;
This is part of the general push to deprecate register_sysctl_paths and register_sysctl_table. This patchset completely removes register_sysctl_table and replaces it with register_sysctl effectively transitioning 5 base paths ("kernel", "vm", "fs", "dev" and "debug") to the new call. Besides removing the actuall function, I also removed it from the checks done in check-sysctl-docs. Testing for this change was done in the same way as with previous sysctl replacement patches: I made sure that the result of `find /proc/sys/ | sha1sum` was the same before and after the patchset. Have pushed this through 0-day. Waiting on results.. Feedback greatly appreciated. Best Joel Joel Granados (2): sysctl: Refactor base paths registrations sysctl: Remove register_sysctl_table fs/proc/proc_sysctl.c | 70 --------------------------------------- fs/sysctls.c | 9 +++-- include/linux/sysctl.h | 23 ------------- kernel/sysctl.c | 13 +++----- scripts/check-sysctl-docs | 10 ------ 5 files changed, 10 insertions(+), 115 deletions(-)