linux-kernel - Re: [PATCH rcu 11/12] torture: Flush printk() buffers before powering off

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: <20220621210103.GO1790663@paulmck-ThinkPad-P17-Gen-1>
Date:   Tue, 21 Jun 2022 14:01:03 -0700
From:   "Paul E. McKenney" <paulmck@...nel.org>
To:     John Ogness <john.ogness@...utronix.de>
Cc:     rcu@...r.kernel.org, linux-kernel@...r.kernel.org,
        kernel-team@...com, rostedt@...dmis.org
Subject: Re: [PATCH rcu 11/12] torture: Flush printk() buffers before
 powering off

On Tue, Jun 21, 2022 at 10:58:27PM +0206, John Ogness wrote:
> On 2022-06-21, "Paul E. McKenney" <paulmck@...nel.org> wrote:
> > The patch below will cause rcutorture to implicitly test this
> > functionality, unless told otherwise, for example, by using the
> > --bootargs "torture.printk_shutdown_bug_workaround" kvm.sh
> > argument.
> >
> > Thoughts?
> 
> I feel like this is dirtying the torture.* bootarg namespace a
> bit. Also, I am not sure how useful it is as a dynamic option. I assume
> that users would generally avoid using it, so its very existence might
> just be more noise in the documentation and code. It is an unusual
> feature:
> 
> "In case some bug shows up, here is a flag to avoid it."
> 
> I personally would just drop the patch and rely on a correctly
> functional kernel. But I am also not an rcutorture user. If _you_ think
> that such a flag is useful, feel free to include the patch.

Fair points!

The main value to me is to avoid me having to pile through code to relearn
the incantation pr_flush(1000, true).  Which, now that you mention it,
could be dealt with by adding a comment:

	// pr_flush(1000, true); // If needed to flush printk() buffers.

This definitely will not go into the upcoming merge window, so there is
plenty of time to give it some thought.

> > commit 204bf1e2a5a2fb68c15b4b64793ad0896db6f705
> > Author: Paul E. McKenney <paulmck@...nel.org>
> > Date:   Tue Jun 21 11:02:25 2022 -0700
> >
> >     torture: Optionally flush printk() buffers before powering off
> >     
> >     The rcutorture test suite produces quite a bit of console output at
> >     the end of a test.  This means that the new-in-2022 printk() kthreads
> >     are likely to be in the process of flushing output at the time of the
> >     torture_shutdown() function's call to kernel_power_off().  Normally,
> >     rcutorture relies on printk() to flush any pending output upon shutdown,
> >     the better to detect bugs in this area, for example, the one introduced
> >     by 8e274732115f ("printk: extend console_lock for per-console locking").
> >     However, once such a bug is detected and reported, it is necessary to
> >     test the rest of the system, without noise from the already-reported bug.
> >     
> >     This commit therefore adds a torture.printk_shutdown_bug_workaround
> >     kernel parameter, which causes torture_shutdown() to invoke pr_flush(),
> >     and print an informative message on the console, immediately before
> >     invoking kernel_power_off().  When this kernel parameter is not specified,
> >     it is up to printk() to flush its own buffers.
> >     
> >     Suggested-by: John Ogness <john.ogness@...utronix.de>
> >     Signed-off-by: Paul E. McKenney <paulmck@...nel.org>
> 
> Reviewed-by: John Ogness <john.ogness@...utronix.de>

Thank you!

							Thanx, Paul