linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Petr Mladek <pmladek@suse.com>
To: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Tejun Heo <tj@kernel.org>,
	akpm@linux-foundation.org, linux-mm@kvack.org,
	Cong Wang <xiyou.wangcong@gmail.com>,
	Dave Hansen <dave.hansen@intel.com>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Mel Gorman <mgorman@suse.de>, Michal Hocko <mhocko@kernel.org>,
	Vlastimil Babka <vbabka@suse.cz>,
	Peter Zijlstra <peterz@infradead.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Jan Kara <jack@suse.cz>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>,
	rostedt@home.goodmis.org, Byungchul Park <byungchul.park@lge.com>,
	Pavel Machek <pavel@ucw.cz>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 0/2] printk: Console owner and waiter logic cleanup
Date: Tue, 16 Jan 2018 11:13:55 +0100	[thread overview]
Message-ID: <20180116101355.iy7q3pqxzzlpdiht@pathway.suse.cz> (raw)
In-Reply-To: <20180116022349.GD6607@jagdpanzerIV>

On Tue 2018-01-16 11:23:49, Sergey Senozhatsky wrote:
> On (01/15/18 15:45), Petr Mladek wrote:
> > > I think adding the preempt_disable() would fix printk() but let non
> > > printk console_unlock() still preempt.
> > 
> > I would personally remove cond_resched() from console_unlock()
> > completely.
> 
> hmm, not so sure. I think it's there for !PREEMPT systems which have
> to print a lot of messages. the case I'm speaking about in particular
> is when we register a CON_PRINTBUFFER console and need to console_unlock()
> (flush) all of the messages we currently have in the logbuf. we better
> have that cond_resched() there, I think.

Good point. I agree that we should keep the cond_resched() there
at least for now.


> > Sleeping in console_unlock() increases the chance that more messages
> > would need to be handled. And more importantly it reduces the chance
> > of a successful handover.
> > 
> > As a result, the caller might spend there very long time, it might
> > be getting increasingly far behind. There is higher risk of lost
> > messages. Also the eventual taker might have too much to proceed
> > in preemption disabled context.
> 
> yes.
> 
> > Removing cond_resched() is in sync with printk() priorities.
> 
> hmm, not sure. we have sleeping console_lock()->console_unlock() path
> for PREEMPT kernels, that cond_resched() makes the !PREEMPT kernels to
> have the same sleeping console_lock()->console_unlock().
> 
> printk()->console_unlock() seems to be a pretty independent thing,
> unfortunately (!), yet sleeping console_lock()->console_unlock()
> messes up with it a lot.

IMHO, the problem here is that console_lock is used to synchronize
too many things. It would be great to separate printk() duties
into a separate lock in the long term.

Anyway, I see it the following way. Most console_lock() callers
do the following things:

void foo()
{
	console_lock()
	foo_specific_work();
	console_unlock();
}

where console_unlock() flushes the printk buffer before actually
releasing the lock.

IMHO, it would make sense if flushing the printk buffer behaves
the same when called either from printk() or from any other path.
I mean that it should be aggressive and allow an effective
hand off.

It should be safe as long as foo_specific_work() does not take
too much time.

>From other side. The cond_resched() in console_unlock() should
be obsoleted by the hand-shake code.


> > The highest one is to get the messages out.
> > 
> > Finally, removing cond_resched() should make the behavior more
> > predictable (never preempted)
> 
> but we are always preempted in PREEMPT kernels when the current
> console_sem owner acquired the lock via console_lock(), not via
> console_trylock(). cond_resched() does the same, but for !PREEMPT.

I agree that the situation is more complicated for cond_resched()
called after console_lock(). I do not resist on removing it now.

Just one more thing. The time axe looks like:

+ cond_resched added into console_unlock in v4.5-rc1, Jan 15, 2016
     (commit 8d91f8b15361dfb438ab6)

+ preemtion enabled in printk in, v4.6-rc1, Mar 17, 2016
     (commit 6b97a20d3a7909daa0662)

They both were obvious solutions that helped to reduce the risk
of soft-lockups. The first one handled evidently safe scenarios.
The second one was even more aggressive. I would say that
they both were more or less add-hoc solutions that did not
take into account the other side effects (delaying output,
even loosing messages).

I would not say that one is a diametric difference between them.
Therefore if we remove one for a reason, we should think about
reverting the other as well. But again. I am fine if we remove
only one now.

Does this make any sense?

Best Regard,
Petr

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  parent reply	other threads:[~2018-01-16 10:14 UTC|newest]

Thread overview: 140+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-10 13:24 [PATCH v5 0/2] printk: Console owner and waiter logic cleanup Petr Mladek
2018-01-10 13:24 ` [PATCH v5 1/2] printk: Add console owner and waiter logic to load balance console writes Petr Mladek
2018-01-10 16:50   ` Steven Rostedt
2018-01-12 16:54   ` Steven Rostedt
2018-01-12 17:11     ` Steven Rostedt
2018-01-17 19:13       ` Rasmus Villemoes
2018-01-17 19:33         ` Steven Rostedt
2018-01-19  9:51         ` Sergey Senozhatsky
2018-01-18 22:03     ` Pavel Machek
2018-01-19  0:20       ` Steven Rostedt
2018-01-17  2:19   ` Byungchul Park
2018-01-17  4:54     ` Byungchul Park
2018-01-17  7:34     ` Byungchul Park
2018-01-17 12:04     ` Petr Mladek
2018-01-18  1:53       ` Byungchul Park
2018-01-18  1:57         ` Byungchul Park
2018-01-18  2:19         ` Steven Rostedt
2018-01-18  4:01           ` Byungchul Park
2018-01-18 15:21             ` Steven Rostedt
2018-01-19  2:37               ` Byungchul Park
2018-01-19  3:27                 ` Steven Rostedt
2018-01-22  2:31                   ` Byungchul Park
2018-01-10 13:24 ` [PATCH v5 2/2] printk: Hide console waiter logic into helpers Petr Mladek
2018-01-10 17:52   ` Steven Rostedt
2018-01-11 12:03     ` Petr Mladek
2018-01-12 15:37       ` Steven Rostedt
2018-01-12 16:08         ` Petr Mladek
2018-01-12 16:36           ` Steven Rostedt
2018-01-15 16:08             ` Petr Mladek
2018-01-16  5:05               ` Sergey Senozhatsky
2018-01-10 14:05 ` [PATCH v5 0/2] printk: Console owner and waiter logic cleanup Tejun Heo
2018-01-10 16:29   ` Petr Mladek
2018-01-10 17:02     ` Tejun Heo
2018-01-10 18:21       ` Peter Zijlstra
2018-01-10 18:30         ` Tejun Heo
2018-01-10 18:41           ` Peter Zijlstra
2018-01-10 19:05             ` Tejun Heo
2018-01-11  5:15         ` Sergey Senozhatsky
2018-01-10 18:22       ` Steven Rostedt
2018-01-10 18:36         ` Tejun Heo
2018-01-10 18:40       ` Mathieu Desnoyers
2018-01-11  7:36         ` Sergey Senozhatsky
2018-01-11 11:24           ` Petr Mladek
2018-01-11 13:19             ` Sergey Senozhatsky
2018-01-24  9:36       ` Peter Zijlstra
2018-01-24 18:46         ` Tejun Heo
2018-05-09  8:58       ` Sergey Senozhatsky
2018-01-10 18:54     ` Steven Rostedt
2018-01-11  5:10     ` Sergey Senozhatsky
2018-01-10 18:05   ` Steven Rostedt
2018-01-10 18:12     ` Tejun Heo
2018-01-10 18:14       ` Tejun Heo
2018-01-10 18:45         ` Steven Rostedt
2018-01-10 18:41       ` Steven Rostedt
2018-01-10 18:57         ` Tejun Heo
2018-01-10 19:17           ` Steven Rostedt
2018-01-10 19:34             ` Tejun Heo
2018-01-10 19:44               ` Steven Rostedt
2018-01-10 22:44                 ` Tejun Heo
2018-01-11  5:35             ` Sergey Senozhatsky
2018-01-11  4:58     ` Sergey Senozhatsky
2018-01-11  9:34       ` Petr Mladek
2018-01-11 10:38         ` Sergey Senozhatsky
2018-01-11 11:50           ` Petr Mladek
2018-01-11 16:29           ` Steven Rostedt
2018-01-12  1:30             ` Steven Rostedt
2018-01-12  2:55               ` Steven Rostedt
2018-01-12  4:20                 ` Steven Rostedt
2018-01-16 19:44                 ` Tejun Heo
2018-01-17  9:12                   ` Petr Mladek
2018-01-17 15:15                     ` Tejun Heo
2018-01-17 17:12                       ` Steven Rostedt
2018-01-17 18:42                         ` Steven Rostedt
2018-01-19 18:20                           ` Steven Rostedt
2018-01-20  7:14                             ` Sergey Senozhatsky
2018-01-20 15:49                               ` Steven Rostedt
2018-01-21 14:15                                 ` Sergey Senozhatsky
2018-01-21 21:04                                   ` Steven Rostedt
2018-01-22  8:56                                     ` Sergey Senozhatsky
2018-01-22 10:28                                       ` Sergey Senozhatsky
2018-01-22 10:36                                         ` Sergey Senozhatsky
2018-01-23  6:40                                   ` Sergey Senozhatsky
2018-01-23  7:05                                     ` Sergey Senozhatsky
2018-01-23  7:31                                     ` Sergey Senozhatsky
2018-01-23 14:56                                     ` Steven Rostedt
2018-01-23 15:21                                       ` Sergey Senozhatsky
2018-01-23 15:41                                         ` Steven Rostedt
2018-01-23 15:43                                           ` Tejun Heo
2018-01-23 16:12                                             ` Sergey Senozhatsky
2018-01-23 16:13                                             ` Steven Rostedt
2018-01-23 17:21                                               ` Tejun Heo
2018-04-23  5:35                                             ` Sergey Senozhatsky
2018-01-23 16:01                                           ` Sergey Senozhatsky
2018-01-23 16:24                                             ` Steven Rostedt
2018-01-24  2:11                                               ` Sergey Senozhatsky
2018-01-24  2:52                                                 ` Steven Rostedt
2018-01-24  4:44                                                   ` Sergey Senozhatsky
2018-01-23 17:22                                             ` Tejun Heo
2018-01-20 12:19                             ` Tejun Heo
2018-01-20 14:51                               ` Steven Rostedt
2018-01-17 20:05                         ` Tejun Heo
2018-01-18  5:43                           ` Sergey Senozhatsky
2018-01-18 11:51                           ` Petr Mladek
2018-01-18  5:42                         ` Sergey Senozhatsky
2018-01-12  3:12               ` Sergey Senozhatsky
2018-01-12  2:56             ` Sergey Senozhatsky
2018-01-12  3:21               ` Steven Rostedt
2018-01-12 10:05                 ` Sergey Senozhatsky
2018-01-12 12:21                   ` Steven Rostedt
2018-01-12 12:55                     ` Petr Mladek
2018-01-13  7:31                       ` Sergey Senozhatsky
2018-01-15  8:51                         ` Petr Mladek
2018-01-15  9:48                           ` Sergey Senozhatsky
2018-01-16  5:16                           ` Sergey Senozhatsky
2018-01-16  9:08                             ` Petr Mladek
2018-01-15 12:08                       ` Steven Rostedt
2018-01-16  4:51                         ` Sergey Senozhatsky
2018-01-13  7:28                     ` Sergey Senozhatsky
2018-01-15 10:17                       ` Petr Mladek
2018-01-15 11:50                         ` Petr Mladek
2018-01-16  6:10                           ` Sergey Senozhatsky
2018-01-16  9:36                             ` Petr Mladek
2018-01-16 10:10                               ` Sergey Senozhatsky
2018-01-16 16:06                             ` Steven Rostedt
2018-01-16  5:23                         ` Sergey Senozhatsky
2018-01-15 12:06                       ` Steven Rostedt
2018-01-15 14:45                         ` Petr Mladek
2018-01-16  2:23                           ` Sergey Senozhatsky
2018-01-16  4:47                             ` Sergey Senozhatsky
2018-01-16 10:19                               ` Petr Mladek
2018-01-17  2:24                                 ` Sergey Senozhatsky
2018-01-16 15:45                               ` Steven Rostedt
2018-01-17  2:18                                 ` Sergey Senozhatsky
2018-01-17 13:04                                   ` Petr Mladek
2018-01-17 15:24                                     ` Steven Rostedt
2018-01-18  4:31                                     ` Sergey Senozhatsky
2018-01-18 15:22                                       ` Steven Rostedt
2018-01-16 10:13                             ` Petr Mladek [this message]
2018-01-17  6:29                               ` Sergey Senozhatsky
2018-01-16  1:46                         ` Sergey Senozhatsky

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=20180116101355.iy7q3pqxzzlpdiht@pathway.suse.cz \
    --to=pmladek@suse.com \
    --cc=akpm@linux-foundation.org \
    --cc=byungchul.park@lge.com \
    --cc=dave.hansen@intel.com \
    --cc=hannes@cmpxchg.org \
    --cc=jack@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mgorman@suse.de \
    --cc=mhocko@kernel.org \
    --cc=pavel@ucw.cz \
    --cc=penguin-kernel@I-love.SAKURA.ne.jp \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=rostedt@home.goodmis.org \
    --cc=sergey.senozhatsky.work@gmail.com \
    --cc=sergey.senozhatsky@gmail.com \
    --cc=tj@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=vbabka@suse.cz \
    --cc=xiyou.wangcong@gmail.com \
    /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).