The prior message is confusing users, which is the exact opposite of the
goal. Try and make it clear (without needing to look at the kernel
source) that the message is indicating one of the following
situations:
1. the param is misspelled
2. the param is not valid due to the kernel configuration
3. the param is intended for init but isn't after the '--'
delineator on the command line
On that same topic, also make it clear that these params are passed to
init still despite not being after the delineator.
Fixes: 86d1919a4fb0 ("init: print out unknown kernel parameters")
Signed-off-by: Andrew Halaney <[email protected]>
---
I'm not a huge fan of the wall of text this adds, but at the same time
I've had a few people come to me with confusion about the message and
concern that userspace isn't getting the params (not the case here, it's
just a cosmetic message). I'm open to better ideas on how to express
what I describe in the commit message, or if people think the message is
more confusing than useful a full revert would be ok with me too
(although I do think it is useful personally).
Thanks,
Andrew
init/main.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/init/main.c b/init/main.c
index ee4d3e1b3eb9..8dc88c2386ee 100644
--- a/init/main.c
+++ b/init/main.c
@@ -925,6 +925,10 @@ static void __init print_unknown_bootoptions(void)
for (p = &envp_init[2]; *p; p++)
end += sprintf(end, " %s", *p);
+ pr_notice("The kernel command line has unknown parameters. They are either\n");
+ pr_notice("misspelled, not valid for the current kernel configuration,\n");
+ pr_notice("or are meant for init but are not after the '--' delineator. They will\n");
+ pr_notice("be passed to init along with those after '--' on the command line.\n");
pr_notice("Unknown command line parameters:%s\n", unknown_options);
memblock_free(unknown_options, len);
}
--
2.31.1
On Tue, 12 Oct 2021 16:35:23 -0500
Andrew Halaney <[email protected]> wrote:
> --- a/init/main.c
> +++ b/init/main.c
> @@ -925,6 +925,10 @@ static void __init print_unknown_bootoptions(void)
> for (p = &envp_init[2]; *p; p++)
> end += sprintf(end, " %s", *p);
>
> + pr_notice("The kernel command line has unknown parameters. They are either\n");
> + pr_notice("misspelled, not valid for the current kernel configuration,\n");
> + pr_notice("or are meant for init but are not after the '--' delineator. They will\n");
> + pr_notice("be passed to init along with those after '--' on the command line.\n");
> pr_notice("Unknown command line parameters:%s\n", unknown_options);
> memblock_free(unknown_options, len);
What about just changing it to simply say:
pr_notice("Unknown kernel command line parameters "%s", will be passed to user space.\n",
unknown_options);
-- Steve
On 10/12/21 5:01 PM, Steven Rostedt wrote:
> On Tue, 12 Oct 2021 16:35:23 -0500
> Andrew Halaney <[email protected]> wrote:
>
>> --- a/init/main.c
>> +++ b/init/main.c
>> @@ -925,6 +925,10 @@ static void __init print_unknown_bootoptions(void)
>> for (p = &envp_init[2]; *p; p++)
>> end += sprintf(end, " %s", *p);
>>
>> + pr_notice("The kernel command line has unknown parameters. They are either\n");
>> + pr_notice("misspelled, not valid for the current kernel configuration,\n");
>> + pr_notice("or are meant for init but are not after the '--' delineator. They will\n");
>> + pr_notice("be passed to init along with those after '--' on the command line.\n");
>> pr_notice("Unknown command line parameters:%s\n", unknown_options);
>> memblock_free(unknown_options, len);
>
> What about just changing it to simply say:
>
> pr_notice("Unknown kernel command line parameters "%s", will be passed to user space.\n",
> unknown_options);
>
Yes, that's much more palatable.
thanks.
--
~Randy
On Tue, 12 Oct 2021 17:18:32 -0700
Randy Dunlap <[email protected]> wrote:
> On 10/12/21 5:01 PM, Steven Rostedt wrote:
> > On Tue, 12 Oct 2021 16:35:23 -0500
> > Andrew Halaney <[email protected]> wrote:
> >
> >> --- a/init/main.c
> >> +++ b/init/main.c
> >> @@ -925,6 +925,10 @@ static void __init print_unknown_bootoptions(void)
> >> for (p = &envp_init[2]; *p; p++)
> >> end += sprintf(end, " %s", *p);
> >>
> >> + pr_notice("The kernel command line has unknown parameters. They are either\n");
> >> + pr_notice("misspelled, not valid for the current kernel configuration,\n");
> >> + pr_notice("or are meant for init but are not after the '--' delineator. They will\n");
> >> + pr_notice("be passed to init along with those after '--' on the command line.\n");
> >> pr_notice("Unknown command line parameters:%s\n", unknown_options);
> >> memblock_free(unknown_options, len);
> >
> > What about just changing it to simply say:
> >
> > pr_notice("Unknown kernel command line parameters "%s", will be passed to user space.\n",
> > unknown_options);
> >
>
> Yes, that's much more palatable.
Thanks.
Andrew (Halaney, not Morton),
Feel free to send a v2 with the above text, and just add:
Suggested-by: Steven Rostedt (VMware) <[email protected]>
-- Steve
On Tue, Oct 12, 2021 at 05:18:32PM -0700, Randy Dunlap wrote:
> On 10/12/21 5:01 PM, Steven Rostedt wrote:
> > On Tue, 12 Oct 2021 16:35:23 -0500
> > Andrew Halaney <[email protected]> wrote:
> >
> > > --- a/init/main.c
> > > +++ b/init/main.c
> > > @@ -925,6 +925,10 @@ static void __init print_unknown_bootoptions(void)
> > > for (p = &envp_init[2]; *p; p++)
> > > end += sprintf(end, " %s", *p);
> > > + pr_notice("The kernel command line has unknown parameters. They are either\n");
> > > + pr_notice("misspelled, not valid for the current kernel configuration,\n");
> > > + pr_notice("or are meant for init but are not after the '--' delineator. They will\n");
> > > + pr_notice("be passed to init along with those after '--' on the command line.\n");
> > > pr_notice("Unknown command line parameters:%s\n", unknown_options);
> > > memblock_free(unknown_options, len);
> >
> > What about just changing it to simply say:
> >
> > pr_notice("Unknown kernel command line parameters "%s", will be passed to user space.\n",
> > unknown_options);
> >
>
> Yes, that's much more palatable.
>
> thanks.
> --
> ~Randy
>
Heh, that's basically what the users suggested too but I wasn't the
biggest fan since I didn't think it highlighted the points I tried to
make. I guess I'm just too verbose :P
I'll spin a v2 with that considering everyone likes that form more.
Thanks,
Andrew
On Wed, 13 Oct 2021 07:56:38 -0500
Andrew Halaney <[email protected]> wrote:
> I'll spin a v2 with that considering everyone likes that form more.
Right, because we don't need a novel in the kernel to be printed ;-)
Just enough info that makes sense, and have comments or documentation
elsewhere that can explain more details.
-- Steve
On Wed, Oct 13, 2021 at 09:03:33AM -0400, Steven Rostedt wrote:
> Just enough info that makes sense, and have comments or documentation
> elsewhere that can explain more details.
Yes, as we do, for example in:
f77f420d3475 ("x86/msr: Add a pointer to an URL which contains further details")
--
Regards/Gruss,
Boris.
SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg