linux-kernel - Re: [PATCH][RFC] printk: make pr

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 24 Aug 2016 10:19:20 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
Cc:     Jan Kara <jack@...e.cz>, Kay Sievers <kay@...y.org>,
        Tejun Heo <tj@...nel.org>, Calvin Owens <calvinowens@...com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        linux-kernel@...r.kernel.org,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>
Subject: Re: [PATCH][RFC] printk: make pr_cont buffer per-cpu

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