LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
To: Petr Mladek <pmladek@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	akpm@linux-foundation.org, linux-mm@kvack.org,
	Peter Zijlstra <peterz@infradead.org>, Jan Kara <jack@suse.cz>,
	Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>,
	Tejun Heo <tj@kernel.org>,
	linux-kernel@vger.kernel.org,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Subject: Re: [PATCH] printk: Ratelimit messages printed by console drivers
Date: Fri, 20 Apr 2018 11:15:11 +0900	[thread overview]
Message-ID: <20180420021511.GB6397@jagdpanzerIV> (raw)
In-Reply-To: <20180419125353.lawdc3xna5oqlq7k@pathway.suse.cz>

On (04/19/18 14:53), Petr Mladek wrote:
> > > > 
> > > > Besides 100 lines is absolutely not enough for any real lockdep splat.
> > > > My call would be - up to 1000 lines in a 1 minute interval.
> 
> But this would break the intention of this patch.

You picked an arbitrary value and now you are saying that any other
value will not work?

> Come on guys! The first reaction how to fix the infinite loop was
> to fix the console drivers and remove the recursive messages. We are
> talking about messages that should not be there or they should
> get replaced by WARN_ONCE(), print_once() or so. This patch only
> give us a chance to see the problem and do not blow up immediately.
> 
> I am fine with increasing the number of lines. But we need to keep
> the timeout long. In fact, 1 hour is still rather short from my POV.

Disagree.

I saw 3 or 4 lockdep reports coming from console drivers. "100 lines"
is way too restrictive. I want to have a complete report; not the first
50 lines, not the first 103 lines, which would "hint" me that "hey, there
is something wrong there, but you are on your own to figure out the rest".

> > > Well, if we want to basically turn printk_safe() into printk_safe_ratelimited().
> > > I'm not so sure about it.
> 
> No, it is not about printk_safe(). The ratelimit is active when
> console_owner == current. It triggers when printk() is called
> inside

"console_owner == current" is exactly the point when we call console
drivers and add scheduler, networking, timekeeping, etc. locks to the
picture. And so far all of the lockdeps reports that we had were from
call_console_drivers(). So it very much is about printk_safe().

> > > Besides the patch also rate limits printk_nmi->logbuf - the logbuf
> > > PRINTK_NMI_DEFERRED_CONTEXT_MASK bypass, which is way too important
> > > to rate limit it - for no reason.
> 
> Again. It has the effect only when console_owner == current. It means
> that it affects "only" NMIs that interrupt console_unlock() when calling
> console drivers.

What is your objection here? NMIs can come anytime.

> > One more thing,
> > I'd really prefer to rate limit the function which flushes per-CPU
> > printk_safe buffers; not the function that appends new messages to
> > the per-CPU printk_safe buffers.
> 
> I wonder if this opinion is still valid after explaining the
> dependency on printk_safe(). In each case, it sounds weird
> to block printk_safe buffers with some "unwanted" messages.
> Or maybe I miss something.

I'm not following.

The fact that some consoles under some circumstances can add unwanted
messages to the buffer does not look like a good enough reason to start
rate limiting _all_ messages and to potentially discard the _important_
ones.

	-ss

  reply	other threads:[~2018-04-20  2:15 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-13 12:47 Petr Mladek
2018-04-13 14:12 ` Steven Rostedt
2018-04-14  2:35   ` Sergey Senozhatsky
2018-04-16  1:47     ` Sergey Senozhatsky
2018-04-16  4:25       ` Sergey Senozhatsky
2018-04-19 12:53         ` Petr Mladek
2018-04-20  2:15           ` Sergey Senozhatsky [this message]
2018-04-20  9:12             ` Petr Mladek
2018-04-20 12:04               ` Steven Rostedt
2018-04-20 14:01                 ` Petr Mladek
2018-04-20 14:17                   ` Steven Rostedt
2018-04-20 14:19                     ` Steven Rostedt
2018-04-20 14:57                     ` Petr Mladek
2018-04-20 15:13                       ` Steven Rostedt
2018-04-23 10:32                         ` Petr Mladek
2018-04-23 11:36                           ` Steven Rostedt
2018-04-23 12:45                             ` Petr Mladek
2018-04-25  5:31                               ` Sergey Senozhatsky
2018-04-26  9:42                                 ` Petr Mladek
2018-04-27 10:22                                   ` Sergey Senozhatsky
2018-05-09 12:00                                     ` Petr Mladek
2018-05-09 12:59                                       ` Steven Rostedt
2019-02-26 10:24                                       ` Tetsuo Handa
2019-02-28  4:45                                         ` Sergey Senozhatsky
2018-04-23  5:21               ` Sergey Senozhatsky
2018-04-23 12:26                 ` Petr Mladek
2018-04-23 13:00                   ` 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=20180420021511.GB6397@jagdpanzerIV \
    --to=sergey.senozhatsky.work@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=jack@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=penguin-kernel@I-love.SAKURA.ne.jp \
    --cc=peterz@infradead.org \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=sergey.senozhatsky@gmail.com \
    --cc=tj@kernel.org \
    --subject='Re: [PATCH] printk: Ratelimit messages printed by console drivers' \
    /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

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