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

From: Petr Mladek <pmladek@suse.com>
To: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Cc: 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: Thu, 18 Aug 2016 11:33:29 +0200	[thread overview]
Message-ID: <20160818093329.GL13300@pathway.suse.cz> (raw)
In-Reply-To: <20160818022712.GB500@swordfish>

On Thu 2016-08-18 11:27:12, Sergey Senozhatsky wrote:
> 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.

Sounds fine to me.

> > 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 [*]
> > 
> > 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.

Good point. That changes my preferences :-)

> 
> > 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.

Also we need to make sure that everything will be done on a single CPU
as the printk_func is per-CPU variable.

> > 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.

I would need to see the code how it looks and if is really works.
But yes, it seems that this is the way to go.

Best Regards,
Petr