Re: [PATCH RT 2/2] watchdog, rt: prevent deferral of watchdogd wakeup

[Guenter Roeck]

On 09/28/2018 02:03 PM, Julia Cartwright wrote:
> When PREEMPT_RT_FULL is enabled, all hrtimer expiry functions are
> deferred for execution into the context of ktimersoftd unless otherwise
> annotated.
>
> Deferring the expiry of the hrtimer used by the watchdog core, however,
> is a waste, as the callback does nothing but queue a kthread work item
> and wakeup watchdogd.
>
> It's worst then that, too: the deferral through ktimersoftd also means
> that for correct behavior a user must adjust the scheduling parameters
> of both watchdogd _and_ ktimersoftd, which is unnecessary and has other
> side effects (like causing unrelated expiry functions to execute at
> potentially elevated priority).
>
> Instead, mark the hrtimer used by the watchdog core as being _HARD to
> allow it's execution directly from hardirq context.  The work done in
> this expiry function is well-bounded and minimal.
>
> A user still must adjust the scheduling parameters of the watchdogd
> to be correct w.r.t. their application needs.
>
> Cc: Guenter Roeck <linux@xxxxxxxxxxxx>
> Reported-and-tested-by: Steffen Trumtrar <s.trumtrar@xxxxxxxxxxxxxx>
> Reported-by: Tim Sander <tim@xxxxxxxxxxxxxxx>
> Signed-off-by: Julia Cartwright <julia@xxxxxx>

Acked-by: Guenter Roeck <linux@xxxxxxxxxxxx>

> ---
>   drivers/watchdog/watchdog_dev.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
> index ffbdc4642ea5..9c2b3e5cebdc 100644
> --- a/drivers/watchdog/watchdog_dev.c
> +++ b/drivers/watchdog/watchdog_dev.c
> @@ -945,7 +945,7 @@ static int watchdog_cdev_register(struct watchdog_device *wdd, dev_t devno)
>   		return -ENODEV;
>   
>   	kthread_init_work(&wd_data->work, watchdog_ping_work);
> -	hrtimer_init(&wd_data->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> +	hrtimer_init(&wd_data->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_HARD);
>   	wd_data->timer.function = watchdog_timer_expired;
>   
>   	if (wdd->id == 0) {