public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Kay Sievers <kay@vrfy.org>
To: Ingo Molnar <mingo@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"kay.sievers" <kay.sievers@vrfy.org>,
	Fengguang Wu <fengguang.wu@intel.com>,
	Ingo Molnar <mingo@elte.hu>
Subject: Re: [PATCH] printk: Add printk_flush() to force buffered text to console
Date: Sat, 23 Jun 2012 13:47:32 +0200	[thread overview]
Message-ID: <1340452052.1784.40.camel@mop> (raw)
In-Reply-To: <20120623061313.GA21895@gmail.com>

On Sat, 2012-06-23 at 08:13 +0200, Ingo Molnar wrote:
> * Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > On Thu, 21 Jun 2012 19:52:03 -0400
> > Steven Rostedt <rostedt@goodmis.org> wrote:
> > 
> > > But with the new printk() changes, text without a newline 
> > > gets buffered and does not print out to the console at the 
> > > location of the printk.
> > 
> > uh, how about we fix that?  The old behaviour was good, the 
> > new behaviour is noxious.
> 
> Absolutely.
> 
> pr_flush() really seems to be a workaround.
> 
> > Please idenfity these "new printk() changes".  Was the new 
> > noxiousness an unavoidable effect of them?
> 
> Fundamentally:
> 
> e2ae715d66bf kmsg - kmsg_dump() use iterator to receive log buffer content
> c313af145b9b printk() - isolate KERN_CONT users from ordinary complete lines
> 3ce9a7c0ac28 printk() - restore prefix/timestamp printing for multi-newline strings
> 649e6ee33f73 printk() - restore timestamp printing at console output
> 5c5d5ca51abd printk() - do not merge continuation lines of different threads
> 7f3a781d6fd8 printk - fix compilation for CONFIG_PRINTK=n
> 5fc3249068c1 kmsg: use do_div() to divide 64bit integer
> e11fea92e13f kmsg: export printk records to the /dev/kmsg interface
> 7ff9554bb578 printk: convert byte-buffer to variable-length record buffer
> 
> Should we revert them or can they be fixed sanely? Kay seems to 
> be busy with other things so I guess a revert is the best we can 
> do. Greg, Kay?

I just don't have a better idea than Joe or Steven.

The conversion from the unreliable byte-buffer to records changes the
behavior a little in some areas. We are atomic at the line level now and
not at an sometimes unpredictable byte stream position which can just be
moved forward by adding another few bytes.

The usual continuation line user should all be buffered to make things
generally more reliable and less racy. And we really had these problems
in the past, and substantially improved the common use-cases now.

But yes, it seems to come into our way with the current output format of
the kernel self-tests, and we need to do something about it. But I don't
think it should be going back to the much-too-simple and unreliable
byte-stream buffer.

We need make some sort of a compromise for the self-tests here, I guess.
I think the trade of using one of these options for the self-tests to
cope with the new model is justified:
- flush explicitly when it is needed, by a global flag or with a
  message prefix
- printing a second full line that says: test foo OK, instead
  of appending the OK to the former line
- printing one full line, and suppress the later OK entirely. The OK
  might not be needed at all; we will find out that the machine
  has not crashed at that point and a message in case of an error
  could be sufficient in most cases.

Reverting the changes now would remove:
- proper device identifiers attached to a message, which allows
  tools for the first time to know the contexts of the kernel messages
- 100% message integrity of structured messages, and single line
  prints, and if only one continuation line user prints at a time.
  We never mix continuation users with full line users, like the
  old buffer did.
- very reliable continuation line prints, because of the buffering.
  The only race that can still happen is several cont users racing
  against each other, which is not too likely
- 100% granularity at line level during userspace buffer access,
  crash dumping; we never copy around half or overwritten lines,
  like the old buffer
- the ring buffer prunes only entire messages not just the half
  of a line like the old byte buffer
- sequence numbers allow us to track the state of the read and write
  position in the buffer, so we never copy in-the-meantime already
  overwritten messages around
- sequence numbers which allow userspace to reconnect to the old reading
  position and process only new messages, and find if messages got
  overwritten
- support for multiple concurrent readers with live update of the kernel
  log stream

This seems like a trade of a rather cosmetic output-format issue for
kernel self-tests, against general reliability, integrity and usefulness
of the rest of the kernel message users.

The record buffer is a huge step forward for the system logging, and it
allows us for the first time to make the kernel log useful to consumers
other than humans, and is key part of providing important information to
the system management.

An explicit flush for the self-test like users is unfortunate, and
inconvenient, and surely not nice, but I think it's still the better
overall deal.

Thanks,
Kay


  parent reply	other threads:[~2012-06-23 11:47 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-21 23:52 [PATCH] printk: Add printk_flush() to force buffered text to console Steven Rostedt
2012-06-22  7:17 ` Ingo Molnar
2012-06-22 10:45   ` Steven Rostedt
2012-06-22  8:24 ` Joe Perches
2012-06-22 10:48   ` Steven Rostedt
2012-06-22 21:54 ` Andrew Morton
2012-06-22 23:41   ` Steven Rostedt
2012-06-22 23:49     ` Andrew Morton
2012-06-22 23:56       ` Steven Rostedt
2012-06-23  6:13   ` Ingo Molnar
2012-06-23  7:44     ` Joe Perches
2012-06-25  8:45       ` Ingo Molnar
2012-06-25 16:53         ` Joe Perches
2012-06-23 11:47     ` Kay Sievers [this message]
2012-06-23 12:04       ` Fengguang Wu
2012-06-23 15:28       ` Joe Perches
2012-06-23 16:56         ` Kay Sievers
2012-06-25  9:09       ` [PATCH] printk: Revert the buffered-printk() changes for now Ingo Molnar
2012-06-25 10:06         ` Kay Sievers
2012-06-25 13:42           ` Steven Rostedt
2012-06-25 14:07           ` Ingo Molnar
2012-06-25 14:48             ` Steven Rostedt
2012-06-25 16:40               ` Greg Kroah-Hartman
2012-06-27  5:52                 ` Ingo Molnar
2012-06-27  5:59                   ` Joe Perches
2012-07-06 11:04                     ` Ingo Molnar
  -- strict thread matches above, loose matches on Subject: below --
2012-06-14  4:46 [PATCH] printk: Add printk_flush() to force buffered text to console Steven Rostedt
2012-06-14  4:59 ` Greg Kroah-Hartman
2012-06-14 11:01   ` Steven Rostedt
2012-06-14 15:41     ` Greg Kroah-Hartman
2012-06-14 17:07       ` Steven Rostedt
2012-06-15  4:22         ` Fengguang Wu
2012-06-15  4:30           ` Greg Kroah-Hartman
2012-06-15  4:37             ` Fengguang Wu
2012-06-15 12:04             ` Ingo Molnar
2012-06-15 23:13               ` Greg Kroah-Hartman
2012-06-15 23:53                 ` Steven Rostedt
2012-06-18 23:03                   ` Greg Kroah-Hartman
2012-06-19  1:28                     ` Steven Rostedt
2012-06-20 12:25                       ` Ingo Molnar
2012-06-21 17:13                         ` Greg Kroah-Hartman
2012-06-21 17:41                           ` Steven Rostedt
2012-06-21 18:17                             ` Joe Perches
2012-06-21 18:22                               ` Joe Perches
2012-06-21 18:29                               ` Steven Rostedt
2012-06-21 18:39                                 ` Joe Perches
2012-06-21 18:49                                   ` Steven Rostedt
2012-06-21 18:55                                     ` Joe Perches
2012-06-21 19:38                                       ` Steven Rostedt
2012-06-16  6:59                 ` Ingo Molnar
2012-06-16 12:51                   ` Joe Perches
2012-06-16 15:38                     ` Greg Kroah-Hartman
2012-06-16 15:40                   ` Greg Kroah-Hartman

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=1340452052.1784.40.camel@mop \
    --to=kay@vrfy.org \
    --cc=akpm@linux-foundation.org \
    --cc=fengguang.wu@intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=kay.sievers@vrfy.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=mingo@kernel.org \
    --cc=rostedt@goodmis.org \
    --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