Re: [PATCH 1/3] printk: use CONFIG_CONSOLE_LOGLEVEL_* directly

Masahiro Yamada
2021-02-02T23:06:17Z

On Tue, Feb 2, 2021 at 7:09 PM Sergey Senozhatsky
<sergey.senozhatsky@gmail.com> wrote:
>
> On (21/02/02 16:02), Masahiro Yamada wrote:
> >
> > CONSOLE_LOGLEVEL_DEFAULT is nothing more than a shorthand of
> > CONFIG_CONSOLE_LOGLEVEL_DEFAULT.
> >
> > When you change CONFIG_CONSOLE_LOGLEVEL_DEFAULT from Kconfig, almost
> > all objects are rebuilt because CONFIG_CONSOLE_LOGLEVEL_DEFAULT is
> > used in <linux/printk.h>, which is included from most of source files.
> >
> > In fact, there are only 4 users of CONSOLE_LOGLEVEL_DEFAULT:
> >
> > arch/x86/platform/uv/uv_nmi.c
> > drivers/firmware/efi/libstub/efi-stub-helper.c
> > drivers/tty/sysrq.c
> > kernel/printk/printk.c
> >
> > So, when you change CONFIG_CONSOLE_LOGLEVEL_DEFAULT and rebuild the
> > kernel, it is enough to recompile those 4 files.
>
> Do you change CONFIG_CONSOLE_LOGLEVEL_DEFAULT so often that it becomes a
> problem?
>
> -ss

<linux/printk.h> is one of most included headers,
so it is worth downsizing.

CONSOLE_LOGLEVEL_DEFAULT is not such a parameter
that printk() users need to know.

Changing CONFIG_CONSOLE_LOGLEVEL_DEFAULT results in
the rebuilds of the entire tree, which is a flag of
bad code structure.

So, this is not only CONSOLE_LOGLEVEL_DEFAULT.
<linux/printk.h> contains parameters
and func declarations that printk() users
do not need to know.

Examples:
CONSOLE_LOGLEVEL_DEFAULT
log_buf_addr_get()
log_buf_len_get()
oops_in_progress
...

They are only needed for those who want
to more closely get access to
the printk internals.

Ideally, such parameters and func
declarations can go to the subsystems'
local header (kernel/printk/internal.h)
but when it is not possible,
they can be separated out to
a different header.

I can see a similar idea in the consumer/provider
model in several subsystems.

Consumers and providers are often orthogonal,
and de-coupling them clarifies
who needs what.

See other subsystems, for example,

<linux/clk.h> - clk consumer
<linux/clk-provider.h> - clk provider

--
Best Regards
Masahiro Yamada