On Tue, Nov 05, 2019 at 03:45:06PM +0100, Sebastian Andrzej Siewior wrote: > From: Julia Cartwright <julia@xxxxxx> > > When PREEMPT_RT is enabled, all hrtimer expiry functions are > deferred for execution into the context of ksoftirqd 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 ksoftirqd also means > that for correct behavior a user must adjust the scheduling parameters > of both watchdogd _and_ ksoftirqd, 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. > > Link: https://lkml.kernel.org/r/0e02d8327aeca344096c246713033887bc490dd7.1538089180.git.julia@xxxxxx > 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> > [bigeasy: use only HRTIMER_MODE_REL_HARD] > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx> Looks ok for mainline. Reviewed-by: Guenter Roeck <linux@xxxxxxxxxxxx> > --- > drivers/watchdog/watchdog_dev.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c > index dbd2ad4c92948..d3acc0a7256ca 100644 > --- a/drivers/watchdog/watchdog_dev.c > +++ b/drivers/watchdog/watchdog_dev.c > @@ -158,7 +158,8 @@ static inline void watchdog_update_worker(struct watchdog_device *wdd) > ktime_t t = watchdog_next_keepalive(wdd); > > if (t > 0) > - hrtimer_start(&wd_data->timer, t, HRTIMER_MODE_REL); > + hrtimer_start(&wd_data->timer, t, > + HRTIMER_MODE_REL_HARD); > } else { > hrtimer_cancel(&wd_data->timer); > } > @@ -177,7 +178,7 @@ static int __watchdog_ping(struct watchdog_device *wdd) > if (ktime_after(earliest_keepalive, now)) { > hrtimer_start(&wd_data->timer, > ktime_sub(earliest_keepalive, now), > - HRTIMER_MODE_REL); > + HRTIMER_MODE_REL_HARD); > return 0; > } > > @@ -971,7 +972,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) { > @@ -1019,7 +1020,8 @@ static int watchdog_cdev_register(struct watchdog_device *wdd, dev_t devno) > __module_get(wdd->ops->owner); > kref_get(&wd_data->kref); > if (handle_boot_enabled) > - hrtimer_start(&wd_data->timer, 0, HRTIMER_MODE_REL); > + hrtimer_start(&wd_data->timer, 0, > + HRTIMER_MODE_REL_HARD); > else > pr_info("watchdog%d running and kernel based pre-userspace handler disabled\n", > wdd->id); > -- > 2.24.0 >
