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
