Received-SPF: pass (google.com: domain of linux-kernel+bounces-9556-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
From: John Ogness <john.ogness@linutronix.de>
To: Douglas Anderson <dianders@chromium.org>, Andrew Morton
 <akpm@linux-foundation.org>
Cc: Petr Mladek <pmladek@suse.com>, Li Zhe <lizhe.67@bytedance.com>, Pingfan
 Liu <kernelfans@gmail.com>, Lecopzer Chen <lecopzer.chen@mediatek.com>,
 Douglas Anderson <dianders@chromium.org>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/4] watchdog/hardlockup: Use
 printk_cpu_sync_get_irqsave() to serialize reporting
In-Reply-To: <20231220131534.3.I6ff691b3b40f0379bc860f80c6e729a0485b5247@changeid>
References: <20231220211640.2023645-1-dianders@chromium.org>
 <20231220131534.3.I6ff691b3b40f0379bc860f80c6e729a0485b5247@changeid>
Date: Fri, 22 Dec 2023 10:48:49 +0106
Message-ID: <87h6kak1ye.fsf@jogness.linutronix.de>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain

On 2023-12-20, Douglas Anderson <dianders@chromium.org> wrote:
> The interleaving problem was less bad with the "buddy" hardlockup
> detector. With "buddy" we always end up calling
> `trigger_single_cpu_backtrace(cpu)` on some CPU other than the running
> one. trigger_single_cpu_backtrace() always at least serializes the
> individual stack crawls because it eventually uses
> printk_cpu_sync_get_irqsave(). Unfortunately the fact that
> trigger_single_cpu_backtrace() eventually calls
> printk_cpu_sync_get_irqsave() (on a different CPU) means that we have
> to drop the "lock" before calling it and we can't fully serialize all
> printouts associated with a given hardlockup.

I think that is good enough. Otherwise there would need to be some kind
of CPU handshaking to ensure things are synchronized correctly in case
multiple CPUs have triggered the situation.

> However, we still do get
> the advantage of serializing the output of print_modules() and
> print_irqtrace_events().
>
> Aside from serializing hardlockups from each other, this change also
> has the advantage of serializing hardlockups and softlockups from each
> other if they happen to happen at the same time since they are both
> using the same "lock".
>
> Even though nobody is expected to hang while holding the lock
> associated with printk_cpu_sync_get_irqsave(), out of an abundance of
> caution, we don't call printk_cpu_sync_get_irqsave() until after we
> print out about the hardlockup. This makes extra sure that, even if
> printk_cpu_sync_get_irqsave() somehow never runs we at least print
> that we saw the hardlockup.

I agree with calling printk() before trying to acquire ownership of the
cpu_sync.

> This is different than the choice made for
> softlockup because hardlockup is really our last resort.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>

Reviewed-by: John Ogness <john.ogness@linutronix.de>