Thread (8 messages) 8 messages, 5 authors, 2021-02-04

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

From: Petr Mladek <pmladek@suse.com>
Date: 2021-02-04 10:16:53
Also in: dri-devel, linux-fbdev, lkml, platform-driver-x86

On Thu 2021-02-04 06:51:09, Masahiro Yamada wrote:
On Thu, Feb 4, 2021 at 12:23 AM Petr Mladek [off-list ref] wrote:
quoted
On Tue 2021-02-02 09:44:22, John Ogness wrote:
quoted
On 2021-02-02, Masahiro Yamada [off-list ref] wrote:
quoted
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.

Remove the CONSOLE_LOGLEVEL_DEFAULT definition from <linux/printk.h>,
and use CONFIG_CONSOLE_LOGLEVEL_DEFAULT directly.
With commit a8fe19ebfbfd ("kernel/printk: use symbolic defines for
console loglevels") it can be seen that various drivers used to
hard-code their own values. The introduction of the macros in an
intuitive location (include/linux/printk.h) made it easier for authors
to find/use the various available printk settings and thresholds.

Technically there is no problem using Kconfig macros directly. But will
authors bother to hunt down available Kconfig settings? Or will they
only look in printk.h to see what is available?

IMHO if code wants to use settings from a foreign subsystem, it should
be taking those from headers of that subsystem, rather than using some
Kconfig settings from that subsystem. Headers exist to make information
available to external code. Kconfig (particularly for a subsystem) exist
to configure that subsystem.
I agree with this this view.

I have never seen a policy to restrict
the use of CONFIG options in relevant
subsystem headers.
I would say that it is a common sense. But I admit that I did not look
at the code in detail. See below.
quoted
What about using default_console_loglevel() in the external code?
It reads the value from an array. This value is initialized to
CONSOLE_LOGLEVEL_DEFAULT and never modified later.
I do not think default_console_loglevel()
is a perfect constant
because it can be modified via
/proc/sys/kernel/printk
And that is the problem. I somehow expected that the external code
wanted to have the currently valid value and not the prebuilt one.

When I look closely:

  + arch/x86/platform/uv/uv_nmi.c
  + drivers/firmware/efi/libstub/efi-stub-helper.c

    These use the value to statically initialize global variables that
    might later be modified by subsystem-specific kernel parameters.

    CONFIG_CONSOLE_LOGLEVEL_DEFAULT is acceptable here from my POV.
    The build dependency sucks. And it is not worth any too complicated
    solution.


  + drivers/tty/sysrq.c

    The intention here is to use the highest console loglevel so that
    people really see them. It used to be hardcoded "7". sysrq is
    typically the last chance to get some information from the system.

    We actually want to use the hardcoded "7" here. But we should
    define it via a macro in printk.h, e.g.

     #define CONSOLE_LOGLEVEL_ALL_NORMAL 7 /* all non-debugging messages */

     or

     #define CONSOLE_LOGLEVEL_NO_DEBUG 7  /* all non-debugging messages */

Best Regards,
Petr
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help