Thread (22 messages) 22 messages, 3 authors, 2023-02-27

Re: [PATCH RFC bootconfig] Allow forcing unconditional bootconfig processing

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Date: 2023-02-27 23:29:44
Also in: lkml

On Mon, 27 Feb 2023 08:56:28 -0800
"Paul E. McKenney" [off-list ref] wrote:
On Mon, Feb 27, 2023 at 08:16:32AM +0900, Masami Hiramatsu wrote:
quoted
On Fri, 24 Feb 2023 17:19:10 -0800
"Paul E. McKenney" [off-list ref] wrote:
quoted
On Sat, Feb 25, 2023 at 09:58:11AM +0900, Masami Hiramatsu wrote:
quoted
On Fri, 24 Feb 2023 08:33:07 -0800
"Paul E. McKenney" [off-list ref] wrote:
quoted
On Sat, Feb 25, 2023 at 01:13:06AM +0900, Masami Hiramatsu wrote:
quoted
Hi Geert,

On Fri, 24 Feb 2023 09:31:50 +0100
Geert Uytterhoeven [off-list ref] wrote:
quoted
Hi Paul,

On Sat, Jan 7, 2023 at 5:33 PM Paul E. McKenney [off-list ref] wrote:
quoted
On Sun, Jan 08, 2023 at 12:22:15AM +0900, Masami Hiramatsu wrote:
quoted
BTW, maybe CONFIG_BOOT_CONFIG_EMBED is better to select this.
(or at least recommend to enable this)
Like this?

                                                        Thanx, Paul

------------------------------------------------------------------------

commit d09a1505c51a70da38b34ac38062977299aef742
Author: Paul E. McKenney [off-list ref]
Date:   Sat Jan 7 08:09:22 2023 -0800

    bootconfig: Default BOOT_CONFIG_FORCE to y if BOOT_CONFIG_EMBED

    When a kernel is built with CONFIG_BOOT_CONFIG_EMBED=y, the intention
    will normally be to unconditionally provide the specified kernel-boot
    arguments to the kernel, as opposed to requiring a separately provided
    bootconfig parameter.  Therefore, make the BOOT_CONFIG_FORCE Kconfig
    option default to y in kernels built with CONFIG_BOOT_CONFIG_EMBED=y.

    The old semantics may be obtained by manually overriding this default.

    Suggested-by: Masami Hiramatsu [off-list ref]
    Signed-off-by: Paul E. McKenney [off-list ref]
diff --git a/init/Kconfig b/init/Kconfig
index 0fb19fa0edba9..97a0f14d9020d 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1379,6 +1379,7 @@ config BOOT_CONFIG
 config BOOT_CONFIG_FORCE
        bool "Force unconditional bootconfig processing"
        depends on BOOT_CONFIG
+       default y if BOOT_CONFIG_EMBED
        help
          With this Kconfig option set, BOOT_CONFIG processing is carried
          out even when the "bootconfig" kernel-boot parameter is omitted.
Thanks for your patch, which is now commit 6ded8a28ed80e4cc
("bootconfig: Default BOOT_CONFIG_FORCE to y if BOOT_CONFIG_EMBED").

After this change, an all{mod,yes}config kernel has:

    CONFIG_BOOT_CONFIG_FORCE=y
    CONFIG_BOOT_CONFIG_EMBED=y
    CONFIG_BOOT_CONFIG_EMBED_FILE=""

Will this actually work? I haven't tried booting such a kernel yet.
Yeah, good question. It is same as when you boot the kernel with 'bootconfig'
but do not add the bootconfig file to initrd. You may see below message
on boot log, but kernel boots normally. :)

 'bootconfig' found on command line, but no bootconfig found

(Maybe it is better to fix the message, because if BOOT_CONFIG_FORCE=y, this
will be shown without 'bootconfig' on command line.)
I just tried it again, and for me it just silently ignores the bootconfig
setup.  Which is what I recall happening when I tried it when creating
the patch.

Here is the .config file pieces of interest:

CONFIG_BOOT_CONFIG=y
CONFIG_BOOT_CONFIG_FORCE=y
CONFIG_BOOT_CONFIG_EMBED=y
CONFIG_BOOT_CONFIG_EMBED_FILE=""

Anyone else seeing something different?
Hmm, from the code, I think you'll see that message in early console log.

In init/main.c:

----
#ifdef CONFIG_BOOT_CONFIG
/* Is bootconfig on command line? */
static bool bootconfig_found = IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE);
static size_t initargs_offs;
#else
----
And
----
static void __init setup_boot_config(void)
{
...
        strscpy(tmp_cmdline, boot_command_line, COMMAND_LINE_SIZE);
        err = parse_args("bootconfig", tmp_cmdline, NULL, 0, 0, 0, NULL,
                         bootconfig_params);

        if (IS_ERR(err) || !bootconfig_found)
                return;

        /* parse_args() stops at the next param of '--' and returns an address */
        if (err)
                initargs_offs = err - tmp_cmdline;

        if (!data) {
                pr_err("'bootconfig' found on command line, but no bootconfig found\n");
                return;
        }
----

Thus, if CONFIG_BOOT_CONFIG_FORCE=y, the process passes the below check

        if (IS_ERR(err) || !bootconfig_found)
                return;

But since we have an empty 'data', the error should be printed.
And you are quite right, the runs without data files did get me this:

'bootconfig' found on command line, but no bootconfig found

Please accept my apologies for my confusion.
No problem :), so should we skip this message if CONFIG_BOOT_CONFIG_FORCE=y,
because user may not pass 'bootconfig'?

Or, may be we can make it;

 "Skip bootconfig, because no bootconfig data found."

so that user can notice they forget to set up bootconfig data?
Good point, the current message could be quite confusing.  Me, I already
knew what was happening, so I just looked for the change in console-log
output.  ;-)

How about something like this?

	"No bootconfig data provided, so skipping bootconfig"

But as you say, keeping the current message in kernels that have been
built with CONFIG_BOOT_CONFIG_FORCE=n.
That sounds good to me. OK, let me update that.

Thank you,
							Thanx, Paul
quoted
Thank you,

quoted
							Thanx, Paul
quoted
Thank you,
quoted
							Thanx, Paul
quoted
Thank you!
quoted
Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

-- 
Masami Hiramatsu (Google) [off-list ref]

-- 
Masami Hiramatsu (Google) [off-list ref]

-- 
Masami Hiramatsu (Google) [off-list ref]

-- 
Masami Hiramatsu (Google) [off-list ref]
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help