Re: [PATCH v4 4/6] kernel/watchdog: Adapt the watchdog_hld interface for async model

[Lecopzer Chen <lecopzer.chen@mediatek.com>]

> On Thu 2022-04-28 00:13:38, Lecopzer Chen wrote:
> > When lockup_detector_init()->watchdog_nmi_probe(), PMU may be not ready
> > yet. E.g. on arm64, PMU is not ready until
> > device_initcall(armv8_pmu_driver_init).  And it is deeply integrated
> > with the driver model and cpuhp. Hence it is hard to push this
> > initialization before smp_init().
> > 
> > But it is easy to take an opposite approach and try to initialize
> > the watchdog once again later.
> > The delayed probe is called using workqueues. It need to allocate
> > memory and must be proceed in a normal context.
> > The delayed probe is able to use if watchdog_nmi_probe() returns
> > non-zero which means the return code returned when PMU is not ready yet.
> > 
> > Provide an API - retry_lockup_detector_init() for anyone who needs
> > to delayed init lockup detector if they had ever failed at
> > lockup_detector_init().
> > 
> > The original assumption is: nobody should use delayed probe after
> > lockup_detector_check() which has __init attribute.
> > That is, anyone uses this API must call between lockup_detector_init()
> > and lockup_detector_check(), and the caller must have __init attribute
> > 
> > --- a/kernel/watchdog.c
> > +++ b/kernel/watchdog.c
> > +/*
> > + * retry_lockup_detector_init - retry init lockup detector if possible.
> > + *
> > + * Retry hardlockup detector init. It is useful when it requires some
> > + * functionality that has to be initialized later on a particular
> > + * platform.
> > + */
> > +void __init retry_lockup_detector_init(void)
> > +{
> > +	/* Must be called before late init calls */
> > +	if (!allow_lockup_detector_init_retry)
> > +		return;
> > +
> > +	queue_work_on(__smp_processor_id(), system_wq, &detector_work);
> 
> Just a small nit. This can be simplified by calling:
> 
> 	schedule_work(&detector_work);
> 
> It uses "system_wq" that uses CPU-bound workers. It prefers
> the current CPU. But the exact CPU is not important. Any CPU-bound
> worker is enough.

Thanks!! I'll tweak this on -rc1
> 
> > +}
> > +
> 
> With the above change, feel free to use:
> 
> Reviewed-by: Petr Mladek <pmladek@suse.com>

Really appreciate your review and idea, thank you ver much.


BRs,
Lecopzer