Re: [PATCH 2/2] workqueue: implement lockup detector

[Tejun Heo <tj@kernel.org>]

Hello, Ulrich.

On Thu, Dec 03, 2015 at 03:12:20PM -0500, Ulrich Obergfell wrote:
> I share Don's concern about connecting the soft lockup detector and the
> workqueue watchdog to the same kernel parameter in /proc. I would feel
> more comfortable if the workqueue watchdog had its dedicated parameter.

Sure, separating the knobs out isn't difficult.  I still don't like
the idea of having multiple set of similar knobs controlling about the
same thing tho.

For example, let's say there's a user who boots with "nosoftlockup"
explicitly.  I'm pretty sure the user wouldn't be intending to keep
workqueue watchdog running.  The same goes for threshold adjustments,
so here's my question.  What are the reasons for the concern?  What
are we worrying about?

> The patched watchdog_enable_all_cpus() function disables the workqueue watchdog
> unconditionally at [1]. However, the workqueue watchdog remains disabled if the
> code path [2] is executed (and wq_watchdog_thresh is not updated as well).

Oops, you're right.

> And another question that comes to my mind is: Would the workqueue watchdog
> participate in the lockup detector suspend/resume mechanism, and if yes, how
> would it be integrated into this ?

>From the usage, I can't quite tell what the purpose of the mechanism
is.  The only user seems to be fixup_ht_bug() and when it fails it
says "failed to disable PMU erratum BJ122, BV98, HSD29 workaround" so
if you could give me a pointer, it'd be great.  But at any rate, if
shutting down watchdog is all that's necessary, it shouldn't be a
problem to integrate.

Thanks.

-- 
tejun