Re: [PATCH] watchdog: Fix possible soft lockup warning at bootup

[Waiman Long]

On 1/2/20 3:12 PM, Waiman Long wrote:
> On 1/2/20 3:08 PM, Andrew Morton wrote:
>> On Thu,  2 Jan 2020 10:41:49 -0500 Waiman Long <longman@xxxxxxxxxx> wrote:
>>
>>> It was found that watchdog soft lockup warning was displayed on some
>>> arm64 server systems at bootup time:
>>>
>>> ...
>>>
>>> Further analysis of the situation revealed that the smp_init() call
>>> itself took more than 20s for that 2-socket 56-core and 224-thread
>>> server.
>>>
>>>  [    0.115632] CPU1: Booted secondary processor 0x0000000100 [0x431f0af1]
>>>    :
>>>  [   27.177282] CPU223: Booted secondary processor 0x0000011b03 [0x431f0af1]
>>>
>>> By adding some instrumentation code, it was found that for cpu 14,
>>> watchdog_enable() was called early with a timestamp of 1. The first
>>> watchdog timer callback for that cpu, however, happened really late at
>>> the above 25s timestamp mark causing the watchdog logic to treat the
>>> delay as a soft lockup.
>>>
>>> On another arm64 system that doesn't show the soft lockup warning, the
>>> watchdog timer callback happened earlier at the 5s timestamp mark with
>>> the watchdog thread invoked shortly after that.
>>>
>>> The reason why there was such a delay in the first watchdog timer
>>> callback for that particular system wasn't fully known yet.
>> Mysteries are unwelcome.  Are you continuing to investigate this?
> Yes, I will do some more investigation as to why it took so long.

As stated in my v2 patch, the soft lockup problem is caused by a clock
slowdown in during the init_smp() call. This is likely to be hardware
specific and I don't have enough information about the hardware to do an
in-depth investigation of this slow down. So I opt to do just a delay of
the init call as a workaround.


>>> Given
>>> the fact that smp_init() can run for a long time on some systems,
>>> it is probably more appropriate to enable the watchdog function after
>>> smp_init() instead of before it.
>>>
>>> Another way is to leave watchdog_touch_ts at 0 in watchdog_enable()
>>> while the system is at the booting stage. Either one of those should
>>> be able to eliminate the soft lockup warning on bootup.
>>>
>>> ...
>>>
>>> --- a/kernel/watchdog.c
>>> +++ b/kernel/watchdog.c
>>> @@ -496,7 +496,9 @@ static void watchdog_enable(unsigned int cpu)
>>>  		      HRTIMER_MODE_REL_PINNED_HARD);
>>>  
>>>  	/* Initialize timestamp */
>>> -	__touch_watchdog();
>>> +	if (system_state != SYSTEM_BOOTING)
>>> +		__touch_watchdog();
>> A comment which explains the system_state test would be appropriate
>> here.
> Will do so.

This change is not actually necessary. So it is taken out from the v2 patch.

Cheers,
Longman