linux-kernel - Re: [PATCH 3/4] watchdog/hardlockup: Use printk_cpu_sync_get

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: <87h6kak1ye.fsf@jogness.linutronix.de>
Date: Fri, 22 Dec 2023 10:48:49 +0106
From: John Ogness <john.ogness@...utronix.de>
To: Douglas Anderson <dianders@...omium.org>, Andrew Morton
 <akpm@...ux-foundation.org>
Cc: Petr Mladek <pmladek@...e.com>, Li Zhe <lizhe.67@...edance.com>, Pingfan
 Liu <kernelfans@...il.com>, Lecopzer Chen <lecopzer.chen@...iatek.com>,
 Douglas Anderson <dianders@...omium.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/4] watchdog/hardlockup: Use
 printk_cpu_sync_get_irqsave() to serialize reporting

On 2023-12-20, Douglas Anderson <dianders@...omium.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@...omium.org>

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