From: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
To: Petr Mladek <pmladek@suse.com>
Cc: John Ogness <john.ogness@linutronix.de>,
Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
Steven Rostedt <rostedt@goodmis.org>,
Linus Torvalds <torvalds@linux-foundation.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Thomas Gleixner <tglx@linutronix.de>,
linux-kernel@vger.kernel.org
Subject: Re: recursion handling: Re: [PATCH next v2 3/3] printk: remove logbuf_lock, add syslog_lock
Date: Sat, 5 Dec 2020 13:25:19 +0900 [thread overview]
Message-ID: <X8sLr4snLX9DB3I8@jagdpanzerIV.localdomain> (raw)
In-Reply-To: <X8pfX/qPBuY360k/@alley>
On (20/12/04 17:10), Petr Mladek wrote:
[..]
> char *get_printk_counter_by_ctx()
> {
> int ctx = 0;
>
> if (in_nmi)
> ctx = 1;
>
> if (!printk_percpu_data_ready())
> return &printk_count_early[ctx];
>
> return this_cpu_ptr(printk_count[ctx]);
> }
>
> > +
> > + return count;
> > +}
> > +
> > +static bool printk_enter(unsigned long *flags)
> > +{
> > + char *count;
> > +
> > + local_irq_save(*flags);
> > + count = get_printk_count();
> > + /* Only 1 level of recursion allowed. */
>
> We should allow at least some level of recursion. Otherwise, we would
> not see warnings printed from vsprintf code.
One level of recursion seems reasonable on one hand, but on the other
hand I also wonder if we can get useful info from recursion levels 2
and higher. Would be great to maybe list possible scenarios. vsprintf()
still call re-enter printk() -- WARN_ONCE()-s in the code -- external
format specifiers handlers also can. Would we need to let two levels of
recursion printk->vsprintf->printk->???->printk or one is just enough?
It also would make sense to add the lost messages counter to per-CPU
recursion counter struct, to count the number of times we bailout
of printk due to recursion limit. So that we'll at least have
"%d recursive printk messages lost" hints.
Overall...
I wonder where does the "limit printk recursion" come from? printk_safe
doesn't impose any strict limits (print_context is limited, but still)
and we've been running it for years now; have we ever seen any reports
of printk recursion overflows?
> > + if (*count > 1) {
> > + local_irq_restore(*flags);
> > + return false;
> > + }
> > + (*count)++;
> > +
> > + return true;
> > +}
>
> This should be unified with printk_context, printk_nmi_enter(),
> printk_nmi_exit(). It does not make sense to have two separate
> printk context counters.
Agreed.
> Or is there any plan to remove printk_safe and printk_context?
That's a good point. This patch set and printk_safe answer the
same question in different ways, as far as I understand it. The
question is "Why do we want to track printk recursion"? This patch
set merely wants to, correct me if I'm wrong, avoid the very deep
vprintk_store() recursion stacks (which is a subset of printk()
recursion superset):
vprintk_store()
{
if (!printk_enter())
return;
vsprintf/prb
print_exit();
}
And that's pretty much it, at least for the time being.
printk_safe()'s answer is - we don't want to re-enter parts of
the kernel that sit in the core, behind the scenes, and that are
not ready to be re-entered. Things like
printk()
down_console_sem()
down()
raw_spin_lock_irqsave(&sem->lock)
printk()
down_console_sem()
down()
raw_spin_lock_irqsave(&sem->lock)
-ss
next prev parent reply other threads:[~2020-12-05 4:27 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-01 20:53 [PATCH next v2 0/3] printk: remove logbuf_lock John Ogness
2020-12-01 20:53 ` [PATCH next v2 1/3] printk: inline log_output(),log_store() in vprintk_store() John Ogness
2020-12-03 15:57 ` Petr Mladek
2020-12-03 16:25 ` John Ogness
2020-12-04 6:13 ` Sergey Senozhatsky
2020-12-04 8:26 ` Petr Mladek
2020-12-01 20:53 ` [PATCH next v2 2/3] printk: change @clear_seq to atomic64_t John Ogness
2020-12-04 9:12 ` Petr Mladek
2020-12-06 20:23 ` John Ogness
2020-12-07 9:34 ` Peter Zijlstra
2020-12-07 10:03 ` John Ogness
2020-12-07 12:56 ` Peter Zijlstra
2020-12-07 12:56 ` Petr Mladek
2020-12-07 16:46 ` David Laight
2020-12-08 20:34 ` Sergey Senozhatsky
2020-12-08 22:30 ` John Ogness
2020-12-09 1:04 ` Sergey Senozhatsky
2020-12-09 8:16 ` Peter Zijlstra
2020-12-09 9:22 ` Sergey Senozhatsky
2020-12-09 10:46 ` Sergey Senozhatsky
2020-12-09 11:00 ` Peter Zijlstra
2020-12-09 11:28 ` Sergey Senozhatsky
2020-12-09 12:29 ` Peter Zijlstra
2020-12-09 8:07 ` Peter Zijlstra
2020-12-01 20:53 ` [PATCH next v2 3/3] printk: remove logbuf_lock, add syslog_lock John Ogness
2020-12-04 6:41 ` Sergey Senozhatsky
2020-12-06 20:44 ` John Ogness
2020-12-04 15:52 ` devkmsg: was " Petr Mladek
2020-12-06 20:51 ` John Ogness
2020-12-07 9:56 ` Petr Mladek
2020-12-04 15:57 ` syslog: was: " Petr Mladek
2020-12-06 21:06 ` John Ogness
2020-12-07 10:01 ` Petr Mladek
2020-12-04 16:10 ` recursion handling: " Petr Mladek
2020-12-05 4:25 ` Sergey Senozhatsky [this message]
2020-12-06 22:08 ` John Ogness
2020-12-05 9:41 ` Sergey Senozhatsky
2020-12-06 22:17 ` John Ogness
2020-12-06 21:44 ` John Ogness
2020-12-07 11:17 ` Petr Mladek
2020-12-04 16:15 ` vprintk_store: was: " Petr Mladek
2020-12-06 22:30 ` John Ogness
2020-12-07 12:46 ` Petr Mladek
2020-12-04 16:19 ` consoles: " Petr Mladek
2020-12-05 4:39 ` Sergey Senozhatsky
2020-12-07 9:50 ` Petr Mladek
2020-12-08 20:51 ` 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=X8sLr4snLX9DB3I8@jagdpanzerIV.localdomain \
--to=sergey.senozhatsky@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=john.ogness@linutronix.de \
--cc=linux-kernel@vger.kernel.org \
--cc=pmladek@suse.com \
--cc=rostedt@goodmis.org \
--cc=sergey.senozhatsky.work@gmail.com \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.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).