All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC 0/2] printk: Official way to mute consoles
@ 2020-10-22 11:42 Petr Mladek
  2020-10-22 11:42 ` [RFC 1/2] printk: Add kernel parameter: mute_console Petr Mladek
  2020-10-22 11:42 ` RFC 2/2] printk: Restore and document obsolete ways to disable console output Petr Mladek
  0 siblings, 2 replies; 15+ messages in thread
From: Petr Mladek @ 2020-10-22 11:42 UTC (permalink / raw)
  To: Sergey Senozhatsky, Steven Rostedt, John Ogness
  Cc: Linus Torvalds, Guenter Roeck, Shreyas Joshi, shreyasjoshi15,
	Greg Kroah-Hartman, Sergey Senozhatsky, linux-kernel,
	Petr Mladek

The long discussion about handling empty console= came up with several
problems. This patchset tries to solve the two original problems
with empty console="" parameter:

  + Prevent the potential crash by registering console for /dev/console.
  + Prevent the performance regression by muting the consoles.

IMHO, the patchset makes sense on its own. It fixes a regression. It seems
that people want this functionality [1][2][3].

Note that there are still some problems that might be solved later:

  + Invalid console=bla name might still prevent registering any console.

  + The kernel should not crash even when /dev/console does not point
    to any real console. 

  + Should we add some fallback for stdin, stdout, and stderr when /dev/console
    can't be opened? For example, /dev/null?
    
  + How user space handle missing none-console associated with /dev/console?

[1] https://www.programmersought.com/article/19374022450/
[2] https://developer.toradex.com/knowledge-base/how-to-disable-enable-debug-messages-in-linux
[3] https://unix.stackexchange.com/questions/117926/try-to-disable-console-output-console-null-doesnt-work


Petr Mladek (2):
  printk: Add kernel parameter: mute_console
  printk: Restore and document obsolete ways to disable console output

 .../admin-guide/kernel-parameters.txt         | 11 +++++++
 kernel/printk/printk.c                        | 30 +++++++++++++++++--
 2 files changed, 39 insertions(+), 2 deletions(-)

-- 
2.26.2


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

* [RFC 1/2] printk: Add kernel parameter: mute_console
  2020-10-22 11:42 [RFC 0/2] printk: Official way to mute consoles Petr Mladek
@ 2020-10-22 11:42 ` Petr Mladek
  2020-10-22 13:10   ` John Ogness
                     ` (3 more replies)
  2020-10-22 11:42 ` RFC 2/2] printk: Restore and document obsolete ways to disable console output Petr Mladek
  1 sibling, 4 replies; 15+ messages in thread
From: Petr Mladek @ 2020-10-22 11:42 UTC (permalink / raw)
  To: Sergey Senozhatsky, Steven Rostedt, John Ogness
  Cc: Linus Torvalds, Guenter Roeck, Shreyas Joshi, shreyasjoshi15,
	Greg Kroah-Hartman, Sergey Senozhatsky, linux-kernel,
	Petr Mladek

Users use various undocumented ways how to completely disable
console output. It is usually done by passing an invalid console
name, for example, console="", console=null.

It mostly works but just by chance. The console name is added to the list
of preferred consoles and the variable "preferred_console" is set.
As a result, any register_console() fails because the driver name
does not match the invalid name. And the console is not used as
a fallback because "preferred_console" is set.

It stops working, when another console is defined on the command line
or another way, for example, by SPCR or a device tree.

More importantly, the above approach might break the system. /dev/console
is used as stdin, stdout, and stderr for the init process [0]

Why yet another command line option?

console_loglevel is not reliable. It is manipulated also by user space
when it configures log daemons.

People might also want to just disable the kernel messages but still
use the console for login.

[0] https://lore.kernel.org/r/20201006025935.GA597@jagdpanzerIV.localdomain

Suggested-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 .../admin-guide/kernel-parameters.txt         |  6 ++++++
 kernel/printk/printk.c                        | 21 ++++++++++++++++++-
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 02d4adbf98d2..52b9e7f5468d 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2974,6 +2974,12 @@
 			Used for mtrr cleanup. It is spare mtrr entries number.
 			Set to 2 or more if your graphical card needs more.
 
+	mute_console	[KNL]
+			Completely disable printing of kernel messages to
+			the console. It can still be used as stdin, stdout,
+			and stderr for the init process. Also it can be used
+			for login.
+
 	n2=		[NET] SDL Inc. RISCom/N2 synchronous serial card
 
 	netdev=		[NET] Network devices parameters
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index fe64a49344bf..63fb96630767 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -1207,6 +1207,19 @@ void __init setup_log_buf(int early)
 	memblock_free(__pa(new_log_buf), new_log_buf_len);
 }
 
+static bool mute_console;
+
+static int __init mute_console_setup(char *str)
+{
+	mute_console = true;
+	pr_info("All consoles muted.\n");
+
+	return 0;
+}
+
+early_param("mute_console", mute_console_setup);
+module_param(mute_console, bool, 0644);
+
 static bool __read_mostly ignore_loglevel;
 
 static int __init ignore_loglevel_setup(char *str)
@@ -1224,7 +1237,13 @@ MODULE_PARM_DESC(ignore_loglevel,
 
 static bool suppress_message_printing(int level)
 {
-	return (level >= console_loglevel && !ignore_loglevel);
+	if (unlikely(mute_console))
+		return true;
+
+	if (unlikely(ignore_loglevel))
+		return false;
+
+	return (level >= console_loglevel);
 }
 
 #ifdef CONFIG_BOOT_PRINTK_DELAY
-- 
2.26.2


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

* RFC 2/2] printk: Restore and document obsolete ways to disable console output
  2020-10-22 11:42 [RFC 0/2] printk: Official way to mute consoles Petr Mladek
  2020-10-22 11:42 ` [RFC 1/2] printk: Add kernel parameter: mute_console Petr Mladek
@ 2020-10-22 11:42 ` Petr Mladek
  2020-10-22 14:23   ` Guenter Roeck
  1 sibling, 1 reply; 15+ messages in thread
From: Petr Mladek @ 2020-10-22 11:42 UTC (permalink / raw)
  To: Sergey Senozhatsky, Steven Rostedt, John Ogness
  Cc: Linus Torvalds, Guenter Roeck, Shreyas Joshi, shreyasjoshi15,
	Greg Kroah-Hartman, Sergey Senozhatsky, linux-kernel,
	Petr Mladek

The commit 48021f98130880dd7 ("printk: handle blank console arguments
passed in.") prevented crash caused by empty console= parameter value.

Unfortunately, this value is widely used on Chromebooks to disable
the console output. The above commit caused performance regression
because the messages were pushed on slow console even though nobody
was watching it.

"mute_console" kernel parameter has been introduced as a proper and
official was to disable console output. It avoids the performance
problem by suppressing all kernel messages. Also avoids the crash
by registering the default console.

Make console="" behave the same as "mute_console" to avoid
the performance regression on existing Chromebooks.

Do the same also for console=null which seem to be another widely
suggested and non-official way to disable the console output.

Suggested-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 Documentation/admin-guide/kernel-parameters.txt | 5 +++++
 kernel/printk/printk.c                          | 9 ++++++++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 52b9e7f5468d..14472f674a89 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -670,11 +670,16 @@
 		hvc<n>	Use the hypervisor console device <n>. This is for
 			both Xen and PowerPC hypervisors.
 
+		null
+		""	Obsolete way to disable console output. Please,
+			use "mute_console" instead.
+
 		If the device connected to the port is not a TTY but a braille
 		device, prepend "brl," before the device type, for instance
 			console=brl,ttyS0
 		For now, only VisioBraille is supported.
 
+
 	console_msg_format=
 			[KNL] Change console messages format
 		default
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 63fb96630767..08c50d8ba110 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2208,8 +2208,15 @@ static int __init console_setup(char *str)
 	char *s, *options, *brl_options = NULL;
 	int idx;
 
-	if (str[0] == 0)
+	/*
+	 * console="" or console=null have been suggested as a way to
+	 * disable console output. It worked just by chance and was not
+	 * reliable. It has been obsoleted by "mute_console" parameter.
+	 */
+	if (str[0] == 0 || strcmp(str, "null") == 0) {
+		mute_console = true;
 		return 1;
+	}
 
 	if (_braille_console_setup(&str, &brl_options))
 		return 1;
-- 
2.26.2


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

* Re: [RFC 1/2] printk: Add kernel parameter: mute_console
  2020-10-22 11:42 ` [RFC 1/2] printk: Add kernel parameter: mute_console Petr Mladek
@ 2020-10-22 13:10   ` John Ogness
  2020-10-22 14:15     ` Guenter Roeck
  2020-10-22 13:45   ` Steven Rostedt
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 15+ messages in thread
From: John Ogness @ 2020-10-22 13:10 UTC (permalink / raw)
  To: Petr Mladek, Sergey Senozhatsky, Steven Rostedt
  Cc: Linus Torvalds, Guenter Roeck, Shreyas Joshi, shreyasjoshi15,
	Greg Kroah-Hartman, Sergey Senozhatsky, linux-kernel,
	Petr Mladek

On 2020-10-22, Petr Mladek <pmladek@suse.com> wrote:
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 02d4adbf98d2..52b9e7f5468d 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2974,6 +2974,12 @@
>  			Used for mtrr cleanup. It is spare mtrr entries number.
>  			Set to 2 or more if your graphical card needs more.
>  
> +	mute_console	[KNL]
> +			Completely disable printing of kernel messages to
> +			the console. It can still be used as stdin, stdout,
> +			and stderr for the init process. Also it can be used
> +			for login.

IMHO it would make more sense for this to be a console option:

    console=ttyS0,115200,mute

Then other consoles could still exist that are not muted.

On a side note, I am considering proposing something similar for my
printk-rework efforts. Once console printers are moved to kthreads, some
users may not care about latencies and instead prefer synchronous
printing. My idea for this is to provide a "sync" option for the
console:

    console=ttyS0,115200,sync

John Ogness

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

* Re: [RFC 1/2] printk: Add kernel parameter: mute_console
  2020-10-22 11:42 ` [RFC 1/2] printk: Add kernel parameter: mute_console Petr Mladek
  2020-10-22 13:10   ` John Ogness
@ 2020-10-22 13:45   ` Steven Rostedt
  2020-10-22 14:22     ` Guenter Roeck
  2020-10-23  9:46     ` Petr Mladek
  2020-10-23  0:33   ` Sergey Senozhatsky
  2020-10-23 15:59   ` Linus Torvalds
  3 siblings, 2 replies; 15+ messages in thread
From: Steven Rostedt @ 2020-10-22 13:45 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Sergey Senozhatsky, John Ogness, Linus Torvalds, Guenter Roeck,
	Shreyas Joshi, shreyasjoshi15, Greg Kroah-Hartman,
	Sergey Senozhatsky, linux-kernel

On Thu, 22 Oct 2020 13:42:27 +0200
Petr Mladek <pmladek@suse.com> wrote:

> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index fe64a49344bf..63fb96630767 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -1207,6 +1207,19 @@ void __init setup_log_buf(int early)
>  	memblock_free(__pa(new_log_buf), new_log_buf_len);
>  }
>  
> +static bool mute_console;
> +
> +static int __init mute_console_setup(char *str)
> +{
> +	mute_console = true;
> +	pr_info("All consoles muted.\n");
> +
> +	return 0;
> +}
> +
> +early_param("mute_console", mute_console_setup);
> +module_param(mute_console, bool, 0644);
> +

Why have both early_param and module_param? What's the purpose of
module_param? Usually that's there to just set a variable, without a need
for another interface. But if you have early_param() that sets
mute_console, isn't that redundant?

-- Steve

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

* Re: [RFC 1/2] printk: Add kernel parameter: mute_console
  2020-10-22 13:10   ` John Ogness
@ 2020-10-22 14:15     ` Guenter Roeck
  2020-10-22 14:53       ` John Ogness
  0 siblings, 1 reply; 15+ messages in thread
From: Guenter Roeck @ 2020-10-22 14:15 UTC (permalink / raw)
  To: John Ogness, Petr Mladek, Sergey Senozhatsky, Steven Rostedt
  Cc: Linus Torvalds, Shreyas Joshi, shreyasjoshi15,
	Greg Kroah-Hartman, Sergey Senozhatsky, linux-kernel

On 10/22/20 6:10 AM, John Ogness wrote:
> On 2020-10-22, Petr Mladek <pmladek@suse.com> wrote:
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index 02d4adbf98d2..52b9e7f5468d 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -2974,6 +2974,12 @@
>>  			Used for mtrr cleanup. It is spare mtrr entries number.
>>  			Set to 2 or more if your graphical card needs more.
>>  
>> +	mute_console	[KNL]
>> +			Completely disable printing of kernel messages to
>> +			the console. It can still be used as stdin, stdout,

"to all consoles"

>> +			and stderr for the init process. Also it can be used
>> +			for login.
> 
> IMHO it would make more sense for this to be a console option:
> 
>     console=ttyS0,115200,mute
> 
> Then other consoles could still exist that are not muted.
> 
Then why specify this console in the first place ?

> On a side note, I am considering proposing something similar for my
> printk-rework efforts. Once console printers are moved to kthreads, some
> users may not care about latencies and instead prefer synchronous
> printing. My idea for this is to provide a "sync" option for the
> console:
> 
>     console=ttyS0,115200,sync
> 

The whole point of the exercise is to disable all consoles, including default
ones which are not explicitly specified on the command line. The above would
mean that each potential console would have to be muted. That isn't really
scalable for a build system which has to handle many different console devices.

Guenter

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

* Re: [RFC 1/2] printk: Add kernel parameter: mute_console
  2020-10-22 13:45   ` Steven Rostedt
@ 2020-10-22 14:22     ` Guenter Roeck
  2020-10-23  9:46     ` Petr Mladek
  1 sibling, 0 replies; 15+ messages in thread
From: Guenter Roeck @ 2020-10-22 14:22 UTC (permalink / raw)
  To: Steven Rostedt, Petr Mladek
  Cc: Sergey Senozhatsky, John Ogness, Linus Torvalds, Shreyas Joshi,
	shreyasjoshi15, Greg Kroah-Hartman, Sergey Senozhatsky,
	linux-kernel

On 10/22/20 6:45 AM, Steven Rostedt wrote:
> On Thu, 22 Oct 2020 13:42:27 +0200
> Petr Mladek <pmladek@suse.com> wrote:
> 
>> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
>> index fe64a49344bf..63fb96630767 100644
>> --- a/kernel/printk/printk.c
>> +++ b/kernel/printk/printk.c
>> @@ -1207,6 +1207,19 @@ void __init setup_log_buf(int early)
>>  	memblock_free(__pa(new_log_buf), new_log_buf_len);
>>  }
>>  
>> +static bool mute_console;
>> +
>> +static int __init mute_console_setup(char *str)
>> +{
>> +	mute_console = true;
>> +	pr_info("All consoles muted.\n");
>> +
>> +	return 0;
>> +}
>> +
>> +early_param("mute_console", mute_console_setup);
>> +module_param(mute_console, bool, 0644);
>> +
> 
> Why have both early_param and module_param? What's the purpose of
> module_param? Usually that's there to just set a variable, without a need
> for another interface. But if you have early_param() that sets
> mute_console, isn't that redundant?
> 

For me the question was why we would need an extra module parameter
in the first place instead of just making "console=" and "console=null"
official. It doesn't really matter for us as long as "console=" is still
supported, I just don't see the point.

Thanks,
Guenter

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

* Re: RFC 2/2] printk: Restore and document obsolete ways to disable console output
  2020-10-22 11:42 ` RFC 2/2] printk: Restore and document obsolete ways to disable console output Petr Mladek
@ 2020-10-22 14:23   ` Guenter Roeck
  0 siblings, 0 replies; 15+ messages in thread
From: Guenter Roeck @ 2020-10-22 14:23 UTC (permalink / raw)
  To: Petr Mladek, Sergey Senozhatsky, Steven Rostedt, John Ogness
  Cc: Linus Torvalds, Shreyas Joshi, shreyasjoshi15,
	Greg Kroah-Hartman, Sergey Senozhatsky, linux-kernel

On 10/22/20 4:42 AM, Petr Mladek wrote:
> The commit 48021f98130880dd7 ("printk: handle blank console arguments
> passed in.") prevented crash caused by empty console= parameter value.
> 
> Unfortunately, this value is widely used on Chromebooks to disable
> the console output. The above commit caused performance regression
> because the messages were pushed on slow console even though nobody
> was watching it.
> 
> "mute_console" kernel parameter has been introduced as a proper and
> official was to disable console output. It avoids the performance
> problem by suppressing all kernel messages. Also avoids the crash
> by registering the default console.
> 
> Make console="" behave the same as "mute_console" to avoid
> the performance regression on existing Chromebooks.
> 
> Do the same also for console=null which seem to be another widely
> suggested and non-official way to disable the console output.
> 
> Suggested-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> Signed-off-by: Petr Mladek <pmladek@suse.com>
> ---
>  Documentation/admin-guide/kernel-parameters.txt | 5 +++++
>  kernel/printk/printk.c                          | 9 ++++++++-
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 52b9e7f5468d..14472f674a89 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -670,11 +670,16 @@
>  		hvc<n>	Use the hypervisor console device <n>. This is for
>  			both Xen and PowerPC hypervisors.
>  
> +		null
> +		""	Obsolete way to disable console output. Please,
> +			use "mute_console" instead.
> +
>  		If the device connected to the port is not a TTY but a braille
>  		device, prepend "brl," before the device type, for instance
>  			console=brl,ttyS0
>  		For now, only VisioBraille is supported.
>  
> +

stray whitespace change

>  	console_msg_format=
>  			[KNL] Change console messages format
>  		default
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 63fb96630767..08c50d8ba110 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2208,8 +2208,15 @@ static int __init console_setup(char *str)
>  	char *s, *options, *brl_options = NULL;
>  	int idx;
>  
> -	if (str[0] == 0)
> +	/*
> +	 * console="" or console=null have been suggested as a way to
> +	 * disable console output. It worked just by chance and was not
> +	 * reliable. It has been obsoleted by "mute_console" parameter.
> +	 */
> +	if (str[0] == 0 || strcmp(str, "null") == 0) {
> +		mute_console = true;
>  		return 1;
> +	}
>  
>  	if (_braille_console_setup(&str, &brl_options))
>  		return 1;
> 


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

* Re: [RFC 1/2] printk: Add kernel parameter: mute_console
  2020-10-22 14:15     ` Guenter Roeck
@ 2020-10-22 14:53       ` John Ogness
  2020-10-23 11:49         ` Petr Mladek
  0 siblings, 1 reply; 15+ messages in thread
From: John Ogness @ 2020-10-22 14:53 UTC (permalink / raw)
  To: Guenter Roeck, Petr Mladek, Sergey Senozhatsky, Steven Rostedt
  Cc: Linus Torvalds, Shreyas Joshi, shreyasjoshi15,
	Greg Kroah-Hartman, Sergey Senozhatsky, linux-kernel

On 2020-10-22, Guenter Roeck <linux@roeck-us.net> wrote:
>>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>>> index 02d4adbf98d2..52b9e7f5468d 100644
>>> --- a/Documentation/admin-guide/kernel-parameters.txt
>>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>>> @@ -2974,6 +2974,12 @@
>>>  			Used for mtrr cleanup. It is spare mtrr entries number.
>>>  			Set to 2 or more if your graphical card needs more.
>>>  
>>> +	mute_console	[KNL]
>>> +			Completely disable printing of kernel messages to
>>> +			the console. It can still be used as stdin, stdout,
>
> "to all consoles"
>
>>> +			and stderr for the init process. Also it can be used
>>> +			for login.
>> 
>> IMHO it would make more sense for this to be a console option:
>> 
>>     console=ttyS0,115200,mute
>> 
>> Then other consoles could still exist that are not muted.
>> 
> Then why specify this console in the first place ?

Because you may still want to use it for stdin/stdout/stderr of PID 1
and/or logins. I understood "mute" as meaning the user does not want to
see kernel logging. But everything else should still work as usual.

>> On a side note, I am considering proposing something similar for my
>> printk-rework efforts. Once console printers are moved to kthreads, some
>> users may not care about latencies and instead prefer synchronous
>> printing. My idea for this is to provide a "sync" option for the
>> console:
>> 
>>     console=ttyS0,115200,sync
>
> The whole point of the exercise is to disable all consoles, including
> default ones which are not explicitly specified on the command line.

In that case I think specifying something like:

    console=null

makes that most sense. I think implementing a "null console" driver
would be quite simple. Then there would be no need for special handling
in the printk subsystem.

John Ogness

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

* Re: [RFC 1/2] printk: Add kernel parameter: mute_console
  2020-10-22 11:42 ` [RFC 1/2] printk: Add kernel parameter: mute_console Petr Mladek
  2020-10-22 13:10   ` John Ogness
  2020-10-22 13:45   ` Steven Rostedt
@ 2020-10-23  0:33   ` Sergey Senozhatsky
  2020-10-23 12:11     ` Petr Mladek
  2020-10-23 15:59   ` Linus Torvalds
  3 siblings, 1 reply; 15+ messages in thread
From: Sergey Senozhatsky @ 2020-10-23  0:33 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Sergey Senozhatsky, Steven Rostedt, John Ogness, Linus Torvalds,
	Guenter Roeck, Shreyas Joshi, shreyasjoshi15, Greg Kroah-Hartman,
	Sergey Senozhatsky, linux-kernel

On (20/10/22 13:42), Petr Mladek wrote:
> +static bool mute_console;
> +
> +static int __init mute_console_setup(char *str)
> +{
> +	mute_console = true;
> +	pr_info("All consoles muted.\n");
> +
> +	return 0;
> +}

First of all, thanks a lot for picking this up and for the patch set!

I've several thoughts and comments below.

>  static bool suppress_message_printing(int level)
>  {
> -	return (level >= console_loglevel && !ignore_loglevel);
> +	if (unlikely(mute_console))
> +		return true;
> +
> +	if (unlikely(ignore_loglevel))
> +		return false;
> +
> +	return (level >= console_loglevel);
>  }

This is one way of doing it. Another one is to clear CON_ENABLED bit
from all consoles (upon registration), one upside of this is that we
will signal user-space that consoles are disabled/muted (by removing
the E flag from /proc/consoles).

But, if I'm mistaken, but this mutes only printk side, consoles still
have uart running:
	printf -> tty -> uart -> serial_driver_IRQ() -> TX
	seriaal_driver_IRQ() -> RX -> uart -> tty

so user space, in theory, still can push messages to slow consoles,
AFAIU.

Thinking more about it. We are still relying on the fact that there is
anything registered as console driver, which is not necessarily the case,
we can have NULL console drivers list. So how about having a dummy struct
console in printk, with NOP read/write and NOP tty_driver and NOP
tty_operations. So that when init calls filp_open("/dev/console") and
we can't give tty anything but NULL, we'd just give tty back the dummy
NOP device.

	-ss

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

* Re: [RFC 1/2] printk: Add kernel parameter: mute_console
  2020-10-22 13:45   ` Steven Rostedt
  2020-10-22 14:22     ` Guenter Roeck
@ 2020-10-23  9:46     ` Petr Mladek
  1 sibling, 0 replies; 15+ messages in thread
From: Petr Mladek @ 2020-10-23  9:46 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Sergey Senozhatsky, John Ogness, Linus Torvalds, Guenter Roeck,
	Shreyas Joshi, shreyasjoshi15, Greg Kroah-Hartman,
	Sergey Senozhatsky, linux-kernel

On Thu 2020-10-22 09:45:12, Steven Rostedt wrote:
> On Thu, 22 Oct 2020 13:42:27 +0200
> Petr Mladek <pmladek@suse.com> wrote:
> 
> > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> > index fe64a49344bf..63fb96630767 100644
> > --- a/kernel/printk/printk.c
> > +++ b/kernel/printk/printk.c
> > @@ -1207,6 +1207,19 @@ void __init setup_log_buf(int early)
> >  	memblock_free(__pa(new_log_buf), new_log_buf_len);
> >  }
> >  
> > +static bool mute_console;
> > +
> > +static int __init mute_console_setup(char *str)
> > +{
> > +	mute_console = true;
> > +	pr_info("All consoles muted.\n");
> > +
> > +	return 0;
> > +}
> > +
> > +early_param("mute_console", mute_console_setup);
> > +module_param(mute_console, bool, 0644);
> > +
> 
> Why have both early_param and module_param? What's the purpose of
> module_param? Usually that's there to just set a variable, without a need
> for another interface. But if you have early_param() that sets
> mute_console, isn't that redundant?

I was surprised as well. But both seem to be needed:

   + early_param allows to enable it on the command line.
     Note that module_param would need to be set via
     printk.mute_console

   + module_param() allows to modify the state at runtime

IMHO, both should be possible. It is supposed to be used similar
way like ignore_loglevel.

Best Regards,
Petr

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

* Re: [RFC 1/2] printk: Add kernel parameter: mute_console
  2020-10-22 14:53       ` John Ogness
@ 2020-10-23 11:49         ` Petr Mladek
  2020-10-23 14:52           ` Guenter Roeck
  0 siblings, 1 reply; 15+ messages in thread
From: Petr Mladek @ 2020-10-23 11:49 UTC (permalink / raw)
  To: John Ogness
  Cc: Guenter Roeck, Sergey Senozhatsky, Steven Rostedt,
	Linus Torvalds, Shreyas Joshi, shreyasjoshi15,
	Greg Kroah-Hartman, Sergey Senozhatsky, linux-kernel

On Thu 2020-10-22 16:59:15, John Ogness wrote:
> On 2020-10-22, Guenter Roeck <linux@roeck-us.net> wrote:
> > The whole point of the exercise is to disable all consoles, including
> > default ones which are not explicitly specified on the command line.
> 
> In that case I think specifying something like:
> 
>     console=null
> 
> makes that most sense. I think implementing a "null console" driver
> would be quite simple. Then there would be no need for special handling
> in the printk subsystem.

Heh, it actually already exists and has been created for exactly this
purpose, see the commit 3117ff13f104e98b05b6 ("tty: Add NULL TTY
driver").

Regarding the interface:

   + console=null or console= is OK when people do not want consoles
     at all

   + mute_console (or another extra parameter) would be needed if
     people wanted to have login console.

It is true that nobody asked for the login support. So, the null
console should be enough for now.

Best Regards,
Petr

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

* Re: [RFC 1/2] printk: Add kernel parameter: mute_console
  2020-10-23  0:33   ` Sergey Senozhatsky
@ 2020-10-23 12:11     ` Petr Mladek
  0 siblings, 0 replies; 15+ messages in thread
From: Petr Mladek @ 2020-10-23 12:11 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Steven Rostedt, John Ogness, Linus Torvalds, Guenter Roeck,
	Shreyas Joshi, shreyasjoshi15, Greg Kroah-Hartman,
	Sergey Senozhatsky, linux-kernel

On Fri 2020-10-23 09:33:34, Sergey Senozhatsky wrote:
> On (20/10/22 13:42), Petr Mladek wrote:
> > +static bool mute_console;
> > +
> > +static int __init mute_console_setup(char *str)
> > +{
> > +	mute_console = true;
> > +	pr_info("All consoles muted.\n");
> > +
> > +	return 0;
> > +}
> 
> First of all, thanks a lot for picking this up and for the patch set!
> 
> I've several thoughts and comments below.
> 
> >  static bool suppress_message_printing(int level)
> >  {
> > -	return (level >= console_loglevel && !ignore_loglevel);
> > +	if (unlikely(mute_console))
> > +		return true;
> > +
> > +	if (unlikely(ignore_loglevel))
> > +		return false;
> > +
> > +	return (level >= console_loglevel);
> >  }
> 
> This is one way of doing it. Another one is to clear CON_ENABLED bit
> from all consoles (upon registration), one upside of this is that we
> will signal user-space that consoles are disabled/muted (by removing
> the E flag from /proc/consoles).

Hmm, CON_ENABLED is used by suspend/resume code unconditionaly. We
would need another flag to define the state after resume.

Well, it is true that CON_ENABLED has the same effect. Messages are
not printed to the console. So, introducing another variable is
likely overkill.

> Thinking more about it. We are still relying on the fact that there is
> anything registered as console driver, which is not necessarily the case,
> we can have NULL console drivers list. So how about having a dummy struct
> console in printk, with NOP read/write and NOP tty_driver and NOP
> tty_operations. So that when init calls filp_open("/dev/console") and
> we can't give tty anything but NULL, we'd just give tty back the dummy
> NOP device.

Yup, this seems to be the best solution.

Best Regards,
Petr

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

* Re: [RFC 1/2] printk: Add kernel parameter: mute_console
  2020-10-23 11:49         ` Petr Mladek
@ 2020-10-23 14:52           ` Guenter Roeck
  0 siblings, 0 replies; 15+ messages in thread
From: Guenter Roeck @ 2020-10-23 14:52 UTC (permalink / raw)
  To: Petr Mladek, John Ogness
  Cc: Sergey Senozhatsky, Steven Rostedt, Linus Torvalds,
	Shreyas Joshi, shreyasjoshi15, Greg Kroah-Hartman,
	Sergey Senozhatsky, linux-kernel

On 10/23/20 4:49 AM, Petr Mladek wrote:
> On Thu 2020-10-22 16:59:15, John Ogness wrote:
>> On 2020-10-22, Guenter Roeck <linux@roeck-us.net> wrote:
>>> The whole point of the exercise is to disable all consoles, including
>>> default ones which are not explicitly specified on the command line.
>>
>> In that case I think specifying something like:
>>
>>     console=null
>>
>> makes that most sense. I think implementing a "null console" driver
>> would be quite simple. Then there would be no need for special handling
>> in the printk subsystem.
> 
> Heh, it actually already exists and has been created for exactly this
> purpose, see the commit 3117ff13f104e98b05b6 ("tty: Add NULL TTY
> driver").
> 

Ok with me to use that, as long as we add code that maps console=
and console=null to console=ttynull.

Guenter

> Regarding the interface:
> 
>    + console=null or console= is OK when people do not want consoles
>      at all
> 
>    + mute_console (or another extra parameter) would be needed if
>      people wanted to have login console.
> 
> It is true that nobody asked for the login support. So, the null
> console should be enough for now.
> 
> Best Regards,
> Petr
> 


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

* Re: [RFC 1/2] printk: Add kernel parameter: mute_console
  2020-10-22 11:42 ` [RFC 1/2] printk: Add kernel parameter: mute_console Petr Mladek
                     ` (2 preceding siblings ...)
  2020-10-23  0:33   ` Sergey Senozhatsky
@ 2020-10-23 15:59   ` Linus Torvalds
  3 siblings, 0 replies; 15+ messages in thread
From: Linus Torvalds @ 2020-10-23 15:59 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Sergey Senozhatsky, Steven Rostedt, John Ogness, Guenter Roeck,
	Shreyas Joshi, shreyasjoshi15, Greg Kroah-Hartman,
	Sergey Senozhatsky, Linux Kernel Mailing List

On Thu, Oct 22, 2020 at 4:42 AM Petr Mladek <pmladek@suse.com> wrote:
>
> Users use various undocumented ways how to completely disable
> console output. It is usually done by passing an invalid console
> name, for example, console="", console=null.
>
> It mostly works but just by chance.

Honestly, since that 'console=""' seems to be out in the wild, I think
we might as well just (a) document it, and (b) make sure it works by
more than chance.

That said, I also like John Ogness' suggestion to have it as a
per-console option to mute a particular console. At least that seems
like a perfectly fine extension.

I don't really see the point of a whole new "mute_console" option,
considering that people already figured out an alternate way to do it
that we'd have to support going forward anyway. Just make that the
standard.

             Linus

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

end of thread, other threads:[~2020-10-23 15:59 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-22 11:42 [RFC 0/2] printk: Official way to mute consoles Petr Mladek
2020-10-22 11:42 ` [RFC 1/2] printk: Add kernel parameter: mute_console Petr Mladek
2020-10-22 13:10   ` John Ogness
2020-10-22 14:15     ` Guenter Roeck
2020-10-22 14:53       ` John Ogness
2020-10-23 11:49         ` Petr Mladek
2020-10-23 14:52           ` Guenter Roeck
2020-10-22 13:45   ` Steven Rostedt
2020-10-22 14:22     ` Guenter Roeck
2020-10-23  9:46     ` Petr Mladek
2020-10-23  0:33   ` Sergey Senozhatsky
2020-10-23 12:11     ` Petr Mladek
2020-10-23 15:59   ` Linus Torvalds
2020-10-22 11:42 ` RFC 2/2] printk: Restore and document obsolete ways to disable console output Petr Mladek
2020-10-22 14:23   ` Guenter Roeck

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.