Re: [PATCH] powerpc/watchdog: disable watchdog when watchdog_thresh=0

["Luiz Capitulino (Red Hat)" <luiz.capitulino@linux.dev>]

On 11/6/26 5:26 AM, Michael Ellerman wrote:
> On 4/11/26 2:40 pm, Luiz Capitulino (Red Hat) wrote:
>> On powerpc, trying to disable the hardlockup detector with:
>>
>>   # echo 0 > /proc/sys/kernel/watchdog_thresh
>>
>> causes an immediate and endless hardlockup detection, eventually leading
>> to an unusable system (splat example below).
> 
> ...
>> This condition is already checked in __lockup_detector_reconfigure() for
>> the soft lockup detector and the arch-independent hardlockup detector,
>> what's missing is powerpc performing the same check in start_watchdog().
> 
> Arguably setting watchdog_thresh to 0 should mean watchdog_enabled becomes false.
> Then there wouldn't need to be extra checks for thresh == 0 anywhere.
> 
> But if the existing code already checks for thresh == 0 then this is fine.

Yes, the common code in kernel/watchdog.c actually checks for both.

Thanks for the review.

> 
>> Fixes: 2104180a5369 ("powerpc/64s: implement arch-specific hardlockup watchdog")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Luiz Capitulino (Red Hat) <luiz.capitulino@linux.dev>
>> ---
>> Notes:
>> - This issue was not discovered by AI. We have a new test-case in our CI that
>>    sets watchdog_thresh=0, when running on powerpc we get the described issue
>> - This issue was initially reproduced on Power10, but I developed the fix
>>    on Power9 (which is what I have access now)
>>
>>   arch/powerpc/kernel/watchdog.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/arch/powerpc/kernel/watchdog.c b/arch/powerpc/kernel/watchdog.c
>> index 582ee3fd71c1..44f178e07531 100644
>> --- a/arch/powerpc/kernel/watchdog.c
>> +++ b/arch/powerpc/kernel/watchdog.c
>> @@ -487,6 +487,9 @@ static void start_watchdog(void *arg)
>>       if (!(watchdog_enabled & WATCHDOG_HARDLOCKUP_ENABLED))
>>           return;
>> +    if (!watchdog_thresh)
>> +        return;
>> +
>>       if (!cpumask_test_cpu(cpu, &watchdog_cpumask))
>>           return;
> 
> 
> Fix looks good to me.
> 
> Reviewed-by: Michael Ellerman <mpe@kernel.org>
> 
> cheers