Re: [for-next][PATCH 04/26] bootconfig: Add Extra Boot Config support

[Masami Hiramatsu]

On Thu, 6 Feb 2020 18:20:15 +0100
Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:

> Hi Hiramatsu-san,
> 
> On Thu, Feb 6, 2020 at 3:42 PM Masami Hiramatsu <mhiramat@xxxxxxxxxx> wrote:
> > On Thu, 6 Feb 2020 12:54:05 +0100
> > Borislav Petkov <bp@xxxxxxxxx> wrote:
> > > On Tue, Jan 14, 2020 at 04:03:20PM -0500, Steven Rostedt wrote:
> > > > diff --git a/init/Kconfig b/init/Kconfig
> > > > index a34064a031a5..63450d3bbf12 100644
> > > > --- a/init/Kconfig
> > > > +++ b/init/Kconfig
> > > > @@ -1215,6 +1215,17 @@ source "usr/Kconfig"
> > > >
> > > >  endif
> > > >
> > > > +config BOOT_CONFIG
> > > > +   bool "Boot config support"
> > > > +   select LIBXBC
> > > > +   default y
> > >
> > > Any particular reason this is default y? Why should it be enabled by
> > > default on all boxes?
> >
> > Oh, you are not the first person asked that :)
> >
> > https://lkml.org/lkml/2019/12/9/563
> >
> > And yes, I think this is important that will useful for most developers
> > and admins. Since the bootconfig already covers kernel and init options,
> > this can be a new standard way to pass args to kernel boot.
> >
> > And as I reported above thread, the memory footpoint of view, most code
> > and working memory are released after boot. Also, as Linus's suggested,
> > now this feature is enabled only if user gives "bootconfig" on the kernel
> > command line. So the side effect is minimized.
> 
> With m68k/atari_defconfig, bloat-o-meter says:
> 
>     add/remove: 39/0 grow/shrink: 2/0 up/down: 13086/0 (13086)
> 
> which is IMHO not that small for a "default y" option that may or may not
> be used.
> 
> Especially:
> 
>         Function                                     old     new   delta
>     xbc_nodes                                      -    8192   +8192
> 
> Any chance xbc_nodes can be allocated dynamically, and only when needed?

Yes, I think we can use memblock to allocate it. However, this xbc_nodes is
__init_data, which is released right after boot. So I think it should be
OK except for your system doesn't have user space...

> Yes, there are industrial products running Linux on a current ARM SoC
> using the builtin 8 or 10 MiB of SRAM (+ XIP for the kernel), so these
> definitely want to say CONFIG_BOOT_CONFIG=n.

I think for such products the kernel must be tuned with custom config,
and you can say CONFIG_BOOT_CONFIG=n. That is a configurable feature.

Thank you,

> 
> Thanks!
> 
> add/remove: 39/0 grow/shrink: 2/0 up/down: 13086/0 (13086)
> Function                                     old     new   delta
> xbc_nodes                                      -    8192   +8192
> xbc_init                                       -     854    +854
> start_kernel                                1020    1580    +560
> copy_xbc_key_value_list                        -     362    +362
> __xbc_parse_value                              -     324    +324
> xbc_snprint_cmdline                            -     294    +294
> xbc_node_compose_key_after                     -     266    +266
> xbc_namebuf                                    -     256    +256
> xbc_add_sibling                                -     254    +254
> xbc_node_find_next_leaf                        -     186    +186
> xbc_node_find_child                            -     176    +176
> __xbc_add_key                                  -     162    +162
> proc_boot_config_init                          -     124    +124
> xbc_make_cmdline                               -     122    +122
> xbc_node_find_value                            -     104    +104
> xbc_node_find_next_key_value                   -     102    +102
> __xbc_close_brace                              -      92     +92
> find_match_node                                -      80     +80
> __xbc_parse_keys                               -      78     +78
> xbc_node_get_data                              -      64     +64
> xbc_parse_key                                  -      60     +60
> skip_comment                                   -      48     +48
> xbc_destroy_all                                -      42     +42
> xbc_node_get_parent                            -      38     +38
> xbc_parse_error                                -      36     +36
> xbc_node_get_child                             -      34     +34
> xbc_node_get_next                              -      32     +32
> boot_config_checksum                           -      30     +30
> boot_config_proc_show                          -      26     +26
> xbc_root_node                                  -      20     +20
> kzalloc.constprop                            810     830     +20
> xbc_node_index                                 -      14     +14
> xbc_node_num                                   -       4      +4
> xbc_data_size                                  -       4      +4
> xbc_data                                       -       4      +4
> saved_boot_config                              -       4      +4
> last_parent                                    -       4      +4
> extra_init_args                                -       4      +4
> extra_command_line                             -       4      +4
> __initcall_proc_boot_config_init5              -       4      +4
> xbc_debug_dump                                 -       2      +2
> Total: Before=3688860, After=3701946, chg +0.35%
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
> 
> 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 <mhiramat@xxxxxxxxxx>