linux-kernel - Re: [PATCH] printk: stop spining waiter when console resume to flush prb

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <YJj9JdhgL88ivHVy@alley>
Date:   Mon, 10 May 2021 11:30:13 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     luojiaxing <luojiaxing@...wei.com>
Cc:     sergey.senozhatsky@...il.com, rostedt@...dmis.org,
        john.ogness@...utronix.de, linux-kernel@...r.kernel.org,
        Sergey Senozhatsky <senozhatsky@...omium.org>,
        linuxarm@...wei.com, bobo.shaobowang@...wei.com
Subject: Re: [PATCH] printk: stop spining waiter when console resume to flush
 prb

On Mon 2021-05-10 15:41:31, luojiaxing wrote:
> 
> On 2021/5/6 21:39, Petr Mladek wrote:
> Hi, Petr, I test your patch and I think it needs to make some modifications
> to fix the problem.
> 
> 
> My test method is as follows:
> Kernel thread A causes the console to enter suspend and then resume it 10
> seconds later.
> Kernel thread B repeatedly invokes dev_info() for 15 seconds after the
> console suspend.

Could you please provide the test code?

If kthread B starts invoking dev_info() after console_resume() then it
has nothing to do with suspend/resume. It can happen anytime that a
process starts a flood of printk() calls.

Also, do you see this problem in the real life, please?
What motivated you to investigate this scenario, please?

> > >From 574e844f512c9f450e64832f09cc389bc9915f83 Mon Sep 17 00:00:00 2001
> > From: Petr Mladek <pmladek@...e.com>
> > Date: Thu, 6 May 2021 12:40:56 +0200
> > Subject: [PATCH] printk: Prevent softlockup when resuming console
> > 
> > Many printk messages might get accumulated when consoles were suspended.
> > They are proceed when console_unlock() is called in resume_console().
> > 
> > --- a/kernel/printk/printk.c
> > +++ b/kernel/printk/printk.c
> > @@ -2637,13 +2636,15 @@ void console_unlock(void)
> >   		 * finish. This task can not be preempted if there is a
> >   		 * waiter waiting to take over.
> >   		 */
> > -		console_lock_spinning_enable();
> > +		if (spinning_enabled)
> > +			console_lock_spinning_enable();
> 
> 
> change to
> 
> 
> +               if (!spinning_enabled) {
> +                       raw_spin_lock(&console_owner_lock);
> +                       WRITE_ONCE(console_waiter, true);
> +                       raw_spin_unlock(&console_owner_lock);
> +               }
> 

IMHO, both variants have the same result and behavior:

console_trylock_spinning() has the following condition:

	if (!waiter && owner && owner != current) {
		WRITE_ONCE(console_waiter, true);
		spin = true;
	}

My variant causes that @owner will stay "NULL".
Your variant causes that @waiter will be "true"

In both cases, the condition fails and spin will stay "false"
which means that any parallel printk() will not spin.

Best Regards,
Petr