Re: [RFC][PATCH v8 1/2] printk: Make printk() completely async

Sergey Senozhatsky
2016-04-01T09:36:46Z

Hello Petr,

On (04/01/16 10:59), Petr Mladek wrote:
[..]
> CPU0 CPU1
>
> printk()
>
> if (printk_kthread)
> # fails and need_flush_console
> # stays false
>
> init_printk_kthread()
> # put printk_thread into
> # run queue
> printk_kthread = ...;
>
> if (!in_panic && printk_kthread)
> wake_up_process(printk_kthread);
>
>
> # printk kthread finally gets
> # scheduled
> printk_kthread_func()
>
> set_current_state(TASK_INTERRUPTIBLE);
> if (!need_flush_console)
> schedule();
>
> => printk_kthread is happily sleeping without calling console.

oohh, that tiny race. well, looks quite harmless, it's unlikely that
we had printk()-s up until late_initcall(init_printk_kthread) and not
a single one ever after. but good find!

so the check
if (printk_kthread)
need_flush_console = 1

can be replaced with
if (!printk_sync)
need_flush_console = 1

or... may be dropped.

> I do not see any code that will modify need_flush_console when
> printk.synchronous is modified at runtime.

printk.synchronous is RO; no runtime modification.

> I know that all this is rather theoretical. My main point is to remove
> unnecessary checks that make the code harder to read and does not bring
> any big advantage.

my point is that those checks are just .loads, which help to avoid
spurious .stores from various CPUs.

CPU1 CPU2 CPU3 ... CPU1024

lock logbuf
need_flush=1
unlock logbuf
lock logbuf
need_flush=1
unlock logbuf
lock logbuf
need_flush=1
unlock logbuf
wakeup kthread
...
lock logbuf
need_flush=1
unlock logbuf

isn't it a bit useless need_flush=1 traffic?

-ss