Re: [PATCH 2/2] printk: wake up klogd in vprintk_emit

[Petr Mladek <pmladek@suse.com>]

On Sat 2018-04-14 12:01:45, Sergey Senozhatsky wrote:
> We wake up klogd very late - only when current console_sem owner
> is done pushing pending kernel messages to the serial/net consoles.
> In some cases this results in lost syslog messages, because kernel
> log buffer is a circular buffer and if we don't wakeup syslog long
> enough there are chances that logbuf simply will wrap around.
> 
> The patch moves the klog wake up call to vprintk_emit(), which is
> the only legit way for a kernel message to appear in the logbuf,
> right before we attempt to grab the console_sem (possibly spinning
> on it waiting for the hand off) and call console drivers.
> 
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> ---
>  kernel/printk/printk.c | 14 ++------------
>  1 file changed, 2 insertions(+), 12 deletions(-)
> 
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 2f4af216bd6e..86f0b337cbf6 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -1888,6 +1888,7 @@ asmlinkage int vprintk_emit(int facility, int level,
>  
>  	printed_len = log_output(facility, level, lflags, dict, dictlen, text, text_len);
>  
> +	wake_up_klogd();
>  	logbuf_unlock_irqrestore(flags);

The change makes perfect sense and I am fine with the idea. I just
wonder if there is a strong reason to do the wake_up before
releasing the logbuf_lock. It makes an assumption that it needs
to be synchronized by logbuf_lock.

In fact, I would feel more comfortable if we move this to the end
of vprintk_emit() right before return printk_len. This will be
more close to the current behavior (console first). But it will
still wakeup klogd much earlier and regularly if there is
a flood of messages.

Best Regards,
Petr