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:   Tue, 18 Jul 2023 12:39:33 +0900
From:   Sergey Senozhatsky <senozhatsky@chromium.org>
To:     John Ogness <john.ogness@linutronix.de>
Cc:     Petr Mladek <pmladek@suse.com>,
        Sergey Senozhatsky <senozhatsky@chromium.org>,
        Steven Rostedt <rostedt@goodmis.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH printk v3 2/7] printk: Reduce console_unblank() usage in
 unsafe scenarios
Message-ID: <20230718033933.GC955071@google.com>
References: <20230717194607.145135-1-john.ogness@linutronix.de>
 <20230717194607.145135-3-john.ogness@linutronix.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230717194607.145135-3-john.ogness@linutronix.de>
Precedence: bulk

On (23/07/17 21:52), John Ogness wrote:
> 
> A semaphore is not NMI-safe, even when using down_trylock(). Both
> down_trylock() and up() are using internal spinlocks and up()
> might even call wake_up_process().
> 
> In the panic() code path it gets even worse because the internal
> spinlocks of the semaphore may have been taken by a CPU that has
> been stopped.
> 
> To reduce the risk of deadlocks caused by the console semaphore in
> the panic path, make the following changes:
> 
> - First check if any consoles have implemented the unblank()
>   callback. If not, then there is no reason to take the console
>   semaphore anyway. (This check is also useful for the non-panic
>   path since the locking/unlocking of the console lock can be
>   quite expensive due to console printing.)
> 
> - If the panic path is in NMI context, bail out without attempting
>   to take the console semaphore or calling any unblank() callbacks.
>   Bailing out is acceptable because console_unblank() would already
>   bail out if the console semaphore is contended. The alternative of
>   ignoring the console semaphore and calling the unblank() callbacks
>   anyway is a bad idea because these callbacks are also not NMI-safe.
> 
> If consoles with unblank() callbacks exist and console_unblank() is
> called from a non-NMI panic context, it will still attempt a
> down_trylock(). This could still result in a deadlock if one of the
> stopped CPUs is holding the semaphore internal spinlock. But this
> is a risk that the kernel has been (and continues to be) willing
> to take.
> 
> Signed-off-by: John Ogness <john.ogness@linutronix.de>

Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>