All of lore.kernel.org
 help / color / mirror / Atom feed
From: Petr Mladek <pmladek@suse.com>
To: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Cc: Jan Kara <jack@suse.cz>, Kay Sievers <kay@vrfy.org>,
	Tejun Heo <tj@kernel.org>, Calvin Owens <calvinowens@fb.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Subject: Re: [PATCH][RFC] printk: make pr_cont buffer per-cpu
Date: Wed, 24 Aug 2016 10:19:20 +0200	[thread overview]
Message-ID: <20160824081919.GE2504@pathway.suse.cz> (raw)
In-Reply-To: <20160824011420.GA452@swordfish>

On Wed 2016-08-24 10:14:20, Sergey Senozhatsky wrote:
> Hello,
> 
> On (08/23/16 13:47), Petr Mladek wrote:
> [..]
> > >  	if (!(lflags & LOG_NEWLINE)) {
> > > +		if (!this_cpu_read(cont_printing)) {
> > > +			if (system_state == SYSTEM_RUNNING) {
> > > +				this_cpu_write(cont_printing, true);
> > > +				preempt_disable();
> > > +			}
> > > +		}
> > 
> > I am afraid that this is not acceptable. It means that printk() will have
> > an unexpected side effect. The missing "\n" at the end of a printed
> > string would disable preemption. See below for more.
> 
> missing '\n' must WARN about "sched while atomic" eventually, so it
> shouldn't go unnoticed or stay hidden.

Well, it will still force people to rebuilt a test kernel because they
forget to use '\n" and the test kernel is unusable.

IMHO, the connection between '\n' and preemption is not
intuitive and hard to spot. We should do our best to avoid it.


> > I think that cont lines should be a corner case. There should be only
> > a limited use of them. We should not make too complicated things to
> > support them. Also printk() must not get harder to use because of them.
> > I still see a messed output rather as a cosmetic problem in compare with
> > possible possible deadlocks or hung tasks.
> 
> oh, I would love it if pr_cont() was never used in SMP. but this is not
> the case, unfortunately. and, ironically, where pr_cont really matters
> is debugging -- for instance, look at arch/x86/kernel/dumpstack_{32,64}.c
> show_regs() or show_stack_log_lvl()

Sure but how big is the problem in the daily life? I have never heard
colleagues complaining about messed cont lines. It is rare and it
is always possible to restore it. Checking BUG/WARN messages is
a rather hard detective work anyway.

The most painful situation would be if backtraces from different
CPUs are mixed. But there will be problem even with mixed lines.
Fortunately, this usually happens when printing backtraces from
all CPUs in NMI and the output is serialized.


> well, I do understand what you mean and agree with it, but I'm
> afraid pr_cont() kinda matters after all and people *probably*
> expect it to be SMP safe (I'm not entirely sure whether all of
> those pr_cont() calls were put there with the idea that the
> output can be messed up and quite hard to read).

This was even worse before the cont lines buffer.

Sigh, I do not feel experienced enough to decide about this.
I wonder if this is rather theoretical problem or if there
are many real complains about it.

I feel that we might be trapped by a perfectionism. Perfect output
would be great. But it must not make printk() hard to use
in the daily life.

Best Regards,
Petr

  reply	other threads:[~2016-08-24  8:33 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-22 15:40 [PATCH][RFC] printk: make pr_cont buffer per-cpu Sergey Senozhatsky
2016-08-22 16:10 ` Joe Perches
2016-08-23  1:10   ` Sergey Senozhatsky
2016-08-23  5:18 ` Sergey Senozhatsky
2016-08-23 11:47   ` Petr Mladek
2016-08-24  1:14     ` Sergey Senozhatsky
2016-08-24  8:19       ` Petr Mladek [this message]
2016-08-24 14:27         ` Sergey Senozhatsky
2016-08-25 21:27           ` Petr Mladek
2016-08-25 21:33             ` Petr Mladek

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=20160824081919.GE2504@pathway.suse.cz \
    --to=pmladek@suse.com \
    --cc=akpm@linux-foundation.org \
    --cc=calvinowens@fb.com \
    --cc=jack@suse.cz \
    --cc=kay@vrfy.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sergey.senozhatsky.work@gmail.com \
    --cc=sergey.senozhatsky@gmail.com \
    --cc=tj@kernel.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 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.