linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Petr Mladek <pmladek@suse.com>
To: David Laight <David.Laight@aculab.com>
Cc: John Ogness <john.ogness@linutronix.de>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [RFC PATCH 1/5] printk: implement pr_cont_t
Date: Tue, 25 Aug 2020 15:10:41 +0200	[thread overview]
Message-ID: <20200825131041.GV4353@alley> (raw)
In-Reply-To: <fb47baa77ff940e99224feac85a2f2d7@AcuMS.aculab.com>

On Thu 2020-08-20 12:33:23, David Laight wrote:
> From: Petr Mladek
> > Sent: 20 August 2020 11:16
> ...
> > Now that I think about it. This is the biggest problem with any temporary buffer
> > for pr_cont() lines. I am more and more convinced that we should just
> > _keep the current behavior_. It is not ideal. But sometimes mixed
> > messages are always better than lost ones.
> 
> Maybe a marker to say 'more expected' might be useful.
> OTOH lack of a trailing '\n' probably signifies that a
> pr_cont() is likely to be next.

The problem is the "probably". Lack of trailing '\n' might also mean
that the author did not care. Note that newline is not strictly
required at the moment. The next message is concatenated only when
pr_cont() is used from the same process.

I would personally hate printk when I debugged some hard-to-reproduce
bug, finally succeeded, and some message was missing just because
of a missing newline.


> Unexpected pr_cont() could be output with a leading "... "
> to help indicate the message is a continuation.

Interesting idea. It might help to catch broken code. Well, I am still
not sure that people would appreciate this printk() behavior.


> > That said, some printk() API using local buffer would be still
> > valuable for pieces of code where people really want to avoid
> > mixed lines. But it should be optional and people should be
> > aware of the risks.
> 
> That could be very useful if it supported multi-line output.
> Thing like the stack backtrace code could use it avoid
> the mess that happens when multiple processes generate
> tracebacks at the same time.

Honestly, I am going to push against it. I agree that would be useful
but it is an evil path.

There has to be some limits. Backtraces are often printed in situation
where the buffer could not get allocated dynamically. There always
be a situation when a buffer will not be big enough.

If we add this feature, people will want to use it also for other
purposes that might need even bigger buffers and the size might
be even harder to predict.

And the temporary buffer is only part of the problem. The message also
has to be stored into the lockless ringbuffer, show on consoles, passed
to userspace, stored during panic by pstore.

The printk() design already is very complicated. And multiple lines
support would stretch it in yet another dimension.

And it is going to be less important. We are going to always store the
information about the printk caller. So that it will be much easier
to grep related lines.

Best Regards,
Petr

  reply	other threads:[~2020-08-25 13:10 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-19 23:26 [RFC PATCH 0/5] printk: new log_cont implementation John Ogness
2020-08-19 23:26 ` [RFC PATCH 1/5] printk: implement pr_cont_t John Ogness
2020-08-20  0:33   ` Joe Perches
2020-08-20  7:44     ` David Laight
2020-08-20  7:59       ` Joe Perches
2020-08-20  8:49         ` David Laight
2020-08-20  9:18           ` John Ogness
2020-08-20 16:03       ` Joe Perches
2020-08-20 10:16   ` Petr Mladek
2020-08-20 12:33     ` David Laight
2020-08-25 13:10       ` Petr Mladek [this message]
2020-08-25 13:38         ` David Laight
2020-08-25 15:32           ` Petr Mladek
2020-08-24  2:08     ` Sergey Senozhatsky
2020-08-24  5:37   ` Sergey Senozhatsky
2020-08-19 23:26 ` [RFC PATCH 2/5] sysrq: use pr_cont_t for cont messages John Ogness
2020-08-20  1:03   ` Linus Torvalds
2020-08-20 17:48     ` Joe Perches
2020-08-20 17:53       ` Linus Torvalds
2020-08-20 22:11       ` John Ogness
2020-08-20 22:36         ` Joe Perches
2020-08-19 23:26 ` [RFC PATCH 3/5] workqueue: " John Ogness
2020-08-19 23:26 ` [RFC PATCH 4/5] locking/selftest: " John Ogness
2020-08-19 23:26 ` [RFC PATCH 5/5] lockdep: " 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=20200825131041.GV4353@alley \
    --to=pmladek@suse.com \
    --cc=David.Laight@aculab.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=john.ogness@linutronix.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=sergey.senozhatsky.work@gmail.com \
    --cc=sergey.senozhatsky@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).