LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Sergey Senozhatsky <sergey.senozhatsky@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: Mon, 23 Apr 2018 22:00:02 +0900	[thread overview]
Message-ID: <20180423130002.GA465@tigerII.localdomain> (raw)
In-Reply-To: <20180423122600.sozmrytiasd32bhc@pathway.suse.cz>

On (04/23/18 14:26), Petr Mladek wrote:
> need to see the problems and be able to debug them. BTW: I wrote
> this in the patch description.
> 
> > A very quick googling:
> 
> Nice collection. Just note that the useful (ratelimited) information
> always fits into the first 100 lines in all these examples:

I'm *very and really* sorry to ask that, but are you serious now?
Apologies if I'm not getting a joke here, or something.

[..]
> > Throttling down that error mechanism to 100 lines
> > per hour, or 1000 lines per hour is unlikely will be welcomed.
> 
> I wonder if you have bigger problems with the number of lines
> or with the length of the period.
> 
> We simply _must_ limit the number of lines. Otherwise we would
> never be able to break an _infinite_ loop.

Give me examples of such messages, and please do explain why you were
not able to rate-limit them and instead decided to introduce a system
wide printk() rate-limit.

> > Among all the patches and proposal that we saw so far, one stands out - it's
> > the original Tejun's patch [offloading to work queue]. Because it has zero
> > interference with the existing call_console_drivers()->printk()
> > channels.
> 
> The only problem is that it does not solve the infinite loop.

Same as above. I'm not kidding. I really want to know what unfixable&endless
messages you are talking about? May I take look at the backtraces?

> > What is so special about this case that we decided to screw up printk()
> > instead?
> 
> Also messages from console drivers are about printk debugging. There must
> be some limitations by definition.

No. Check the links that I found after _literally_ 5 seconds of googling.
Tons of messages are coming from core kernel code. Nothing to do with
the debugging. It *is* a valid and widely used error reporting channel.
End of story.


SERIOUSLY. PLEASE (!) - don't turn printk() into rate-limited printk().
Don't introduce that HUGE regression. Let's handle it the same way as we
always do - let's look at the logs, and rate-limit misbehaving code.


> > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> > index 247808333ba4..484c456c095a 100644
> > --- a/kernel/printk/printk.c
> > +++ b/kernel/printk/printk.c
> > @@ -2385,9 +2385,11 @@ void console_unlock(void)
> >  		 */
> >  		console_lock_spinning_enable();
> >  
> > +		__printk_safe_exit();
> >  		stop_critical_timings();	/* don't trace print latency */
> >  		call_console_drivers(ext_text, ext_len, text, len);
> >  		start_critical_timings();
> > +		__printk_safe_enter();
> 
> Is this by intention? What is the reason to call
> console_lock_spinning_disable_and_check() in printk_safe() context, please?

Yes, it is. console_lock_spinning_enable() is touching console_owner_lock:
an `internal' printk lock -- which we also touch in vprintk_emit(). As such
[internal printk lock] it must be accessed under printk_safe(), by definition.

> >  		if (console_lock_spinning_disable_and_check()) {
> >  			printk_safe_exit_irqrestore(flags);

	-ss

      reply	other threads:[~2018-04-23 13:00 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
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 [this message]

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=20180423130002.GA465@tigerII.localdomain \
    --to=sergey.senozhatsky@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.work@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).