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

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

[Masahiro Yamada]

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 this, the build system will rebuild the minimal number of objects.

Steps to confirm it:

  [1] Do the full build
  [2] Change CONFIG_CONSOLE_LOGLEVEL_DEFAULT from 'make menuconfig' etc.
  [3] Rebuild

  $ make
    SYNC    include/config/auto.conf
    CALL    scripts/checksyscalls.sh
    CALL    scripts/atomic/check-atomics.sh
    DESCEND  objtool
    CHK     include/generated/compile.h
    CC      kernel/printk/printk.o
    AR      kernel/printk/built-in.a
    AR      kernel/built-in.a
    CC      drivers/tty/sysrq.o
    AR      drivers/tty/built-in.a
    CC      drivers/firmware/efi/libstub/efi-stub-helper.o
    STUBCPY drivers/firmware/efi/libstub/efi-stub-helper.stub.o
    AR      drivers/firmware/efi/libstub/lib.a
    AR      drivers/built-in.a
    GEN     .version
    CHK     include/generated/compile.h
    UPD     include/generated/compile.h
    CC      init/version.o
    AR      init/built-in.a
    LD      vmlinux.o
    ...

For the same reason, do likewise for CONSOLE_LOGLEVEL_QUIET and
MESSAGE_LOGLEVEL_DEFAULT.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 arch/x86/platform/uv/uv_nmi.c                  |  2 +-
 drivers/firmware/efi/libstub/efi-stub-helper.c |  6 +++---
 drivers/tty/sysrq.c                            |  4 ++--
 drivers/video/fbdev/core/fbcon.c               |  2 +-
 drivers/video/fbdev/efifb.c                    |  2 +-
 include/linux/printk.h                         | 10 ----------
 init/main.c                                    |  2 +-
 kernel/printk/printk.c                         |  6 +++---
 8 files changed, 12 insertions(+), 22 deletions(-)

diff --git a/arch/x86/platform/uv/uv_nmi.c b/arch/x86/platform/uv/uv_nmi.c
index eafc530c8767..4751299c7416 100644
--- a/arch/x86/platform/uv/uv_nmi.c
+++ b/arch/x86/platform/uv/uv_nmi.c
@@ -100,7 +100,7 @@ static cpumask_var_t uv_nmi_cpu_mask;
  * Default is all stack dumps go to the console and buffer.
  * Lower level to send to log buffer only.
  */
-static int uv_nmi_loglevel = CONSOLE_LOGLEVEL_DEFAULT;
+static int uv_nmi_loglevel = CONFIG_CONSOLE_LOGLEVEL_DEFAULT;
 module_param_named(dump_loglevel, uv_nmi_loglevel, int, 0644);
 
 /*
diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index aa8da0a49829..3e8d8f706589 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -12,7 +12,7 @@
 #include <linux/ctype.h>
 #include <linux/efi.h>
 #include <linux/kernel.h>
-#include <linux/printk.h> /* For CONSOLE_LOGLEVEL_* */
+#include <linux/printk.h> /* For CONSOLE_LOGLEVEL_DEBUG */
 #include <asm/efi.h>
 #include <asm/setup.h>
 
@@ -21,7 +21,7 @@
 bool efi_nochunk;
 bool efi_nokaslr = !IS_ENABLED(CONFIG_RANDOMIZE_BASE);
 bool efi_noinitrd;
-int efi_loglevel = CONSOLE_LOGLEVEL_DEFAULT;
+int efi_loglevel = CONFIG_CONSOLE_LOGLEVEL_DEFAULT;
 bool efi_novamap;
 
 static bool efi_nosoftreserve;
@@ -213,7 +213,7 @@ efi_status_t efi_parse_options(char const *cmdline)
 		if (!strcmp(param, "nokaslr")) {
 			efi_nokaslr = true;
 		} else if (!strcmp(param, "quiet")) {
-			efi_loglevel = CONSOLE_LOGLEVEL_QUIET;
+			efi_loglevel = CONFIG_CONSOLE_LOGLEVEL_QUIET;
 		} else if (!strcmp(param, "noinitrd")) {
 			efi_noinitrd = true;
 		} else if (!strcmp(param, "efi") && val) {
diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index 959f9e121cc6..e0ae7793155e 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -103,7 +103,7 @@ static void sysrq_handle_loglevel(int key)
 	int i;
 
 	i = key - '0';
-	console_loglevel = CONSOLE_LOGLEVEL_DEFAULT;
+	console_loglevel = CONFIG_CONSOLE_LOGLEVEL_DEFAULT;
 	pr_info("Loglevel set to %d\n", i);
 	console_loglevel = i;
 }
@@ -584,7 +584,7 @@ void __handle_sysrq(int key, bool check_mask)
 	 * routing in the consumers of /proc/kmsg.
 	 */
 	orig_log_level = console_loglevel;
-	console_loglevel = CONSOLE_LOGLEVEL_DEFAULT;
+	console_loglevel = CONFIG_CONSOLE_LOGLEVEL_DEFAULT;
 
         op_p = __sysrq_get_key_op(key);
         if (op_p) {
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index bf61598bf1c3..75b97268663f 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -1043,7 +1043,7 @@ static void fbcon_init(struct vc_data *vc, int init)
 
 	info = registered_fb[con2fb_map[vc->vc_num]];
 
-	if (logo_shown < 0 && console_loglevel <= CONSOLE_LOGLEVEL_QUIET)
+	if (logo_shown < 0 && console_loglevel <= CONFIG_CONSOLE_LOGLEVEL_QUIET)
 		logo_shown = FBCON_LOGO_DONTSHOW;
 
 	if (vc != svc || logo_shown == FBCON_LOGO_DONTSHOW ||
diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
index e57c00824965..dfb234a0a59d 100644
--- a/drivers/video/fbdev/efifb.c
+++ b/drivers/video/fbdev/efifb.c
@@ -160,7 +160,7 @@ static void efifb_show_boot_graphics(struct fb_info *info)
 	}
 
 	/* Avoid flashing the logo if we're going to print std probe messages */
-	if (console_loglevel > CONSOLE_LOGLEVEL_QUIET)
+	if (console_loglevel > CONFIG_CONSOLE_LOGLEVEL_QUIET)
 		return;
 
 	/* bgrt_tab.status is unreliable, so we don't check it */
diff --git a/include/linux/printk.h b/include/linux/printk.h
index fe7eb2351610..fd34b3aa2f90 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -46,22 +46,12 @@ static inline const char *printk_skip_headers(const char *buffer)
 
 #define CONSOLE_EXT_LOG_MAX	8192
 
-/* printk's without a loglevel use this.. */
-#define MESSAGE_LOGLEVEL_DEFAULT CONFIG_MESSAGE_LOGLEVEL_DEFAULT
-
 /* We show everything that is MORE important than this.. */
 #define CONSOLE_LOGLEVEL_SILENT  0 /* Mum's the word */
 #define CONSOLE_LOGLEVEL_MIN	 1 /* Minimum loglevel we let people use */
 #define CONSOLE_LOGLEVEL_DEBUG	10 /* issue debug messages */
 #define CONSOLE_LOGLEVEL_MOTORMOUTH 15	/* You can't shut this one up */
 
-/*
- * Default used to be hard-coded at 7, quiet used to be hardcoded at 4,
- * we're now allowing both to be set from kernel config.
- */
-#define CONSOLE_LOGLEVEL_DEFAULT CONFIG_CONSOLE_LOGLEVEL_DEFAULT
-#define CONSOLE_LOGLEVEL_QUIET	 CONFIG_CONSOLE_LOGLEVEL_QUIET
-
 extern int console_printk[];
 
 #define console_loglevel (console_printk[0])
diff --git a/init/main.c b/init/main.c
index c68d784376ca..4dfcbf7f24c6 100644
--- a/init/main.c
+++ b/init/main.c
@@ -236,7 +236,7 @@ static int __init debug_kernel(char *str)
 
 static int __init quiet_kernel(char *str)
 {
-	console_loglevel = CONSOLE_LOGLEVEL_QUIET;
+	console_loglevel = CONFIG_CONSOLE_LOGLEVEL_QUIET;
 	return 0;
 }
 
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 5a95c688621f..92b93340905c 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -61,10 +61,10 @@
 #include "internal.h"
 
 int console_printk[4] = {
-	CONSOLE_LOGLEVEL_DEFAULT,	/* console_loglevel */
-	MESSAGE_LOGLEVEL_DEFAULT,	/* default_message_loglevel */
+	CONFIG_CONSOLE_LOGLEVEL_DEFAULT,	/* console_loglevel */
+	CONFIG_MESSAGE_LOGLEVEL_DEFAULT,	/* default_message_loglevel */
 	CONSOLE_LOGLEVEL_MIN,		/* minimum_console_loglevel */
-	CONSOLE_LOGLEVEL_DEFAULT,	/* default_console_loglevel */
+	CONFIG_CONSOLE_LOGLEVEL_DEFAULT,	/* default_console_loglevel */
 };
 EXPORT_SYMBOL_GPL(console_printk);
 
-- 
2.27.0

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

[John Ogness]

On 2021-02-02, Masahiro Yamada <masahiroy@kernel.org> 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.
>
> 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.

But my feeling on this may be misguided. Is it generally accepted in the
kernel that any code can use Kconfig settings of any other code?

John Ogness

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

[Sergey Senozhatsky]

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

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

[Greg Kroah-Hartman]

On Tue, Feb 02, 2021 at 04:02:16PM +0900, 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.
> 
> Remove the CONSOLE_LOGLEVEL_DEFAULT definition from <linux/printk.h>,
> and use CONFIG_CONSOLE_LOGLEVEL_DEFAULT directly.
> 
> With this, the build system will rebuild the minimal number of objects.
> 
> Steps to confirm it:
> 
>   [1] Do the full build
>   [2] Change CONFIG_CONSOLE_LOGLEVEL_DEFAULT from 'make menuconfig' etc.
>   [3] Rebuild
> 
>   $ make
>     SYNC    include/config/auto.conf
>     CALL    scripts/checksyscalls.sh
>     CALL    scripts/atomic/check-atomics.sh
>     DESCEND  objtool
>     CHK     include/generated/compile.h
>     CC      kernel/printk/printk.o
>     AR      kernel/printk/built-in.a
>     AR      kernel/built-in.a
>     CC      drivers/tty/sysrq.o
>     AR      drivers/tty/built-in.a
>     CC      drivers/firmware/efi/libstub/efi-stub-helper.o
>     STUBCPY drivers/firmware/efi/libstub/efi-stub-helper.stub.o
>     AR      drivers/firmware/efi/libstub/lib.a
>     AR      drivers/built-in.a
>     GEN     .version
>     CHK     include/generated/compile.h
>     UPD     include/generated/compile.h
>     CC      init/version.o
>     AR      init/built-in.a
>     LD      vmlinux.o
>     ...
> 
> For the same reason, do likewise for CONSOLE_LOGLEVEL_QUIET and
> MESSAGE_LOGLEVEL_DEFAULT.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
> 
>  arch/x86/platform/uv/uv_nmi.c                  |  2 +-
>  drivers/firmware/efi/libstub/efi-stub-helper.c |  6 +++---
>  drivers/tty/sysrq.c                            |  4 ++--
>  drivers/video/fbdev/core/fbcon.c               |  2 +-
>  drivers/video/fbdev/efifb.c                    |  2 +-
>  include/linux/printk.h                         | 10 ----------
>  init/main.c                                    |  2 +-
>  kernel/printk/printk.c                         |  6 +++---
>  8 files changed, 12 insertions(+), 22 deletions(-)

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

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

[Masahiro Yamada]

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

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

[Petr Mladek]

On Tue 2021-02-02 09:44:22, John Ogness wrote:
> On 2021-02-02, Masahiro Yamada <masahiroy@kernel.org> 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.
> >
> > 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.

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.

Best Regards,
Petr

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

[Masahiro Yamada]

On Thu, Feb 4, 2021 at 12:23 AM Petr Mladek <pmladek@suse.com> wrote:
>
> On Tue 2021-02-02 09:44:22, John Ogness wrote:
> > On 2021-02-02, Masahiro Yamada <masahiroy@kernel.org> 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.
> > >
> > > 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.



> 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


I am not sure if it works either.

Some code may not be linked to vmlinux.
drivers/firmware/efi/libstub/efi-stub-helper.c




> Best Regards,
> Petr




--
Best Regards
Masahiro Yamada

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

[Petr Mladek]

On Thu 2021-02-04 06:51:09, Masahiro Yamada wrote:
> On Thu, Feb 4, 2021 at 12:23 AM Petr Mladek <pmladek@suse.com> wrote:
> >
> > On Tue 2021-02-02 09:44:22, John Ogness wrote:
> > > On 2021-02-02, Masahiro Yamada <masahiroy@kernel.org> 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.
> > > >
> > > > 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.

> > 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