Re: [GIT PULL] bootconfig: Updates for 6.6

Re: [GIT PULL] bootconfig: Updates for 6.6

[Linus Torvalds]

On Fri, 1 Sept 2023 at 18:10, Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
>  - fs/proc: Add /proc/raw_cmdline for boot loader arguments.

Honestly, I pulled this, and then I unpulled again.

This seems such a *silly* thing. It's also actively confusing, since
this "raw" file internally in the kernel is called
"boot_command_line", vs the "saved_command_line" of the regular one,
and we also have 'extra_command_line' and 'static_command_line' etc,
so where does this all end?

So the name doesn't even make any sense. It's not "raw" in any sense
of the word. It just happens to be the one that came from the boot
side.

In other words, this smells like a complete hack to me. It makes no
sense, and it should *not* be added to the top-level /proc filesystem
as some kind of fundamental file.

And not only is it a special case that isn't worthy of adding to the
top-level /proc directory, it only has _one_ special case user that
could possibly care.

And this is all self-inflicted pain because the bootconfig code
corrupted the original command line, and decided to expose that
corrupted thing in /proc/cmdline.

So because you made a mess of it originally, you're now making a
*bigger* mess of this all.

No, thank you. The way to fix a mess is not to make it worse. And this
just makes things worse.

I suspect the fix should always have been to make the extra stuff be
somehow clear so that you can parse it. Not make another file that has
the exact same contents for most people.

Maybe a marker of "this is the end of the 'extra command line", the
same way we have that "--" for "this is the end of the stuff the
kernel should parse".

           Linus

Re: [GIT PULL] bootconfig: Updates for 6.6

[Masami Hiramatsu]

Hi Linus,

Thanks for the comment.
Let us reconsider how it should be handled.

On Sat, 2 Sep 2023 11:35:00 -0700
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Fri, 1 Sept 2023 at 18:10, Masami Hiramatsu <mhiramat@kernel.org> wrote:
> >
> >  - fs/proc: Add /proc/raw_cmdline for boot loader arguments.
> 
> Honestly, I pulled this, and then I unpulled again.
> 
> This seems such a *silly* thing. It's also actively confusing, since
> this "raw" file internally in the kernel is called
> "boot_command_line", vs the "saved_command_line" of the regular one,
> and we also have 'extra_command_line' and 'static_command_line' etc,
> so where does this all end?
>
> So the name doesn't even make any sense. It's not "raw" in any sense
> of the word. It just happens to be the one that came from the boot
> side.

OK.

> 
> In other words, this smells like a complete hack to me. It makes no
> sense, and it should *not* be added to the top-level /proc filesystem
> as some kind of fundamental file.

Got it.

> 
> And not only is it a special case that isn't worthy of adding to the
> top-level /proc directory, it only has _one_ special case user that
> could possibly care.

Indeed. For the most users, /proc/raw_cmdline will be the same as
/proc/cmdline.

> 
> And this is all self-inflicted pain because the bootconfig code
> corrupted the original command line, and decided to expose that
> corrupted thing in /proc/cmdline.
> 
> So because you made a mess of it originally, you're now making a
> *bigger* mess of this all.
> 
> No, thank you. The way to fix a mess is not to make it worse. And this
> just makes things worse.

OK.

> 
> I suspect the fix should always have been to make the extra stuff be
> somehow clear so that you can parse it. Not make another file that has
> the exact same contents for most people.
> 
> Maybe a marker of "this is the end of the 'extra command line", the
> same way we have that "--" for "this is the end of the stuff the
> kernel should parse".

Yeah, I thought that, but it seems that any marker may introduce
another problem.

If the user need to know the bootloader command line string when the
bootconfig is enabled, what about adding a special line to the
/proc/bootconfig, e.g.

bootloader.parameters = "<params from bootloader>"

In this case, it will be only shown when the bootconfig is enabled, and
normal /proc/cmdline user does not need to care about that. Of course
bootconfig can drop "bootloader" items.


Thank you,

-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [GIT PULL] bootconfig: Updates for 6.6

[Masami Hiramatsu]

On Sun, 3 Sep 2023 13:16:19 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:

> If the user need to know the bootloader command line string when the
> bootconfig is enabled, what about adding a special line to the
> /proc/bootconfig, e.g.
> 
> bootloader.parameters = "<params from bootloader>"
> 
> In this case, it will be only shown when the bootconfig is enabled, and
> normal /proc/cmdline user does not need to care about that. Of course
> bootconfig can drop "bootloader" items.

Or more better for both bootconfig and reader, it is easy to add bootloader
parameter as a comment.

# Parameters from bootloader:
# root=UUID=ac0f0548-a69d-43ca-a06b-7db01bcbd5ad ro quiet ...

Thank you,

-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [GIT PULL] bootconfig: Updates for 6.6

[Paul E. McKenney]

On Sun, Sep 03, 2023 at 05:32:21PM +0900, Masami Hiramatsu wrote:
> On Sun, 3 Sep 2023 13:16:19 +0900
> Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> 
> > If the user need to know the bootloader command line string when the
> > bootconfig is enabled, what about adding a special line to the
> > /proc/bootconfig, e.g.
> > 
> > bootloader.parameters = "<params from bootloader>"
> > 
> > In this case, it will be only shown when the bootconfig is enabled, and
> > normal /proc/cmdline user does not need to care about that. Of course
> > bootconfig can drop "bootloader" items.
> 
> Or more better for both bootconfig and reader, it is easy to add bootloader
> parameter as a comment.
> 
> # Parameters from bootloader:
> # root=UUID=ac0f0548-a69d-43ca-a06b-7db01bcbd5ad ro quiet ...

From what I can see, this would work quite well!

							Thanx, Paul