Re: [RFC][PATCHv2 7/8] printk: add printk emergency_mode parameter

[Petr Mladek <pmladek@suse.com>]

On Wed 2017-03-29 18:25:10, Sergey Senozhatsky wrote:
> This param permits user-space to forcibly on/off printk emergency
> mode via /sys/module/printk/parameters/emergency_mode node.
> 
> We have annotated sections in the kernel that switch printk to
> emergency, but there might be places/cases when user space would
> want to have printk operate in emergency mode all the time.

Thanks a lot for the parameter.
 
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> ---
>  kernel/printk/printk.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 1927b5cb5cbe..0d96839bb450 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -455,7 +455,7 @@ static struct task_struct *printk_kthread __read_mostly;
>  static atomic_t printk_emergency __read_mostly;
>  /*
>   * Disable printk_kthread permanently. Unlike `oops_in_progress'
> - * it doesn't go back to 0.
> + * it doesn't go back to 0 (unless enforced by user-space).
>   */
>  static bool printk_kthread_disabled __read_mostly;
>  
> @@ -483,6 +483,24 @@ void printk_emergency_end(void)
>  	atomic_dec(&printk_emergency);
>  }
>  
> +static int printk_kthread_disabled_set(const char *val,
> +					const struct kernel_param *kp)
> +{
> +	return param_set_bool(val, kp);
> +}
> +
> +static const struct kernel_param_ops printk_kthread_disabled_ops = {
> +	.set = printk_kthread_disabled_set,
> +	.get = param_get_bool,
> +};
> +
> +module_param_cb(emergency_mode,
> +		&printk_kthread_disabled_ops,
> +		&printk_kthread_disabled,
> +		0644);
> +MODULE_PARM_DESC(emergency_mode,
> +		"don't offload message printing to printk kthread");

I wonder if we could make this easier. Something like:

static bool printk_force_emergency;
module_param_named(force_emergency, printk_force_emergency,
		   bool, S_IRUGO | S_IWUSR);

and use it instead of printk_kthread_disabled variable. It was
confusing anyway. You already mentioned that it did not
stop the kthread.

Also the relation between the sysfs entry and printk code
will be cleaner. People might thing that emergency_mode
shows the immediate value of printk_emergency variable.

Best Regards,
Petr