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

[Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>]

Hello,

really sorry for very long reply.

On (08/12/16 11:44), Petr Mladek wrote:
[..]
> IMHO, this is fine. We force the synchronous mode in critical
> situations anyway.

yes, I think it makes sense to lower the priority (we also have
briefly discussed this in private emails with Viresh). I'd still
prefer to have forced sync-printk on suspend/hibernate/etc., though.

> But I was curious if we could hit a printk from the wake_up_process().
> The change above causes using the fair scheduler and there is
> the following call chain [*]
> 
>   vprintk_emit()
>   -> wake_up_process()
>    -> try_to_wake_up()
>     -> ttwu_queue()
>      -> ttwu_do_activate()
>       -> ttwu_activate()
>        -> activate_task()
> 	-> enqueue_task()
> 	 -> enqueue_task_fair()	    via p->sched_class->enqueue_task
> 	  -> cfs_rq_of()
> 	   -> task_of()
> 	    -> WARN_ON_ONCE(!entity_is_task(se))
> 
> We should never trigger this because printk_kthread is a task.
> But what if the date gets inconsistent?
> 
> Then there is the following chain:
> 
>   vprintk_emit()
>   -> wake_up_process()
>    -> try_to_wake_up()
>     -> ttwu_queue()
>      -> ttwu_do_activate()
>       -> ttwu_activate()
>        -> activate_task()
> 	-> enqueue_task()
> 	 -> enqueue_task_fair()	    via p->sched_class->enqueue_task
> 	  ->hrtick_update()
> 	   -> hrtick_start_fair()
> 	    -> WARN_ON(task_rq(p) != rq)
> 
> This looks like another paranoid consistency check that might be
> triggered when the scheduler gets messed.
> 
> I see few possible solutions:
> 
> 1. Replace the WARN_ONs by printk_deferred().
> 
>    This is the usual solution but it would make debugging less convenient.

what I did internally was a combination of #1 and #3: I introduced a
dump_stack_deferred() function which is basically (almost) a copy-past
of dump_stack() from lib/dump_stack.c with the difference that it calls
printk_deferred(). and added a WARN_ON_DEFERRED() macro.


> 2. Force synchronous printk inside WARN()/BUG() macros.

will it help? semaphore up() calls wake_up_process() regardless the context.
not to mention that we still may have spin_dump() enabled.


> 3. Force printk_deferred() inside WARN()/BUG() macros via the per-CPU
>    printk_func.
> 
>    It might be elegant. But we do not want this outside the scheduler
>    code. Therefore we would need special variants of  WARN_*_SCHED()
>    BUG_*_SCHED() macros.
>
> I personally prefer the 2nd solution. What do you think about it,
> please?

I personally think a combo of #1 and #3 is a bit better than plain #2.

	-ss