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

From: Jan Kara <jack@suse.cz>
To: Petr Mladek <pmladek@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
	Viresh Kumar <viresh.kumar@linaro.org>, Jan Kara <jack@suse.cz>,
	Andrew Morton <akpm@linux-foundation.org>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Jan Kara <jack@suse.com>, Tejun Heo <tj@kernel.org>,
	Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Byungchul Park <byungchul.park@lge.com>,
	vlevenetz@mm-sol.com,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: [PATCH v10 1/2] printk: Make printk() completely async
Date: Fri, 19 Aug 2016 21:00:07 +0200	[thread overview]
Message-ID: <20160819190007.GA8275@quack2.suse.cz> (raw)
In-Reply-To: <20160819095455.GR13300@pathway.suse.cz>

On Fri 19-08-16 11:54:55, Petr Mladek wrote:
> On Fri 2016-08-19 15:32:36, Sergey Senozhatsky wrote:
> > On (08/18/16 12:56), Petr Mladek wrote:
> > > The advantage of the printk_func trick is that it is transparent.
> > > You do not need to modify any existing functions used by WARN()/BUG()
> > > macros.
> > 
> > good point.
> > 
> > so something like below, perhaps. I'm less sure about
> > deferred BUG()/BUG_ON():
> > 
> > #define DEFERRED_BUG() do { 		\
> > 	printk_deferred_enter();	\
> > 	BUG();				\
> > 	printk_deferred_exit();		\
> > } while (0)				\
> > 
> > #define DEFERRED_BUG_ON(condition) do { \
> > 	printk_deferred_enter();	\
> > 	BUG_ON(condition);		\
> > 	printk_deferred_exit();		\
> > } while (0)
> > 
> > depending on .config BUG() may never return back -- passing control
> > to do_exit(), so printk_deferred_exit() won't be executed. thus we
> > probably need to have a per-cpu variable that would indicate that
> > we are in deferred_bug. hm... but do we really need deferred BUG()
> > in the first place?
> 
> Good question. I am not aware of any BUG_ON() that would be called from
> wake_up_process() but it is hard to check everything.
> 
> A conservative approach would be to force synchronous printk from
> BUG_ON().

Just a quick thought: Cannot we just do printk_deferred_enter() when we are
about to call into the scheduler from printk code and printk_deferred_exit()
when leaving it? That would look like the least error-prone way how
handling this kind of recursion...

OTOH there's also the other possible direction for the recursion when we
are in the scheduler, holding some scheduler locks, decide to WARN which
enters printk, that ends up calling wake_up_process() which deadlocks
on scheduler locks... I don't see how to handle this type of recursion
inside the printk code itself easily and so far the answer was - use
printk_deferred() in the scheduler and don't use WARN...

Hum, maybe we could add lockdep annotation to a WARN_ON and BUG_ON macros so
that it would grab and release console_sem (even if the condition is false).
That way we'd get lockdep splats for all the possible WARN_ON and BUG_ON
calls that could deadlock.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR