| Message ID | 20200326181606.7027-2-vbabka@suse.cz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC,v3,1/2] kernel/sysctl: support setting sysctl parameters from kernel command line | expand |
On Thu, Mar 26, 2020 at 07:16:06PM +0100, Vlastimil Babka wrote: > We can now handle sysctl parameters on kernel command line, but historically > some parameters introduced their own command line equivalent, which we don't > want to remove for compatibility reasons. We can however convert them to the > generic infrastructure with a table translating the legacy command line > parameters to their sysctl names, and removing the one-off param handlers. > > This patch adds the support and makes the first conversion to demonstrate it, > on the (deprecated) numa_zonelist_order parameter. > > Signed-off-by: Vlastimil Babka <vbabka@suse.cz> > --- > Changes in v3: > - constify some things according to Kees > - expand the comment of sysctl_aliases to note on different timings > > fs/proc/proc_sysctl.c | 48 ++++++++++++++++++++++++++++++++++++------- > mm/page_alloc.c | 9 -------- > 2 files changed, 41 insertions(+), 16 deletions(-) > > diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c > index 8ee3273e4540..3a861e0a7c7e 100644 > --- a/fs/proc/proc_sysctl.c > +++ b/fs/proc/proc_sysctl.c > @@ -1729,6 +1729,37 @@ int __init proc_sys_init(void) > > struct vfsmount *proc_mnt = NULL; > > +struct sysctl_alias { > + const char *kernel_param; > + const char *sysctl_param; > +}; > + > +/* > + * Historically some settings had both sysctl and a command line parameter. > + * With the generic sysctl. parameter support, we can handle them at a single > + * place and only keep the historical name for compatibility. This is not meant > + * to add brand new aliases. When adding existing aliases, consider whether > + * the possibly different moment of changing the value (e.g. from early_param > + * to the moment do_sysctl_args() is called) is an issue for the specific > + * parameter. > + */ > +static const struct sysctl_alias sysctl_aliases[] = { > + {"numa_zonelist_order", "vm.numa_zonelist_order" }, > + { } > +}; > + > +const char *sysctl_find_alias(char *param) This should be "static" too. > +{ > + const struct sysctl_alias *alias; > + > + for (alias = &sysctl_aliases[0]; alias->kernel_param != NULL; alias++) { > + if (strcmp(alias->kernel_param, param) == 0) > + return alias->sysctl_param; > + } > + > + return NULL; > +} > + > /* Set sysctl value passed on kernel command line. */ > static int process_sysctl_arg(char *param, char *val, > const char *unused, void *arg) > @@ -1741,15 +1772,18 @@ static int process_sysctl_arg(char *param, char *val, > loff_t pos = 0; > ssize_t wret; > > - if (strncmp(param, "sysctl", sizeof("sysctl") - 1)) > - return 0; > - > - param += sizeof("sysctl") - 1; > + if (strncmp(param, "sysctl", sizeof("sysctl") - 1) == 0) { > + param += sizeof("sysctl") - 1; > > - if (param[0] != '/' && param[0] != '.') > - return 0; > + if (param[0] != '/' && param[0] != '.') > + return 0; > > - param++; > + param++; > + } else { > + param = (char *) sysctl_find_alias(param); > + if (!param) > + return 0; > + } > > if (!proc_mnt) { > proc_fs_type = get_fs_type("proc"); > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 3c4eb750a199..de7a134b1b8a 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -5460,15 +5460,6 @@ static int __parse_numa_zonelist_order(char *s) > return 0; > } > > -static __init int setup_numa_zonelist_order(char *s) > -{ > - if (!s) > - return 0; > - > - return __parse_numa_zonelist_order(s); > -} > -early_param("numa_zonelist_order", setup_numa_zonelist_order); > - > char numa_zonelist_order[] = "Node"; > > /* > -- > 2.25.1 > Otherwise, yay, I love it! :)
On Thu, Mar 26, 2020 at 01:34:08PM -0700, Kees Cook wrote: > On Thu, Mar 26, 2020 at 07:16:06PM +0100, Vlastimil Babka wrote: > > We can now handle sysctl parameters on kernel command line, but historically > > some parameters introduced their own command line equivalent, which we don't > > want to remove for compatibility reasons. We can however convert them to the > > generic infrastructure with a table translating the legacy command line > > parameters to their sysctl names, and removing the one-off param handlers. > > > > This patch adds the support and makes the first conversion to demonstrate it, > > on the (deprecated) numa_zonelist_order parameter. > > > > Signed-off-by: Vlastimil Babka <vbabka@suse.cz> > > --- > > Changes in v3: > > - constify some things according to Kees > > - expand the comment of sysctl_aliases to note on different timings > > > > fs/proc/proc_sysctl.c | 48 ++++++++++++++++++++++++++++++++++++------- > > mm/page_alloc.c | 9 -------- > > 2 files changed, 41 insertions(+), 16 deletions(-) > > > > diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c > > index 8ee3273e4540..3a861e0a7c7e 100644 > > --- a/fs/proc/proc_sysctl.c > > +++ b/fs/proc/proc_sysctl.c > > @@ -1729,6 +1729,37 @@ int __init proc_sys_init(void) > > > > struct vfsmount *proc_mnt = NULL; > > > > +struct sysctl_alias { > > + const char *kernel_param; > > + const char *sysctl_param; > > +}; > > + > > +/* > > + * Historically some settings had both sysctl and a command line parameter. > > + * With the generic sysctl. parameter support, we can handle them at a single > > + * place and only keep the historical name for compatibility. This is not meant > > + * to add brand new aliases. When adding existing aliases, consider whether > > + * the possibly different moment of changing the value (e.g. from early_param > > + * to the moment do_sysctl_args() is called) is an issue for the specific > > + * parameter. > > + */ > > +static const struct sysctl_alias sysctl_aliases[] = { > > + {"numa_zonelist_order", "vm.numa_zonelist_order" }, > > + { } > > +}; > > + > > +const char *sysctl_find_alias(char *param) > > This should be "static" too. > > > +{ > > + const struct sysctl_alias *alias; > > + > > + for (alias = &sysctl_aliases[0]; alias->kernel_param != NULL; alias++) { > > + if (strcmp(alias->kernel_param, param) == 0) > > + return alias->sysctl_param; > > + } > > + > > + return NULL; > > +} > > + > > /* Set sysctl value passed on kernel command line. */ > > static int process_sysctl_arg(char *param, char *val, > > const char *unused, void *arg) > > @@ -1741,15 +1772,18 @@ static int process_sysctl_arg(char *param, char *val, > > loff_t pos = 0; > > ssize_t wret; > > > > - if (strncmp(param, "sysctl", sizeof("sysctl") - 1)) Somewhat off-topic but in some projects I use macro-helpers to make that more transparent. So this would become a constant expression: #define STRLITERALLEN(x) (sizeof(""x"") - 1) or #define STRLEN(x) (sizeof(""x"") - 1) But I guess that's a matter of style/taste. Christian
diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c index 8ee3273e4540..3a861e0a7c7e 100644 --- a/fs/proc/proc_sysctl.c +++ b/fs/proc/proc_sysctl.c @@ -1729,6 +1729,37 @@ int __init proc_sys_init(void) struct vfsmount *proc_mnt = NULL; +struct sysctl_alias { + const char *kernel_param; + const char *sysctl_param; +}; + +/* + * Historically some settings had both sysctl and a command line parameter. + * With the generic sysctl. parameter support, we can handle them at a single + * place and only keep the historical name for compatibility. This is not meant + * to add brand new aliases. When adding existing aliases, consider whether + * the possibly different moment of changing the value (e.g. from early_param + * to the moment do_sysctl_args() is called) is an issue for the specific + * parameter. + */ +static const struct sysctl_alias sysctl_aliases[] = { + {"numa_zonelist_order", "vm.numa_zonelist_order" }, + { } +}; + +const char *sysctl_find_alias(char *param) +{ + const struct sysctl_alias *alias; + + for (alias = &sysctl_aliases[0]; alias->kernel_param != NULL; alias++) { + if (strcmp(alias->kernel_param, param) == 0) + return alias->sysctl_param; + } + + return NULL; +} + /* Set sysctl value passed on kernel command line. */ static int process_sysctl_arg(char *param, char *val, const char *unused, void *arg) @@ -1741,15 +1772,18 @@ static int process_sysctl_arg(char *param, char *val, loff_t pos = 0; ssize_t wret; - if (strncmp(param, "sysctl", sizeof("sysctl") - 1)) - return 0; - - param += sizeof("sysctl") - 1; + if (strncmp(param, "sysctl", sizeof("sysctl") - 1) == 0) { + param += sizeof("sysctl") - 1; - if (param[0] != '/' && param[0] != '.') - return 0; + if (param[0] != '/' && param[0] != '.') + return 0; - param++; + param++; + } else { + param = (char *) sysctl_find_alias(param); + if (!param) + return 0; + } if (!proc_mnt) { proc_fs_type = get_fs_type("proc"); diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 3c4eb750a199..de7a134b1b8a 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -5460,15 +5460,6 @@ static int __parse_numa_zonelist_order(char *s) return 0; } -static __init int setup_numa_zonelist_order(char *s) -{ - if (!s) - return 0; - - return __parse_numa_zonelist_order(s); -} -early_param("numa_zonelist_order", setup_numa_zonelist_order); - char numa_zonelist_order[] = "Node"; /*
We can now handle sysctl parameters on kernel command line, but historically some parameters introduced their own command line equivalent, which we don't want to remove for compatibility reasons. We can however convert them to the generic infrastructure with a table translating the legacy command line parameters to their sysctl names, and removing the one-off param handlers. This patch adds the support and makes the first conversion to demonstrate it, on the (deprecated) numa_zonelist_order parameter. Signed-off-by: Vlastimil Babka <vbabka@suse.cz> --- Changes in v3: - constify some things according to Kees - expand the comment of sysctl_aliases to note on different timings fs/proc/proc_sysctl.c | 48 ++++++++++++++++++++++++++++++++++++------- mm/page_alloc.c | 9 -------- 2 files changed, 41 insertions(+), 16 deletions(-)