Re: [PATCH] init/main.c: Print all command line when boot
On Tue, 19 May 2020 11:29:46 +0800
王程刚 wrote:
> Function pr_notice print max length maybe less than the command line length,
> need more times to print all.
> For example, arm64 has 2048 bytes command line length, but printk maximum
> length is only 1024 bytes.
Good catch, and if you use bootconfig, you can expand it longer than that.
>
> Signed-off-by: Chenggang Wang
> ---
> init/main.c | 12 +++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/init/main.c b/init/main.c
> index 03371976d387..4cf676cc3305 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -825,6 +825,16 @@ void __init __weak arch_call_rest_init(void)
> rest_init();
> }
>
> +static void __init print_cmdline(void)
> +{
> + const char *prefix = "Kernel command line: ";
> + int len = -strlen(prefix);
> +
> + len += pr_notice("%s%s\n", prefix, boot_command_line);
Why don't you use saved_command_line here? Those can be different
and the effective one is saved_command_line.
> + while (boot_command_line[len])
> + len += pr_notice("%s\n", _command_line[len]);
Also, don't append "\n" unless you are sure there is an actual
option separator (not a space, because the option can be quoted.)
Thank you,
--
Masami Hiramatsu
Re: [PATCH] init/main.c: Print all command line when boot
On Mon, May 18, 2020 at 10:09:34PM -0700, Joe Perches wrote:
> On Mon, 2020-05-18 at 20:44 -0700, Andrew Morton wrote:
> > On Tue, 19 May 2020 11:29:46 +0800 王程刚 wrote:
> >
> > > Function pr_notice print max length maybe less than the command line
> > > length,
> > > need more times to print all.
> > > For example, arm64 has 2048 bytes command line length, but printk maximum
> > > length is only 1024 bytes.
> >
> > I can see why that might be a problem!
> >
> > > --- a/init/main.c
> > > +++ b/init/main.c
> > > @@ -825,6 +825,16 @@ void __init __weak arch_call_rest_init(void)
> > > rest_init();
> > > }
> > >
> > > +static void __init print_cmdline(void)
> > > +{
> > > + const char *prefix = "Kernel command line: ";
> >
> > const char prefix[] = "...";
> >
> > might generate slightly more efficient code.
> >
> > > + int len = -strlen(prefix);
> >
> > hm, tricky. What the heck does printk() actually return to the caller?
> > Seems that we forgot to document this, and there are so many different
> > paths which a printk call can take internally that I'm not confident
> > that they all got it right!
>
> There is no use of the return value of any pr_ or
> dev_ or netdev_
> All the pr_ mechanisms (as functions) should return void.
> https://lore.kernel.org/lkml/1466739971-30399-1-git-send-email-...@perches.com/
>
> > > + len += pr_notice("%s%s\n", prefix, boot_command_line);
> > > + while (boot_command_line[len])
> > > + len += pr_notice("%s\n", _command_line[len]);
> > > +}
>
> More likely it'd be better to use a strlen(boot_command_line)
> and perhaps do something like print multiple lines with args
> using strchr(, ' ') at some largish value, say 132 or 256 chars
> maximum per line.
>
>
>
Should it use pr_cont to print the subsequent lines?
Re: [PATCH] init/main.c: Print all command line when boot
On Mon, 2020-05-18 at 20:44 -0700, Andrew Morton wrote:
> On Tue, 19 May 2020 11:29:46 +0800 王程刚 wrote:
>
> > Function pr_notice print max length maybe less than the command line length,
> > need more times to print all.
> > For example, arm64 has 2048 bytes command line length, but printk maximum
> > length is only 1024 bytes.
>
> I can see why that might be a problem!
>
> > --- a/init/main.c
> > +++ b/init/main.c
> > @@ -825,6 +825,16 @@ void __init __weak arch_call_rest_init(void)
> > rest_init();
> > }
> >
> > +static void __init print_cmdline(void)
> > +{
> > + const char *prefix = "Kernel command line: ";
>
> const char prefix[] = "...";
>
> might generate slightly more efficient code.
>
> > + int len = -strlen(prefix);
>
> hm, tricky. What the heck does printk() actually return to the caller?
> Seems that we forgot to document this, and there are so many different
> paths which a printk call can take internally that I'm not confident
> that they all got it right!
There is no use of the return value of any pr_ or
dev_ or netdev_ mechanisms (as functions) should return void.
https://lore.kernel.org/lkml/1466739971-30399-1-git-send-email-...@perches.com/
> > + len += pr_notice("%s%s\n", prefix, boot_command_line);
> > + while (boot_command_line[len])
> > + len += pr_notice("%s\n", _command_line[len]);
> > +}
More likely it'd be better to use a strlen(boot_command_line)
and perhaps do something like print multiple lines with args
using strchr(, ' ') at some largish value, say 132 or 256 chars
maximum per line.
Re: [PATCH] init/main.c: Print all command line when boot
On Tue, 19 May 2020 11:29:46 +0800 王程刚 wrote:
> Function pr_notice print max length maybe less than the command line length,
> need more times to print all.
> For example, arm64 has 2048 bytes command line length, but printk maximum
> length is only 1024 bytes.
I can see why that might be a problem!
> --- a/init/main.c
> +++ b/init/main.c
> @@ -825,6 +825,16 @@ void __init __weak arch_call_rest_init(void)
> rest_init();
> }
>
> +static void __init print_cmdline(void)
> +{
> + const char *prefix = "Kernel command line: ";
const char prefix[] = "...";
might generate slightly more efficient code.
> + int len = -strlen(prefix);
hm, tricky. What the heck does printk() actually return to the caller?
Seems that we forgot to document this, and there are so many different
paths which a printk call can take internally that I'm not confident
that they all got it right!
> + len += pr_notice("%s%s\n", prefix, boot_command_line);
> + while (boot_command_line[len])
> + len += pr_notice("%s\n", _command_line[len]);
> +}
Did you really intend to insert a \n into the output every 1024'th
character?
And what effect does this additional \n have upon the code logic?
Doesn't this cause the printk() return value to be one greater than
expected each time it is called?
>
> ...
>
[PATCH] init/main.c: Print all command line when boot
Function pr_notice print max length maybe less than the command line length,
need more times to print all.
For example, arm64 has 2048 bytes command line length, but printk maximum
length is only 1024 bytes.
Signed-off-by: Chenggang Wang
---
init/main.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/init/main.c b/init/main.c
index 03371976d387..4cf676cc3305 100644
--- a/init/main.c
+++ b/init/main.c
@@ -825,6 +825,16 @@ void __init __weak arch_call_rest_init(void)
rest_init();
}
+static void __init print_cmdline(void)
+{
+ const char *prefix = "Kernel command line: ";
+ int len = -strlen(prefix);
+
+ len += pr_notice("%s%s\n", prefix, boot_command_line);
+ while (boot_command_line[len])
+ len += pr_notice("%s\n", _command_line[len]);
+}
+
asmlinkage __visible void __init start_kernel(void)
{
char *command_line;
@@ -858,7 +868,7 @@ asmlinkage __visible void __init start_kernel(void)
build_all_zonelists(NULL);
page_alloc_init();
- pr_notice("Kernel command line: %s\n", saved_command_line);
+ print_cmdline();
/* parameters may set static keys */
jump_label_init();
parse_early_param();
--
2.20.1
