From: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
To: Petr Mladek <pmladek@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>,
Steven Rostedt <rostedt@goodmis.org>,
Peter Zijlstra <peterz@infradead.org>,
Andrew Morton <akpm@linux-foundation.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Jiri Slaby <jslaby@suse.cz>,
linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] printk: Correctly handle preemption in console_unlock()
Date: Mon, 16 Jan 2017 20:58:44 +0900 [thread overview]
Message-ID: <20170116115844.GA405@tigerII.localdomain> (raw)
In-Reply-To: <20170116113834.GF20462@pathway.suse.cz>
On (01/16/17 12:38), Petr Mladek wrote:
[..]
> > > Now, @console_may_schedule is not cleared when we call
> > > console_trylock() and jump back to the "again" goto label.
> > > This has become a problem, since the commit 6b97a20d3a7909daa066
> > > ("printk: set may_schedule for some of console_trylock() callers").
> >
> > so I think I'd prefer to revert that commit.
> >
> > the reason I added the commit in question was to reduce the number of
> > printk() soft lockups that I observed back then. however, it obviously
> > didn't solve all of the printk() problems.
>
> Interesting idea!
>
> > now printk() is moving in a
> > completely different direction in term of lockups and deadlocks. there
> > will be no console_trylock() call in vprintk_emit() at all. we will
> > either do console_lock() from scheduleable printk_kthread or
> > console_trylock() from IRQ work. so 6b97a20d3a7909daa066 didn't buy us
> > a lot, and it still doesn't (+ it introduced a bug).
>
> Well, console_trylock() still will be there for the sync mode.
> Or do I miss anything?
you mean in console_unlock()? there we inherit may_schedule from the
original console_sem lock path, which sould be console_lock() in async
printk case (IOW, preemptible).
other then that - from printk POV, I don't think we will care that much.
anything that directly calls console_lock()/console_trylock will be doing
console_unlock(). those paths are not addressed by async printk anyway.
I have some plans on addressing it, as you know, but that's a later work.
so let's return good ol' bhaviour:
-- console_trylock is always "no resched"
-- console_lock is always "enable resched" (regardless of
console_trylock calls from console_unlock()).
> > apart from that, Tetsuo wasn't really happy with the patch
> > http://www.spinics.net/lists/linux-mm/msg103099.html
>
> The complain is questionable. If a code is sensitive for preemption,
> it should disable preemption.
>
> Another question is if people expect that printk() would call
> cond_resched() or preempt.
my assumption would be that probably people expect printk to work
asap.
[..]
> This would revert the change only for non-preemptive kernel.
>
> The commit 6b97a20d3a7909daa06625 ("printk: set may_schedule for some
> of console_trylock() callers" also enabled preemption which still
> affects preemtible kernel.
>
> Do we want to behave differently in preemptive and non-preemtive
> kernel?
not sure I'm following here. in non-preemptible kernels console_trylock()
always sets console_may_schedule to 0, just like it did before. in
preemptible kernels we now will also set console_may_schedule to 0.
just like before.
in any case, we return back the old behavior. there should be no issues (tm)
-ss
next prev parent reply other threads:[~2017-01-16 11:59 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-01-13 13:15 [PATCH] printk: Correctly handle preemption in console_unlock() Petr Mladek
2017-01-13 16:05 ` Steven Rostedt
2017-01-16 11:00 ` Petr Mladek
2017-01-18 5:45 ` Sergey Senozhatsky
2017-01-18 7:21 ` Sergey Senozhatsky
2017-01-25 12:34 ` Petr Mladek
2017-01-14 6:28 ` Sergey Senozhatsky
2017-01-16 11:38 ` Petr Mladek
2017-01-16 11:58 ` Sergey Senozhatsky [this message]
2017-01-16 12:48 ` Petr Mladek
2017-01-16 13:26 ` Sergey Senozhatsky
2017-01-16 13:43 ` Sergey Senozhatsky
2017-01-16 14:14 ` Petr Mladek
2017-01-16 15:19 ` Sergey Senozhatsky
2017-01-16 15:43 ` Sergey Senozhatsky
2017-01-16 16:35 ` Petr Mladek
2017-01-16 13:41 ` Tetsuo Handa
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170116115844.GA405@tigerII.localdomain \
--to=sergey.senozhatsky@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=gregkh@linuxfoundation.org \
--cc=jslaby@suse.cz \
--cc=linux-fbdev@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=penguin-kernel@I-love.SAKURA.ne.jp \
--cc=peterz@infradead.org \
--cc=pmladek@suse.com \
--cc=rostedt@goodmis.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).