Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 13 Jul 2023 17:53:09 +0200
From:   Petr Mladek <pmladek@suse.com>
To:     John Ogness <john.ogness@linutronix.de>
Cc:     Sergey Senozhatsky <senozhatsky@chromium.org>,
        Steven Rostedt <rostedt@goodmis.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        linux-kernel@vger.kernel.org,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: [PATCH printk v2 4/5] printk: Add per-console suspended state
Message-ID: <ZLAd5cRNmOv-_1vW@alley>
References: <20230710134524.25232-1-john.ogness@linutronix.de>
 <20230710134524.25232-5-john.ogness@linutronix.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230710134524.25232-5-john.ogness@linutronix.de>
Precedence: bulk

On Mon 2023-07-10 15:51:23, John Ogness wrote:
> Currently the global @console_suspended is used to determine if
> consoles are in a suspended state. Its primary purpose is to allow
> usage of the console_lock when suspended without causing console
> printing. It is synchronized by the console_lock.
> 
> Rather than relying on the console_lock to determine suspended
> state, make it an official per-console state that is set within
> console->flags. This allows the state to be queried via SRCU.
> 
> Remove @console_suspended. Console printing will still be avoided
> when suspended because console_is_usable() returns false when
> the new suspended flag is set for that console.
> 
> Signed-off-by: John Ogness <john.ogness@linutronix.de>

Looks good to me:

Reviewed-by: Petr Mladek <pmladek@suse.com>

I have double checked the history. suspend_console() was added
into v2.6.18-rc1 by the commit ("Add support for suspending and
resuming the whole console subsystem").

The above commit added "secondary_console_sem". It was taken
by acquire_console_sem() instead of the normal "console_sem"
when "console_suspended" was set. It means that the normal
"console_sem" really was not taken.

The "secondary_console_sem" was removed in v2.6.29-rc6 by the commit
("PM: Fix suspend_console and resume_console to use only one
semaphore"). It solved races between code taking "console_sem"
and code "secondary_console_sem". This commit kept the handling of
"console_locked". It was not set when console_suspended was set
even though "console_sem" was actually taken.

IMHO, it was a bug in the commit removing "secondary_console_sem".
But it probably never caused any issues.

Anyway, this patch makes "console_locked" handling sane. And if some
tty code relies on the insane logic then it should get fixed.

Best Regards,
Petr