Re: [PATCH v8 18/21] printk: Deconstruct kernel.printk into discrete sysctl controls

[Petr Mladek <pmladek@suse.com>]

On Mon 2025-12-15 11:08:12, Joel Granados wrote:
> On Fri, Dec 12, 2025 at 04:24:17PM +0100, Petr Mladek wrote:
> > Adding Joel into Cc. Joel, see the original patch at
> > https://lore.kernel.org/all/c3e5cc507eb3fd7db0a002f31d7e47d764cad176.1764272407.git.chris@chrisdown.name/
> Thx for adding me. Please CC me in the next versions of this series as
> the interface might change (slightly) again. By having this on my radar
> we can better coordinate (if needed) linux-next and PRs for Linus.
> > 
> > On Fri 2025-11-28 03:44:25, Chris Down wrote:
> > > Introduce two new sysctl interfaces for configuring global loglevels:
> > > 
> > > --- a/include/linux/sysctl.h
> > > +++ b/include/linux/sysctl.h
> > > @@ -235,6 +235,13 @@ extern struct ctl_table_header *register_sysctl_mount_point(const char *path);
> > >  
> > >  void do_sysctl_args(void);
> > >  bool sysctl_is_alias(char *param);
> > > +int do_proc_dointvec_conv(bool *negp, unsigned long *lvalp, int *valp,
> > > +			  int write, void *data);
> > > +int do_proc_dointvec(const struct ctl_table *table, int write,
> > > +		     void *buffer, size_t *lenp, loff_t *ppos,
> > > +		     int (*conv)(bool *negp, unsigned long *lvalp, int *valp,
> > > +				 int write, void *data),
> > > +		     void *data);
> > >  int do_proc_douintvec(const struct ctl_table *table, int write,
> > >  		      void *buffer, size_t *lenp, loff_t *ppos,
> > >  		      int (*conv)(unsigned long *lvalp,
> > 
> > This hunk can be removed in v6.19. There is another interface in 6.19,
> > see below.
> > 
> > > diff --git a/kernel/printk/sysctl.c b/kernel/printk/sysctl.c
> > > index da77f3f5c1fe..034739939a61 100644
> > > --- a/kernel/printk/sysctl.c
> > > +++ b/kernel/printk/sysctl.c
> > > @@ -11,6 +11,9 @@
> > >  
> > >  static const int ten_thousand = 10000;
> > >  
> > > +static int min_msg_loglevel = LOGLEVEL_EMERG;
> > > +static int max_msg_loglevel = LOGLEVEL_DEBUG;
> > > +
> > >  static int proc_dointvec_minmax_sysadmin(const struct ctl_table *table, int write,
> > >  				void *buffer, size_t *lenp, loff_t *ppos)
> > >  {
> > > @@ -20,6 +23,50 @@ static int proc_dointvec_minmax_sysadmin(const struct ctl_table *table, int writ
> > >  	return proc_dointvec_minmax(table, write, buffer, lenp, ppos);
> > >  }
> > 
> > The new way is to define the needed helpers using macros. We need this:
> > 
> > static SYSCTL_USER_TO_KERN_INT_CONV(, SYSCTL_CONV_IDENTITY)
> > static SYSCTL_KERN_TO_USER_INT_CONV(, SYSCTL_CONV_IDENTITY)
> > static SYSCTL_INT_CONV_CUSTOM(, sysctl_user_to_kern_int_conv,
> > 			      sysctl_kern_to_user_int_conv, false)
> > 
> > 
> > > +static int do_proc_dointvec_console_loglevel(bool *negp, unsigned long *lvalp,
> > > +					     int *valp,
> > > +					     int write, void *data)
> > 
> > There parameters have got renamed and @table is passed instead or @data:
> > 
> > static int do_proc_dointvec_console_loglevel(bool *negp, unsigned long *u_ptr,
> > 					     int *k_ptr, int dir,
> > 					     const struct ctl_table *table)
> > 
> > 
> > > +{
> > > +	int level, ret;
> > > +
> > > +	/*
> > > +	 * If writing, first do so via a temporary local int so we can
> > > +	 * bounds-check it before touching *valp.
> > > +	 */
> > > +	int *intp = write ? &level : valp;
> > 
> > The direction is newly checked by macro:
> > 
> > 	int *int_ptr = SYSCTL_USER_TO_KERN(dir) ? &level : k_ptr;
> > 
> > > +	ret = do_proc_dointvec_conv(negp, lvalp, intp, write, data);
> > 
> > The following helper is defined by the above mentioned macros:
> > 
> > 	ret = do_proc_int_conv(negp, u_ptr, int_ptr, dir, table);
> > 
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	if (write) {
> > 
> > The new way:
> > 
> > 	if (SYSCTL_USER_TO_KERN(dir)) {
> > 
> > > +		if (level != console_clamp_loglevel(level))
> > > +			return -ERANGE;
> > > +
> > > +		/*
> > > +		 * Honour the administrator-configured minimum console
> > > +		 * loglevel (third element of kernel.printk).  This mirrors
> > > +		 * the syslog() and sysfs control paths so that once the floor
> > > +		 * is raised we do not let this sysctl silently bypass it.
> > > +		 */
> > > +		if (minimum_console_loglevel > CONSOLE_LOGLEVEL_MIN &&
> > > +		    level < minimum_console_loglevel)
> > > +			level = minimum_console_loglevel;
> > > +
> > > +		WRITE_ONCE(*valp, level);
> > 
> > New parameter name:
> > 
> > 		WRITE_ONCE(*k_ptr, level);
> > 
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static int proc_dointvec_console_loglevel(const struct ctl_table *table,
> > > +					  int write, void *buffer, size_t *lenp,
> > > +					  loff_t *ppos)
> > > +{
> > > +	return do_proc_dointvec(table, write, buffer, lenp, ppos,
> > > +			       do_proc_dointvec_console_loglevel, NULL);
> > 
> > There is a new function where the last NULL parameter is not longer passed:
> > 
> > 	return proc_dointvec_conv(table, write, buffer, lenp, ppos,
> > 			       do_proc_dointvec_console_loglevel);
> > 
> > > +}
> > > +
> > >  static const struct ctl_table printk_sysctls[] = {
> > >  	{
> > >  		.procname	= "printk",
> > 
> > 
> > Here are the above described changes made by diff:
> The changes look ok,

Great.

> but there is a lot of context that I'm missing from
> just looking at this change. Will there be another version re-based on
> top of v19-rc1? 

Yes, we need v9. Chris, please add Joel into Cc.

Thanks a lot for checking.

Best Regards,
Petr