linux-kernel - Re: [PATCH printk v2 09/12] printk: add functions to prefer direct printing

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <YlQkC0jddnjb8Mbr@alley>
Date:   Mon, 11 Apr 2022 14:50:19 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     Helge Deller <deller@....de>
Cc:     John Ogness <john.ogness@...utronix.de>,
        Sergey Senozhatsky <senozhatsky@...omium.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        linux-kernel@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jirislaby@...nel.org>,
        "Paul E. McKenney" <paulmck@...nel.org>,
        Frederic Weisbecker <frederic@...nel.org>,
        Neeraj Upadhyay <quic_neeraju@...cinc.com>,
        Josh Triplett <josh@...htriplett.org>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        Lai Jiangshan <jiangshanlai@...il.com>,
        Joel Fernandes <joel@...lfernandes.org>,
        Kees Cook <keescook@...omium.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Luis Chamberlain <mcgrof@...nel.org>,
        Wei Liu <wei.liu@...nel.org>,
        Xiaoming Ni <nixiaoming@...wei.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Marco Elver <elver@...gle.com>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        Shawn Guo <shawn.guo@...aro.org>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Matti Vaittinen <matti.vaittinen@...rohmeurope.com>,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        Phil Auld <pauld@...hat.com>,
        Juri Lelli <juri.lelli@...hat.com>,
        Wang Qing <wangqing@...o.com>, rcu@...r.kernel.org
Subject: Re: [PATCH printk v2 09/12] printk: add functions to prefer direct
 printing

On Thu 2022-04-07 22:20:30, Helge Deller wrote:
> On 4/7/22 22:04, John Ogness wrote:
> > On 2022-04-07, Helge Deller <deller@....de> wrote:
> >> In my case - while I debug low-level kernel code - I then just need to
> >> use pr_warn() or pr_emerg() and get it printed non-threadened. That's
> >> sufficient for me.
> >
> > Actually, no. The loglevel does not determine if a message is direct
> > printed or not. By "warn" I was referring to WARN_ON(condition).
> 
> Oh, then there was a misunderstanding on my side.
> 
> > If you are debugging low-level kernel code, you usually will _want_
> > threaded printing. The timestamps match the printk() call, so you will
> > get accurate logs. And the runtime performance of your low-level kernel
> > code will not be significantly affected by the printk() call.
> 
> That really much depends on what you debug.
> Currently I'm debugging some CPU hotplug stuff, and I really want my
> debug info printed immediately, otherwise it's too late to analyze what's
> going wrong. I can imaginge other use cases like trying to find memory
> leaks are similiar.

So, it is not about severity of the messages but about the context,
in this case the suspend. We try to address this. 10th patch from
this patchset adds:

static inline bool allow_direct_printing(void)
{
	return (!printk_kthreads_available ||
		system_state > SYSTEM_RUNNING ||
		oops_in_progress ||
		atomic_read(&printk_prefer_direct));
}

It means that printk() inside the SYSTEM_SUSPEND state will
automatically try to handle the console directly.

I intentionally mention "try" because printk() uses console_trylock().
It fails when anyone else already does the printing, including
a kthread. trylock() is needed because printk() must be usable also
in atomic context. It has worked this way for more than two decades.
It came with SMP support.


> > If for some reason you really want non-threaded printing, the patch we
> > are discussing creates new functions to manually activate it:
> >
> > printk_prefer_direct_enter();
> > pr_info("debugging\n");
> > printk_prefer_direct_exit();
> 
> That's quite unhandy. At least I would have thought that pr_emerg() would
> do that call sequence.
> Wouldn't it make sense to make pr_emerg() work unthreadened, as it's
> used quite seldom and only in special situations...

It is true that pr_emerg() is relatively special.

But I would really like to avoid a situation where developers
use non-appropriate printk level just to get the message
directly.

Instead, we should allow using the direct context in some
situation easily. We could also add a command line parameter
to disable the kthreads completely.

Best Regards,
Petr