| Message ID | 20230728033701.817094-2-paulmck@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC,bootconfig] 2/2] fs/proc: Add /proc/cmdline_image for embedded arguments | expand |
Hi Paul, On Thu, 27 Jul 2023 20:37:01 -0700 "Paul E. McKenney" <paulmck@kernel.org> wrote: > In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will show > all kernel boot parameters, both those supplied by the boot loader and > those embedded in the kernel image. This works well for those who just > want to see all of the kernel boot parameters, but is not helpful to those > who need to see only those parameters that were embedded into the kernel > image. This is especially important in situations where there are many > kernel images for different kernel versions and kernel configurations, > all of which opens the door to a great deal of human error. There is /proc/bootconfig file which shows all bootconfig entries and is formatted as easily filter by grep (or any other line-based commands). (e.g. `grep ^kernel\\. /proc/cmdline` will filter all kernel cmdline parameters in the bootconfig) Could you clarify the reason why you need a dump of bootconfig file? Thank you, > > Therefore, provide a /proc/cmdline_image file that shows only those kernel > boot parameters that were embedded in the kernel image. The output > is in boot-image format, which allows easy reconcilation against the > boot-config source file. > > Why put this in /proc? Because it is quite similar to /proc/cmdline, so > it makes sense to put it in the same place that /proc/cmdline is located. > > Signed-off-by: Paul E. McKenney <paulmck@kernel.org> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Alexey Dobriyan <adobriyan@gmail.com> > Cc: Masami Hiramatsu <mhiramat@kernel.org> > Cc: <linux-fsdevel@vger.kernel.org> > > Signed-off-by: Paul E. McKenney <paulmck@kernel.org> > --- > fs/proc/cmdline.c | 12 ++++++++++++ > include/linux/init.h | 11 ++++++----- > init/main.c | 9 +++++++++ > 3 files changed, 27 insertions(+), 5 deletions(-) > > diff --git a/fs/proc/cmdline.c b/fs/proc/cmdline.c > index 1d0ef9d2949d..4ab5223198cb 100644 > --- a/fs/proc/cmdline.c > +++ b/fs/proc/cmdline.c > @@ -20,6 +20,15 @@ static int cmdline_load_proc_show(struct seq_file *m, void *v) > return 0; > } > > +static int cmdline_image_proc_show(struct seq_file *m, void *v) > +{ > +#ifdef CONFIG_BOOT_CONFIG_FORCE > + seq_puts(m, saved_bootconfig_string); > + seq_putc(m, '\n'); > +#endif > + return 0; > +} > + > static int __init proc_cmdline_init(void) > { > struct proc_dir_entry *pde; > @@ -31,6 +40,9 @@ static int __init proc_cmdline_init(void) > pde = proc_create_single("cmdline_load", 0, NULL, cmdline_load_proc_show); > pde_make_permanent(pde); > pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1; > + pde = proc_create_single("cmdline_image", 0, NULL, cmdline_image_proc_show); > + pde_make_permanent(pde); > + pde->size = strnlen(saved_bootconfig_string, COMMAND_LINE_SIZE) + 1; > } > return 0; > } > diff --git a/include/linux/init.h b/include/linux/init.h > index 29e75bbe7984..c075983c5015 100644 > --- a/include/linux/init.h > +++ b/include/linux/init.h > @@ -14,7 +14,7 @@ > #define __noinitretpoline > #endif > > -/* These macros are used to mark some functions or > +/* These macros are used to mark some functions or > * initialized data (doesn't apply to uninitialized data) > * as `initialization' functions. The kernel can take this > * as hint that the function is used only during the initialization > @@ -22,7 +22,7 @@ > * > * Usage: > * For functions: > - * > + * > * You should add __init immediately before the function name, like: > * > * static void __init initme(int x, int y) > @@ -148,6 +148,7 @@ extern char boot_command_line[]; > extern char *saved_command_line; > extern unsigned int saved_command_line_len; > extern unsigned int reset_devices; > +extern char saved_bootconfig_string[]; > > /* used by init/main.c */ > void setup_arch(char **); > @@ -184,7 +185,7 @@ extern void (*late_time_init)(void); > extern bool initcall_debug; > > #endif > - > + > #ifndef MODULE > > #ifndef __ASSEMBLY__ > @@ -192,8 +193,8 @@ extern bool initcall_debug; > /* > * initcalls are now grouped by functionality into separate > * subsections. Ordering inside the subsections is determined > - * by link order. > - * For backwards compatibility, initcall() puts the call in > + * by link order. > + * For backwards compatibility, initcall() puts the call in > * the device init subsection. > * > * The `id' arg to __define_initcall() is needed so that multiple initcalls > diff --git a/init/main.c b/init/main.c > index 2121685c479a..981170da0b1c 100644 > --- a/init/main.c > +++ b/init/main.c > @@ -146,6 +146,11 @@ static char *extra_command_line; > /* Extra init arguments */ > static char *extra_init_args; > > +/* Untouched boot-config string */ > +#ifdef CONFIG_BOOT_CONFIG_FORCE > +char saved_bootconfig_string[COMMAND_LINE_SIZE] __ro_after_init; > +#endif > + > #ifdef CONFIG_BOOT_CONFIG > /* Is bootconfig on command line? */ > static bool bootconfig_found; > @@ -435,6 +440,10 @@ static void __init setup_boot_config(void) > return; > } > > +#ifdef CONFIG_BOOT_CONFIG_FORCE > + strncpy(saved_bootconfig_string, data, COMMAND_LINE_SIZE); > +#endif > + > if (size >= XBC_DATA_MAX) { > pr_err("bootconfig size %ld greater than max size %d\n", > (long)size, XBC_DATA_MAX); > -- > 2.40.1 >
On Sat, Jul 29, 2023 at 11:23:46PM +0900, Masami Hiramatsu wrote: > Hi Paul, > > On Thu, 27 Jul 2023 20:37:01 -0700 > "Paul E. McKenney" <paulmck@kernel.org> wrote: > > > In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will show > > all kernel boot parameters, both those supplied by the boot loader and > > those embedded in the kernel image. This works well for those who just > > want to see all of the kernel boot parameters, but is not helpful to those > > who need to see only those parameters that were embedded into the kernel > > image. This is especially important in situations where there are many > > kernel images for different kernel versions and kernel configurations, > > all of which opens the door to a great deal of human error. > > There is /proc/bootconfig file which shows all bootconfig entries and is > formatted as easily filter by grep (or any other line-based commands). > (e.g. `grep ^kernel\\. /proc/cmdline` will filter all kernel cmdline > parameters in the bootconfig) > Could you clarify the reason why you need a dump of bootconfig file? Because I was unaware of /proc/bootconfig? ;-) So how about if I replace this patch of mine with the following? And thank you for pointing me at /proc/bootconfig. Thanx, Paul ------------------------------------------------------------------------ diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst index 98c43c5ef1ee..832d66d4e396 100644 --- a/Documentation/filesystems/proc.rst +++ b/Documentation/filesystems/proc.rst @@ -684,6 +684,7 @@ files are there, and which are missing. File Content ============ =============================================================== apm Advanced power management info + bootconfig Kernel command line obtained from boot config (5.5) buddyinfo Kernel memory allocator information (see text) (2.5) bus Directory containing bus specific information cmdline Kernel command line, both from bootloader and embedded
On Sat, 29 Jul 2023 08:41:46 -0700 "Paul E. McKenney" <paulmck@kernel.org> wrote: > On Sat, Jul 29, 2023 at 11:23:46PM +0900, Masami Hiramatsu wrote: > > Hi Paul, > > > > On Thu, 27 Jul 2023 20:37:01 -0700 > > "Paul E. McKenney" <paulmck@kernel.org> wrote: > > > > > In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will show > > > all kernel boot parameters, both those supplied by the boot loader and > > > those embedded in the kernel image. This works well for those who just > > > want to see all of the kernel boot parameters, but is not helpful to those > > > who need to see only those parameters that were embedded into the kernel > > > image. This is especially important in situations where there are many > > > kernel images for different kernel versions and kernel configurations, > > > all of which opens the door to a great deal of human error. > > > > There is /proc/bootconfig file which shows all bootconfig entries and is > > formatted as easily filter by grep (or any other line-based commands). > > (e.g. `grep ^kernel\\. /proc/cmdline` will filter all kernel cmdline > > parameters in the bootconfig) > > Could you clarify the reason why you need a dump of bootconfig file? > > Because I was unaware of /proc/bootconfig? ;-) Oh :) > > So how about if I replace this patch of mine with the following? Yes, I missed to update the proc.rst. Thanks! > > And thank you for pointing me at /proc/bootconfig. > > Thanx, Paul > > ------------------------------------------------------------------------ > > diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst > index 98c43c5ef1ee..832d66d4e396 100644 > --- a/Documentation/filesystems/proc.rst > +++ b/Documentation/filesystems/proc.rst > @@ -684,6 +684,7 @@ files are there, and which are missing. > File Content > ============ =============================================================== > apm Advanced power management info > + bootconfig Kernel command line obtained from boot config (5.5) > buddyinfo Kernel memory allocator information (see text) (2.5) > bus Directory containing bus specific information > cmdline Kernel command line, both from bootloader and embedded
On Thu, Jul 27, 2023 at 08:37:01PM -0700, Paul E. McKenney wrote: > In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will show > all kernel boot parameters, both those supplied by the boot loader and > those embedded in the kernel image. This works well for those who just > want to see all of the kernel boot parameters, but is not helpful to those > who need to see only those parameters that were embedded into the kernel > image. This is especially important in situations where there are many > kernel images for different kernel versions and kernel configurations, > all of which opens the door to a great deal of human error. > > Therefore, provide a /proc/cmdline_image file that shows only those kernel > boot parameters that were embedded in the kernel image. The output > is in boot-image format, which allows easy reconcilation against the > boot-config source file. > > Why put this in /proc? Because it is quite similar to /proc/cmdline, so > it makes sense to put it in the same place that /proc/cmdline is located. > > Signed-off-by: Paul E. McKenney <paulmck@kernel.org> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Alexey Dobriyan <adobriyan@gmail.com> > Cc: Masami Hiramatsu <mhiramat@kernel.org> > Cc: <linux-fsdevel@vger.kernel.org> > > Signed-off-by: Paul E. McKenney <paulmck@kernel.org> > --- > fs/proc/cmdline.c | 12 ++++++++++++ > include/linux/init.h | 11 ++++++----- > init/main.c | 9 +++++++++ Same thing, Please if possible put /proc/x into fs/proc/x.c so that it is easier to find source. Not all /proc follows this convention but still. I don't like this name too (but less than the other one). Is it Boot Image Format (BIF). If yes, maybe add it as /proc/cmdline.bif ? I don't know what's the good name.
On Fri, Aug 04, 2023 at 08:28:08PM +0300, Alexey Dobriyan wrote: > On Thu, Jul 27, 2023 at 08:37:01PM -0700, Paul E. McKenney wrote: > > In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will show > > all kernel boot parameters, both those supplied by the boot loader and > > those embedded in the kernel image. This works well for those who just > > want to see all of the kernel boot parameters, but is not helpful to those > > who need to see only those parameters that were embedded into the kernel > > image. This is especially important in situations where there are many > > kernel images for different kernel versions and kernel configurations, > > all of which opens the door to a great deal of human error. > > > > Therefore, provide a /proc/cmdline_image file that shows only those kernel > > boot parameters that were embedded in the kernel image. The output > > is in boot-image format, which allows easy reconcilation against the > > boot-config source file. > > > > Why put this in /proc? Because it is quite similar to /proc/cmdline, so > > it makes sense to put it in the same place that /proc/cmdline is located. > > > > Signed-off-by: Paul E. McKenney <paulmck@kernel.org> > > Cc: Andrew Morton <akpm@linux-foundation.org> > > Cc: Alexey Dobriyan <adobriyan@gmail.com> > > Cc: Masami Hiramatsu <mhiramat@kernel.org> > > Cc: <linux-fsdevel@vger.kernel.org> > > > > Signed-off-by: Paul E. McKenney <paulmck@kernel.org> > > --- > > fs/proc/cmdline.c | 12 ++++++++++++ > > include/linux/init.h | 11 ++++++----- > > init/main.c | 9 +++++++++ > > Same thing, > > Please if possible put /proc/x into fs/proc/x.c so that it is easier to > find source. Not all /proc follows this convention but still. > > I don't like this name too (but less than the other one). > Is it Boot Image Format (BIF). If yes, maybe add it as /proc/cmdline.bif ? > > I don't know what's the good name. It turns out that the already existing /proc/bootconfig makes this new /proc/bootconfig_image unnecessary, so I have dropped this patch. Imagine my embarrassment upon learning that /proc/bootconfig has been around since v5.5! ;-) Thanx, Paul
diff --git a/fs/proc/cmdline.c b/fs/proc/cmdline.c index 1d0ef9d2949d..4ab5223198cb 100644 --- a/fs/proc/cmdline.c +++ b/fs/proc/cmdline.c @@ -20,6 +20,15 @@ static int cmdline_load_proc_show(struct seq_file *m, void *v) return 0; } +static int cmdline_image_proc_show(struct seq_file *m, void *v) +{ +#ifdef CONFIG_BOOT_CONFIG_FORCE + seq_puts(m, saved_bootconfig_string); + seq_putc(m, '\n'); +#endif + return 0; +} + static int __init proc_cmdline_init(void) { struct proc_dir_entry *pde; @@ -31,6 +40,9 @@ static int __init proc_cmdline_init(void) pde = proc_create_single("cmdline_load", 0, NULL, cmdline_load_proc_show); pde_make_permanent(pde); pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1; + pde = proc_create_single("cmdline_image", 0, NULL, cmdline_image_proc_show); + pde_make_permanent(pde); + pde->size = strnlen(saved_bootconfig_string, COMMAND_LINE_SIZE) + 1; } return 0; } diff --git a/include/linux/init.h b/include/linux/init.h index 29e75bbe7984..c075983c5015 100644 --- a/include/linux/init.h +++ b/include/linux/init.h @@ -14,7 +14,7 @@ #define __noinitretpoline #endif -/* These macros are used to mark some functions or +/* These macros are used to mark some functions or * initialized data (doesn't apply to uninitialized data) * as `initialization' functions. The kernel can take this * as hint that the function is used only during the initialization @@ -22,7 +22,7 @@ * * Usage: * For functions: - * + * * You should add __init immediately before the function name, like: * * static void __init initme(int x, int y) @@ -148,6 +148,7 @@ extern char boot_command_line[]; extern char *saved_command_line; extern unsigned int saved_command_line_len; extern unsigned int reset_devices; +extern char saved_bootconfig_string[]; /* used by init/main.c */ void setup_arch(char **); @@ -184,7 +185,7 @@ extern void (*late_time_init)(void); extern bool initcall_debug; #endif - + #ifndef MODULE #ifndef __ASSEMBLY__ @@ -192,8 +193,8 @@ extern bool initcall_debug; /* * initcalls are now grouped by functionality into separate * subsections. Ordering inside the subsections is determined - * by link order. - * For backwards compatibility, initcall() puts the call in + * by link order. + * For backwards compatibility, initcall() puts the call in * the device init subsection. * * The `id' arg to __define_initcall() is needed so that multiple initcalls diff --git a/init/main.c b/init/main.c index 2121685c479a..981170da0b1c 100644 --- a/init/main.c +++ b/init/main.c @@ -146,6 +146,11 @@ static char *extra_command_line; /* Extra init arguments */ static char *extra_init_args; +/* Untouched boot-config string */ +#ifdef CONFIG_BOOT_CONFIG_FORCE +char saved_bootconfig_string[COMMAND_LINE_SIZE] __ro_after_init; +#endif + #ifdef CONFIG_BOOT_CONFIG /* Is bootconfig on command line? */ static bool bootconfig_found; @@ -435,6 +440,10 @@ static void __init setup_boot_config(void) return; } +#ifdef CONFIG_BOOT_CONFIG_FORCE + strncpy(saved_bootconfig_string, data, COMMAND_LINE_SIZE); +#endif + if (size >= XBC_DATA_MAX) { pr_err("bootconfig size %ld greater than max size %d\n", (long)size, XBC_DATA_MAX);