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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 18 Aug 2016 11:27:12 +0900
From:	Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
To:	Petr Mladek <pmladek@...e.com>
Cc:	Viresh Kumar <viresh.kumar@...aro.org>, Jan Kara <jack@...e.cz>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
	Jan Kara <jack@...e.com>, Tejun Heo <tj@...nel.org>,
	Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Byungchul Park <byungchul.park@....com>,
	Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
	vlevenetz@...sol.com,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [PATCH v10 1/2] printk: Make printk() completely async

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