All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Ogness <john.ogness@linutronix.de>
To: Sergey Senozhatsky <senozhatsky@chromium.org>
Cc: Petr Mladek <pmladek@suse.com>,
	Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH next v1 1/3] printk: track/limit recursion
Date: Mon, 22 Mar 2021 11:53:30 +0100	[thread overview]
Message-ID: <87mtuvmpcl.fsf@jogness.linutronix.de> (raw)
In-Reply-To: <YFba1Fje6+TeIiGW@google.com>

On 2021-03-21, Sergey Senozhatsky <senozhatsky@chromium.org> wrote:
>> @@ -2055,6 +2122,9 @@ int vprintk_store(int facility, int level,
>>  	 */
>>  	ts_nsec = local_clock();
>>  
>> +	if (!printk_enter_irqsave(&irqflags))
>> +		return 0;
>
> I guess it can be interesting to somehow signal us that we had
> printk() recursion overflow, and how many messages we lost.

Honestly, if we hit 3 levels of recursion, we are probably dealing with
an infinite recursion issue. I do not see the value of counting the
overflows in that case. The logged messages at that recursion level
would ben enough to point us to the problem.

> 3 levels of recursion seem like reasonable limit, but I maybe wouldn't
> mind one extra level.

With 3 levels, we will see all the messages of:

    printk -> WARN_ON -> WARN_ON -> WARN_ON

Keep in mind that each additional level causes the reading of the logs
to be significantly more complex. Each level increases the output
exponentially:

   for every line1 in 1st_WARN_ON {
      for every line2 in 2nd_WARN_ON {
         for every line3 in 3rd_WARN_ON {
            print $line3
         }
         print $line2
      }
      print $line1
   }
   print $line0

IMHO 2 levels is enough because we should _never_ hit 2 levels of
recursion. If we do, the log output at that second level should be
enough to point to the bug. IMHO printing a third level just makes
things unnecessarily difficult to read. (My series uses 3 levels as a
compromise on my part. I would prefer reducing it to 2.)

> And maybe we could add some sort of message prefix for high levels of
> recursion nesting (levels 3+), so that things should not be normal
> will be on the radars and, possibly, will be reported.

I considered this, but am very hesitant to change the output
format. Also, the CUT_HERE usage (combined with PRINTK_CALLER) seem to
be enough.

John Ogness

  reply	other threads:[~2021-03-22 10:54 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-16 23:33 [PATCH next v1 0/3] printk: remove safe buffers John Ogness
2021-03-16 23:33 ` John Ogness
2021-03-16 23:33 ` John Ogness
2021-03-16 23:33 ` [PATCH next v1 1/3] printk: track/limit recursion John Ogness
2021-03-21  5:34   ` Sergey Senozhatsky
2021-03-22 10:53     ` John Ogness [this message]
2021-03-22 11:13       ` Sergey Senozhatsky
2021-03-22 15:07         ` Petr Mladek
2021-03-22 14:49   ` Petr Mladek
2021-03-23 21:32     ` John Ogness
2021-03-24  8:41       ` Petr Mladek
2021-03-16 23:33 ` [PATCH next v1 2/3] printk: remove safe buffers John Ogness
2021-03-16 23:33   ` John Ogness
2021-03-16 23:33   ` John Ogness
2021-03-21  5:26   ` Sergey Senozhatsky
2021-03-21  5:26     ` Sergey Senozhatsky
2021-03-21  5:26     ` Sergey Senozhatsky
2021-03-22 11:16     ` John Ogness
2021-03-22 11:16       ` John Ogness
2021-03-22 11:16       ` John Ogness
2021-03-22 18:02       ` Petr Mladek
2021-03-22 18:02         ` Petr Mladek
2021-03-22 18:02         ` Petr Mladek
2021-03-22 21:58         ` John Ogness
2021-03-22 21:58           ` John Ogness
2021-03-22 21:58           ` John Ogness
2021-03-23  9:46           ` Petr Mladek
2021-03-23  9:46             ` Petr Mladek
2021-03-23  9:46             ` Petr Mladek
2021-03-23 10:47   ` Petr Mladek
2021-03-23 10:47     ` Petr Mladek
2021-03-23 10:47     ` Petr Mladek
2021-03-26 11:12     ` John Ogness
2021-03-26 11:12       ` John Ogness
2021-03-26 11:12       ` John Ogness
2021-03-29 10:04       ` Petr Mladek
2021-03-29 10:04         ` Petr Mladek
2021-03-29 10:04         ` Petr Mladek
2021-03-29 15:10         ` John Ogness
2021-03-29 15:10           ` John Ogness
2021-03-29 15:10           ` John Ogness
2021-03-29 15:13           ` John Ogness
2021-03-29 15:13             ` John Ogness
2021-03-29 15:13             ` John Ogness
2021-03-16 23:33 ` [PATCH next v1 3/3] printk: convert @syslog_lock to spin_lock John Ogness
2021-03-23 12:01   ` Petr Mladek
2021-03-26 11:23     ` John Ogness

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=87mtuvmpcl.fsf@jogness.linutronix.de \
    --to=john.ogness@linutronix.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=senozhatsky@chromium.org \
    --cc=sergey.senozhatsky.work@gmail.com \
    --cc=sergey.senozhatsky@gmail.com \
    --cc=tglx@linutronix.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.