Re: [PATCH v8 14/21] printk: console: Introduce sysfs interface for per-console loglevels

[Petr Mladek <pmladek@suse.com>]

On Fri 2025-11-28 03:44:06, Chris Down wrote:
> A sysfs interface under /sys/class/console/ is created that permits
> viewing and configuring per-console attributes. This is the main
> interface with which we expect users to interact with and configure
> per-console loglevels.
> 
> Each console device now has its own directory (for example,
> /sys/class/console/ttyS0/) containing the following attributes:
> 
> - effective_loglevel (ro): The effective loglevel for the console after
>   considering all loglevel authorities (e.g., global loglevel,
>   per-console loglevel).
> - effective_loglevel_source (ro): The source of the effective loglevel
>   (e.g., local, global, ignore_loglevel).
> - loglevel (rw): The per-console loglevel. Writing a value between 0
>   (KERN_EMERG) and 8 (KERN_DEBUG + 1) sets the per-console loglevel.
>   Writing -1 disables the per-console loglevel.
> 
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -200,6 +200,24 @@ static int __init control_devkmsg(char *str)
>  }
>  __setup("printk.devkmsg=", control_devkmsg);
>  
> +/**
> + * console_clamp_loglevel - Clamp a loglevel to valid console loglevel range
> + *
> + * @level: The loglevel to clamp
> + *
> + * Console loglevels must be within the range [LOGLEVEL_ALERT, LOGLEVEL_DEBUG + 1].
> + * This function clamps a given level to this valid range.
> + *
> + * Note: This does not allow LOGLEVEL_EMERG (0) for per-console loglevels, as
> + * level 0 is reserved for emergency messages that should always go to all consoles.
> + *
> + * Return: The clamped loglevel value
> + */
> +int console_clamp_loglevel(int level)
> +{
> +	return clamp(level, LOGLEVEL_ALERT, LOGLEVEL_DEBUG + 1);
> +}
> +
>  char devkmsg_log_str[DEVKMSG_STR_MAX_SIZE] = "ratelimit";
>  #if defined(CONFIG_PRINTK) && defined(CONFIG_SYSCTL)
>  int devkmsg_sysctl_set_loglvl(const struct ctl_table *table, int write,
> --- /dev/null
> +++ b/kernel/printk/sysfs.c
[...]
> +static ssize_t loglevel_store(struct device *dev, struct device_attribute *attr,
> +			      const char *buf, size_t size)
> +{
> +	struct console *con = dev_get_drvdata(dev);
> +	ssize_t ret;
> +	int level;
> +
> +	ret = kstrtoint(buf, 10, &level);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* -1 means "use global loglevel" */
> +	if (level == -1)
> +		goto out;
> +
> +	/*
> +	 * Reject level 0 (KERN_EMERG) - per-console loglevel must be > 0.
> +	 * Emergency messages should go to all consoles, so they cannot be
> +	 * filtered per-console.
> +	 */
> +	if (level == 0)
> +		return -ERANGE;

Nit: The above check is not needed. It is handled and explained in
     console_clamp_loglevel() called right below.

> +
> +	if (console_clamp_loglevel(level) != level)
> +		return -ERANGE;
> +
> +	/*
> +	 * If the system has a minimum console loglevel set (via sysctl or
> +	 * kernel parameter), enforce it. This prevents setting per-console
> +	 * loglevels below the system minimum.
> +	 */
> +	if (minimum_console_loglevel > CONSOLE_LOGLEVEL_MIN &&
> +	    level < minimum_console_loglevel)
> +		return -ERANGE;
> +
> +out:
> +	console_sysfs_write_loglevel(con, level);
> +	return size;
> +}

Otherwise, it looks good to me. With the duplicit check removed:

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

Best Regards,
Petr