linux-kernel - Re: [PATCH 1/2] kernel/watchdog: add /sys/kernel/{hard,soft}lockup

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: <CAKPOu+9-O7kfV4-qMLF0bDCzYc7O4AEB5Edf_4ehGd02CvHN=Q@mail.gmail.com>
Date: Fri, 2 May 2025 17:12:46 +0200
From: Max Kellermann <max.kellermann@...os.com>
To: Doug Anderson <dianders@...omium.org>
Cc: akpm@...ux-foundation.org, song@...nel.org, joel.granados@...nel.org, 
	cminyard@...sta.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] kernel/watchdog: add /sys/kernel/{hard,soft}lockup_count

On Fri, May 2, 2025 at 4:40 PM Doug Anderson <dianders@...omium.org> wrote:
> Hmmm. I suspect this has more to do with the fact that both hard and
> soft lockups nearly always end up being fatal. ...but I guess
> technically they could be recovered from and the kernel can be
> configured not to panic, so I guess it would be OK to add something
> like this.

This isn't really about recovering from such a lockup but about
detecting it easily+quickly with monitoring tools. A machine that had
a soft lockup still works (for some definition of "works"), and you
cannot detect this condition without parsing dmesg.

softlockup_panic is a bigger hammer with pros and cons of its own.

> It feels like there would be a better place for these to go than
> straight in `/sys/kernel`, though I don't really know it. Maybe
> someone else on this thread has opinions? Any chance they could go in
> "debugfs"?

"oops_count" and "warn_count" in the same directory is prior art.
 I don't think this belongs in debugfs, as debugfs is not considered a
stable ABI, and this feature really isn't about debugging but about
measuring server health.

> Please no embedded ifdefs like this. I personally wouldn't hate it if
> the "unsigned int" was simply always defined, but if we want to keep
> it only defined for sysfs then please use a function to increment this
> that's declared as a static inline noop in the case that sysfs is off.

That would just move the ifdef somewhere else with more code lines,
for something as trivial as incrementing an integer. It's a question
of taste, and I'll follow whatever coding style the maintainers
prefer.