| Message ID | 20230728033701.817094-1-paulmck@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC,bootconfig] 1/2] fs/proc: Add /proc/cmdline_load for boot loader arguments | expand |
Hi Paul, Just a couple of nits: On Thu, 27 Jul 2023 20:37:00 -0700 "Paul E. McKenney" <paulmck@kernel.org> wrote: > > [ sfr: Apply kernel test robot feedback. ] This was a fix for my own build testing (I am not a bot (yet) :-)). > diff --git a/include/linux/init.h b/include/linux/init.h > index 266c3e1640d4..29e75bbe7984 100644 > --- a/include/linux/init.h > +++ b/include/linux/init.h > @@ -112,6 +112,7 @@ > #define __REFCONST .section ".ref.rodata", "a" > > #ifndef __ASSEMBLY__ > + Please remove this added blank line.
On Fri, Jul 28, 2023 at 02:06:35PM +1000, Stephen Rothwell wrote: > Hi Paul, > > Just a couple of nits: > > On Thu, 27 Jul 2023 20:37:00 -0700 "Paul E. McKenney" <paulmck@kernel.org> wrote: > > > > [ sfr: Apply kernel test robot feedback. ] > > This was a fix for my own build testing (I am not a bot (yet) :-)). Ah! This says "Stephen Rothwell made a change in response to kernel test robot feedback". I saw your change and saw the kernel test robot complaint, and gave the robot the benefit of the doubt. But if you saw this build bug yourself and object to crediting kernel test robot for also spotting it, please let me know and I will remove this line. > > diff --git a/include/linux/init.h b/include/linux/init.h > > index 266c3e1640d4..29e75bbe7984 100644 > > --- a/include/linux/init.h > > +++ b/include/linux/init.h > > @@ -112,6 +112,7 @@ > > #define __REFCONST .section ".ref.rodata", "a" > > > > #ifndef __ASSEMBLY__ > > + > > Please remove this added blank line. Good eyes, done! I should also split out the modifications to include/linux/init.h, which are now whitespace-only, shouldn't I? Thanx, Paul
Hi Paul, On Thu, 27 Jul 2023 20:37:00 -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 supplied by the boot loader. > This is especially important when these parameters are presented to the > boot loader by automation that might gather them from diverse sources. > > Therefore, provide a /proc/cmdline_load file that shows only those kernel > boot parameters supplied by the boot loader. If I understand correctly, /proc/cmdline_load is something like /proc/cmdline_load - `/proc/bootconfig | grep ^kernel\\.`. BTW, what about CONFIG_CMDLINE? We already have that Kconfig and it is also merged with the command line specified by boot loader. Should we also expose that? (when CONFIG_CMDLINE_OVERRIDE=y, we don't need it because cmdline is always overridden by the CONFIG_CMDLINE) Unfortunatelly, this option is implemented in each arch init, so we have to change all of them... Thank you, > > 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. > > [ sfr: Apply kernel test robot feedback. ] > > Co-developed-by: Stephen Rothwell <sfr@canb.auug.org.au> > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> > Co-developed-by: Arnd Bergmann <arnd@kernel.org> > Signed-off-by: Arnd Bergmann <arnd@kernel.org> > Signed-off-by: Paul E. McKenney <paulmck@kernel.org> > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > 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> > --- > fs/proc/cmdline.c | 13 +++++++++++++ > include/linux/init.h | 3 ++- > init/main.c | 2 +- > 3 files changed, 16 insertions(+), 2 deletions(-) > > diff --git a/fs/proc/cmdline.c b/fs/proc/cmdline.c > index a6f76121955f..1d0ef9d2949d 100644 > --- a/fs/proc/cmdline.c > +++ b/fs/proc/cmdline.c > @@ -3,6 +3,7 @@ > #include <linux/init.h> > #include <linux/proc_fs.h> > #include <linux/seq_file.h> > +#include <asm/setup.h> > #include "internal.h" > > static int cmdline_proc_show(struct seq_file *m, void *v) > @@ -12,6 +13,13 @@ static int cmdline_proc_show(struct seq_file *m, void *v) > return 0; > } > > +static int cmdline_load_proc_show(struct seq_file *m, void *v) > +{ > + seq_puts(m, boot_command_line); > + seq_putc(m, '\n'); > + return 0; > +} > + > static int __init proc_cmdline_init(void) > { > struct proc_dir_entry *pde; > @@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void) > pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show); > pde_make_permanent(pde); > pde->size = saved_command_line_len + 1; > + if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) { > + 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; > + } > return 0; > } > fs_initcall(proc_cmdline_init); > diff --git a/include/linux/init.h b/include/linux/init.h > index 266c3e1640d4..29e75bbe7984 100644 > --- a/include/linux/init.h > +++ b/include/linux/init.h > @@ -112,6 +112,7 @@ > #define __REFCONST .section ".ref.rodata", "a" > > #ifndef __ASSEMBLY__ > + > /* > * Used for initialization calls.. > */ > @@ -143,7 +144,7 @@ struct file_system_type; > > /* Defined in init/main.c */ > extern int do_one_initcall(initcall_t fn); > -extern char __initdata boot_command_line[]; > +extern char boot_command_line[]; FYI, boot_command_line[] is mixture of built-in cmdline string with bootloader cmdline string. > extern char *saved_command_line; > extern unsigned int saved_command_line_len; > extern unsigned int reset_devices; > diff --git a/init/main.c b/init/main.c > index ad920fac325c..2121685c479a 100644 > --- a/init/main.c > +++ b/init/main.c > @@ -135,7 +135,7 @@ EXPORT_SYMBOL(system_state); > void (*__initdata late_time_init)(void); > > /* Untouched command line saved by arch-specific code. */ > -char __initdata boot_command_line[COMMAND_LINE_SIZE]; > +char boot_command_line[COMMAND_LINE_SIZE] __ro_after_init; > /* Untouched saved command line (eg. for /proc) */ > char *saved_command_line __ro_after_init; > unsigned int saved_command_line_len __ro_after_init; > -- > 2.40.1 >
On Sat, Jul 29, 2023 at 11:29:29PM +0900, Masami Hiramatsu wrote: > Hi Paul, > > On Thu, 27 Jul 2023 20:37:00 -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 supplied by the boot loader. > > This is especially important when these parameters are presented to the > > boot loader by automation that might gather them from diverse sources. > > > > Therefore, provide a /proc/cmdline_load file that shows only those kernel > > boot parameters supplied by the boot loader. > > If I understand correctly, /proc/cmdline_load is something like > /proc/cmdline_load - `/proc/bootconfig | grep ^kernel\\.`. Yes, very much something like that. For one use case, suppose you have a kernel that gets some boot parameters from the boot loader and some from bootconfig. If you want to kexec() into a new kernel, you must tell kexec() what the kernel boot parameters are. However, you must *not* tell kexec() about any of the current kernel's parameters that came from bootconfig, because those should instead be supplied by the new kernel being kexec()ed into. So you must pass in only those parameters that came from the boot loader, hence my proposed /proc/cmdline_load. > BTW, what about CONFIG_CMDLINE? We already have that Kconfig and it is also > merged with the command line specified by boot loader. Should we also > expose that? (when CONFIG_CMDLINE_OVERRIDE=y, we don't need it because > cmdline is always overridden by the CONFIG_CMDLINE) Unfortunatelly, this > option is implemented in each arch init, so we have to change all of them... The use case is embedded systems, right? I have no idea whether they have a use case requiring this. Do those sorts of embedded systems use kexec()? (I don't know of any that do, but then again, I haven't been looking.) This arch init is in setup_arch(), correct? If so, one option is to make start_kernel() or something that it invokes make a copy of the command line just before invoking setup_arch(). Full disclosure: I have not yet looked at all the ins and outs of CONFIG_CMDLINE, so this suggestion should be viewed with appropriate skepticism. > Thank you, > > > > > 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. > > > > [ sfr: Apply kernel test robot feedback. ] > > > > Co-developed-by: Stephen Rothwell <sfr@canb.auug.org.au> > > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> > > Co-developed-by: Arnd Bergmann <arnd@kernel.org> > > Signed-off-by: Arnd Bergmann <arnd@kernel.org> > > Signed-off-by: Paul E. McKenney <paulmck@kernel.org> > > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > > 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> > > --- > > fs/proc/cmdline.c | 13 +++++++++++++ > > include/linux/init.h | 3 ++- > > init/main.c | 2 +- > > 3 files changed, 16 insertions(+), 2 deletions(-) > > > > diff --git a/fs/proc/cmdline.c b/fs/proc/cmdline.c > > index a6f76121955f..1d0ef9d2949d 100644 > > --- a/fs/proc/cmdline.c > > +++ b/fs/proc/cmdline.c > > @@ -3,6 +3,7 @@ > > #include <linux/init.h> > > #include <linux/proc_fs.h> > > #include <linux/seq_file.h> > > +#include <asm/setup.h> > > #include "internal.h" > > > > static int cmdline_proc_show(struct seq_file *m, void *v) > > @@ -12,6 +13,13 @@ static int cmdline_proc_show(struct seq_file *m, void *v) > > return 0; > > } > > > > +static int cmdline_load_proc_show(struct seq_file *m, void *v) > > +{ > > + seq_puts(m, boot_command_line); > > + seq_putc(m, '\n'); > > + return 0; > > +} > > + > > static int __init proc_cmdline_init(void) > > { > > struct proc_dir_entry *pde; > > @@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void) > > pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show); > > pde_make_permanent(pde); > > pde->size = saved_command_line_len + 1; > > + if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) { > > + 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; > > + } > > return 0; > > } > > fs_initcall(proc_cmdline_init); > > diff --git a/include/linux/init.h b/include/linux/init.h > > index 266c3e1640d4..29e75bbe7984 100644 > > --- a/include/linux/init.h > > +++ b/include/linux/init.h > > @@ -112,6 +112,7 @@ > > #define __REFCONST .section ".ref.rodata", "a" > > > > #ifndef __ASSEMBLY__ > > + > > /* > > * Used for initialization calls.. > > */ > > @@ -143,7 +144,7 @@ struct file_system_type; > > > > /* Defined in init/main.c */ > > extern int do_one_initcall(initcall_t fn); > > -extern char __initdata boot_command_line[]; > > +extern char boot_command_line[]; > > FYI, boot_command_line[] is mixture of built-in cmdline string with > bootloader cmdline string. So if we also need to separate out the CONFIG_CMDLINE arguments, then /proc/cmdline_load will need to come from some string saved off before the CONFIG_CMDLINE processing, correct? I would expect that to be a separate patch series, but if it is needed, I would be happy to look into setting it up, as long as I am in the area. My tests indicate that boot_command_line[] doesn't contain any bootconfig (and opposed to CONFIG_CMDLINE) arguments, but I could easily have missed some other corner-case configuration. And thank you for looking this over! Thanx, Paul > > extern char *saved_command_line; > > extern unsigned int saved_command_line_len; > > extern unsigned int reset_devices; > > diff --git a/init/main.c b/init/main.c > > index ad920fac325c..2121685c479a 100644 > > --- a/init/main.c > > +++ b/init/main.c > > @@ -135,7 +135,7 @@ EXPORT_SYMBOL(system_state); > > void (*__initdata late_time_init)(void); > > > > /* Untouched command line saved by arch-specific code. */ > > -char __initdata boot_command_line[COMMAND_LINE_SIZE]; > > +char boot_command_line[COMMAND_LINE_SIZE] __ro_after_init; > > /* Untouched saved command line (eg. for /proc) */ > > char *saved_command_line __ro_after_init; > > unsigned int saved_command_line_len __ro_after_init; > > -- > > 2.40.1 > > > > > -- > Masami Hiramatsu (Google) <mhiramat@kernel.org>
On Sat, 29 Jul 2023 09:16:56 -0700 "Paul E. McKenney" <paulmck@kernel.org> wrote: > On Sat, Jul 29, 2023 at 11:29:29PM +0900, Masami Hiramatsu wrote: > > Hi Paul, > > > > On Thu, 27 Jul 2023 20:37:00 -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 supplied by the boot loader. > > > This is especially important when these parameters are presented to the > > > boot loader by automation that might gather them from diverse sources. > > > > > > Therefore, provide a /proc/cmdline_load file that shows only those kernel > > > boot parameters supplied by the boot loader. > > > > If I understand correctly, /proc/cmdline_load is something like > > /proc/cmdline_load - `/proc/bootconfig | grep ^kernel\\.`. ^^^ /proc/cmdline - `/proc/bootconfig | grep ^kernel\\.` > > Yes, very much something like that. > > For one use case, suppose you have a kernel that gets some boot parameters > from the boot loader and some from bootconfig. If you want to kexec() > into a new kernel, you must tell kexec() what the kernel boot parameters > are. However, you must *not* tell kexec() about any of the current > kernel's parameters that came from bootconfig, because those should > instead be supplied by the new kernel being kexec()ed into. > > So you must pass in only those parameters that came from the boot loader, > hence my proposed /proc/cmdline_load. Ah, I got it. Indeed, for kexec, we need to drop the options from the bootconfig. > > > BTW, what about CONFIG_CMDLINE? We already have that Kconfig and it is also > > merged with the command line specified by boot loader. Should we also > > expose that? (when CONFIG_CMDLINE_OVERRIDE=y, we don't need it because > > cmdline is always overridden by the CONFIG_CMDLINE) Unfortunatelly, this > > option is implemented in each arch init, so we have to change all of them... > > The use case is embedded systems, right? I have no idea whether they > have a use case requiring this. Do those sorts of embedded systems > use kexec()? (I don't know of any that do, but then again, I haven't > been looking.) Not sure, I guess it is possible to use kexec() for kdump or warm reboot, but it should be rare and we can expand this if someone need it. > > This arch init is in setup_arch(), correct? If so, one option is to > make start_kernel() or something that it invokes make a copy of the > command line just before invoking setup_arch(). Full disclosure: I > have not yet looked at all the ins and outs of CONFIG_CMDLINE, so this > suggestion should be viewed with appropriate skepticism. Yeah, maybe it is the best way to do. Anyway, I understand the reason why we need this interface. Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> Thank you! > > > Thank you, > > > > > > > > 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. > > > > > > [ sfr: Apply kernel test robot feedback. ] > > > > > > Co-developed-by: Stephen Rothwell <sfr@canb.auug.org.au> > > > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> > > > Co-developed-by: Arnd Bergmann <arnd@kernel.org> > > > Signed-off-by: Arnd Bergmann <arnd@kernel.org> > > > Signed-off-by: Paul E. McKenney <paulmck@kernel.org> > > > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > > > 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> > > > --- > > > fs/proc/cmdline.c | 13 +++++++++++++ > > > include/linux/init.h | 3 ++- > > > init/main.c | 2 +- > > > 3 files changed, 16 insertions(+), 2 deletions(-) > > > > > > diff --git a/fs/proc/cmdline.c b/fs/proc/cmdline.c > > > index a6f76121955f..1d0ef9d2949d 100644 > > > --- a/fs/proc/cmdline.c > > > +++ b/fs/proc/cmdline.c > > > @@ -3,6 +3,7 @@ > > > #include <linux/init.h> > > > #include <linux/proc_fs.h> > > > #include <linux/seq_file.h> > > > +#include <asm/setup.h> > > > #include "internal.h" > > > > > > static int cmdline_proc_show(struct seq_file *m, void *v) > > > @@ -12,6 +13,13 @@ static int cmdline_proc_show(struct seq_file *m, void *v) > > > return 0; > > > } > > > > > > +static int cmdline_load_proc_show(struct seq_file *m, void *v) > > > +{ > > > + seq_puts(m, boot_command_line); > > > + seq_putc(m, '\n'); > > > + return 0; > > > +} > > > + > > > static int __init proc_cmdline_init(void) > > > { > > > struct proc_dir_entry *pde; > > > @@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void) > > > pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show); > > > pde_make_permanent(pde); > > > pde->size = saved_command_line_len + 1; > > > + if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) { > > > + 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; > > > + } > > > return 0; > > > } > > > fs_initcall(proc_cmdline_init); > > > diff --git a/include/linux/init.h b/include/linux/init.h > > > index 266c3e1640d4..29e75bbe7984 100644 > > > --- a/include/linux/init.h > > > +++ b/include/linux/init.h > > > @@ -112,6 +112,7 @@ > > > #define __REFCONST .section ".ref.rodata", "a" > > > > > > #ifndef __ASSEMBLY__ > > > + > > > /* > > > * Used for initialization calls.. > > > */ > > > @@ -143,7 +144,7 @@ struct file_system_type; > > > > > > /* Defined in init/main.c */ > > > extern int do_one_initcall(initcall_t fn); > > > -extern char __initdata boot_command_line[]; > > > +extern char boot_command_line[]; > > > > FYI, boot_command_line[] is mixture of built-in cmdline string with > > bootloader cmdline string. > > So if we also need to separate out the CONFIG_CMDLINE arguments, then > /proc/cmdline_load will need to come from some string saved off before > the CONFIG_CMDLINE processing, correct? I would expect that to be a > separate patch series, but if it is needed, I would be happy to look > into setting it up, as long as I am in the area. > > My tests indicate that boot_command_line[] doesn't contain any bootconfig > (and opposed to CONFIG_CMDLINE) arguments, but I could easily have missed > some other corner-case configuration. > > And thank you for looking this over! > > Thanx, Paul > > > > extern char *saved_command_line; > > > extern unsigned int saved_command_line_len; > > > extern unsigned int reset_devices; > > > diff --git a/init/main.c b/init/main.c > > > index ad920fac325c..2121685c479a 100644 > > > --- a/init/main.c > > > +++ b/init/main.c > > > @@ -135,7 +135,7 @@ EXPORT_SYMBOL(system_state); > > > void (*__initdata late_time_init)(void); > > > > > > /* Untouched command line saved by arch-specific code. */ > > > -char __initdata boot_command_line[COMMAND_LINE_SIZE]; > > > +char boot_command_line[COMMAND_LINE_SIZE] __ro_after_init; > > > /* Untouched saved command line (eg. for /proc) */ > > > char *saved_command_line __ro_after_init; > > > unsigned int saved_command_line_len __ro_after_init; > > > -- > > > 2.40.1 > > > > > > > > > -- > > Masami Hiramatsu (Google) <mhiramat@kernel.org>
On Sun, Jul 30, 2023 at 10:58:44AM +0900, Masami Hiramatsu wrote: > On Sat, 29 Jul 2023 09:16:56 -0700 > "Paul E. McKenney" <paulmck@kernel.org> wrote: > > > On Sat, Jul 29, 2023 at 11:29:29PM +0900, Masami Hiramatsu wrote: > > > Hi Paul, > > > > > > On Thu, 27 Jul 2023 20:37:00 -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 supplied by the boot loader. > > > > This is especially important when these parameters are presented to the > > > > boot loader by automation that might gather them from diverse sources. > > > > > > > > Therefore, provide a /proc/cmdline_load file that shows only those kernel > > > > boot parameters supplied by the boot loader. > > > > > > If I understand correctly, /proc/cmdline_load is something like > > > /proc/cmdline_load - `/proc/bootconfig | grep ^kernel\\.`. > > ^^^ /proc/cmdline - `/proc/bootconfig | grep ^kernel\\.` Heh! My mind autocorrected without me noticing. ;-) > > Yes, very much something like that. > > > > For one use case, suppose you have a kernel that gets some boot parameters > > from the boot loader and some from bootconfig. If you want to kexec() > > into a new kernel, you must tell kexec() what the kernel boot parameters > > are. However, you must *not* tell kexec() about any of the current > > kernel's parameters that came from bootconfig, because those should > > instead be supplied by the new kernel being kexec()ed into. > > > > So you must pass in only those parameters that came from the boot loader, > > hence my proposed /proc/cmdline_load. > > Ah, I got it. Indeed, for kexec, we need to drop the options from > the bootconfig. > > > > BTW, what about CONFIG_CMDLINE? We already have that Kconfig and it is also > > > merged with the command line specified by boot loader. Should we also > > > expose that? (when CONFIG_CMDLINE_OVERRIDE=y, we don't need it because > > > cmdline is always overridden by the CONFIG_CMDLINE) Unfortunatelly, this > > > option is implemented in each arch init, so we have to change all of them... > > > > The use case is embedded systems, right? I have no idea whether they > > have a use case requiring this. Do those sorts of embedded systems > > use kexec()? (I don't know of any that do, but then again, I haven't > > been looking.) > > Not sure, I guess it is possible to use kexec() for kdump or warm reboot, > but it should be rare and we can expand this if someone need it. Works for me! > > This arch init is in setup_arch(), correct? If so, one option is to > > make start_kernel() or something that it invokes make a copy of the > > command line just before invoking setup_arch(). Full disclosure: I > > have not yet looked at all the ins and outs of CONFIG_CMDLINE, so this > > suggestion should be viewed with appropriate skepticism. > > Yeah, maybe it is the best way to do. > Anyway, I understand the reason why we need this interface. > > Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> Thank you! I will apply your Acked-by on my next rebase. Thanx, Paul > Thank you! > > > > > > Thank you, > > > > > > > > > > > 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. > > > > > > > > [ sfr: Apply kernel test robot feedback. ] > > > > > > > > Co-developed-by: Stephen Rothwell <sfr@canb.auug.org.au> > > > > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> > > > > Co-developed-by: Arnd Bergmann <arnd@kernel.org> > > > > Signed-off-by: Arnd Bergmann <arnd@kernel.org> > > > > Signed-off-by: Paul E. McKenney <paulmck@kernel.org> > > > > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > > > > 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> > > > > --- > > > > fs/proc/cmdline.c | 13 +++++++++++++ > > > > include/linux/init.h | 3 ++- > > > > init/main.c | 2 +- > > > > 3 files changed, 16 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/fs/proc/cmdline.c b/fs/proc/cmdline.c > > > > index a6f76121955f..1d0ef9d2949d 100644 > > > > --- a/fs/proc/cmdline.c > > > > +++ b/fs/proc/cmdline.c > > > > @@ -3,6 +3,7 @@ > > > > #include <linux/init.h> > > > > #include <linux/proc_fs.h> > > > > #include <linux/seq_file.h> > > > > +#include <asm/setup.h> > > > > #include "internal.h" > > > > > > > > static int cmdline_proc_show(struct seq_file *m, void *v) > > > > @@ -12,6 +13,13 @@ static int cmdline_proc_show(struct seq_file *m, void *v) > > > > return 0; > > > > } > > > > > > > > +static int cmdline_load_proc_show(struct seq_file *m, void *v) > > > > +{ > > > > + seq_puts(m, boot_command_line); > > > > + seq_putc(m, '\n'); > > > > + return 0; > > > > +} > > > > + > > > > static int __init proc_cmdline_init(void) > > > > { > > > > struct proc_dir_entry *pde; > > > > @@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void) > > > > pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show); > > > > pde_make_permanent(pde); > > > > pde->size = saved_command_line_len + 1; > > > > + if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) { > > > > + 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; > > > > + } > > > > return 0; > > > > } > > > > fs_initcall(proc_cmdline_init); > > > > diff --git a/include/linux/init.h b/include/linux/init.h > > > > index 266c3e1640d4..29e75bbe7984 100644 > > > > --- a/include/linux/init.h > > > > +++ b/include/linux/init.h > > > > @@ -112,6 +112,7 @@ > > > > #define __REFCONST .section ".ref.rodata", "a" > > > > > > > > #ifndef __ASSEMBLY__ > > > > + > > > > /* > > > > * Used for initialization calls.. > > > > */ > > > > @@ -143,7 +144,7 @@ struct file_system_type; > > > > > > > > /* Defined in init/main.c */ > > > > extern int do_one_initcall(initcall_t fn); > > > > -extern char __initdata boot_command_line[]; > > > > +extern char boot_command_line[]; > > > > > > FYI, boot_command_line[] is mixture of built-in cmdline string with > > > bootloader cmdline string. > > > > So if we also need to separate out the CONFIG_CMDLINE arguments, then > > /proc/cmdline_load will need to come from some string saved off before > > the CONFIG_CMDLINE processing, correct? I would expect that to be a > > separate patch series, but if it is needed, I would be happy to look > > into setting it up, as long as I am in the area. > > > > My tests indicate that boot_command_line[] doesn't contain any bootconfig > > (and opposed to CONFIG_CMDLINE) arguments, but I could easily have missed > > some other corner-case configuration. > > > > And thank you for looking this over! > > > > Thanx, Paul > > > > > > extern char *saved_command_line; > > > > extern unsigned int saved_command_line_len; > > > > extern unsigned int reset_devices; > > > > diff --git a/init/main.c b/init/main.c > > > > index ad920fac325c..2121685c479a 100644 > > > > --- a/init/main.c > > > > +++ b/init/main.c > > > > @@ -135,7 +135,7 @@ EXPORT_SYMBOL(system_state); > > > > void (*__initdata late_time_init)(void); > > > > > > > > /* Untouched command line saved by arch-specific code. */ > > > > -char __initdata boot_command_line[COMMAND_LINE_SIZE]; > > > > +char boot_command_line[COMMAND_LINE_SIZE] __ro_after_init; > > > > /* Untouched saved command line (eg. for /proc) */ > > > > char *saved_command_line __ro_after_init; > > > > unsigned int saved_command_line_len __ro_after_init; > > > > -- > > > > 2.40.1 > > > > > > > > > > > > > -- > > > Masami Hiramatsu (Google) <mhiramat@kernel.org> > > > -- > Masami Hiramatsu (Google) <mhiramat@kernel.org>
On Thu, Jul 27, 2023 at 08:37:00PM -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 supplied by the boot loader. > This is especially important when these parameters are presented to the > boot loader by automation that might gather them from diverse sources. > > Therefore, provide a /proc/cmdline_load file that shows only those kernel > boot parameters supplied by the boot loader. > +static int cmdline_load_proc_show(struct seq_file *m, void *v) > +{ > + seq_puts(m, boot_command_line); > + seq_putc(m, '\n'); > + return 0; > +} > + > static int __init proc_cmdline_init(void) > { > struct proc_dir_entry *pde; > @@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void) > pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show); > pde_make_permanent(pde); > pde->size = saved_command_line_len + 1; > + if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) { > + 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; > + } Please add it as separate fs/proc/cmdline_load.c file so that name of the file matches name of the /proc file. The name "cmdline_load" is kind of non-descriptive. Mentioning "bootloader" somewhere should improve things.
On Fri, Aug 04, 2023 at 08:23:20PM +0300, Alexey Dobriyan wrote: > On Thu, Jul 27, 2023 at 08:37:00PM -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 supplied by the boot loader. > > This is especially important when these parameters are presented to the > > boot loader by automation that might gather them from diverse sources. > > > > Therefore, provide a /proc/cmdline_load file that shows only those kernel > > boot parameters supplied by the boot loader. > > > +static int cmdline_load_proc_show(struct seq_file *m, void *v) > > +{ > > + seq_puts(m, boot_command_line); > > + seq_putc(m, '\n'); > > + return 0; > > +} > > + > > static int __init proc_cmdline_init(void) > > { > > struct proc_dir_entry *pde; > > @@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void) > > pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show); > > pde_make_permanent(pde); > > pde->size = saved_command_line_len + 1; > > + if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) { > > + 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; > > + } > > Please add it as separate fs/proc/cmdline_load.c file so that name of > the file matches name of the /proc file. Thank you, will do! > The name "cmdline_load" is kind of non-descriptive. Mentioning "bootloader" > somewhere should improve things. If we can all quickly come to agreement on a name, I can of course easily change it. /proc/cmdline_bootloader? Better than /proc/cmdline_from_bootloader, I suppose. /proc/cmdline_bootldr? /proc/bootloader by analogy with /proc/bootconfig? Something else? Thanx, Paul
On Fri, Aug 04, 2023 at 10:36:17AM -0700, Paul E. McKenney wrote: > On Fri, Aug 04, 2023 at 08:23:20PM +0300, Alexey Dobriyan wrote: > > On Thu, Jul 27, 2023 at 08:37:00PM -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 supplied by the boot loader. > > > This is especially important when these parameters are presented to the > > > boot loader by automation that might gather them from diverse sources. > > > > > > Therefore, provide a /proc/cmdline_load file that shows only those kernel > > > boot parameters supplied by the boot loader. > > > > > +static int cmdline_load_proc_show(struct seq_file *m, void *v) > > > +{ > > > + seq_puts(m, boot_command_line); > > > + seq_putc(m, '\n'); > > > + return 0; > > > +} > > > + > > > static int __init proc_cmdline_init(void) > > > { > > > struct proc_dir_entry *pde; > > > @@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void) > > > pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show); > > > pde_make_permanent(pde); > > > pde->size = saved_command_line_len + 1; > > > + if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) { > > > + 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; > > > + } > > > > Please add it as separate fs/proc/cmdline_load.c file so that name of > > the file matches name of the /proc file. > > Thank you, will do! And here is an untested sneak preview, which I will fold into the original after testing. Thoughts? Thanx, Paul ------------------------------------------------------------------------ diff --git a/fs/proc/Makefile b/fs/proc/Makefile index bd08616ed8ba..094f3102eb9f 100644 --- a/fs/proc/Makefile +++ b/fs/proc/Makefile @@ -34,3 +34,4 @@ proc-$(CONFIG_PROC_VMCORE) += vmcore.o proc-$(CONFIG_PRINTK) += kmsg.o proc-$(CONFIG_PROC_PAGE_MONITOR) += page.o proc-$(CONFIG_BOOT_CONFIG) += bootconfig.o +proc-$(CONFIG_BOOT_CONFIG_FORCE) += cmdline_load.o diff --git a/fs/proc/cmdline.c b/fs/proc/cmdline.c index 1d0ef9d2949d..082def2c1cc6 100644 --- a/fs/proc/cmdline.c +++ b/fs/proc/cmdline.c @@ -13,13 +13,6 @@ static int cmdline_proc_show(struct seq_file *m, void *v) return 0; } -static int cmdline_load_proc_show(struct seq_file *m, void *v) -{ - seq_puts(m, boot_command_line); - seq_putc(m, '\n'); - return 0; -} - static int __init proc_cmdline_init(void) { struct proc_dir_entry *pde; @@ -27,11 +20,6 @@ static int __init proc_cmdline_init(void) pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show); pde_make_permanent(pde); pde->size = saved_command_line_len + 1; - if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) { - 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; - } return 0; } fs_initcall(proc_cmdline_init); diff --git a/fs/proc/cmdline_load.c b/fs/proc/cmdline_load.c new file mode 100644 index 000000000000..e3dccb3441ce --- /dev/null +++ b/fs/proc/cmdline_load.c @@ -0,0 +1,25 @@ +// SPDX-License-Identifier: GPL-2.0 +#include <linux/fs.h> +#include <linux/init.h> +#include <linux/proc_fs.h> +#include <linux/seq_file.h> +#include <asm/setup.h> +#include "internal.h" + +static int cmdline_load_proc_show(struct seq_file *m, void *v) +{ + seq_puts(m, boot_command_line); + seq_putc(m, '\n'); + return 0; +} + +static int __init proc_cmdline_load_init(void) +{ + struct proc_dir_entry *pde; + + 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; + return 0; +} +fs_initcall(proc_cmdline_load_init);
On Fri, 4 Aug 2023 10:36:17 -0700 "Paul E. McKenney" <paulmck@kernel.org> wrote: > On Fri, Aug 04, 2023 at 08:23:20PM +0300, Alexey Dobriyan wrote: > > On Thu, Jul 27, 2023 at 08:37:00PM -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 supplied by the boot loader. > > > This is especially important when these parameters are presented to the > > > boot loader by automation that might gather them from diverse sources. > > > > > > Therefore, provide a /proc/cmdline_load file that shows only those kernel > > > boot parameters supplied by the boot loader. > > > > > +static int cmdline_load_proc_show(struct seq_file *m, void *v) > > > +{ > > > + seq_puts(m, boot_command_line); > > > + seq_putc(m, '\n'); > > > + return 0; > > > +} > > > + > > > static int __init proc_cmdline_init(void) > > > { > > > struct proc_dir_entry *pde; > > > @@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void) > > > pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show); > > > pde_make_permanent(pde); > > > pde->size = saved_command_line_len + 1; > > > + if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) { > > > + 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; > > > + } > > > > Please add it as separate fs/proc/cmdline_load.c file so that name of > > the file matches name of the /proc file. > > Thank you, will do! > > > The name "cmdline_load" is kind of non-descriptive. Mentioning "bootloader" > > somewhere should improve things. > > If we can all quickly come to agreement on a name, I can of course easily > change it. > > /proc/cmdline_bootloader? Better than /proc/cmdline_from_bootloader, > I suppose. /proc/cmdline_bootldr? /proc/bootloader by analogy with > /proc/bootconfig? Something else? What about "/proc/raw_cmdline" ? Thank you, > > Thanx, Paul
On Mon, Aug 07, 2023 at 11:44:55AM +0900, Masami Hiramatsu wrote: > On Fri, 4 Aug 2023 10:36:17 -0700 > "Paul E. McKenney" <paulmck@kernel.org> wrote: > > > On Fri, Aug 04, 2023 at 08:23:20PM +0300, Alexey Dobriyan wrote: > > > On Thu, Jul 27, 2023 at 08:37:00PM -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 supplied by the boot loader. > > > > This is especially important when these parameters are presented to the > > > > boot loader by automation that might gather them from diverse sources. > > > > > > > > Therefore, provide a /proc/cmdline_load file that shows only those kernel > > > > boot parameters supplied by the boot loader. > > > > > > > +static int cmdline_load_proc_show(struct seq_file *m, void *v) > > > > +{ > > > > + seq_puts(m, boot_command_line); > > > > + seq_putc(m, '\n'); > > > > + return 0; > > > > +} > > > > + > > > > static int __init proc_cmdline_init(void) > > > > { > > > > struct proc_dir_entry *pde; > > > > @@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void) > > > > pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show); > > > > pde_make_permanent(pde); > > > > pde->size = saved_command_line_len + 1; > > > > + if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) { > > > > + 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; > > > > + } > > > > > > Please add it as separate fs/proc/cmdline_load.c file so that name of > > > the file matches name of the /proc file. > > > > Thank you, will do! > > > > > The name "cmdline_load" is kind of non-descriptive. Mentioning "bootloader" > > > somewhere should improve things. > > > > If we can all quickly come to agreement on a name, I can of course easily > > change it. > > > > /proc/cmdline_bootloader? Better than /proc/cmdline_from_bootloader, > > I suppose. /proc/cmdline_bootldr? /proc/bootloader by analogy with > > /proc/bootconfig? Something else? > > What about "/proc/raw_cmdline" ? That would work of me! Any objections to /proc/raw_cmdline? Going once... Thanx, Paul
On Sun, Aug 06, 2023 at 09:39:28PM -0700, Paul E. McKenney wrote: > On Mon, Aug 07, 2023 at 11:44:55AM +0900, Masami Hiramatsu wrote: > > On Fri, 4 Aug 2023 10:36:17 -0700 > > "Paul E. McKenney" <paulmck@kernel.org> wrote: > > > > > On Fri, Aug 04, 2023 at 08:23:20PM +0300, Alexey Dobriyan wrote: > > > > On Thu, Jul 27, 2023 at 08:37:00PM -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 supplied by the boot loader. > > > > > This is especially important when these parameters are presented to the > > > > > boot loader by automation that might gather them from diverse sources. > > > > > > > > > > Therefore, provide a /proc/cmdline_load file that shows only those kernel > > > > > boot parameters supplied by the boot loader. > > > > > > > > > +static int cmdline_load_proc_show(struct seq_file *m, void *v) > > > > > +{ > > > > > + seq_puts(m, boot_command_line); > > > > > + seq_putc(m, '\n'); > > > > > + return 0; > > > > > +} > > > > > + > > > > > static int __init proc_cmdline_init(void) > > > > > { > > > > > struct proc_dir_entry *pde; > > > > > @@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void) > > > > > pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show); > > > > > pde_make_permanent(pde); > > > > > pde->size = saved_command_line_len + 1; > > > > > + if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) { > > > > > + 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; > > > > > + } > > > > > > > > Please add it as separate fs/proc/cmdline_load.c file so that name of > > > > the file matches name of the /proc file. > > > > > > Thank you, will do! > > > > > > > The name "cmdline_load" is kind of non-descriptive. Mentioning "bootloader" > > > > somewhere should improve things. > > > > > > If we can all quickly come to agreement on a name, I can of course easily > > > change it. > > > > > > /proc/cmdline_bootloader? Better than /proc/cmdline_from_bootloader, > > > I suppose. /proc/cmdline_bootldr? /proc/bootloader by analogy with > > > /proc/bootconfig? Something else? > > > > What about "/proc/raw_cmdline" ? > > That would work of me! > > Any objections to /proc/raw_cmdline? > > Going once... Going twice... If I don't hear otherwise, /proc/raw_cmdline is is on Monday August 14 PDT. Thanx, Paul
On Sat, Aug 12, 2023 at 04:30:41PM -0700, Paul E. McKenney wrote: > On Sun, Aug 06, 2023 at 09:39:28PM -0700, Paul E. McKenney wrote: > > On Mon, Aug 07, 2023 at 11:44:55AM +0900, Masami Hiramatsu wrote: > > > On Fri, 4 Aug 2023 10:36:17 -0700 > > > "Paul E. McKenney" <paulmck@kernel.org> wrote: > > > > > > > On Fri, Aug 04, 2023 at 08:23:20PM +0300, Alexey Dobriyan wrote: > > > > > On Thu, Jul 27, 2023 at 08:37:00PM -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 supplied by the boot loader. > > > > > > This is especially important when these parameters are presented to the > > > > > > boot loader by automation that might gather them from diverse sources. > > > > > > > > > > > > Therefore, provide a /proc/cmdline_load file that shows only those kernel > > > > > > boot parameters supplied by the boot loader. > > > > > > > > > > > +static int cmdline_load_proc_show(struct seq_file *m, void *v) > > > > > > +{ > > > > > > + seq_puts(m, boot_command_line); > > > > > > + seq_putc(m, '\n'); > > > > > > + return 0; > > > > > > +} > > > > > > + > > > > > > static int __init proc_cmdline_init(void) > > > > > > { > > > > > > struct proc_dir_entry *pde; > > > > > > @@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void) > > > > > > pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show); > > > > > > pde_make_permanent(pde); > > > > > > pde->size = saved_command_line_len + 1; > > > > > > + if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) { > > > > > > + 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; > > > > > > + } > > > > > > > > > > Please add it as separate fs/proc/cmdline_load.c file so that name of > > > > > the file matches name of the /proc file. > > > > > > > > Thank you, will do! > > > > > > > > > The name "cmdline_load" is kind of non-descriptive. Mentioning "bootloader" > > > > > somewhere should improve things. > > > > > > > > If we can all quickly come to agreement on a name, I can of course easily > > > > change it. > > > > > > > > /proc/cmdline_bootloader? Better than /proc/cmdline_from_bootloader, > > > > I suppose. /proc/cmdline_bootldr? /proc/bootloader by analogy with > > > > /proc/bootconfig? Something else? > > > > > > What about "/proc/raw_cmdline" ? > > > > That would work of me! > > > > Any objections to /proc/raw_cmdline? > > > > Going once... > > Going twice... > > If I don't hear otherwise, /proc/raw_cmdline is is on Monday August 14 PDT. And gone! Please see below for the updated version. Thanx, Paul ------------------------------------------------------------------------ commit 0fe10f0d1873a6f6e287c0c5b45e9203b0e33c83 Author: Paul E. McKenney <paulmck@kernel.org> Date: Fri Jul 21 16:05:38 2023 -0700 fs/proc: Add /proc/raw_cmdline for boot loader arguments 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 supplied by the boot loader. This is especially important when these parameters are presented to the boot loader by automation that might gather them from diverse sources. It is also useful when booting the next kernel via kexec(), in which case it is necessary to supply only those kernel command-line arguments from the boot loader, and most definitely not those that were embedded into the current kernel. Therefore, provide a /proc/raw_cmdline file that shows only those kernel boot parameters supplied by the boot loader. Why put this in /proc? Because it is quite similar to /proc/cmdline, and /proc/bootconfig, so it makes sense to put it in the same place that those files are located. [ sfr: Apply kernel test robot feedback. ] [ paulmck: Apply Randy Dunlap feedback. ] [ paulmck: Apply naming feedback from Alexey Dobriyan and Masami Hiramatsu. ] Co-developed-by: Stephen Rothwell <sfr@canb.auug.org.au> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> Co-developed-by: Arnd Bergmann <arnd@kernel.org> Signed-off-by: Arnd Bergmann <arnd@kernel.org> Signed-off-by: Paul E. McKenney <paulmck@kernel.org> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Alexey Dobriyan <adobriyan@gmail.com> Cc: <linux-fsdevel@vger.kernel.org> diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst index 75a8c899ebcc..61419270c38f 100644 --- a/Documentation/filesystems/proc.rst +++ b/Documentation/filesystems/proc.rst @@ -723,6 +723,7 @@ files are there, and which are missing. partitions Table of partitions known to the system pci Deprecated info of PCI bus (new way -> /proc/bus/pci/, decoupled by lspci (2.4) + raw_cmdline Kernel command line obtained from kernel image (6.6) rtc Real time clock scsi SCSI info (see text) slabinfo Slab pool info diff --git a/fs/proc/Makefile b/fs/proc/Makefile index bd08616ed8ba..6182296f3c6b 100644 --- a/fs/proc/Makefile +++ b/fs/proc/Makefile @@ -34,3 +34,4 @@ proc-$(CONFIG_PROC_VMCORE) += vmcore.o proc-$(CONFIG_PRINTK) += kmsg.o proc-$(CONFIG_PROC_PAGE_MONITOR) += page.o proc-$(CONFIG_BOOT_CONFIG) += bootconfig.o +proc-$(CONFIG_BOOT_CONFIG_FORCE) += raw_cmdline.o diff --git a/fs/proc/raw_cmdline.c b/fs/proc/raw_cmdline.c new file mode 100644 index 000000000000..2e19eb89fc8e --- /dev/null +++ b/fs/proc/raw_cmdline.c @@ -0,0 +1,25 @@ +// SPDX-License-Identifier: GPL-2.0 +#include <linux/fs.h> +#include <linux/init.h> +#include <linux/proc_fs.h> +#include <linux/seq_file.h> +#include <asm/setup.h> +#include "internal.h" + +static int raw_cmdline_proc_show(struct seq_file *m, void *v) +{ + seq_puts(m, boot_command_line); + seq_putc(m, '\n'); + return 0; +} + +static int __init proc_raw_cmdline_init(void) +{ + struct proc_dir_entry *pde; + + pde = proc_create_single("raw_cmdline", 0, NULL, raw_cmdline_proc_show); + pde_make_permanent(pde); + pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1; + return 0; +} +fs_initcall(proc_raw_cmdline_init); diff --git a/include/linux/init.h b/include/linux/init.h index 266c3e1640d4..29e75bbe7984 100644 --- a/include/linux/init.h +++ b/include/linux/init.h @@ -112,6 +112,7 @@ #define __REFCONST .section ".ref.rodata", "a" #ifndef __ASSEMBLY__ + /* * Used for initialization calls.. */ @@ -143,7 +144,7 @@ struct file_system_type; /* Defined in init/main.c */ extern int do_one_initcall(initcall_t fn); -extern char __initdata boot_command_line[]; +extern char boot_command_line[]; extern char *saved_command_line; extern unsigned int saved_command_line_len; extern unsigned int reset_devices; diff --git a/init/main.c b/init/main.c index ad920fac325c..2121685c479a 100644 --- a/init/main.c +++ b/init/main.c @@ -135,7 +135,7 @@ EXPORT_SYMBOL(system_state); void (*__initdata late_time_init)(void); /* Untouched command line saved by arch-specific code. */ -char __initdata boot_command_line[COMMAND_LINE_SIZE]; +char boot_command_line[COMMAND_LINE_SIZE] __ro_after_init; /* Untouched saved command line (eg. for /proc) */ char *saved_command_line __ro_after_init; unsigned int saved_command_line_len __ro_after_init;
On Mon, 14 Aug 2023 16:08:29 -0700 "Paul E. McKenney" <paulmck@kernel.org> wrote: > On Sat, Aug 12, 2023 at 04:30:41PM -0700, Paul E. McKenney wrote: > > On Sun, Aug 06, 2023 at 09:39:28PM -0700, Paul E. McKenney wrote: > > > On Mon, Aug 07, 2023 at 11:44:55AM +0900, Masami Hiramatsu wrote: > > > > On Fri, 4 Aug 2023 10:36:17 -0700 > > > > "Paul E. McKenney" <paulmck@kernel.org> wrote: > > > > > > > > > On Fri, Aug 04, 2023 at 08:23:20PM +0300, Alexey Dobriyan wrote: > > > > > > On Thu, Jul 27, 2023 at 08:37:00PM -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 supplied by the boot loader. > > > > > > > This is especially important when these parameters are presented to the > > > > > > > boot loader by automation that might gather them from diverse sources. > > > > > > > > > > > > > > Therefore, provide a /proc/cmdline_load file that shows only those kernel > > > > > > > boot parameters supplied by the boot loader. > > > > > > > > > > > > > +static int cmdline_load_proc_show(struct seq_file *m, void *v) > > > > > > > +{ > > > > > > > + seq_puts(m, boot_command_line); > > > > > > > + seq_putc(m, '\n'); > > > > > > > + return 0; > > > > > > > +} > > > > > > > + > > > > > > > static int __init proc_cmdline_init(void) > > > > > > > { > > > > > > > struct proc_dir_entry *pde; > > > > > > > @@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void) > > > > > > > pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show); > > > > > > > pde_make_permanent(pde); > > > > > > > pde->size = saved_command_line_len + 1; > > > > > > > + if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) { > > > > > > > + 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; > > > > > > > + } > > > > > > > > > > > > Please add it as separate fs/proc/cmdline_load.c file so that name of > > > > > > the file matches name of the /proc file. > > > > > > > > > > Thank you, will do! > > > > > > > > > > > The name "cmdline_load" is kind of non-descriptive. Mentioning "bootloader" > > > > > > somewhere should improve things. > > > > > > > > > > If we can all quickly come to agreement on a name, I can of course easily > > > > > change it. > > > > > > > > > > /proc/cmdline_bootloader? Better than /proc/cmdline_from_bootloader, > > > > > I suppose. /proc/cmdline_bootldr? /proc/bootloader by analogy with > > > > > /proc/bootconfig? Something else? > > > > > > > > What about "/proc/raw_cmdline" ? > > > > > > That would work of me! > > > > > > Any objections to /proc/raw_cmdline? > > > > > > Going once... > > > > Going twice... > > > > If I don't hear otherwise, /proc/raw_cmdline is is on Monday August 14 PDT. > > And gone! > > Please see below for the updated version. OK, I'll pick this. Thanks! > > Thanx, Paul > > ------------------------------------------------------------------------ > > commit 0fe10f0d1873a6f6e287c0c5b45e9203b0e33c83 > Author: Paul E. McKenney <paulmck@kernel.org> > Date: Fri Jul 21 16:05:38 2023 -0700 > > fs/proc: Add /proc/raw_cmdline for boot loader arguments > > 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 supplied by the boot loader. > This is especially important when these parameters are presented to the > boot loader by automation that might gather them from diverse sources. > It is also useful when booting the next kernel via kexec(), in which > case it is necessary to supply only those kernel command-line arguments > from the boot loader, and most definitely not those that were embedded > into the current kernel. > > Therefore, provide a /proc/raw_cmdline file that shows only those kernel > boot parameters supplied by the boot loader. > > Why put this in /proc? Because it is quite similar to /proc/cmdline, > and /proc/bootconfig, so it makes sense to put it in the same place that > those files are located. > > [ sfr: Apply kernel test robot feedback. ] > [ paulmck: Apply Randy Dunlap feedback. ] > [ paulmck: Apply naming feedback from Alexey Dobriyan and Masami Hiramatsu. ] > > Co-developed-by: Stephen Rothwell <sfr@canb.auug.org.au> > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> > Co-developed-by: Arnd Bergmann <arnd@kernel.org> > Signed-off-by: Arnd Bergmann <arnd@kernel.org> > Signed-off-by: Paul E. McKenney <paulmck@kernel.org> > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Alexey Dobriyan <adobriyan@gmail.com> > Cc: <linux-fsdevel@vger.kernel.org> > > diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst > index 75a8c899ebcc..61419270c38f 100644 > --- a/Documentation/filesystems/proc.rst > +++ b/Documentation/filesystems/proc.rst > @@ -723,6 +723,7 @@ files are there, and which are missing. > partitions Table of partitions known to the system > pci Deprecated info of PCI bus (new way -> /proc/bus/pci/, > decoupled by lspci (2.4) > + raw_cmdline Kernel command line obtained from kernel image (6.6) > rtc Real time clock > scsi SCSI info (see text) > slabinfo Slab pool info > diff --git a/fs/proc/Makefile b/fs/proc/Makefile > index bd08616ed8ba..6182296f3c6b 100644 > --- a/fs/proc/Makefile > +++ b/fs/proc/Makefile > @@ -34,3 +34,4 @@ proc-$(CONFIG_PROC_VMCORE) += vmcore.o > proc-$(CONFIG_PRINTK) += kmsg.o > proc-$(CONFIG_PROC_PAGE_MONITOR) += page.o > proc-$(CONFIG_BOOT_CONFIG) += bootconfig.o > +proc-$(CONFIG_BOOT_CONFIG_FORCE) += raw_cmdline.o > diff --git a/fs/proc/raw_cmdline.c b/fs/proc/raw_cmdline.c > new file mode 100644 > index 000000000000..2e19eb89fc8e > --- /dev/null > +++ b/fs/proc/raw_cmdline.c > @@ -0,0 +1,25 @@ > +// SPDX-License-Identifier: GPL-2.0 > +#include <linux/fs.h> > +#include <linux/init.h> > +#include <linux/proc_fs.h> > +#include <linux/seq_file.h> > +#include <asm/setup.h> > +#include "internal.h" > + > +static int raw_cmdline_proc_show(struct seq_file *m, void *v) > +{ > + seq_puts(m, boot_command_line); > + seq_putc(m, '\n'); > + return 0; > +} > + > +static int __init proc_raw_cmdline_init(void) > +{ > + struct proc_dir_entry *pde; > + > + pde = proc_create_single("raw_cmdline", 0, NULL, raw_cmdline_proc_show); > + pde_make_permanent(pde); > + pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1; > + return 0; > +} > +fs_initcall(proc_raw_cmdline_init); > diff --git a/include/linux/init.h b/include/linux/init.h > index 266c3e1640d4..29e75bbe7984 100644 > --- a/include/linux/init.h > +++ b/include/linux/init.h > @@ -112,6 +112,7 @@ > #define __REFCONST .section ".ref.rodata", "a" > > #ifndef __ASSEMBLY__ > + > /* > * Used for initialization calls.. > */ > @@ -143,7 +144,7 @@ struct file_system_type; > > /* Defined in init/main.c */ > extern int do_one_initcall(initcall_t fn); > -extern char __initdata boot_command_line[]; > +extern char boot_command_line[]; > extern char *saved_command_line; > extern unsigned int saved_command_line_len; > extern unsigned int reset_devices; > diff --git a/init/main.c b/init/main.c > index ad920fac325c..2121685c479a 100644 > --- a/init/main.c > +++ b/init/main.c > @@ -135,7 +135,7 @@ EXPORT_SYMBOL(system_state); > void (*__initdata late_time_init)(void); > > /* Untouched command line saved by arch-specific code. */ > -char __initdata boot_command_line[COMMAND_LINE_SIZE]; > +char boot_command_line[COMMAND_LINE_SIZE] __ro_after_init; > /* Untouched saved command line (eg. for /proc) */ > char *saved_command_line __ro_after_init; > unsigned int saved_command_line_len __ro_after_init;
On Wed, 16 Aug 2023 18:40:03 +0900 Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote: > On Mon, 14 Aug 2023 16:08:29 -0700 > "Paul E. McKenney" <paulmck@kernel.org> wrote: > > > On Sat, Aug 12, 2023 at 04:30:41PM -0700, Paul E. McKenney wrote: > > > On Sun, Aug 06, 2023 at 09:39:28PM -0700, Paul E. McKenney wrote: > > > > On Mon, Aug 07, 2023 at 11:44:55AM +0900, Masami Hiramatsu wrote: > > > > > On Fri, 4 Aug 2023 10:36:17 -0700 > > > > > "Paul E. McKenney" <paulmck@kernel.org> wrote: > > > > > > > > > > > On Fri, Aug 04, 2023 at 08:23:20PM +0300, Alexey Dobriyan wrote: > > > > > > > On Thu, Jul 27, 2023 at 08:37:00PM -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 supplied by the boot loader. > > > > > > > > This is especially important when these parameters are presented to the > > > > > > > > boot loader by automation that might gather them from diverse sources. > > > > > > > > > > > > > > > > Therefore, provide a /proc/cmdline_load file that shows only those kernel > > > > > > > > boot parameters supplied by the boot loader. > > > > > > > > > > > > > > > +static int cmdline_load_proc_show(struct seq_file *m, void *v) > > > > > > > > +{ > > > > > > > > + seq_puts(m, boot_command_line); > > > > > > > > + seq_putc(m, '\n'); > > > > > > > > + return 0; > > > > > > > > +} > > > > > > > > + > > > > > > > > static int __init proc_cmdline_init(void) > > > > > > > > { > > > > > > > > struct proc_dir_entry *pde; > > > > > > > > @@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void) > > > > > > > > pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show); > > > > > > > > pde_make_permanent(pde); > > > > > > > > pde->size = saved_command_line_len + 1; > > > > > > > > + if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) { > > > > > > > > + 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; > > > > > > > > + } > > > > > > > > > > > > > > Please add it as separate fs/proc/cmdline_load.c file so that name of > > > > > > > the file matches name of the /proc file. > > > > > > > > > > > > Thank you, will do! > > > > > > > > > > > > > The name "cmdline_load" is kind of non-descriptive. Mentioning "bootloader" > > > > > > > somewhere should improve things. > > > > > > > > > > > > If we can all quickly come to agreement on a name, I can of course easily > > > > > > change it. > > > > > > > > > > > > /proc/cmdline_bootloader? Better than /proc/cmdline_from_bootloader, > > > > > > I suppose. /proc/cmdline_bootldr? /proc/bootloader by analogy with > > > > > > /proc/bootconfig? Something else? > > > > > > > > > > What about "/proc/raw_cmdline" ? > > > > > > > > That would work of me! > > > > > > > > Any objections to /proc/raw_cmdline? > > > > > > > > Going once... > > > > > > Going twice... > > > > > > If I don't hear otherwise, /proc/raw_cmdline is is on Monday August 14 PDT. > > > > And gone! > > > > Please see below for the updated version. > > OK, I'll pick this. Wait, is it OK to push this through bootconfig tree? Since this is not directly related to the bootconfig, fsdevel maintainer can pick this. I would like to ping to fsdevel people at first. Thank you, > > Thanks! > > > > > Thanx, Paul > > > > ------------------------------------------------------------------------ > > > > commit 0fe10f0d1873a6f6e287c0c5b45e9203b0e33c83 > > Author: Paul E. McKenney <paulmck@kernel.org> > > Date: Fri Jul 21 16:05:38 2023 -0700 > > > > fs/proc: Add /proc/raw_cmdline for boot loader arguments > > > > 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 supplied by the boot loader. > > This is especially important when these parameters are presented to the > > boot loader by automation that might gather them from diverse sources. > > It is also useful when booting the next kernel via kexec(), in which > > case it is necessary to supply only those kernel command-line arguments > > from the boot loader, and most definitely not those that were embedded > > into the current kernel. > > > > Therefore, provide a /proc/raw_cmdline file that shows only those kernel > > boot parameters supplied by the boot loader. > > > > Why put this in /proc? Because it is quite similar to /proc/cmdline, > > and /proc/bootconfig, so it makes sense to put it in the same place that > > those files are located. > > > > [ sfr: Apply kernel test robot feedback. ] > > [ paulmck: Apply Randy Dunlap feedback. ] > > [ paulmck: Apply naming feedback from Alexey Dobriyan and Masami Hiramatsu. ] > > > > Co-developed-by: Stephen Rothwell <sfr@canb.auug.org.au> > > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> > > Co-developed-by: Arnd Bergmann <arnd@kernel.org> > > Signed-off-by: Arnd Bergmann <arnd@kernel.org> > > Signed-off-by: Paul E. McKenney <paulmck@kernel.org> > > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > > Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > Cc: Andrew Morton <akpm@linux-foundation.org> > > Cc: Alexey Dobriyan <adobriyan@gmail.com> > > Cc: <linux-fsdevel@vger.kernel.org> > > > > diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst > > index 75a8c899ebcc..61419270c38f 100644 > > --- a/Documentation/filesystems/proc.rst > > +++ b/Documentation/filesystems/proc.rst > > @@ -723,6 +723,7 @@ files are there, and which are missing. > > partitions Table of partitions known to the system > > pci Deprecated info of PCI bus (new way -> /proc/bus/pci/, > > decoupled by lspci (2.4) > > + raw_cmdline Kernel command line obtained from kernel image (6.6) > > rtc Real time clock > > scsi SCSI info (see text) > > slabinfo Slab pool info > > diff --git a/fs/proc/Makefile b/fs/proc/Makefile > > index bd08616ed8ba..6182296f3c6b 100644 > > --- a/fs/proc/Makefile > > +++ b/fs/proc/Makefile > > @@ -34,3 +34,4 @@ proc-$(CONFIG_PROC_VMCORE) += vmcore.o > > proc-$(CONFIG_PRINTK) += kmsg.o > > proc-$(CONFIG_PROC_PAGE_MONITOR) += page.o > > proc-$(CONFIG_BOOT_CONFIG) += bootconfig.o > > +proc-$(CONFIG_BOOT_CONFIG_FORCE) += raw_cmdline.o > > diff --git a/fs/proc/raw_cmdline.c b/fs/proc/raw_cmdline.c > > new file mode 100644 > > index 000000000000..2e19eb89fc8e > > --- /dev/null > > +++ b/fs/proc/raw_cmdline.c > > @@ -0,0 +1,25 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +#include <linux/fs.h> > > +#include <linux/init.h> > > +#include <linux/proc_fs.h> > > +#include <linux/seq_file.h> > > +#include <asm/setup.h> > > +#include "internal.h" > > + > > +static int raw_cmdline_proc_show(struct seq_file *m, void *v) > > +{ > > + seq_puts(m, boot_command_line); > > + seq_putc(m, '\n'); > > + return 0; > > +} > > + > > +static int __init proc_raw_cmdline_init(void) > > +{ > > + struct proc_dir_entry *pde; > > + > > + pde = proc_create_single("raw_cmdline", 0, NULL, raw_cmdline_proc_show); > > + pde_make_permanent(pde); > > + pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1; > > + return 0; > > +} > > +fs_initcall(proc_raw_cmdline_init); > > diff --git a/include/linux/init.h b/include/linux/init.h > > index 266c3e1640d4..29e75bbe7984 100644 > > --- a/include/linux/init.h > > +++ b/include/linux/init.h > > @@ -112,6 +112,7 @@ > > #define __REFCONST .section ".ref.rodata", "a" > > > > #ifndef __ASSEMBLY__ > > + > > /* > > * Used for initialization calls.. > > */ > > @@ -143,7 +144,7 @@ struct file_system_type; > > > > /* Defined in init/main.c */ > > extern int do_one_initcall(initcall_t fn); > > -extern char __initdata boot_command_line[]; > > +extern char boot_command_line[]; > > extern char *saved_command_line; > > extern unsigned int saved_command_line_len; > > extern unsigned int reset_devices; > > diff --git a/init/main.c b/init/main.c > > index ad920fac325c..2121685c479a 100644 > > --- a/init/main.c > > +++ b/init/main.c > > @@ -135,7 +135,7 @@ EXPORT_SYMBOL(system_state); > > void (*__initdata late_time_init)(void); > > > > /* Untouched command line saved by arch-specific code. */ > > -char __initdata boot_command_line[COMMAND_LINE_SIZE]; > > +char boot_command_line[COMMAND_LINE_SIZE] __ro_after_init; > > /* Untouched saved command line (eg. for /proc) */ > > char *saved_command_line __ro_after_init; > > unsigned int saved_command_line_len __ro_after_init; > > > -- > Masami Hiramatsu (Google) <mhiramat@kernel.org>
On Thu, Aug 17, 2023 at 12:17:31AM +0900, Masami Hiramatsu wrote: > On Wed, 16 Aug 2023 18:40:03 +0900 > Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote: > > On Mon, 14 Aug 2023 16:08:29 -0700 > > "Paul E. McKenney" <paulmck@kernel.org> wrote: > > > On Sat, Aug 12, 2023 at 04:30:41PM -0700, Paul E. McKenney wrote: > > > > On Sun, Aug 06, 2023 at 09:39:28PM -0700, Paul E. McKenney wrote: > > > > > On Mon, Aug 07, 2023 at 11:44:55AM +0900, Masami Hiramatsu wrote: > > > > > > On Fri, 4 Aug 2023 10:36:17 -0700 > > > > > > "Paul E. McKenney" <paulmck@kernel.org> wrote: > > > > > > > On Fri, Aug 04, 2023 at 08:23:20PM +0300, Alexey Dobriyan wrote: > > > > > > > > On Thu, Jul 27, 2023 at 08:37:00PM -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 supplied by the boot loader. > > > > > > > > > This is especially important when these parameters are presented to the > > > > > > > > > boot loader by automation that might gather them from diverse sources. > > > > > > > > > > > > > > > > > > Therefore, provide a /proc/cmdline_load file that shows only those kernel > > > > > > > > > boot parameters supplied by the boot loader. > > > > > > > > > > > > > > > > > +static int cmdline_load_proc_show(struct seq_file *m, void *v) > > > > > > > > > +{ > > > > > > > > > + seq_puts(m, boot_command_line); > > > > > > > > > + seq_putc(m, '\n'); > > > > > > > > > + return 0; > > > > > > > > > +} > > > > > > > > > + > > > > > > > > > static int __init proc_cmdline_init(void) > > > > > > > > > { > > > > > > > > > struct proc_dir_entry *pde; > > > > > > > > > @@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void) > > > > > > > > > pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show); > > > > > > > > > pde_make_permanent(pde); > > > > > > > > > pde->size = saved_command_line_len + 1; > > > > > > > > > + if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) { > > > > > > > > > + 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; > > > > > > > > > + } > > > > > > > > > > > > > > > > Please add it as separate fs/proc/cmdline_load.c file so that name of > > > > > > > > the file matches name of the /proc file. > > > > > > > > > > > > > > Thank you, will do! > > > > > > > > > > > > > > > The name "cmdline_load" is kind of non-descriptive. Mentioning "bootloader" > > > > > > > > somewhere should improve things. > > > > > > > > > > > > > > If we can all quickly come to agreement on a name, I can of course easily > > > > > > > change it. > > > > > > > > > > > > > > /proc/cmdline_bootloader? Better than /proc/cmdline_from_bootloader, > > > > > > > I suppose. /proc/cmdline_bootldr? /proc/bootloader by analogy with > > > > > > > /proc/bootconfig? Something else? > > > > > > > > > > > > What about "/proc/raw_cmdline" ? > > > > > > > > > > That would work of me! > > > > > > > > > > Any objections to /proc/raw_cmdline? > > > > > > > > > > Going once... > > > > > > > > Going twice... > > > > > > > > If I don't hear otherwise, /proc/raw_cmdline is is on Monday August 14 PDT. > > > > > > And gone! > > > > > > Please see below for the updated version. > > > > OK, I'll pick this. > > Wait, is it OK to push this through bootconfig tree? Since this is not directly > related to the bootconfig, fsdevel maintainer can pick this. > I would like to ping to fsdevel people at first. Whichever path works best works for me! Here are the three commits I have queued, just please let me know when any of them are pulled into some other tree and I will drop them from -rcu. 97edd1291847 ("doc: Update /proc/cmdline documentation to include boot config") 0fe10f0d1873 ("fs/proc: Add /proc/raw_cmdline for boot loader arguments") 9192c6ac7516 ("doc: Add /proc/bootconfig to proc.rst") Thanx, Paul > Thank you, > > > > > Thanks! > > > > > > > > Thanx, Paul > > > > > > ------------------------------------------------------------------------ > > > > > > commit 0fe10f0d1873a6f6e287c0c5b45e9203b0e33c83 > > > Author: Paul E. McKenney <paulmck@kernel.org> > > > Date: Fri Jul 21 16:05:38 2023 -0700 > > > > > > fs/proc: Add /proc/raw_cmdline for boot loader arguments > > > > > > 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 supplied by the boot loader. > > > This is especially important when these parameters are presented to the > > > boot loader by automation that might gather them from diverse sources. > > > It is also useful when booting the next kernel via kexec(), in which > > > case it is necessary to supply only those kernel command-line arguments > > > from the boot loader, and most definitely not those that were embedded > > > into the current kernel. > > > > > > Therefore, provide a /proc/raw_cmdline file that shows only those kernel > > > boot parameters supplied by the boot loader. > > > > > > Why put this in /proc? Because it is quite similar to /proc/cmdline, > > > and /proc/bootconfig, so it makes sense to put it in the same place that > > > those files are located. > > > > > > [ sfr: Apply kernel test robot feedback. ] > > > [ paulmck: Apply Randy Dunlap feedback. ] > > > [ paulmck: Apply naming feedback from Alexey Dobriyan and Masami Hiramatsu. ] > > > > > > Co-developed-by: Stephen Rothwell <sfr@canb.auug.org.au> > > > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> > > > Co-developed-by: Arnd Bergmann <arnd@kernel.org> > > > Signed-off-by: Arnd Bergmann <arnd@kernel.org> > > > Signed-off-by: Paul E. McKenney <paulmck@kernel.org> > > > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > > > Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > > Cc: Andrew Morton <akpm@linux-foundation.org> > > > Cc: Alexey Dobriyan <adobriyan@gmail.com> > > > Cc: <linux-fsdevel@vger.kernel.org> > > > > > > diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst > > > index 75a8c899ebcc..61419270c38f 100644 > > > --- a/Documentation/filesystems/proc.rst > > > +++ b/Documentation/filesystems/proc.rst > > > @@ -723,6 +723,7 @@ files are there, and which are missing. > > > partitions Table of partitions known to the system > > > pci Deprecated info of PCI bus (new way -> /proc/bus/pci/, > > > decoupled by lspci (2.4) > > > + raw_cmdline Kernel command line obtained from kernel image (6.6) > > > rtc Real time clock > > > scsi SCSI info (see text) > > > slabinfo Slab pool info > > > diff --git a/fs/proc/Makefile b/fs/proc/Makefile > > > index bd08616ed8ba..6182296f3c6b 100644 > > > --- a/fs/proc/Makefile > > > +++ b/fs/proc/Makefile > > > @@ -34,3 +34,4 @@ proc-$(CONFIG_PROC_VMCORE) += vmcore.o > > > proc-$(CONFIG_PRINTK) += kmsg.o > > > proc-$(CONFIG_PROC_PAGE_MONITOR) += page.o > > > proc-$(CONFIG_BOOT_CONFIG) += bootconfig.o > > > +proc-$(CONFIG_BOOT_CONFIG_FORCE) += raw_cmdline.o > > > diff --git a/fs/proc/raw_cmdline.c b/fs/proc/raw_cmdline.c > > > new file mode 100644 > > > index 000000000000..2e19eb89fc8e > > > --- /dev/null > > > +++ b/fs/proc/raw_cmdline.c > > > @@ -0,0 +1,25 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +#include <linux/fs.h> > > > +#include <linux/init.h> > > > +#include <linux/proc_fs.h> > > > +#include <linux/seq_file.h> > > > +#include <asm/setup.h> > > > +#include "internal.h" > > > + > > > +static int raw_cmdline_proc_show(struct seq_file *m, void *v) > > > +{ > > > + seq_puts(m, boot_command_line); > > > + seq_putc(m, '\n'); > > > + return 0; > > > +} > > > + > > > +static int __init proc_raw_cmdline_init(void) > > > +{ > > > + struct proc_dir_entry *pde; > > > + > > > + pde = proc_create_single("raw_cmdline", 0, NULL, raw_cmdline_proc_show); > > > + pde_make_permanent(pde); > > > + pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1; > > > + return 0; > > > +} > > > +fs_initcall(proc_raw_cmdline_init); > > > diff --git a/include/linux/init.h b/include/linux/init.h > > > index 266c3e1640d4..29e75bbe7984 100644 > > > --- a/include/linux/init.h > > > +++ b/include/linux/init.h > > > @@ -112,6 +112,7 @@ > > > #define __REFCONST .section ".ref.rodata", "a" > > > > > > #ifndef __ASSEMBLY__ > > > + > > > /* > > > * Used for initialization calls.. > > > */ > > > @@ -143,7 +144,7 @@ struct file_system_type; > > > > > > /* Defined in init/main.c */ > > > extern int do_one_initcall(initcall_t fn); > > > -extern char __initdata boot_command_line[]; > > > +extern char boot_command_line[]; > > > extern char *saved_command_line; > > > extern unsigned int saved_command_line_len; > > > extern unsigned int reset_devices; > > > diff --git a/init/main.c b/init/main.c > > > index ad920fac325c..2121685c479a 100644 > > > --- a/init/main.c > > > +++ b/init/main.c > > > @@ -135,7 +135,7 @@ EXPORT_SYMBOL(system_state); > > > void (*__initdata late_time_init)(void); > > > > > > /* Untouched command line saved by arch-specific code. */ > > > -char __initdata boot_command_line[COMMAND_LINE_SIZE]; > > > +char boot_command_line[COMMAND_LINE_SIZE] __ro_after_init; > > > /* Untouched saved command line (eg. for /proc) */ > > > char *saved_command_line __ro_after_init; > > > unsigned int saved_command_line_len __ro_after_init; > > > > > > -- > > Masami Hiramatsu (Google) <mhiramat@kernel.org> > > > -- > Masami Hiramatsu (Google) <mhiramat@kernel.org>
On Wed, 16 Aug 2023 09:13:30 -0700 "Paul E. McKenney" <paulmck@kernel.org> wrote: > On Thu, Aug 17, 2023 at 12:17:31AM +0900, Masami Hiramatsu wrote: > > On Wed, 16 Aug 2023 18:40:03 +0900 > > Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote: > > > On Mon, 14 Aug 2023 16:08:29 -0700 > > > "Paul E. McKenney" <paulmck@kernel.org> wrote: > > > > On Sat, Aug 12, 2023 at 04:30:41PM -0700, Paul E. McKenney wrote: > > > > > On Sun, Aug 06, 2023 at 09:39:28PM -0700, Paul E. McKenney wrote: > > > > > > On Mon, Aug 07, 2023 at 11:44:55AM +0900, Masami Hiramatsu wrote: > > > > > > > On Fri, 4 Aug 2023 10:36:17 -0700 > > > > > > > "Paul E. McKenney" <paulmck@kernel.org> wrote: > > > > > > > > On Fri, Aug 04, 2023 at 08:23:20PM +0300, Alexey Dobriyan wrote: > > > > > > > > > On Thu, Jul 27, 2023 at 08:37:00PM -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 supplied by the boot loader. > > > > > > > > > > This is especially important when these parameters are presented to the > > > > > > > > > > boot loader by automation that might gather them from diverse sources. > > > > > > > > > > > > > > > > > > > > Therefore, provide a /proc/cmdline_load file that shows only those kernel > > > > > > > > > > boot parameters supplied by the boot loader. > > > > > > > > > > > > > > > > > > > +static int cmdline_load_proc_show(struct seq_file *m, void *v) > > > > > > > > > > +{ > > > > > > > > > > + seq_puts(m, boot_command_line); > > > > > > > > > > + seq_putc(m, '\n'); > > > > > > > > > > + return 0; > > > > > > > > > > +} > > > > > > > > > > + > > > > > > > > > > static int __init proc_cmdline_init(void) > > > > > > > > > > { > > > > > > > > > > struct proc_dir_entry *pde; > > > > > > > > > > @@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void) > > > > > > > > > > pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show); > > > > > > > > > > pde_make_permanent(pde); > > > > > > > > > > pde->size = saved_command_line_len + 1; > > > > > > > > > > + if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) { > > > > > > > > > > + 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; > > > > > > > > > > + } > > > > > > > > > > > > > > > > > > Please add it as separate fs/proc/cmdline_load.c file so that name of > > > > > > > > > the file matches name of the /proc file. > > > > > > > > > > > > > > > > Thank you, will do! > > > > > > > > > > > > > > > > > The name "cmdline_load" is kind of non-descriptive. Mentioning "bootloader" > > > > > > > > > somewhere should improve things. > > > > > > > > > > > > > > > > If we can all quickly come to agreement on a name, I can of course easily > > > > > > > > change it. > > > > > > > > > > > > > > > > /proc/cmdline_bootloader? Better than /proc/cmdline_from_bootloader, > > > > > > > > I suppose. /proc/cmdline_bootldr? /proc/bootloader by analogy with > > > > > > > > /proc/bootconfig? Something else? > > > > > > > > > > > > > > What about "/proc/raw_cmdline" ? > > > > > > > > > > > > That would work of me! > > > > > > > > > > > > Any objections to /proc/raw_cmdline? > > > > > > > > > > > > Going once... > > > > > > > > > > Going twice... > > > > > > > > > > If I don't hear otherwise, /proc/raw_cmdline is is on Monday August 14 PDT. > > > > > > > > And gone! > > > > > > > > Please see below for the updated version. > > > > > > OK, I'll pick this. > > > > Wait, is it OK to push this through bootconfig tree? Since this is not directly > > related to the bootconfig, fsdevel maintainer can pick this. > > I would like to ping to fsdevel people at first. > > Whichever path works best works for me! > > Here are the three commits I have queued, just please let me know when any > of them are pulled into some other tree and I will drop them from -rcu. > > 97edd1291847 ("doc: Update /proc/cmdline documentation to include boot config") > 0fe10f0d1873 ("fs/proc: Add /proc/raw_cmdline for boot loader arguments") > 9192c6ac7516 ("doc: Add /proc/bootconfig to proc.rst") OK, I picked and pushed to linux-trace's bootconfig/for-next branch. Thank you! > > Thanx, Paul > > > Thank you, > > > > > > > > Thanks! > > > > > > > > > > > Thanx, Paul > > > > > > > > ------------------------------------------------------------------------ > > > > > > > > commit 0fe10f0d1873a6f6e287c0c5b45e9203b0e33c83 > > > > Author: Paul E. McKenney <paulmck@kernel.org> > > > > Date: Fri Jul 21 16:05:38 2023 -0700 > > > > > > > > fs/proc: Add /proc/raw_cmdline for boot loader arguments > > > > > > > > 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 supplied by the boot loader. > > > > This is especially important when these parameters are presented to the > > > > boot loader by automation that might gather them from diverse sources. > > > > It is also useful when booting the next kernel via kexec(), in which > > > > case it is necessary to supply only those kernel command-line arguments > > > > from the boot loader, and most definitely not those that were embedded > > > > into the current kernel. > > > > > > > > Therefore, provide a /proc/raw_cmdline file that shows only those kernel > > > > boot parameters supplied by the boot loader. > > > > > > > > Why put this in /proc? Because it is quite similar to /proc/cmdline, > > > > and /proc/bootconfig, so it makes sense to put it in the same place that > > > > those files are located. > > > > > > > > [ sfr: Apply kernel test robot feedback. ] > > > > [ paulmck: Apply Randy Dunlap feedback. ] > > > > [ paulmck: Apply naming feedback from Alexey Dobriyan and Masami Hiramatsu. ] > > > > > > > > Co-developed-by: Stephen Rothwell <sfr@canb.auug.org.au> > > > > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> > > > > Co-developed-by: Arnd Bergmann <arnd@kernel.org> > > > > Signed-off-by: Arnd Bergmann <arnd@kernel.org> > > > > Signed-off-by: Paul E. McKenney <paulmck@kernel.org> > > > > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > > > > Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > > > Cc: Andrew Morton <akpm@linux-foundation.org> > > > > Cc: Alexey Dobriyan <adobriyan@gmail.com> > > > > Cc: <linux-fsdevel@vger.kernel.org> > > > > > > > > diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst > > > > index 75a8c899ebcc..61419270c38f 100644 > > > > --- a/Documentation/filesystems/proc.rst > > > > +++ b/Documentation/filesystems/proc.rst > > > > @@ -723,6 +723,7 @@ files are there, and which are missing. > > > > partitions Table of partitions known to the system > > > > pci Deprecated info of PCI bus (new way -> /proc/bus/pci/, > > > > decoupled by lspci (2.4) > > > > + raw_cmdline Kernel command line obtained from kernel image (6.6) > > > > rtc Real time clock > > > > scsi SCSI info (see text) > > > > slabinfo Slab pool info > > > > diff --git a/fs/proc/Makefile b/fs/proc/Makefile > > > > index bd08616ed8ba..6182296f3c6b 100644 > > > > --- a/fs/proc/Makefile > > > > +++ b/fs/proc/Makefile > > > > @@ -34,3 +34,4 @@ proc-$(CONFIG_PROC_VMCORE) += vmcore.o > > > > proc-$(CONFIG_PRINTK) += kmsg.o > > > > proc-$(CONFIG_PROC_PAGE_MONITOR) += page.o > > > > proc-$(CONFIG_BOOT_CONFIG) += bootconfig.o > > > > +proc-$(CONFIG_BOOT_CONFIG_FORCE) += raw_cmdline.o > > > > diff --git a/fs/proc/raw_cmdline.c b/fs/proc/raw_cmdline.c > > > > new file mode 100644 > > > > index 000000000000..2e19eb89fc8e > > > > --- /dev/null > > > > +++ b/fs/proc/raw_cmdline.c > > > > @@ -0,0 +1,25 @@ > > > > +// SPDX-License-Identifier: GPL-2.0 > > > > +#include <linux/fs.h> > > > > +#include <linux/init.h> > > > > +#include <linux/proc_fs.h> > > > > +#include <linux/seq_file.h> > > > > +#include <asm/setup.h> > > > > +#include "internal.h" > > > > + > > > > +static int raw_cmdline_proc_show(struct seq_file *m, void *v) > > > > +{ > > > > + seq_puts(m, boot_command_line); > > > > + seq_putc(m, '\n'); > > > > + return 0; > > > > +} > > > > + > > > > +static int __init proc_raw_cmdline_init(void) > > > > +{ > > > > + struct proc_dir_entry *pde; > > > > + > > > > + pde = proc_create_single("raw_cmdline", 0, NULL, raw_cmdline_proc_show); > > > > + pde_make_permanent(pde); > > > > + pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1; > > > > + return 0; > > > > +} > > > > +fs_initcall(proc_raw_cmdline_init); > > > > diff --git a/include/linux/init.h b/include/linux/init.h > > > > index 266c3e1640d4..29e75bbe7984 100644 > > > > --- a/include/linux/init.h > > > > +++ b/include/linux/init.h > > > > @@ -112,6 +112,7 @@ > > > > #define __REFCONST .section ".ref.rodata", "a" > > > > > > > > #ifndef __ASSEMBLY__ > > > > + > > > > /* > > > > * Used for initialization calls.. > > > > */ > > > > @@ -143,7 +144,7 @@ struct file_system_type; > > > > > > > > /* Defined in init/main.c */ > > > > extern int do_one_initcall(initcall_t fn); > > > > -extern char __initdata boot_command_line[]; > > > > +extern char boot_command_line[]; > > > > extern char *saved_command_line; > > > > extern unsigned int saved_command_line_len; > > > > extern unsigned int reset_devices; > > > > diff --git a/init/main.c b/init/main.c > > > > index ad920fac325c..2121685c479a 100644 > > > > --- a/init/main.c > > > > +++ b/init/main.c > > > > @@ -135,7 +135,7 @@ EXPORT_SYMBOL(system_state); > > > > void (*__initdata late_time_init)(void); > > > > > > > > /* Untouched command line saved by arch-specific code. */ > > > > -char __initdata boot_command_line[COMMAND_LINE_SIZE]; > > > > +char boot_command_line[COMMAND_LINE_SIZE] __ro_after_init; > > > > /* Untouched saved command line (eg. for /proc) */ > > > > char *saved_command_line __ro_after_init; > > > > unsigned int saved_command_line_len __ro_after_init; > > > > > > > > > -- > > > Masami Hiramatsu (Google) <mhiramat@kernel.org> > > > > > > -- > > Masami Hiramatsu (Google) <mhiramat@kernel.org>
On Sun, Aug 20, 2023 at 11:14:01PM +0900, Masami Hiramatsu wrote: > On Wed, 16 Aug 2023 09:13:30 -0700 > "Paul E. McKenney" <paulmck@kernel.org> wrote: > > On Thu, Aug 17, 2023 at 12:17:31AM +0900, Masami Hiramatsu wrote: > > > On Wed, 16 Aug 2023 18:40:03 +0900 > > > Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote: > > > > On Mon, 14 Aug 2023 16:08:29 -0700 > > > > "Paul E. McKenney" <paulmck@kernel.org> wrote: > > > > > On Sat, Aug 12, 2023 at 04:30:41PM -0700, Paul E. McKenney wrote: > > > > > > On Sun, Aug 06, 2023 at 09:39:28PM -0700, Paul E. McKenney wrote: > > > > > > > On Mon, Aug 07, 2023 at 11:44:55AM +0900, Masami Hiramatsu wrote: > > > > > > > > On Fri, 4 Aug 2023 10:36:17 -0700 > > > > > > > > "Paul E. McKenney" <paulmck@kernel.org> wrote: > > > > > > > > > On Fri, Aug 04, 2023 at 08:23:20PM +0300, Alexey Dobriyan wrote: > > > > > > > > > > On Thu, Jul 27, 2023 at 08:37:00PM -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 supplied by the boot loader. > > > > > > > > > > > This is especially important when these parameters are presented to the > > > > > > > > > > > boot loader by automation that might gather them from diverse sources. > > > > > > > > > > > > > > > > > > > > > > Therefore, provide a /proc/cmdline_load file that shows only those kernel > > > > > > > > > > > boot parameters supplied by the boot loader. > > > > > > > > > > > > > > > > > > > > > +static int cmdline_load_proc_show(struct seq_file *m, void *v) > > > > > > > > > > > +{ > > > > > > > > > > > + seq_puts(m, boot_command_line); > > > > > > > > > > > + seq_putc(m, '\n'); > > > > > > > > > > > + return 0; > > > > > > > > > > > +} > > > > > > > > > > > + > > > > > > > > > > > static int __init proc_cmdline_init(void) > > > > > > > > > > > { > > > > > > > > > > > struct proc_dir_entry *pde; > > > > > > > > > > > @@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void) > > > > > > > > > > > pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show); > > > > > > > > > > > pde_make_permanent(pde); > > > > > > > > > > > pde->size = saved_command_line_len + 1; > > > > > > > > > > > + if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) { > > > > > > > > > > > + 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; > > > > > > > > > > > + } > > > > > > > > > > > > > > > > > > > > Please add it as separate fs/proc/cmdline_load.c file so that name of > > > > > > > > > > the file matches name of the /proc file. > > > > > > > > > > > > > > > > > > Thank you, will do! > > > > > > > > > > > > > > > > > > > The name "cmdline_load" is kind of non-descriptive. Mentioning "bootloader" > > > > > > > > > > somewhere should improve things. > > > > > > > > > > > > > > > > > > If we can all quickly come to agreement on a name, I can of course easily > > > > > > > > > change it. > > > > > > > > > > > > > > > > > > /proc/cmdline_bootloader? Better than /proc/cmdline_from_bootloader, > > > > > > > > > I suppose. /proc/cmdline_bootldr? /proc/bootloader by analogy with > > > > > > > > > /proc/bootconfig? Something else? > > > > > > > > > > > > > > > > What about "/proc/raw_cmdline" ? > > > > > > > > > > > > > > That would work of me! > > > > > > > > > > > > > > Any objections to /proc/raw_cmdline? > > > > > > > > > > > > > > Going once... > > > > > > > > > > > > Going twice... > > > > > > > > > > > > If I don't hear otherwise, /proc/raw_cmdline is is on Monday August 14 PDT. > > > > > > > > > > And gone! > > > > > > > > > > Please see below for the updated version. > > > > > > > > OK, I'll pick this. > > > > > > Wait, is it OK to push this through bootconfig tree? Since this is not directly > > > related to the bootconfig, fsdevel maintainer can pick this. > > > I would like to ping to fsdevel people at first. > > > > Whichever path works best works for me! > > > > Here are the three commits I have queued, just please let me know when any > > of them are pulled into some other tree and I will drop them from -rcu. > > > > 97edd1291847 ("doc: Update /proc/cmdline documentation to include boot config") > > 0fe10f0d1873 ("fs/proc: Add /proc/raw_cmdline for boot loader arguments") > > 9192c6ac7516 ("doc: Add /proc/bootconfig to proc.rst") > > OK, I picked and pushed to linux-trace's bootconfig/for-next branch. Thank you, and I will drop them from -rcu on my next rebase. Thanx, Paul > > > Thank you, > > > > > > > > > > > Thanks! > > > > > > > > > > > > > > Thanx, Paul > > > > > > > > > > ------------------------------------------------------------------------ > > > > > > > > > > commit 0fe10f0d1873a6f6e287c0c5b45e9203b0e33c83 > > > > > Author: Paul E. McKenney <paulmck@kernel.org> > > > > > Date: Fri Jul 21 16:05:38 2023 -0700 > > > > > > > > > > fs/proc: Add /proc/raw_cmdline for boot loader arguments > > > > > > > > > > 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 supplied by the boot loader. > > > > > This is especially important when these parameters are presented to the > > > > > boot loader by automation that might gather them from diverse sources. > > > > > It is also useful when booting the next kernel via kexec(), in which > > > > > case it is necessary to supply only those kernel command-line arguments > > > > > from the boot loader, and most definitely not those that were embedded > > > > > into the current kernel. > > > > > > > > > > Therefore, provide a /proc/raw_cmdline file that shows only those kernel > > > > > boot parameters supplied by the boot loader. > > > > > > > > > > Why put this in /proc? Because it is quite similar to /proc/cmdline, > > > > > and /proc/bootconfig, so it makes sense to put it in the same place that > > > > > those files are located. > > > > > > > > > > [ sfr: Apply kernel test robot feedback. ] > > > > > [ paulmck: Apply Randy Dunlap feedback. ] > > > > > [ paulmck: Apply naming feedback from Alexey Dobriyan and Masami Hiramatsu. ] > > > > > > > > > > Co-developed-by: Stephen Rothwell <sfr@canb.auug.org.au> > > > > > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> > > > > > Co-developed-by: Arnd Bergmann <arnd@kernel.org> > > > > > Signed-off-by: Arnd Bergmann <arnd@kernel.org> > > > > > Signed-off-by: Paul E. McKenney <paulmck@kernel.org> > > > > > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > > > > > Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > > > > Cc: Andrew Morton <akpm@linux-foundation.org> > > > > > Cc: Alexey Dobriyan <adobriyan@gmail.com> > > > > > Cc: <linux-fsdevel@vger.kernel.org> > > > > > > > > > > diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst > > > > > index 75a8c899ebcc..61419270c38f 100644 > > > > > --- a/Documentation/filesystems/proc.rst > > > > > +++ b/Documentation/filesystems/proc.rst > > > > > @@ -723,6 +723,7 @@ files are there, and which are missing. > > > > > partitions Table of partitions known to the system > > > > > pci Deprecated info of PCI bus (new way -> /proc/bus/pci/, > > > > > decoupled by lspci (2.4) > > > > > + raw_cmdline Kernel command line obtained from kernel image (6.6) > > > > > rtc Real time clock > > > > > scsi SCSI info (see text) > > > > > slabinfo Slab pool info > > > > > diff --git a/fs/proc/Makefile b/fs/proc/Makefile > > > > > index bd08616ed8ba..6182296f3c6b 100644 > > > > > --- a/fs/proc/Makefile > > > > > +++ b/fs/proc/Makefile > > > > > @@ -34,3 +34,4 @@ proc-$(CONFIG_PROC_VMCORE) += vmcore.o > > > > > proc-$(CONFIG_PRINTK) += kmsg.o > > > > > proc-$(CONFIG_PROC_PAGE_MONITOR) += page.o > > > > > proc-$(CONFIG_BOOT_CONFIG) += bootconfig.o > > > > > +proc-$(CONFIG_BOOT_CONFIG_FORCE) += raw_cmdline.o > > > > > diff --git a/fs/proc/raw_cmdline.c b/fs/proc/raw_cmdline.c > > > > > new file mode 100644 > > > > > index 000000000000..2e19eb89fc8e > > > > > --- /dev/null > > > > > +++ b/fs/proc/raw_cmdline.c > > > > > @@ -0,0 +1,25 @@ > > > > > +// SPDX-License-Identifier: GPL-2.0 > > > > > +#include <linux/fs.h> > > > > > +#include <linux/init.h> > > > > > +#include <linux/proc_fs.h> > > > > > +#include <linux/seq_file.h> > > > > > +#include <asm/setup.h> > > > > > +#include "internal.h" > > > > > + > > > > > +static int raw_cmdline_proc_show(struct seq_file *m, void *v) > > > > > +{ > > > > > + seq_puts(m, boot_command_line); > > > > > + seq_putc(m, '\n'); > > > > > + return 0; > > > > > +} > > > > > + > > > > > +static int __init proc_raw_cmdline_init(void) > > > > > +{ > > > > > + struct proc_dir_entry *pde; > > > > > + > > > > > + pde = proc_create_single("raw_cmdline", 0, NULL, raw_cmdline_proc_show); > > > > > + pde_make_permanent(pde); > > > > > + pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1; > > > > > + return 0; > > > > > +} > > > > > +fs_initcall(proc_raw_cmdline_init); > > > > > diff --git a/include/linux/init.h b/include/linux/init.h > > > > > index 266c3e1640d4..29e75bbe7984 100644 > > > > > --- a/include/linux/init.h > > > > > +++ b/include/linux/init.h > > > > > @@ -112,6 +112,7 @@ > > > > > #define __REFCONST .section ".ref.rodata", "a" > > > > > > > > > > #ifndef __ASSEMBLY__ > > > > > + > > > > > /* > > > > > * Used for initialization calls.. > > > > > */ > > > > > @@ -143,7 +144,7 @@ struct file_system_type; > > > > > > > > > > /* Defined in init/main.c */ > > > > > extern int do_one_initcall(initcall_t fn); > > > > > -extern char __initdata boot_command_line[]; > > > > > +extern char boot_command_line[]; > > > > > extern char *saved_command_line; > > > > > extern unsigned int saved_command_line_len; > > > > > extern unsigned int reset_devices; > > > > > diff --git a/init/main.c b/init/main.c > > > > > index ad920fac325c..2121685c479a 100644 > > > > > --- a/init/main.c > > > > > +++ b/init/main.c > > > > > @@ -135,7 +135,7 @@ EXPORT_SYMBOL(system_state); > > > > > void (*__initdata late_time_init)(void); > > > > > > > > > > /* Untouched command line saved by arch-specific code. */ > > > > > -char __initdata boot_command_line[COMMAND_LINE_SIZE]; > > > > > +char boot_command_line[COMMAND_LINE_SIZE] __ro_after_init; > > > > > /* Untouched saved command line (eg. for /proc) */ > > > > > char *saved_command_line __ro_after_init; > > > > > unsigned int saved_command_line_len __ro_after_init; > > > > > > > > > > > > -- > > > > Masami Hiramatsu (Google) <mhiramat@kernel.org> > > > > > > > > > -- > > > Masami Hiramatsu (Google) <mhiramat@kernel.org> > > > -- > Masami Hiramatsu (Google) <mhiramat@kernel.org>
diff --git a/fs/proc/cmdline.c b/fs/proc/cmdline.c index a6f76121955f..1d0ef9d2949d 100644 --- a/fs/proc/cmdline.c +++ b/fs/proc/cmdline.c @@ -3,6 +3,7 @@ #include <linux/init.h> #include <linux/proc_fs.h> #include <linux/seq_file.h> +#include <asm/setup.h> #include "internal.h" static int cmdline_proc_show(struct seq_file *m, void *v) @@ -12,6 +13,13 @@ static int cmdline_proc_show(struct seq_file *m, void *v) return 0; } +static int cmdline_load_proc_show(struct seq_file *m, void *v) +{ + seq_puts(m, boot_command_line); + seq_putc(m, '\n'); + return 0; +} + static int __init proc_cmdline_init(void) { struct proc_dir_entry *pde; @@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void) pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show); pde_make_permanent(pde); pde->size = saved_command_line_len + 1; + if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) { + 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; + } return 0; } fs_initcall(proc_cmdline_init); diff --git a/include/linux/init.h b/include/linux/init.h index 266c3e1640d4..29e75bbe7984 100644 --- a/include/linux/init.h +++ b/include/linux/init.h @@ -112,6 +112,7 @@ #define __REFCONST .section ".ref.rodata", "a" #ifndef __ASSEMBLY__ + /* * Used for initialization calls.. */ @@ -143,7 +144,7 @@ struct file_system_type; /* Defined in init/main.c */ extern int do_one_initcall(initcall_t fn); -extern char __initdata boot_command_line[]; +extern char boot_command_line[]; extern char *saved_command_line; extern unsigned int saved_command_line_len; extern unsigned int reset_devices; diff --git a/init/main.c b/init/main.c index ad920fac325c..2121685c479a 100644 --- a/init/main.c +++ b/init/main.c @@ -135,7 +135,7 @@ EXPORT_SYMBOL(system_state); void (*__initdata late_time_init)(void); /* Untouched command line saved by arch-specific code. */ -char __initdata boot_command_line[COMMAND_LINE_SIZE]; +char boot_command_line[COMMAND_LINE_SIZE] __ro_after_init; /* Untouched saved command line (eg. for /proc) */ char *saved_command_line __ro_after_init; unsigned int saved_command_line_len __ro_after_init;