Re: [GIT PULL] bootconfig: Updates for 6.6

[Google]

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@xxxxxxxxxxxxxxxxxxxx> wrote:

> On Fri, 1 Sept 2023 at 18:10, Masami Hiramatsu <mhiramat@xxxxxxxxxx> 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@xxxxxxxxxx>