Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 10 May 2021 11:30:13 +0200
From:   Petr Mladek <pmladek@suse.com>
To:     luojiaxing <luojiaxing@huawei.com>
Cc:     sergey.senozhatsky@gmail.com, rostedt@goodmis.org,
        john.ogness@linutronix.de, linux-kernel@vger.kernel.org,
        Sergey Senozhatsky <senozhatsky@chromium.org>,
        linuxarm@huawei.com, bobo.shaobowang@huawei.com
Subject: Re: [PATCH] printk: stop spining waiter when console resume to flush
 prb
Message-ID: <YJj9JdhgL88ivHVy@alley>
References: <1620288026-5373-1-git-send-email-luojiaxing@huawei.com>
 <YJPxj83F1sBjHHAE@alley>
 <f38cd7b9-22a4-b65d-fd37-2d95fe95fc00@huawei.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <f38cd7b9-22a4-b65d-fd37-2d95fe95fc00@huawei.com>
Precedence: bulk

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@suse.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