diff mbox series

[v6,10/11] printk: Deprecate the kernel.printk sysctl interface

Message ID 993d978d4a59dd370ac39c6c24c9b72c11f4dc74.1730133890.git.chris@chrisdown.name
State New
Headers show
Series printk: console: Per-console loglevels | expand

Commit Message

Chris Down Oct. 28, 2024, 4:45 p.m. UTC
The kernel.printk sysctl interface is deprecated in favour of more
granular and clearer sysctl interfaces for controlling loglevels, namely
kernel.console_loglevel and kernel.default_message_loglevel.

kernel.printk has a number of fairly significant usability problems:

- It takes four values in a specific order, which is not intuitive.
  Speaking from personal experience, even people working on printk
  sometimes need to look up the order of these values, which doesn't
  suggest much in the way of perspicuity.
- There is no validation on the input values, so users can set them to
  invalid or nonsensical values without receiving any errors or
  warnings. This can lead to unpredictable behaviour.
- One of the four values, default_console_loglevel, is not used in the
  kernel (see below), making it redundant and potentially confusing.
- Overall, the interface is complex, hard to understand, and combines
  multiple controls into a single sysctl, which is not ideal. It should
  be separated into distinct controls for clarity.

Setting kernel.printk now calls printk_sysctl_deprecated, which emits a
pr_warn. The warning informs users about the deprecation and suggests
using the new sysctl interfaces instead.

By deprecating kernel.printk, we aim to:

- Encourage users to adopt the new, dedicated sysctl interfaces
  (kernel.console_loglevel and kernel.default_message_loglevel), which
  are more straightforward and easier to understand.
- Improve input validation and error handling, ensuring that users
  receive appropriate feedback when setting invalid values.
- Simplify the configuration of loglevels by exposing only the controls
  that are necessary and relevant.

Signed-off-by: Chris Down <chris@chrisdown.name>
---
 Documentation/admin-guide/sysctl/kernel.rst |  3 +++
 kernel/printk/sysctl.c                      | 14 +++++++++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

Comments

Petr Mladek Nov. 14, 2024, 4:25 p.m. UTC | #1
On Mon 2024-10-28 16:45:59, Chris Down wrote:
> The kernel.printk sysctl interface is deprecated in favour of more
> granular and clearer sysctl interfaces for controlling loglevels, namely
> kernel.console_loglevel and kernel.default_message_loglevel.
> 
> kernel.printk has a number of fairly significant usability problems:
> 
> - It takes four values in a specific order, which is not intuitive.
>   Speaking from personal experience, even people working on printk
>   sometimes need to look up the order of these values, which doesn't
>   suggest much in the way of perspicuity.
> - There is no validation on the input values, so users can set them to
>   invalid or nonsensical values without receiving any errors or
>   warnings. This can lead to unpredictable behaviour.
> - One of the four values, default_console_loglevel, is not used in the
>   kernel (see below), making it redundant and potentially confusing.
> - Overall, the interface is complex, hard to understand, and combines
>   multiple controls into a single sysctl, which is not ideal. It should
>   be separated into distinct controls for clarity.
> 
> Setting kernel.printk now calls printk_sysctl_deprecated, which emits a
> pr_warn. The warning informs users about the deprecation and suggests
> using the new sysctl interfaces instead.
> 
> By deprecating kernel.printk, we aim to:
> 
> - Encourage users to adopt the new, dedicated sysctl interfaces
>   (kernel.console_loglevel and kernel.default_message_loglevel), which
>   are more straightforward and easier to understand.
> - Improve input validation and error handling, ensuring that users
>   receive appropriate feedback when setting invalid values.
> - Simplify the configuration of loglevels by exposing only the controls
>   that are necessary and relevant.
> 
> --- a/kernel/printk/sysctl.c
> +++ b/kernel/printk/sysctl.c
> @@ -7,6 +7,7 @@
>  #include <linux/printk.h>
>  #include <linux/capability.h>
>  #include <linux/ratelimit.h>
> +#include <linux/console.h>
>  #include "internal.h"
>  
>  static const int ten_thousand = 10000;
> @@ -46,13 +47,24 @@ static int printk_console_loglevel(const struct ctl_table *table, int write,
>  	return 0;
>  }
>  
> +static int printk_sysctl_deprecated(const struct ctl_table *table, int write,
> +				    void *buffer, size_t *lenp, loff_t *ppos)
> +{
> +	int res = proc_dointvec(table, write, buffer, lenp, ppos);
> +
> +	if (write)
> +		pr_warn_once("printk: The kernel.printk sysctl is deprecated. Consider using kernel.console_loglevel or kernel.default_message_loglevel instead.\n");
> +
> +	return res;
> +}
> +
>  static struct ctl_table printk_sysctls[] = {
>  	{
>  		.procname	= "printk",
>  		.data		= &console_loglevel,
>  		.maxlen		= 4*sizeof(int),
>  		.mode		= 0644,
> -		.proc_handler	= proc_dointvec,
> +		.proc_handler	= printk_sysctl_deprecated,

I would prefer to follow the existing naming scheme for
custom modifications of proc_dointvec() and call it
"proc_dointvec_printk_deprecated".

>  	},
>  	{
>  		.procname	= "printk_ratelimit",

With this cosmetic change:

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr
diff mbox series

Patch

diff --git a/Documentation/admin-guide/sysctl/kernel.rst b/Documentation/admin-guide/sysctl/kernel.rst
index 8019779b27f6..6027f0057ea3 100644
--- a/Documentation/admin-guide/sysctl/kernel.rst
+++ b/Documentation/admin-guide/sysctl/kernel.rst
@@ -1061,6 +1061,9 @@  Messages without an explicit priority will be printed with this priority.
 printk
 ======
 
+This sysctl is deprecated and will be removed in future. Please consider using
+``kernel.console_loglevel`` or ``kernel.default_message_loglevel`` instead.
+
 The four values in printk denote: ``console_loglevel``,
 ``default_message_loglevel``, ``minimum_console_loglevel`` and
 ``default_console_loglevel`` respectively.
diff --git a/kernel/printk/sysctl.c b/kernel/printk/sysctl.c
index 3bce8b89dacc..42f9e94b1e47 100644
--- a/kernel/printk/sysctl.c
+++ b/kernel/printk/sysctl.c
@@ -7,6 +7,7 @@ 
 #include <linux/printk.h>
 #include <linux/capability.h>
 #include <linux/ratelimit.h>
+#include <linux/console.h>
 #include "internal.h"
 
 static const int ten_thousand = 10000;
@@ -46,13 +47,24 @@  static int printk_console_loglevel(const struct ctl_table *table, int write,
 	return 0;
 }
 
+static int printk_sysctl_deprecated(const struct ctl_table *table, int write,
+				    void *buffer, size_t *lenp, loff_t *ppos)
+{
+	int res = proc_dointvec(table, write, buffer, lenp, ppos);
+
+	if (write)
+		pr_warn_once("printk: The kernel.printk sysctl is deprecated. Consider using kernel.console_loglevel or kernel.default_message_loglevel instead.\n");
+
+	return res;
+}
+
 static struct ctl_table printk_sysctls[] = {
 	{
 		.procname	= "printk",
 		.data		= &console_loglevel,
 		.maxlen		= 4*sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec,
+		.proc_handler	= printk_sysctl_deprecated,
 	},
 	{
 		.procname	= "printk_ratelimit",