All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] printk: use CONFIG_CONSOLE_LOGLEVEL_* directly
@ 2021-02-02  7:02 ` Masahiro Yamada
  0 siblings, 0 replies; 26+ messages in thread
From: Masahiro Yamada @ 2021-02-02  7:02 UTC (permalink / raw)
  To: linux-kernel, Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
	John Ogness
  Cc: Masahiro Yamada, Andy Shevchenko, Ard Biesheuvel,
	Borislav Petkov, Darren Hart, Dimitri Sivanich,
	Greg Kroah-Hartman, H. Peter Anvin, Ingo Molnar, Jiri Slaby,
	Mike Travis, Peter Jones, Russ Anderson, Steve Wahl,
	Thomas Gleixner, dri-devel, linux-efi, linux-fbdev,
	platform-driver-x86, x86

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


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 1/3] printk: use CONFIG_CONSOLE_LOGLEVEL_* directly
@ 2021-02-02  7:02 ` Masahiro Yamada
  0 siblings, 0 replies; 26+ messages in thread
From: Masahiro Yamada @ 2021-02-02  7:02 UTC (permalink / raw)
  To: linux-kernel, Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
	John Ogness
  Cc: Dimitri Sivanich, linux-fbdev, linux-efi, Russ Anderson,
	Steve Wahl, Mike Travis, Greg Kroah-Hartman, Masahiro Yamada,
	Peter Jones, x86, dri-devel, platform-driver-x86, Ingo Molnar,
	Borislav Petkov, H. Peter Anvin, Darren Hart, Thomas Gleixner,
	Jiri Slaby, Ard Biesheuvel, Andy Shevchenko

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 2/3] printk: hard-code CONSOLE_LOGLEVEL_MIN in printk.c
  2021-02-02  7:02 ` Masahiro Yamada
  (?)
@ 2021-02-02  7:02 ` Masahiro Yamada
  2021-02-02 10:06   ` Sergey Senozhatsky
  -1 siblings, 1 reply; 26+ messages in thread
From: Masahiro Yamada @ 2021-02-02  7:02 UTC (permalink / raw)
  To: linux-kernel, Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
	John Ogness
  Cc: Masahiro Yamada

CONSOLE_LOGLEVEL_MIN is only used in kernel/printk/printk.c.

You do not need to expose it to all printk() users.

I could move it to kernel/printk/printk.c, but I do not think this
macro would contribute to the code readability or maintainability.

I just hard-coded it.

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

 include/linux/printk.h | 1 -
 kernel/printk/printk.c | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index fd34b3aa2f90..ceaf0486c01c 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -48,7 +48,6 @@ static inline const char *printk_skip_headers(const char *buffer)
 
 /* 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 */
 
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 92b93340905c..7b50298d52e3 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -63,7 +63,7 @@
 int console_printk[4] = {
 	CONFIG_CONSOLE_LOGLEVEL_DEFAULT,	/* console_loglevel */
 	CONFIG_MESSAGE_LOGLEVEL_DEFAULT,	/* default_message_loglevel */
-	CONSOLE_LOGLEVEL_MIN,		/* minimum_console_loglevel */
+	1,					/* minimum_console_loglevel */
 	CONFIG_CONSOLE_LOGLEVEL_DEFAULT,	/* default_console_loglevel */
 };
 EXPORT_SYMBOL_GPL(console_printk);
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 3/3] printk: move CONSOLE_EXT_LOG_MAX to kernel/printk/printk.c
  2021-02-02  7:02 ` Masahiro Yamada
  (?)
  (?)
@ 2021-02-02  7:02 ` Masahiro Yamada
  2021-02-02  8:44   ` John Ogness
  2021-02-02 12:29     ` kernel test robot
  -1 siblings, 2 replies; 26+ messages in thread
From: Masahiro Yamada @ 2021-02-02  7:02 UTC (permalink / raw)
  To: linux-kernel, Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
	John Ogness
  Cc: Masahiro Yamada

This macro is only used in kernel/printk/printk.c

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

 include/linux/printk.h | 2 --
 kernel/printk/printk.c | 2 ++
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index ceaf0486c01c..d2c9c2a6e471 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -44,8 +44,6 @@ static inline const char *printk_skip_headers(const char *buffer)
 	return buffer;
 }
 
-#define CONSOLE_EXT_LOG_MAX	8192
-
 /* We show everything that is MORE important than this.. */
 #define CONSOLE_LOGLEVEL_SILENT  0 /* Mum's the word */
 #define CONSOLE_LOGLEVEL_DEBUG	10 /* issue debug messages */
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 7b50298d52e3..d83a5860fe93 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -617,6 +617,8 @@ static ssize_t msg_print_ext_body(char *buf, size_t size,
 	return len;
 }
 
+#define CONSOLE_EXT_LOG_MAX	8192
+
 /* /dev/kmsg - userspace message inject/listen interface */
 struct devkmsg_user {
 	u64 seq;
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/3] printk: use CONFIG_CONSOLE_LOGLEVEL_* directly
  2021-02-02  7:02 ` Masahiro Yamada
@ 2021-02-02  8:38   ` John Ogness
  -1 siblings, 0 replies; 26+ messages in thread
From: John Ogness @ 2021-02-02  8:38 UTC (permalink / raw)
  To: Masahiro Yamada, linux-kernel, Petr Mladek, Sergey Senozhatsky,
	Steven Rostedt
  Cc: Masahiro Yamada, Andy Shevchenko, Ard Biesheuvel,
	Borislav Petkov, Darren Hart, Dimitri Sivanich,
	Greg Kroah-Hartman, H. Peter Anvin, Ingo Molnar, Jiri Slaby,
	Mike Travis, Peter Jones, Russ Anderson, Steve Wahl,
	Thomas Gleixner, dri-devel, linux-efi, linux-fbdev,
	platform-driver-x86, x86

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/3] printk: use CONFIG_CONSOLE_LOGLEVEL_* directly
@ 2021-02-02  8:38   ` John Ogness
  0 siblings, 0 replies; 26+ messages in thread
From: John Ogness @ 2021-02-02  8:38 UTC (permalink / raw)
  To: Masahiro Yamada, linux-kernel, Petr Mladek, Sergey Senozhatsky,
	Steven Rostedt
  Cc: Dimitri Sivanich, linux-fbdev, linux-efi, Russ Anderson,
	Steve Wahl, Mike Travis, Greg Kroah-Hartman, Masahiro Yamada,
	Peter Jones, x86, dri-devel, platform-driver-x86, Ingo Molnar,
	Borislav Petkov, H. Peter Anvin, Darren Hart, Thomas Gleixner,
	Jiri Slaby, Ard Biesheuvel, Andy Shevchenko

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 3/3] printk: move CONSOLE_EXT_LOG_MAX to kernel/printk/printk.c
  2021-02-02  7:02 ` [PATCH 3/3] printk: move CONSOLE_EXT_LOG_MAX to kernel/printk/printk.c Masahiro Yamada
@ 2021-02-02  8:44   ` John Ogness
  2021-02-02 10:04     ` Sergey Senozhatsky
  2021-02-02 12:29     ` kernel test robot
  1 sibling, 1 reply; 26+ messages in thread
From: John Ogness @ 2021-02-02  8:44 UTC (permalink / raw)
  To: Masahiro Yamada, linux-kernel, Petr Mladek, Sergey Senozhatsky,
	Steven Rostedt
  Cc: Masahiro Yamada

On 2021-02-02, Masahiro Yamada <masahiroy@kernel.org> wrote:
> This macro is only used in kernel/printk/printk.c

I recently posted a patch [0] that added another macro CONSOLE_LOG_MAX
here. But it also is only used in printk.c. I see no reason why either
should be in the header. Neither my patch nor commit d43ff430f434
("printk: guard the amount written per line by devkmsg_read()") show any
motivation for using printk.h.

I am fine with moving them out. The only consequences could be
out-of-tree modules breaking, but do we care about that?

John Ogness

[0] https://lkml.kernel.org/r/20210126211551.26536-5-john.ogness@linutronix.de

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 3/3] printk: move CONSOLE_EXT_LOG_MAX to kernel/printk/printk.c
  2021-02-02  8:44   ` John Ogness
@ 2021-02-02 10:04     ` Sergey Senozhatsky
  0 siblings, 0 replies; 26+ messages in thread
From: Sergey Senozhatsky @ 2021-02-02 10:04 UTC (permalink / raw)
  To: John Ogness
  Cc: Masahiro Yamada, linux-kernel, Petr Mladek, Sergey Senozhatsky,
	Steven Rostedt

On (21/02/02 09:50), John Ogness wrote:
> 
> The only consequences could be out-of-tree modules breaking,
> but do we care about that?

Yeah, I don't think anyone does. So we are fine here.

	-ss

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/3] printk: hard-code CONSOLE_LOGLEVEL_MIN in printk.c
  2021-02-02  7:02 ` [PATCH 2/3] printk: hard-code CONSOLE_LOGLEVEL_MIN in printk.c Masahiro Yamada
@ 2021-02-02 10:06   ` Sergey Senozhatsky
  2021-02-02 12:51     ` Joe Perches
  0 siblings, 1 reply; 26+ messages in thread
From: Sergey Senozhatsky @ 2021-02-02 10:06 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: linux-kernel, Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
	John Ogness

On (21/02/02 16:02), Masahiro Yamada wrote:
>  include/linux/printk.h | 1 -
>  kernel/printk/printk.c | 2 +-
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/include/linux/printk.h b/include/linux/printk.h
> index fd34b3aa2f90..ceaf0486c01c 100644
> --- a/include/linux/printk.h
> +++ b/include/linux/printk.h
> @@ -48,7 +48,6 @@ static inline const char *printk_skip_headers(const char *buffer)
>  
>  /* 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 */
>  
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 92b93340905c..7b50298d52e3 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -63,7 +63,7 @@
>  int console_printk[4] = {
>  	CONFIG_CONSOLE_LOGLEVEL_DEFAULT,	/* console_loglevel */
>  	CONFIG_MESSAGE_LOGLEVEL_DEFAULT,	/* default_message_loglevel */
> -	CONSOLE_LOGLEVEL_MIN,		/* minimum_console_loglevel */
> +	1,					/* minimum_console_loglevel */
>  	CONFIG_CONSOLE_LOGLEVEL_DEFAULT,	/* default_console_loglevel */

I personally don't think that this improves code readability.

	-ss

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/3] printk: use CONFIG_CONSOLE_LOGLEVEL_* directly
  2021-02-02  7:02 ` Masahiro Yamada
@ 2021-02-02 10:09   ` Sergey Senozhatsky
  -1 siblings, 0 replies; 26+ messages in thread
From: Sergey Senozhatsky @ 2021-02-02 10:09 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: linux-kernel, Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
	John Ogness, Andy Shevchenko, Ard Biesheuvel, Borislav Petkov,
	Darren Hart, Dimitri Sivanich, Greg Kroah-Hartman,
	H. Peter Anvin, Ingo Molnar, Jiri Slaby, Mike Travis,
	Peter Jones, Russ Anderson, Steve Wahl, Thomas Gleixner,
	dri-devel, linux-efi, linux-fbdev, platform-driver-x86, x86

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/3] printk: use CONFIG_CONSOLE_LOGLEVEL_* directly
@ 2021-02-02 10:09   ` Sergey Senozhatsky
  0 siblings, 0 replies; 26+ messages in thread
From: Sergey Senozhatsky @ 2021-02-02 10:09 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: linux-fbdev, linux-efi, Russ Anderson, Steve Wahl, dri-devel,
	platform-driver-x86, Peter Jones, H. Peter Anvin, Jiri Slaby,
	Ard Biesheuvel, x86, Ingo Molnar, Darren Hart, Petr Mladek,
	John Ogness, Mike Travis, Steven Rostedt, Borislav Petkov,
	Thomas Gleixner, Dimitri Sivanich, Greg Kroah-Hartman,
	linux-kernel, Sergey Senozhatsky, Andy Shevchenko

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/3] printk: use CONFIG_CONSOLE_LOGLEVEL_* directly
  2021-02-02  7:02 ` Masahiro Yamada
@ 2021-02-02 10:12   ` Greg Kroah-Hartman
  -1 siblings, 0 replies; 26+ messages in thread
From: Greg Kroah-Hartman @ 2021-02-02 10:12 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: linux-kernel, Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
	John Ogness, Andy Shevchenko, Ard Biesheuvel, Borislav Petkov,
	Darren Hart, Dimitri Sivanich, H. Peter Anvin, Ingo Molnar,
	Jiri Slaby, Mike Travis, Peter Jones, Russ Anderson, Steve Wahl,
	Thomas Gleixner, dri-devel, linux-efi, linux-fbdev,
	platform-driver-x86, x86

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>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/3] printk: use CONFIG_CONSOLE_LOGLEVEL_* directly
@ 2021-02-02 10:12   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 26+ messages in thread
From: Greg Kroah-Hartman @ 2021-02-02 10:12 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: linux-fbdev, linux-efi, Russ Anderson, Steve Wahl, dri-devel,
	platform-driver-x86, Peter Jones, H. Peter Anvin, Jiri Slaby,
	Ard Biesheuvel, x86, Ingo Molnar, Darren Hart, Petr Mladek,
	John Ogness, Mike Travis, Steven Rostedt, Borislav Petkov,
	Thomas Gleixner, Dimitri Sivanich, linux-kernel,
	Sergey Senozhatsky, Andy Shevchenko

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>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 3/3] printk: move CONSOLE_EXT_LOG_MAX to kernel/printk/printk.c
  2021-02-02  7:02 ` [PATCH 3/3] printk: move CONSOLE_EXT_LOG_MAX to kernel/printk/printk.c Masahiro Yamada
@ 2021-02-02 12:29     ` kernel test robot
  2021-02-02 12:29     ` kernel test robot
  1 sibling, 0 replies; 26+ messages in thread
From: kernel test robot @ 2021-02-02 12:29 UTC (permalink / raw)
  To: Masahiro Yamada, linux-kernel, Petr Mladek, Sergey Senozhatsky,
	Steven Rostedt, John Ogness
  Cc: kbuild-all, Masahiro Yamada

[-- Attachment #1: Type: text/plain, Size: 19664 bytes --]

Hi Masahiro,

I love your patch! Yet something to improve:

[auto build test ERROR on linux/master]
[also build test ERROR on efi/next tty/tty-testing tip/x86/core v5.11-rc6 next-20210125]
[cannot apply to pmladek/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Masahiro-Yamada/printk-use-CONFIG_CONSOLE_LOGLEVEL_-directly/20210202-151411
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 2ab38c17aac10bf55ab3efde4c4db3893d8691d2
config: alpha-randconfig-s031-20210202 (attached as .config)
compiler: alpha-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.3-215-g0fb77bb6-dirty
        # https://github.com/0day-ci/linux/commit/35d219bfad62e5008215f996430732aeb52c0652
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Masahiro-Yamada/printk-use-CONFIG_CONSOLE_LOGLEVEL_-directly/20210202-151411
        git checkout 35d219bfad62e5008215f996430732aeb52c0652
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=alpha 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from kernel/printk/printk.c:61:
   kernel/printk/internal.h:59:20: warning: no previous prototype for 'vprintk_func' [-Wmissing-prototypes]
      59 | __printf(1, 0) int vprintk_func(const char *fmt, va_list args) { return 0; }
         |                    ^~~~~~~~~~~~
   kernel/printk/printk.c:175:5: warning: no previous prototype for 'devkmsg_sysctl_set_loglvl' [-Wmissing-prototypes]
     175 | int devkmsg_sysctl_set_loglvl(struct ctl_table *table, int write,
         |     ^~~~~~~~~~~~~~~~~~~~~~~~~
   kernel/printk/printk.c: In function 'console_unlock':
>> kernel/printk/printk.c:2469:23: error: 'CONSOLE_EXT_LOG_MAX' undeclared (first use in this function)
    2469 |  static char ext_text[CONSOLE_EXT_LOG_MAX];
         |                       ^~~~~~~~~~~~~~~~~~~
   kernel/printk/printk.c:2469:23: note: each undeclared identifier is reported only once for each function it appears in
   kernel/printk/printk.c:2469:14: warning: unused variable 'ext_text' [-Wunused-variable]
    2469 |  static char ext_text[CONSOLE_EXT_LOG_MAX];
         |              ^~~~~~~~


vim +/CONSOLE_EXT_LOG_MAX +2469 kernel/printk/printk.c

a8199371afc279 kernel/printk/printk.c Sergey Senozhatsky      2016-03-17  2452  
^1da177e4c3f41 kernel/printk.c        Linus Torvalds          2005-04-16  2453  /**
ac751efa6a0d70 kernel/printk.c        Torben Hohn             2011-01-25  2454   * console_unlock - unlock the console system
^1da177e4c3f41 kernel/printk.c        Linus Torvalds          2005-04-16  2455   *
ac751efa6a0d70 kernel/printk.c        Torben Hohn             2011-01-25  2456   * Releases the console_lock which the caller holds on the console system
^1da177e4c3f41 kernel/printk.c        Linus Torvalds          2005-04-16  2457   * and the console driver list.
^1da177e4c3f41 kernel/printk.c        Linus Torvalds          2005-04-16  2458   *
ac751efa6a0d70 kernel/printk.c        Torben Hohn             2011-01-25  2459   * While the console_lock was held, console output may have been buffered
ac751efa6a0d70 kernel/printk.c        Torben Hohn             2011-01-25  2460   * by printk().  If this is the case, console_unlock(); emits
ac751efa6a0d70 kernel/printk.c        Torben Hohn             2011-01-25  2461   * the output prior to releasing the lock.
^1da177e4c3f41 kernel/printk.c        Linus Torvalds          2005-04-16  2462   *
7f3a781d6fd81e kernel/printk.c        Kay Sievers             2012-05-09  2463   * If there is output waiting, we wake /dev/kmsg and syslog() users.
^1da177e4c3f41 kernel/printk.c        Linus Torvalds          2005-04-16  2464   *
ac751efa6a0d70 kernel/printk.c        Torben Hohn             2011-01-25  2465   * console_unlock(); may be called from any context.
^1da177e4c3f41 kernel/printk.c        Linus Torvalds          2005-04-16  2466   */
ac751efa6a0d70 kernel/printk.c        Torben Hohn             2011-01-25  2467  void console_unlock(void)
^1da177e4c3f41 kernel/printk.c        Linus Torvalds          2005-04-16  2468  {
6fe29354befe4c kernel/printk/printk.c Tejun Heo               2015-06-25 @2469  	static char ext_text[CONSOLE_EXT_LOG_MAX];
70498253186586 kernel/printk.c        Kay Sievers             2012-07-16  2470  	static char text[LOG_LINE_MAX + PREFIX_MAX];
^1da177e4c3f41 kernel/printk.c        Linus Torvalds          2005-04-16  2471  	unsigned long flags;
8d91f8b15361df kernel/printk/printk.c Tejun Heo               2016-01-15  2472  	bool do_cond_resched, retry;
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2473  	struct printk_info info;
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2474  	struct printk_record r;
^1da177e4c3f41 kernel/printk.c        Linus Torvalds          2005-04-16  2475  
557240b48e2dc4 kernel/printk.c        Linus Torvalds          2006-06-19  2476  	if (console_suspended) {
bd8d7cf5b8410f kernel/printk/printk.c Jan Kara                2014-06-04  2477  		up_console_sem();
557240b48e2dc4 kernel/printk.c        Linus Torvalds          2006-06-19  2478  		return;
557240b48e2dc4 kernel/printk.c        Linus Torvalds          2006-06-19  2479  	}
78944e549d3667 kernel/printk.c        Antonino A. Daplas      2006-08-05  2480  
f35efc78add643 kernel/printk/printk.c John Ogness             2020-09-19  2481  	prb_rec_init_rd(&r, &info, text, sizeof(text));
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2482  
8d91f8b15361df kernel/printk/printk.c Tejun Heo               2016-01-15  2483  	/*
257ab443118bff kernel/printk/printk.c Petr Mladek             2017-03-24  2484  	 * Console drivers are called with interrupts disabled, so
8d91f8b15361df kernel/printk/printk.c Tejun Heo               2016-01-15  2485  	 * @console_may_schedule should be cleared before; however, we may
8d91f8b15361df kernel/printk/printk.c Tejun Heo               2016-01-15  2486  	 * end up dumping a lot of lines, for example, if called from
8d91f8b15361df kernel/printk/printk.c Tejun Heo               2016-01-15  2487  	 * console registration path, and should invoke cond_resched()
8d91f8b15361df kernel/printk/printk.c Tejun Heo               2016-01-15  2488  	 * between lines if allowable.  Not doing so can cause a very long
8d91f8b15361df kernel/printk/printk.c Tejun Heo               2016-01-15  2489  	 * scheduling stall on a slow console leading to RCU stall and
8d91f8b15361df kernel/printk/printk.c Tejun Heo               2016-01-15  2490  	 * softlockup warnings which exacerbate the issue with more
8d91f8b15361df kernel/printk/printk.c Tejun Heo               2016-01-15  2491  	 * messages practically incapacitating the system.
257ab443118bff kernel/printk/printk.c Petr Mladek             2017-03-24  2492  	 *
257ab443118bff kernel/printk/printk.c Petr Mladek             2017-03-24  2493  	 * console_trylock() is not able to detect the preemptive
257ab443118bff kernel/printk/printk.c Petr Mladek             2017-03-24  2494  	 * context reliably. Therefore the value must be stored before
547bbf7d214fff kernel/printk/printk.c Randy Dunlap            2020-08-06  2495  	 * and cleared after the "again" goto label.
8d91f8b15361df kernel/printk/printk.c Tejun Heo               2016-01-15  2496  	 */
8d91f8b15361df kernel/printk/printk.c Tejun Heo               2016-01-15  2497  	do_cond_resched = console_may_schedule;
257ab443118bff kernel/printk/printk.c Petr Mladek             2017-03-24  2498  again:
78944e549d3667 kernel/printk.c        Antonino A. Daplas      2006-08-05  2499  	console_may_schedule = 0;
78944e549d3667 kernel/printk.c        Antonino A. Daplas      2006-08-05  2500  
a8199371afc279 kernel/printk/printk.c Sergey Senozhatsky      2016-03-17  2501  	/*
a8199371afc279 kernel/printk/printk.c Sergey Senozhatsky      2016-03-17  2502  	 * We released the console_sem lock, so we need to recheck if
a8199371afc279 kernel/printk/printk.c Sergey Senozhatsky      2016-03-17  2503  	 * cpu is online and (if not) is there at least one CON_ANYTIME
a8199371afc279 kernel/printk/printk.c Sergey Senozhatsky      2016-03-17  2504  	 * console.
a8199371afc279 kernel/printk/printk.c Sergey Senozhatsky      2016-03-17  2505  	 */
a8199371afc279 kernel/printk/printk.c Sergey Senozhatsky      2016-03-17  2506  	if (!can_use_console()) {
a8199371afc279 kernel/printk/printk.c Sergey Senozhatsky      2016-03-17  2507  		console_locked = 0;
a8199371afc279 kernel/printk/printk.c Sergey Senozhatsky      2016-03-17  2508  		up_console_sem();
a8199371afc279 kernel/printk/printk.c Sergey Senozhatsky      2016-03-17  2509  		return;
a8199371afc279 kernel/printk/printk.c Sergey Senozhatsky      2016-03-17  2510  	}
a8199371afc279 kernel/printk/printk.c Sergey Senozhatsky      2016-03-17  2511  
^1da177e4c3f41 kernel/printk.c        Linus Torvalds          2005-04-16  2512  	for (;;) {
6fe29354befe4c kernel/printk/printk.c Tejun Heo               2015-06-25  2513  		size_t ext_len = 0;
3ce9a7c0ac2856 kernel/printk.c        Kay Sievers             2012-05-13  2514  		size_t len;
7ff9554bb578ba kernel/printk.c        Kay Sievers             2012-05-03  2515  
f975237b768279 kernel/printk/printk.c Sergey Senozhatsky      2016-12-27  2516  		printk_safe_enter_irqsave(flags);
f975237b768279 kernel/printk/printk.c Sergey Senozhatsky      2016-12-27  2517  		raw_spin_lock(&logbuf_lock);
084681d14e429c kernel/printk.c        Kay Sievers             2012-06-28  2518  skip:
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2519  		if (!prb_read_valid(prb, console_seq, &r))
7ff9554bb578ba kernel/printk.c        Kay Sievers             2012-05-03  2520  			break;
7ff9554bb578ba kernel/printk.c        Kay Sievers             2012-05-03  2521  
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2522  		if (console_seq != r.info->seq) {
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2523  			console_dropped += r.info->seq - console_seq;
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2524  			console_seq = r.info->seq;
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2525  		}
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2526  
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2527  		if (suppress_message_printing(r.info->level)) {
084681d14e429c kernel/printk.c        Kay Sievers             2012-06-28  2528  			/*
a6ae928c25835c kernel/printk/printk.c Petr Mladek             2018-09-10  2529  			 * Skip record we have buffered and already printed
a6ae928c25835c kernel/printk/printk.c Petr Mladek             2018-09-10  2530  			 * directly to the console when we received it, and
a6ae928c25835c kernel/printk/printk.c Petr Mladek             2018-09-10  2531  			 * record that has level above the console loglevel.
084681d14e429c kernel/printk.c        Kay Sievers             2012-06-28  2532  			 */
084681d14e429c kernel/printk.c        Kay Sievers             2012-06-28  2533  			console_seq++;
084681d14e429c kernel/printk.c        Kay Sievers             2012-06-28  2534  			goto skip;
084681d14e429c kernel/printk.c        Kay Sievers             2012-06-28  2535  		}
649e6ee33f73ba kernel/printk.c        Kay Sievers             2012-05-10  2536  
f92b070f2dc89a kernel/printk/printk.c Petr Mladek             2018-09-13  2537  		/* Output to all consoles once old messages replayed. */
f92b070f2dc89a kernel/printk/printk.c Petr Mladek             2018-09-13  2538  		if (unlikely(exclusive_console &&
f92b070f2dc89a kernel/printk/printk.c Petr Mladek             2018-09-13  2539  			     console_seq >= exclusive_console_stop_seq)) {
f92b070f2dc89a kernel/printk/printk.c Petr Mladek             2018-09-13  2540  			exclusive_console = NULL;
f92b070f2dc89a kernel/printk/printk.c Petr Mladek             2018-09-13  2541  		}
f92b070f2dc89a kernel/printk/printk.c Petr Mladek             2018-09-13  2542  
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2543  		/*
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2544  		 * Handle extended console text first because later
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2545  		 * record_print_text() will modify the record buffer in-place.
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2546  		 */
6fe29354befe4c kernel/printk/printk.c Tejun Heo               2015-06-25  2547  		if (nr_ext_console_drivers) {
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2548  			ext_len = info_print_ext_header(ext_text,
6fe29354befe4c kernel/printk/printk.c Tejun Heo               2015-06-25  2549  						sizeof(ext_text),
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2550  						r.info);
6fe29354befe4c kernel/printk/printk.c Tejun Heo               2015-06-25  2551  			ext_len += msg_print_ext_body(ext_text + ext_len,
6fe29354befe4c kernel/printk/printk.c Tejun Heo               2015-06-25  2552  						sizeof(ext_text) - ext_len,
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2553  						&r.text_buf[0],
74caba7f2a0685 kernel/printk/printk.c John Ogness             2020-09-21  2554  						r.info->text_len,
74caba7f2a0685 kernel/printk/printk.c John Ogness             2020-09-21  2555  						&r.info->dev_info);
6fe29354befe4c kernel/printk/printk.c Tejun Heo               2015-06-25  2556  		}
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2557  		len = record_print_text(&r,
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2558  				console_msg_format & MSG_FORMAT_SYSLOG,
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2559  				printk_time);
7ff9554bb578ba kernel/printk.c        Kay Sievers             2012-05-03  2560  		console_seq++;
07354eb1a74d1e kernel/printk.c        Thomas Gleixner         2009-07-25  2561  		raw_spin_unlock(&logbuf_lock);
7ff9554bb578ba kernel/printk.c        Kay Sievers             2012-05-03  2562  
dbdda842fe96f8 kernel/printk/printk.c Steven Rostedt (VMware  2018-01-10  2563) 		/*
dbdda842fe96f8 kernel/printk/printk.c Steven Rostedt (VMware  2018-01-10  2564) 		 * While actively printing out messages, if another printk()
dbdda842fe96f8 kernel/printk/printk.c Steven Rostedt (VMware  2018-01-10  2565) 		 * were to occur on another CPU, it may wait for this one to
dbdda842fe96f8 kernel/printk/printk.c Steven Rostedt (VMware  2018-01-10  2566) 		 * finish. This task can not be preempted if there is a
dbdda842fe96f8 kernel/printk/printk.c Steven Rostedt (VMware  2018-01-10  2567) 		 * waiter waiting to take over.
dbdda842fe96f8 kernel/printk/printk.c Steven Rostedt (VMware  2018-01-10  2568) 		 */
c162d5b4338d72 kernel/printk/printk.c Petr Mladek             2018-01-12  2569  		console_lock_spinning_enable();
dbdda842fe96f8 kernel/printk/printk.c Steven Rostedt (VMware  2018-01-10  2570) 
81d68a96a39844 kernel/printk.c        Steven Rostedt          2008-05-12  2571  		stop_critical_timings();	/* don't trace print latency */
d9c23523ed98a3 kernel/printk/printk.c Sergey Senozhatsky      2016-12-24  2572  		call_console_drivers(ext_text, ext_len, text, len);
81d68a96a39844 kernel/printk.c        Steven Rostedt          2008-05-12  2573  		start_critical_timings();
dbdda842fe96f8 kernel/printk/printk.c Steven Rostedt (VMware  2018-01-10  2574) 
c162d5b4338d72 kernel/printk/printk.c Petr Mladek             2018-01-12  2575  		if (console_lock_spinning_disable_and_check()) {
c162d5b4338d72 kernel/printk/printk.c Petr Mladek             2018-01-12  2576  			printk_safe_exit_irqrestore(flags);
43a17111c25539 kernel/printk/printk.c Sergey Senozhatsky      2018-04-19  2577  			return;
c162d5b4338d72 kernel/printk/printk.c Petr Mladek             2018-01-12  2578  		}
dbdda842fe96f8 kernel/printk/printk.c Steven Rostedt (VMware  2018-01-10  2579) 
f975237b768279 kernel/printk/printk.c Sergey Senozhatsky      2016-12-27  2580  		printk_safe_exit_irqrestore(flags);
8d91f8b15361df kernel/printk/printk.c Tejun Heo               2016-01-15  2581  
8d91f8b15361df kernel/printk/printk.c Tejun Heo               2016-01-15  2582  		if (do_cond_resched)
8d91f8b15361df kernel/printk/printk.c Tejun Heo               2016-01-15  2583  			cond_resched();
^1da177e4c3f41 kernel/printk.c        Linus Torvalds          2005-04-16  2584  	}
dbdda842fe96f8 kernel/printk/printk.c Steven Rostedt (VMware  2018-01-10  2585) 
^1da177e4c3f41 kernel/printk.c        Linus Torvalds          2005-04-16  2586  	console_locked = 0;
fe3d8ad31cf51b kernel/printk.c        Feng Tang               2011-03-22  2587  
07354eb1a74d1e kernel/printk.c        Thomas Gleixner         2009-07-25  2588  	raw_spin_unlock(&logbuf_lock);
4f2a8d3cf5e048 kernel/printk.c        Peter Zijlstra          2011-06-22  2589  
bd8d7cf5b8410f kernel/printk/printk.c Jan Kara                2014-06-04  2590  	up_console_sem();
4f2a8d3cf5e048 kernel/printk.c        Peter Zijlstra          2011-06-22  2591  
4f2a8d3cf5e048 kernel/printk.c        Peter Zijlstra          2011-06-22  2592  	/*
4f2a8d3cf5e048 kernel/printk.c        Peter Zijlstra          2011-06-22  2593  	 * Someone could have filled up the buffer again, so re-check if there's
4f2a8d3cf5e048 kernel/printk.c        Peter Zijlstra          2011-06-22  2594  	 * something to flush. In case we cannot trylock the console_sem again,
4f2a8d3cf5e048 kernel/printk.c        Peter Zijlstra          2011-06-22  2595  	 * there's a new owner and the console_unlock() from them will do the
4f2a8d3cf5e048 kernel/printk.c        Peter Zijlstra          2011-06-22  2596  	 * flush, no worries.
4f2a8d3cf5e048 kernel/printk.c        Peter Zijlstra          2011-06-22  2597  	 */
07354eb1a74d1e kernel/printk.c        Thomas Gleixner         2009-07-25  2598  	raw_spin_lock(&logbuf_lock);
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2599  	retry = prb_read_valid(prb, console_seq, NULL);
f975237b768279 kernel/printk/printk.c Sergey Senozhatsky      2016-12-27  2600  	raw_spin_unlock(&logbuf_lock);
f975237b768279 kernel/printk/printk.c Sergey Senozhatsky      2016-12-27  2601  	printk_safe_exit_irqrestore(flags);
09dc3cf93f7d16 kernel/printk.c        Peter Zijlstra          2011-12-08  2602  
4f2a8d3cf5e048 kernel/printk.c        Peter Zijlstra          2011-06-22  2603  	if (retry && console_trylock())
4f2a8d3cf5e048 kernel/printk.c        Peter Zijlstra          2011-06-22  2604  		goto again;
^1da177e4c3f41 kernel/printk.c        Linus Torvalds          2005-04-16  2605  }
ac751efa6a0d70 kernel/printk.c        Torben Hohn             2011-01-25  2606  EXPORT_SYMBOL(console_unlock);
^1da177e4c3f41 kernel/printk.c        Linus Torvalds          2005-04-16  2607  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25400 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 3/3] printk: move CONSOLE_EXT_LOG_MAX to kernel/printk/printk.c
@ 2021-02-02 12:29     ` kernel test robot
  0 siblings, 0 replies; 26+ messages in thread
From: kernel test robot @ 2021-02-02 12:29 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 19877 bytes --]

Hi Masahiro,

I love your patch! Yet something to improve:

[auto build test ERROR on linux/master]
[also build test ERROR on efi/next tty/tty-testing tip/x86/core v5.11-rc6 next-20210125]
[cannot apply to pmladek/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Masahiro-Yamada/printk-use-CONFIG_CONSOLE_LOGLEVEL_-directly/20210202-151411
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 2ab38c17aac10bf55ab3efde4c4db3893d8691d2
config: alpha-randconfig-s031-20210202 (attached as .config)
compiler: alpha-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.3-215-g0fb77bb6-dirty
        # https://github.com/0day-ci/linux/commit/35d219bfad62e5008215f996430732aeb52c0652
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Masahiro-Yamada/printk-use-CONFIG_CONSOLE_LOGLEVEL_-directly/20210202-151411
        git checkout 35d219bfad62e5008215f996430732aeb52c0652
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=alpha 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from kernel/printk/printk.c:61:
   kernel/printk/internal.h:59:20: warning: no previous prototype for 'vprintk_func' [-Wmissing-prototypes]
      59 | __printf(1, 0) int vprintk_func(const char *fmt, va_list args) { return 0; }
         |                    ^~~~~~~~~~~~
   kernel/printk/printk.c:175:5: warning: no previous prototype for 'devkmsg_sysctl_set_loglvl' [-Wmissing-prototypes]
     175 | int devkmsg_sysctl_set_loglvl(struct ctl_table *table, int write,
         |     ^~~~~~~~~~~~~~~~~~~~~~~~~
   kernel/printk/printk.c: In function 'console_unlock':
>> kernel/printk/printk.c:2469:23: error: 'CONSOLE_EXT_LOG_MAX' undeclared (first use in this function)
    2469 |  static char ext_text[CONSOLE_EXT_LOG_MAX];
         |                       ^~~~~~~~~~~~~~~~~~~
   kernel/printk/printk.c:2469:23: note: each undeclared identifier is reported only once for each function it appears in
   kernel/printk/printk.c:2469:14: warning: unused variable 'ext_text' [-Wunused-variable]
    2469 |  static char ext_text[CONSOLE_EXT_LOG_MAX];
         |              ^~~~~~~~


vim +/CONSOLE_EXT_LOG_MAX +2469 kernel/printk/printk.c

a8199371afc279 kernel/printk/printk.c Sergey Senozhatsky      2016-03-17  2452  
^1da177e4c3f41 kernel/printk.c        Linus Torvalds          2005-04-16  2453  /**
ac751efa6a0d70 kernel/printk.c        Torben Hohn             2011-01-25  2454   * console_unlock - unlock the console system
^1da177e4c3f41 kernel/printk.c        Linus Torvalds          2005-04-16  2455   *
ac751efa6a0d70 kernel/printk.c        Torben Hohn             2011-01-25  2456   * Releases the console_lock which the caller holds on the console system
^1da177e4c3f41 kernel/printk.c        Linus Torvalds          2005-04-16  2457   * and the console driver list.
^1da177e4c3f41 kernel/printk.c        Linus Torvalds          2005-04-16  2458   *
ac751efa6a0d70 kernel/printk.c        Torben Hohn             2011-01-25  2459   * While the console_lock was held, console output may have been buffered
ac751efa6a0d70 kernel/printk.c        Torben Hohn             2011-01-25  2460   * by printk().  If this is the case, console_unlock(); emits
ac751efa6a0d70 kernel/printk.c        Torben Hohn             2011-01-25  2461   * the output prior to releasing the lock.
^1da177e4c3f41 kernel/printk.c        Linus Torvalds          2005-04-16  2462   *
7f3a781d6fd81e kernel/printk.c        Kay Sievers             2012-05-09  2463   * If there is output waiting, we wake /dev/kmsg and syslog() users.
^1da177e4c3f41 kernel/printk.c        Linus Torvalds          2005-04-16  2464   *
ac751efa6a0d70 kernel/printk.c        Torben Hohn             2011-01-25  2465   * console_unlock(); may be called from any context.
^1da177e4c3f41 kernel/printk.c        Linus Torvalds          2005-04-16  2466   */
ac751efa6a0d70 kernel/printk.c        Torben Hohn             2011-01-25  2467  void console_unlock(void)
^1da177e4c3f41 kernel/printk.c        Linus Torvalds          2005-04-16  2468  {
6fe29354befe4c kernel/printk/printk.c Tejun Heo               2015-06-25 @2469  	static char ext_text[CONSOLE_EXT_LOG_MAX];
70498253186586 kernel/printk.c        Kay Sievers             2012-07-16  2470  	static char text[LOG_LINE_MAX + PREFIX_MAX];
^1da177e4c3f41 kernel/printk.c        Linus Torvalds          2005-04-16  2471  	unsigned long flags;
8d91f8b15361df kernel/printk/printk.c Tejun Heo               2016-01-15  2472  	bool do_cond_resched, retry;
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2473  	struct printk_info info;
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2474  	struct printk_record r;
^1da177e4c3f41 kernel/printk.c        Linus Torvalds          2005-04-16  2475  
557240b48e2dc4 kernel/printk.c        Linus Torvalds          2006-06-19  2476  	if (console_suspended) {
bd8d7cf5b8410f kernel/printk/printk.c Jan Kara                2014-06-04  2477  		up_console_sem();
557240b48e2dc4 kernel/printk.c        Linus Torvalds          2006-06-19  2478  		return;
557240b48e2dc4 kernel/printk.c        Linus Torvalds          2006-06-19  2479  	}
78944e549d3667 kernel/printk.c        Antonino A. Daplas      2006-08-05  2480  
f35efc78add643 kernel/printk/printk.c John Ogness             2020-09-19  2481  	prb_rec_init_rd(&r, &info, text, sizeof(text));
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2482  
8d91f8b15361df kernel/printk/printk.c Tejun Heo               2016-01-15  2483  	/*
257ab443118bff kernel/printk/printk.c Petr Mladek             2017-03-24  2484  	 * Console drivers are called with interrupts disabled, so
8d91f8b15361df kernel/printk/printk.c Tejun Heo               2016-01-15  2485  	 * @console_may_schedule should be cleared before; however, we may
8d91f8b15361df kernel/printk/printk.c Tejun Heo               2016-01-15  2486  	 * end up dumping a lot of lines, for example, if called from
8d91f8b15361df kernel/printk/printk.c Tejun Heo               2016-01-15  2487  	 * console registration path, and should invoke cond_resched()
8d91f8b15361df kernel/printk/printk.c Tejun Heo               2016-01-15  2488  	 * between lines if allowable.  Not doing so can cause a very long
8d91f8b15361df kernel/printk/printk.c Tejun Heo               2016-01-15  2489  	 * scheduling stall on a slow console leading to RCU stall and
8d91f8b15361df kernel/printk/printk.c Tejun Heo               2016-01-15  2490  	 * softlockup warnings which exacerbate the issue with more
8d91f8b15361df kernel/printk/printk.c Tejun Heo               2016-01-15  2491  	 * messages practically incapacitating the system.
257ab443118bff kernel/printk/printk.c Petr Mladek             2017-03-24  2492  	 *
257ab443118bff kernel/printk/printk.c Petr Mladek             2017-03-24  2493  	 * console_trylock() is not able to detect the preemptive
257ab443118bff kernel/printk/printk.c Petr Mladek             2017-03-24  2494  	 * context reliably. Therefore the value must be stored before
547bbf7d214fff kernel/printk/printk.c Randy Dunlap            2020-08-06  2495  	 * and cleared after the "again" goto label.
8d91f8b15361df kernel/printk/printk.c Tejun Heo               2016-01-15  2496  	 */
8d91f8b15361df kernel/printk/printk.c Tejun Heo               2016-01-15  2497  	do_cond_resched = console_may_schedule;
257ab443118bff kernel/printk/printk.c Petr Mladek             2017-03-24  2498  again:
78944e549d3667 kernel/printk.c        Antonino A. Daplas      2006-08-05  2499  	console_may_schedule = 0;
78944e549d3667 kernel/printk.c        Antonino A. Daplas      2006-08-05  2500  
a8199371afc279 kernel/printk/printk.c Sergey Senozhatsky      2016-03-17  2501  	/*
a8199371afc279 kernel/printk/printk.c Sergey Senozhatsky      2016-03-17  2502  	 * We released the console_sem lock, so we need to recheck if
a8199371afc279 kernel/printk/printk.c Sergey Senozhatsky      2016-03-17  2503  	 * cpu is online and (if not) is there at least one CON_ANYTIME
a8199371afc279 kernel/printk/printk.c Sergey Senozhatsky      2016-03-17  2504  	 * console.
a8199371afc279 kernel/printk/printk.c Sergey Senozhatsky      2016-03-17  2505  	 */
a8199371afc279 kernel/printk/printk.c Sergey Senozhatsky      2016-03-17  2506  	if (!can_use_console()) {
a8199371afc279 kernel/printk/printk.c Sergey Senozhatsky      2016-03-17  2507  		console_locked = 0;
a8199371afc279 kernel/printk/printk.c Sergey Senozhatsky      2016-03-17  2508  		up_console_sem();
a8199371afc279 kernel/printk/printk.c Sergey Senozhatsky      2016-03-17  2509  		return;
a8199371afc279 kernel/printk/printk.c Sergey Senozhatsky      2016-03-17  2510  	}
a8199371afc279 kernel/printk/printk.c Sergey Senozhatsky      2016-03-17  2511  
^1da177e4c3f41 kernel/printk.c        Linus Torvalds          2005-04-16  2512  	for (;;) {
6fe29354befe4c kernel/printk/printk.c Tejun Heo               2015-06-25  2513  		size_t ext_len = 0;
3ce9a7c0ac2856 kernel/printk.c        Kay Sievers             2012-05-13  2514  		size_t len;
7ff9554bb578ba kernel/printk.c        Kay Sievers             2012-05-03  2515  
f975237b768279 kernel/printk/printk.c Sergey Senozhatsky      2016-12-27  2516  		printk_safe_enter_irqsave(flags);
f975237b768279 kernel/printk/printk.c Sergey Senozhatsky      2016-12-27  2517  		raw_spin_lock(&logbuf_lock);
084681d14e429c kernel/printk.c        Kay Sievers             2012-06-28  2518  skip:
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2519  		if (!prb_read_valid(prb, console_seq, &r))
7ff9554bb578ba kernel/printk.c        Kay Sievers             2012-05-03  2520  			break;
7ff9554bb578ba kernel/printk.c        Kay Sievers             2012-05-03  2521  
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2522  		if (console_seq != r.info->seq) {
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2523  			console_dropped += r.info->seq - console_seq;
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2524  			console_seq = r.info->seq;
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2525  		}
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2526  
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2527  		if (suppress_message_printing(r.info->level)) {
084681d14e429c kernel/printk.c        Kay Sievers             2012-06-28  2528  			/*
a6ae928c25835c kernel/printk/printk.c Petr Mladek             2018-09-10  2529  			 * Skip record we have buffered and already printed
a6ae928c25835c kernel/printk/printk.c Petr Mladek             2018-09-10  2530  			 * directly to the console when we received it, and
a6ae928c25835c kernel/printk/printk.c Petr Mladek             2018-09-10  2531  			 * record that has level above the console loglevel.
084681d14e429c kernel/printk.c        Kay Sievers             2012-06-28  2532  			 */
084681d14e429c kernel/printk.c        Kay Sievers             2012-06-28  2533  			console_seq++;
084681d14e429c kernel/printk.c        Kay Sievers             2012-06-28  2534  			goto skip;
084681d14e429c kernel/printk.c        Kay Sievers             2012-06-28  2535  		}
649e6ee33f73ba kernel/printk.c        Kay Sievers             2012-05-10  2536  
f92b070f2dc89a kernel/printk/printk.c Petr Mladek             2018-09-13  2537  		/* Output to all consoles once old messages replayed. */
f92b070f2dc89a kernel/printk/printk.c Petr Mladek             2018-09-13  2538  		if (unlikely(exclusive_console &&
f92b070f2dc89a kernel/printk/printk.c Petr Mladek             2018-09-13  2539  			     console_seq >= exclusive_console_stop_seq)) {
f92b070f2dc89a kernel/printk/printk.c Petr Mladek             2018-09-13  2540  			exclusive_console = NULL;
f92b070f2dc89a kernel/printk/printk.c Petr Mladek             2018-09-13  2541  		}
f92b070f2dc89a kernel/printk/printk.c Petr Mladek             2018-09-13  2542  
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2543  		/*
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2544  		 * Handle extended console text first because later
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2545  		 * record_print_text() will modify the record buffer in-place.
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2546  		 */
6fe29354befe4c kernel/printk/printk.c Tejun Heo               2015-06-25  2547  		if (nr_ext_console_drivers) {
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2548  			ext_len = info_print_ext_header(ext_text,
6fe29354befe4c kernel/printk/printk.c Tejun Heo               2015-06-25  2549  						sizeof(ext_text),
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2550  						r.info);
6fe29354befe4c kernel/printk/printk.c Tejun Heo               2015-06-25  2551  			ext_len += msg_print_ext_body(ext_text + ext_len,
6fe29354befe4c kernel/printk/printk.c Tejun Heo               2015-06-25  2552  						sizeof(ext_text) - ext_len,
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2553  						&r.text_buf[0],
74caba7f2a0685 kernel/printk/printk.c John Ogness             2020-09-21  2554  						r.info->text_len,
74caba7f2a0685 kernel/printk/printk.c John Ogness             2020-09-21  2555  						&r.info->dev_info);
6fe29354befe4c kernel/printk/printk.c Tejun Heo               2015-06-25  2556  		}
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2557  		len = record_print_text(&r,
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2558  				console_msg_format & MSG_FORMAT_SYSLOG,
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2559  				printk_time);
7ff9554bb578ba kernel/printk.c        Kay Sievers             2012-05-03  2560  		console_seq++;
07354eb1a74d1e kernel/printk.c        Thomas Gleixner         2009-07-25  2561  		raw_spin_unlock(&logbuf_lock);
7ff9554bb578ba kernel/printk.c        Kay Sievers             2012-05-03  2562  
dbdda842fe96f8 kernel/printk/printk.c Steven Rostedt (VMware  2018-01-10  2563) 		/*
dbdda842fe96f8 kernel/printk/printk.c Steven Rostedt (VMware  2018-01-10  2564) 		 * While actively printing out messages, if another printk()
dbdda842fe96f8 kernel/printk/printk.c Steven Rostedt (VMware  2018-01-10  2565) 		 * were to occur on another CPU, it may wait for this one to
dbdda842fe96f8 kernel/printk/printk.c Steven Rostedt (VMware  2018-01-10  2566) 		 * finish. This task can not be preempted if there is a
dbdda842fe96f8 kernel/printk/printk.c Steven Rostedt (VMware  2018-01-10  2567) 		 * waiter waiting to take over.
dbdda842fe96f8 kernel/printk/printk.c Steven Rostedt (VMware  2018-01-10  2568) 		 */
c162d5b4338d72 kernel/printk/printk.c Petr Mladek             2018-01-12  2569  		console_lock_spinning_enable();
dbdda842fe96f8 kernel/printk/printk.c Steven Rostedt (VMware  2018-01-10  2570) 
81d68a96a39844 kernel/printk.c        Steven Rostedt          2008-05-12  2571  		stop_critical_timings();	/* don't trace print latency */
d9c23523ed98a3 kernel/printk/printk.c Sergey Senozhatsky      2016-12-24  2572  		call_console_drivers(ext_text, ext_len, text, len);
81d68a96a39844 kernel/printk.c        Steven Rostedt          2008-05-12  2573  		start_critical_timings();
dbdda842fe96f8 kernel/printk/printk.c Steven Rostedt (VMware  2018-01-10  2574) 
c162d5b4338d72 kernel/printk/printk.c Petr Mladek             2018-01-12  2575  		if (console_lock_spinning_disable_and_check()) {
c162d5b4338d72 kernel/printk/printk.c Petr Mladek             2018-01-12  2576  			printk_safe_exit_irqrestore(flags);
43a17111c25539 kernel/printk/printk.c Sergey Senozhatsky      2018-04-19  2577  			return;
c162d5b4338d72 kernel/printk/printk.c Petr Mladek             2018-01-12  2578  		}
dbdda842fe96f8 kernel/printk/printk.c Steven Rostedt (VMware  2018-01-10  2579) 
f975237b768279 kernel/printk/printk.c Sergey Senozhatsky      2016-12-27  2580  		printk_safe_exit_irqrestore(flags);
8d91f8b15361df kernel/printk/printk.c Tejun Heo               2016-01-15  2581  
8d91f8b15361df kernel/printk/printk.c Tejun Heo               2016-01-15  2582  		if (do_cond_resched)
8d91f8b15361df kernel/printk/printk.c Tejun Heo               2016-01-15  2583  			cond_resched();
^1da177e4c3f41 kernel/printk.c        Linus Torvalds          2005-04-16  2584  	}
dbdda842fe96f8 kernel/printk/printk.c Steven Rostedt (VMware  2018-01-10  2585) 
^1da177e4c3f41 kernel/printk.c        Linus Torvalds          2005-04-16  2586  	console_locked = 0;
fe3d8ad31cf51b kernel/printk.c        Feng Tang               2011-03-22  2587  
07354eb1a74d1e kernel/printk.c        Thomas Gleixner         2009-07-25  2588  	raw_spin_unlock(&logbuf_lock);
4f2a8d3cf5e048 kernel/printk.c        Peter Zijlstra          2011-06-22  2589  
bd8d7cf5b8410f kernel/printk/printk.c Jan Kara                2014-06-04  2590  	up_console_sem();
4f2a8d3cf5e048 kernel/printk.c        Peter Zijlstra          2011-06-22  2591  
4f2a8d3cf5e048 kernel/printk.c        Peter Zijlstra          2011-06-22  2592  	/*
4f2a8d3cf5e048 kernel/printk.c        Peter Zijlstra          2011-06-22  2593  	 * Someone could have filled up the buffer again, so re-check if there's
4f2a8d3cf5e048 kernel/printk.c        Peter Zijlstra          2011-06-22  2594  	 * something to flush. In case we cannot trylock the console_sem again,
4f2a8d3cf5e048 kernel/printk.c        Peter Zijlstra          2011-06-22  2595  	 * there's a new owner and the console_unlock() from them will do the
4f2a8d3cf5e048 kernel/printk.c        Peter Zijlstra          2011-06-22  2596  	 * flush, no worries.
4f2a8d3cf5e048 kernel/printk.c        Peter Zijlstra          2011-06-22  2597  	 */
07354eb1a74d1e kernel/printk.c        Thomas Gleixner         2009-07-25  2598  	raw_spin_lock(&logbuf_lock);
896fbe20b4e233 kernel/printk/printk.c John Ogness             2020-07-09  2599  	retry = prb_read_valid(prb, console_seq, NULL);
f975237b768279 kernel/printk/printk.c Sergey Senozhatsky      2016-12-27  2600  	raw_spin_unlock(&logbuf_lock);
f975237b768279 kernel/printk/printk.c Sergey Senozhatsky      2016-12-27  2601  	printk_safe_exit_irqrestore(flags);
09dc3cf93f7d16 kernel/printk.c        Peter Zijlstra          2011-12-08  2602  
4f2a8d3cf5e048 kernel/printk.c        Peter Zijlstra          2011-06-22  2603  	if (retry && console_trylock())
4f2a8d3cf5e048 kernel/printk.c        Peter Zijlstra          2011-06-22  2604  		goto again;
^1da177e4c3f41 kernel/printk.c        Linus Torvalds          2005-04-16  2605  }
ac751efa6a0d70 kernel/printk.c        Torben Hohn             2011-01-25  2606  EXPORT_SYMBOL(console_unlock);
^1da177e4c3f41 kernel/printk.c        Linus Torvalds          2005-04-16  2607  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 25400 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/3] printk: hard-code CONSOLE_LOGLEVEL_MIN in printk.c
  2021-02-02 10:06   ` Sergey Senozhatsky
@ 2021-02-02 12:51     ` Joe Perches
  0 siblings, 0 replies; 26+ messages in thread
From: Joe Perches @ 2021-02-02 12:51 UTC (permalink / raw)
  To: Sergey Senozhatsky, Masahiro Yamada
  Cc: linux-kernel, Petr Mladek, Steven Rostedt, John Ogness

On Tue, 2021-02-02 at 19:06 +0900, Sergey Senozhatsky wrote:
> On (21/02/02 16:02), Masahiro Yamada wrote:
> >  include/linux/printk.h | 1 -
> >  kernel/printk/printk.c | 2 +-
> >  2 files changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/include/linux/printk.h b/include/linux/printk.h
> > index fd34b3aa2f90..ceaf0486c01c 100644
> > --- a/include/linux/printk.h
> > +++ b/include/linux/printk.h
> > @@ -48,7 +48,6 @@ static inline const char *printk_skip_headers(const char *buffer)
> >  
> > 
> >  /* 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 */
> >  
> > 
> > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
[]
> > @@ -63,7 +63,7 @@
> >  int console_printk[4] = {
> >  	CONFIG_CONSOLE_LOGLEVEL_DEFAULT,	/* console_loglevel */
> >  	CONFIG_MESSAGE_LOGLEVEL_DEFAULT,	/* default_message_loglevel */
> > -	CONSOLE_LOGLEVEL_MIN,		/* minimum_console_loglevel */
> > +	1,					/* minimum_console_loglevel */
> >  	CONFIG_CONSOLE_LOGLEVEL_DEFAULT,	/* default_console_loglevel */
> 
> I personally don't think that this improves code readability.

Nor maintainability.



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/3] printk: use CONFIG_CONSOLE_LOGLEVEL_* directly
  2021-02-02 10:09   ` Sergey Senozhatsky
@ 2021-02-02 23:04     ` Masahiro Yamada
  -1 siblings, 0 replies; 26+ messages in thread
From: Masahiro Yamada @ 2021-02-02 23:04 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Linux Kernel Mailing List, Petr Mladek, Steven Rostedt,
	John Ogness, Andy Shevchenko, Ard Biesheuvel, Borislav Petkov,
	Darren Hart, Dimitri Sivanich, Greg Kroah-Hartman,
	H. Peter Anvin, Ingo Molnar, Jiri Slaby, Mike Travis,
	Peter Jones, Russ Anderson, Steve Wahl, Thomas Gleixner,
	dri-devel, linux-efi, Linux Fbdev development list,
	platform-driver-x86, X86 ML

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/3] printk: use CONFIG_CONSOLE_LOGLEVEL_* directly
@ 2021-02-02 23:04     ` Masahiro Yamada
  0 siblings, 0 replies; 26+ messages in thread
From: Masahiro Yamada @ 2021-02-02 23:04 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Linux Fbdev development list, linux-efi, Russ Anderson,
	Steve Wahl, dri-devel, platform-driver-x86, Peter Jones,
	H. Peter Anvin, Jiri Slaby, Ard Biesheuvel, X86 ML, Ingo Molnar,
	Darren Hart, Petr Mladek, John Ogness, Mike Travis,
	Steven Rostedt, Borislav Petkov, Thomas Gleixner,
	Dimitri Sivanich, Greg Kroah-Hartman, Linux Kernel Mailing List,
	Andy Shevchenko

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 3/3] printk: move CONSOLE_EXT_LOG_MAX to kernel/printk/printk.c
  2021-02-02 12:29     ` kernel test robot
@ 2021-02-03 13:59       ` Petr Mladek
  -1 siblings, 0 replies; 26+ messages in thread
From: Petr Mladek @ 2021-02-03 13:59 UTC (permalink / raw)
  To: kernel test robot
  Cc: Masahiro Yamada, linux-kernel, Sergey Senozhatsky,
	Steven Rostedt, John Ogness, kbuild-all

On Tue 2021-02-02 20:29:31, kernel test robot wrote:
> Hi Masahiro,
> 
> I love your patch! Yet something to improve:
> 
> [auto build test ERROR on linux/master]
> [also build test ERROR on efi/next tty/tty-testing tip/x86/core v5.11-rc6 next-20210125]
> [cannot apply to pmladek/for-next]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:    https://github.com/0day-ci/linux/commits/Masahiro-Yamada/printk-use-CONFIG_CONSOLE_LOGLEVEL_-directly/20210202-151411
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 2ab38c17aac10bf55ab3efde4c4db3893d8691d2
> config: alpha-randconfig-s031-20210202 (attached as .config)
> compiler: alpha-linux-gcc (GCC) 9.3.0
> reproduce:
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # apt-get install sparse
>         # sparse version: v0.6.3-215-g0fb77bb6-dirty
>         # https://github.com/0day-ci/linux/commit/35d219bfad62e5008215f996430732aeb52c0652
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Masahiro-Yamada/printk-use-CONFIG_CONSOLE_LOGLEVEL_-directly/20210202-151411
>         git checkout 35d219bfad62e5008215f996430732aeb52c0652
>         # save the attached .config to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=alpha 
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All errors (new ones prefixed by >>):
> 
>    In file included from kernel/printk/printk.c:61:
>    kernel/printk/internal.h:59:20: warning: no previous prototype for 'vprintk_func' [-Wmissing-prototypes]
>       59 | __printf(1, 0) int vprintk_func(const char *fmt, va_list args) { return 0; }
>          |                    ^~~~~~~~~~~~
>    kernel/printk/printk.c:175:5: warning: no previous prototype for 'devkmsg_sysctl_set_loglvl' [-Wmissing-prototypes]
>      175 | int devkmsg_sysctl_set_loglvl(struct ctl_table *table, int write,
>          |     ^~~~~~~~~~~~~~~~~~~~~~~~~
>    kernel/printk/printk.c: In function 'console_unlock':
> >> kernel/printk/printk.c:2469:23: error: 'CONSOLE_EXT_LOG_MAX' undeclared (first use in this function)
>     2469 |  static char ext_text[CONSOLE_EXT_LOG_MAX];
>          |                       ^~~~~~~~~~~~~~~~~~~

This code is called also when CONFIG_PRINTK is not enabled.

It is a historic mess. console_lock() is used to synchronize also some
other stuff, especially in tty code, even when printk logging is
not enabled.

It should work to define:

#define CONSOLE_EXT_LOG_MAX 0

in the middle of printk.c, search for:

#else /* CONFIG_PRINTK */

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 3/3] printk: move CONSOLE_EXT_LOG_MAX to kernel/printk/printk.c
@ 2021-02-03 13:59       ` Petr Mladek
  0 siblings, 0 replies; 26+ messages in thread
From: Petr Mladek @ 2021-02-03 13:59 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 3017 bytes --]

On Tue 2021-02-02 20:29:31, kernel test robot wrote:
> Hi Masahiro,
> 
> I love your patch! Yet something to improve:
> 
> [auto build test ERROR on linux/master]
> [also build test ERROR on efi/next tty/tty-testing tip/x86/core v5.11-rc6 next-20210125]
> [cannot apply to pmladek/for-next]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:    https://github.com/0day-ci/linux/commits/Masahiro-Yamada/printk-use-CONFIG_CONSOLE_LOGLEVEL_-directly/20210202-151411
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 2ab38c17aac10bf55ab3efde4c4db3893d8691d2
> config: alpha-randconfig-s031-20210202 (attached as .config)
> compiler: alpha-linux-gcc (GCC) 9.3.0
> reproduce:
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # apt-get install sparse
>         # sparse version: v0.6.3-215-g0fb77bb6-dirty
>         # https://github.com/0day-ci/linux/commit/35d219bfad62e5008215f996430732aeb52c0652
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Masahiro-Yamada/printk-use-CONFIG_CONSOLE_LOGLEVEL_-directly/20210202-151411
>         git checkout 35d219bfad62e5008215f996430732aeb52c0652
>         # save the attached .config to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=alpha 
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All errors (new ones prefixed by >>):
> 
>    In file included from kernel/printk/printk.c:61:
>    kernel/printk/internal.h:59:20: warning: no previous prototype for 'vprintk_func' [-Wmissing-prototypes]
>       59 | __printf(1, 0) int vprintk_func(const char *fmt, va_list args) { return 0; }
>          |                    ^~~~~~~~~~~~
>    kernel/printk/printk.c:175:5: warning: no previous prototype for 'devkmsg_sysctl_set_loglvl' [-Wmissing-prototypes]
>      175 | int devkmsg_sysctl_set_loglvl(struct ctl_table *table, int write,
>          |     ^~~~~~~~~~~~~~~~~~~~~~~~~
>    kernel/printk/printk.c: In function 'console_unlock':
> >> kernel/printk/printk.c:2469:23: error: 'CONSOLE_EXT_LOG_MAX' undeclared (first use in this function)
>     2469 |  static char ext_text[CONSOLE_EXT_LOG_MAX];
>          |                       ^~~~~~~~~~~~~~~~~~~

This code is called also when CONFIG_PRINTK is not enabled.

It is a historic mess. console_lock() is used to synchronize also some
other stuff, especially in tty code, even when printk logging is
not enabled.

It should work to define:

#define CONSOLE_EXT_LOG_MAX 0

in the middle of printk.c, search for:

#else /* CONFIG_PRINTK */

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/3] printk: use CONFIG_CONSOLE_LOGLEVEL_* directly
  2021-02-02  8:38   ` John Ogness
@ 2021-02-03 15:23     ` Petr Mladek
  -1 siblings, 0 replies; 26+ messages in thread
From: Petr Mladek @ 2021-02-03 15:23 UTC (permalink / raw)
  To: John Ogness
  Cc: Masahiro Yamada, linux-kernel, Sergey Senozhatsky,
	Steven Rostedt, Andy Shevchenko, Ard Biesheuvel, Borislav Petkov,
	Darren Hart, Dimitri Sivanich, Greg Kroah-Hartman,
	H. Peter Anvin, Ingo Molnar, Jiri Slaby, Mike Travis,
	Peter Jones, Russ Anderson, Steve Wahl, Thomas Gleixner,
	dri-devel, linux-efi, linux-fbdev, platform-driver-x86, x86

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/3] printk: use CONFIG_CONSOLE_LOGLEVEL_* directly
@ 2021-02-03 15:23     ` Petr Mladek
  0 siblings, 0 replies; 26+ messages in thread
From: Petr Mladek @ 2021-02-03 15:23 UTC (permalink / raw)
  To: John Ogness
  Cc: linux-fbdev, linux-efi, Russ Anderson, Steve Wahl, dri-devel,
	platform-driver-x86, Peter Jones, H. Peter Anvin, Jiri Slaby,
	Ard Biesheuvel, Masahiro Yamada, x86, Ingo Molnar, Darren Hart,
	Mike Travis, Steven Rostedt, Borislav Petkov, Thomas Gleixner,
	Dimitri Sivanich, Greg Kroah-Hartman, linux-kernel,
	Sergey Senozhatsky, Andy Shevchenko

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/3] printk: use CONFIG_CONSOLE_LOGLEVEL_* directly
  2021-02-03 15:23     ` Petr Mladek
@ 2021-02-03 21:51       ` Masahiro Yamada
  -1 siblings, 0 replies; 26+ messages in thread
From: Masahiro Yamada @ 2021-02-03 21:51 UTC (permalink / raw)
  To: Petr Mladek
  Cc: John Ogness, Linux Kernel Mailing List, Sergey Senozhatsky,
	Steven Rostedt, Andy Shevchenko, Ard Biesheuvel, Borislav Petkov,
	Darren Hart, Dimitri Sivanich, Greg Kroah-Hartman,
	H. Peter Anvin, Ingo Molnar, Jiri Slaby, Mike Travis,
	Peter Jones, Russ Anderson, Steve Wahl, Thomas Gleixner,
	dri-devel, linux-efi, Linux Fbdev development list,
	platform-driver-x86, X86 ML

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/3] printk: use CONFIG_CONSOLE_LOGLEVEL_* directly
@ 2021-02-03 21:51       ` Masahiro Yamada
  0 siblings, 0 replies; 26+ messages in thread
From: Masahiro Yamada @ 2021-02-03 21:51 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Linux Fbdev development list, linux-efi, Russ Anderson,
	Steve Wahl, dri-devel, platform-driver-x86, Peter Jones,
	H. Peter Anvin, Jiri Slaby, Ard Biesheuvel, X86 ML, Ingo Molnar,
	Darren Hart, John Ogness, Mike Travis, Steven Rostedt,
	Borislav Petkov, Thomas Gleixner, Dimitri Sivanich,
	Greg Kroah-Hartman, Linux Kernel Mailing List,
	Sergey Senozhatsky, Andy Shevchenko

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/3] printk: use CONFIG_CONSOLE_LOGLEVEL_* directly
  2021-02-03 21:51       ` Masahiro Yamada
@ 2021-02-04 10:16         ` Petr Mladek
  -1 siblings, 0 replies; 26+ messages in thread
From: Petr Mladek @ 2021-02-04 10:16 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: John Ogness, Linux Kernel Mailing List, Sergey Senozhatsky,
	Steven Rostedt, Andy Shevchenko, Ard Biesheuvel, Borislav Petkov,
	Darren Hart, Dimitri Sivanich, Greg Kroah-Hartman,
	H. Peter Anvin, Ingo Molnar, Jiri Slaby, Mike Travis,
	Peter Jones, Russ Anderson, Steve Wahl, Thomas Gleixner,
	dri-devel, linux-efi, Linux Fbdev development list,
	platform-driver-x86, X86 ML

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/3] printk: use CONFIG_CONSOLE_LOGLEVEL_* directly
@ 2021-02-04 10:16         ` Petr Mladek
  0 siblings, 0 replies; 26+ messages in thread
From: Petr Mladek @ 2021-02-04 10:16 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Linux Fbdev development list, linux-efi, Russ Anderson,
	Steve Wahl, dri-devel, platform-driver-x86, Peter Jones,
	H. Peter Anvin, Jiri Slaby, Ard Biesheuvel, X86 ML, Ingo Molnar,
	Darren Hart, John Ogness, Mike Travis, Steven Rostedt,
	Borislav Petkov, Thomas Gleixner, Dimitri Sivanich,
	Greg Kroah-Hartman, Linux Kernel Mailing List,
	Sergey Senozhatsky, Andy Shevchenko

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2021-02-04 11:20 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-02  7:02 [PATCH 1/3] printk: use CONFIG_CONSOLE_LOGLEVEL_* directly Masahiro Yamada
2021-02-02  7:02 ` Masahiro Yamada
2021-02-02  7:02 ` [PATCH 2/3] printk: hard-code CONSOLE_LOGLEVEL_MIN in printk.c Masahiro Yamada
2021-02-02 10:06   ` Sergey Senozhatsky
2021-02-02 12:51     ` Joe Perches
2021-02-02  7:02 ` [PATCH 3/3] printk: move CONSOLE_EXT_LOG_MAX to kernel/printk/printk.c Masahiro Yamada
2021-02-02  8:44   ` John Ogness
2021-02-02 10:04     ` Sergey Senozhatsky
2021-02-02 12:29   ` kernel test robot
2021-02-02 12:29     ` kernel test robot
2021-02-03 13:59     ` Petr Mladek
2021-02-03 13:59       ` Petr Mladek
2021-02-02  8:38 ` [PATCH 1/3] printk: use CONFIG_CONSOLE_LOGLEVEL_* directly John Ogness
2021-02-02  8:38   ` John Ogness
2021-02-03 15:23   ` Petr Mladek
2021-02-03 15:23     ` Petr Mladek
2021-02-03 21:51     ` Masahiro Yamada
2021-02-03 21:51       ` Masahiro Yamada
2021-02-04 10:16       ` Petr Mladek
2021-02-04 10:16         ` Petr Mladek
2021-02-02 10:09 ` Sergey Senozhatsky
2021-02-02 10:09   ` Sergey Senozhatsky
2021-02-02 23:04   ` Masahiro Yamada
2021-02-02 23:04     ` Masahiro Yamada
2021-02-02 10:12 ` Greg Kroah-Hartman
2021-02-02 10:12   ` Greg Kroah-Hartman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.