Re: [PATCH v4 07/17] watchdog/hardlockup: Move perf hardlockup checking/panic to common watchdog.c

On Fri 2023-05-05 09:37:50, Doug Anderson wrote:

Hi,

On Thu, May 4, 2023 at 7:58 PM Nicholas Piggin [off-list ref] wrote:

quoted

On Fri May 5, 2023 at 8:13 AM AEST, Douglas Anderson wrote:

quoted

The perf hardlockup detector works by looking at interrupt counts and
seeing if they change from run to run. The interrupt counts are
managed by the common watchdog code via its watchdog_timer_fn().

Currently the API between the perf detector and the common code is a
function: is_hardlockup(). When the hard lockup detector sees that
function return true then it handles printing out debug info and
inducing a panic if necessary.

Let's change the API a little bit in preparation for the buddy
hardlockup detector. The buddy hardlockup detector wants to print

I think the name change is a gratuitous. Especially since it's now
static.

watchdog_hardlockup_ is a pretty long prefix too, hardlockup_
should be enough?

Seems okay otherwise though.

I went back and forth on names far too much when constructing this
patch series. Mostly I was trying to balance what looked good to me
and what Petr suggested [1]. I'm not super picky about the names and
I'm happy to change them all to a "hardlockup_" prefix. I'd love to
hear Petr's opinion.

Sigh, the original code was a real mess of naming schemes. It is hard
to say how to move forward.

My opinion:

  + watchdog_hardlockup_check(): looks fine. It is consistent with
	watchdog_hardlockup_enable()/disable().

  + watchdog_hardlockup_is_lockedup() is really overly complicated.
	I would personally keep is_hardlockup(). It is static.
	And it will be consitent with is_softlockup().

  + watchdog_hardlockup_interrupt_count() looks better then
	the original. It clearly shows that it makes sense only
	for the hardlockup detector ("bug" fixed by this patch).

	Well, I would personally call it watchdog_hardlockup_kick()
	and remove the comment.


That said, I could live with this patch. It is better than the
original.

Best Regards,
Petr