Re: [PATCH RT 2/2] watchdog, rt: prevent deferral of watchdogd wakeup
On 2018-09-28 21:03:51 [+], 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. … > Signed-off-by: Julia Cartwright did s@HRTIMER_MODE_REL@HRTIMER_MODE_REL_HARD@ and then applied. Thank you Julia. Sebastian
Re: [PATCH RT 2/2] watchdog, rt: prevent deferral of watchdogd wakeup
On 2018-09-28 21:03:51 [+], 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. … > Signed-off-by: Julia Cartwright did s@HRTIMER_MODE_REL@HRTIMER_MODE_REL_HARD@ and then applied. Thank you Julia. Sebastian
Re: [PATCH RT 2/2] watchdog, rt: prevent deferral of watchdogd wakeup
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
Reported-and-tested-by: Steffen Trumtrar
Reported-by: Tim Sander
Signed-off-by: Julia Cartwright
Acked-by: Guenter Roeck
---
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(_data->work, watchdog_ping_work);
- hrtimer_init(_data->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+ hrtimer_init(_data->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_HARD);
wd_data->timer.function = watchdog_timer_expired;
if (wdd->id == 0) {
Re: [PATCH RT 2/2] watchdog, rt: prevent deferral of watchdogd wakeup
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
Reported-and-tested-by: Steffen Trumtrar
Reported-by: Tim Sander
Signed-off-by: Julia Cartwright
Acked-by: Guenter Roeck
---
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(_data->work, watchdog_ping_work);
- hrtimer_init(_data->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+ hrtimer_init(_data->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_HARD);
wd_data->timer.function = watchdog_timer_expired;
if (wdd->id == 0) {
RE: [kbuild-all] [PATCH RT 2/2] watchdog, rt: prevent deferral of watchdogd wakeup
> Subject: Re: [kbuild-all] [PATCH RT 2/2] watchdog, rt: prevent deferral of > watchdogd wakeup > > On 2018-09-29 08:38:55 [+0200], Thomas Gleixner wrote: > > On Sat, 29 Sep 2018, kbuild test robot wrote: > > > [also build test ERROR on v4.19-rc5 next-20180928] > > > [if your patch is applied to the wrong git tree, please drop us a note to > > > help > improve the system] > > > > > > > It's against the RT tree, so it won't work against next or upstream. I > > think it would be good to have a machine parseable tag to describe which > > tree/branch/commit patches are applicable to. If that tag is missing, the > > bot can assume it's against upstream/next. > > I asked the testing team to ignore patches or test against the RT tree > if the patch is tagged [PATCH RT]. I think it worked since I haven't > seen those mails. thanks Sebastian for the input, we will consider this to add to our TODO. > > > Thanks, > > > > tglx > > Sebastian > ___ > kbuild-all mailing list > kbuild-...@lists.01.org > https://lists.01.org/mailman/listinfo/kbuild-all
RE: [kbuild-all] [PATCH RT 2/2] watchdog, rt: prevent deferral of watchdogd wakeup
> Subject: Re: [kbuild-all] [PATCH RT 2/2] watchdog, rt: prevent deferral of > watchdogd wakeup > > On 2018-09-29 08:38:55 [+0200], Thomas Gleixner wrote: > > On Sat, 29 Sep 2018, kbuild test robot wrote: > > > [also build test ERROR on v4.19-rc5 next-20180928] > > > [if your patch is applied to the wrong git tree, please drop us a note to > > > help > improve the system] > > > > > > > It's against the RT tree, so it won't work against next or upstream. I > > think it would be good to have a machine parseable tag to describe which > > tree/branch/commit patches are applicable to. If that tag is missing, the > > bot can assume it's against upstream/next. > > I asked the testing team to ignore patches or test against the RT tree > if the patch is tagged [PATCH RT]. I think it worked since I haven't > seen those mails. thanks Sebastian for the input, we will consider this to add to our TODO. > > > Thanks, > > > > tglx > > Sebastian > ___ > kbuild-all mailing list > kbuild-...@lists.01.org > https://lists.01.org/mailman/listinfo/kbuild-all
Re: [PATCH RT 2/2] watchdog, rt: prevent deferral of watchdogd wakeup
On 2018-09-29 08:38:55 [+0200], Thomas Gleixner wrote: > On Sat, 29 Sep 2018, kbuild test robot wrote: > > [also build test ERROR on v4.19-rc5 next-20180928] > > [if your patch is applied to the wrong git tree, please drop us a note to > > help improve the system] > > > > It's against the RT tree, so it won't work against next or upstream. I > think it would be good to have a machine parseable tag to describe which > tree/branch/commit patches are applicable to. If that tag is missing, the > bot can assume it's against upstream/next. I asked the testing team to ignore patches or test against the RT tree if the patch is tagged [PATCH RT]. I think it worked since I haven't seen those mails. > Thanks, > > tglx Sebastian
Re: [PATCH RT 2/2] watchdog, rt: prevent deferral of watchdogd wakeup
On 2018-09-29 08:38:55 [+0200], Thomas Gleixner wrote: > On Sat, 29 Sep 2018, kbuild test robot wrote: > > [also build test ERROR on v4.19-rc5 next-20180928] > > [if your patch is applied to the wrong git tree, please drop us a note to > > help improve the system] > > > > It's against the RT tree, so it won't work against next or upstream. I > think it would be good to have a machine parseable tag to describe which > tree/branch/commit patches are applicable to. If that tag is missing, the > bot can assume it's against upstream/next. I asked the testing team to ignore patches or test against the RT tree if the patch is tagged [PATCH RT]. I think it worked since I haven't seen those mails. > Thanks, > > tglx Sebastian
Re: [PATCH RT 2/2] watchdog, rt: prevent deferral of watchdogd wakeup
On Sat, 29 Sep 2018, kbuild test robot wrote: > [also build test ERROR on v4.19-rc5 next-20180928] > [if your patch is applied to the wrong git tree, please drop us a note to > help improve the system] > It's against the RT tree, so it won't work against next or upstream. I think it would be good to have a machine parseable tag to describe which tree/branch/commit patches are applicable to. If that tag is missing, the bot can assume it's against upstream/next. Thanks, tglx
Re: [PATCH RT 2/2] watchdog, rt: prevent deferral of watchdogd wakeup
On Sat, 29 Sep 2018, kbuild test robot wrote: > [also build test ERROR on v4.19-rc5 next-20180928] > [if your patch is applied to the wrong git tree, please drop us a note to > help improve the system] > It's against the RT tree, so it won't work against next or upstream. I think it would be good to have a machine parseable tag to describe which tree/branch/commit patches are applicable to. If that tag is missing, the bot can assume it's against upstream/next. Thanks, tglx
Re: [PATCH RT 2/2] watchdog, rt: prevent deferral of watchdogd wakeup
Hi Julia,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.19-rc5 next-20180928]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Julia-Cartwright/kthread-convert-worker-lock-to-raw-spinlock/20180929-052522
config: i386-randconfig-s0-201838 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
drivers//watchdog/watchdog_dev.c: In function 'watchdog_cdev_register':
>> drivers//watchdog/watchdog_dev.c:948:49: error: 'HRTIMER_MODE_REL_HARD'
>> undeclared (first use in this function)
hrtimer_init(_data->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_HARD);
^
drivers//watchdog/watchdog_dev.c:948:49: note: each undeclared identifier is
reported only once for each function it appears in
vim +/HRTIMER_MODE_REL_HARD +948 drivers//watchdog/watchdog_dev.c
919
920 /*
921 * watchdog_cdev_register: register watchdog character device
922 * @wdd: watchdog device
923 * @devno: character device number
924 *
925 * Register a watchdog character device including handling the
legacy
926 * /dev/watchdog node. /dev/watchdog is actually a miscdevice and
927 * thus we set it up like that.
928 */
929
930 static int watchdog_cdev_register(struct watchdog_device *wdd, dev_t
devno)
931 {
932 struct watchdog_core_data *wd_data;
933 int err;
934
935 wd_data = kzalloc(sizeof(struct watchdog_core_data),
GFP_KERNEL);
936 if (!wd_data)
937 return -ENOMEM;
938 kref_init(_data->kref);
939 mutex_init(_data->lock);
940
941 wd_data->wdd = wdd;
942 wdd->wd_data = wd_data;
943
944 if (IS_ERR_OR_NULL(watchdog_kworker))
945 return -ENODEV;
946
947 kthread_init_work(_data->work, watchdog_ping_work);
> 948 hrtimer_init(_data->timer, CLOCK_MONOTONIC,
> HRTIMER_MODE_REL_HARD);
949 wd_data->timer.function = watchdog_timer_expired;
950
951 if (wdd->id == 0) {
952 old_wd_data = wd_data;
953 watchdog_miscdev.parent = wdd->parent;
954 err = misc_register(_miscdev);
955 if (err != 0) {
956 pr_err("%s: cannot register miscdev on minor=%d
(err=%d).\n",
957 wdd->info->identity, WATCHDOG_MINOR,
err);
958 if (err == -EBUSY)
959 pr_err("%s: a legacy watchdog module is
probably present.\n",
960 wdd->info->identity);
961 old_wd_data = NULL;
962 kfree(wd_data);
963 return err;
964 }
965 }
966
967 /* Fill in the data structures */
968 cdev_init(_data->cdev, _fops);
969 wd_data->cdev.owner = wdd->ops->owner;
970
971 /* Add the device */
972 err = cdev_add(_data->cdev, devno, 1);
973 if (err) {
974 pr_err("watchdog%d unable to add device %d:%d\n",
975 wdd->id, MAJOR(watchdog_devt), wdd->id);
976 if (wdd->id == 0) {
977 misc_deregister(_miscdev);
978 old_wd_data = NULL;
979 kref_put(_data->kref,
watchdog_core_data_release);
980 }
981 return err;
982 }
983
984 /* Record time of most recent heartbeat as 'just before now'. */
985 wd_data->last_hw_keepalive = ktime_sub(ktime_get(), 1);
986
987 /*
988 * If the watchdog is running, prevent its driver from being
unloaded,
989 * and schedule an immediate ping.
990 */
991 if (watchdog_hw_running(wdd)) {
992 __module_get(wdd->ops->owner);
993 kref_get(_data->kref);
994 if (handle_boot_enabled)
995 hrtimer_start(_data->timer, 0,
HRTIMER_MODE_REL);
996 else
997 pr_info("watchdog%d running and kernel based
pre-userspace handler disabled\n",
998 wdd->id);
999 }
1000
1001 return 0;
1002 }
1003
---
0-DAY kernel test infrastructureOpen Source Technology Center
Re: [PATCH RT 2/2] watchdog, rt: prevent deferral of watchdogd wakeup
Hi Julia,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.19-rc5 next-20180928]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Julia-Cartwright/kthread-convert-worker-lock-to-raw-spinlock/20180929-052522
config: i386-randconfig-s0-201838 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
drivers//watchdog/watchdog_dev.c: In function 'watchdog_cdev_register':
>> drivers//watchdog/watchdog_dev.c:948:49: error: 'HRTIMER_MODE_REL_HARD'
>> undeclared (first use in this function)
hrtimer_init(_data->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_HARD);
^
drivers//watchdog/watchdog_dev.c:948:49: note: each undeclared identifier is
reported only once for each function it appears in
vim +/HRTIMER_MODE_REL_HARD +948 drivers//watchdog/watchdog_dev.c
919
920 /*
921 * watchdog_cdev_register: register watchdog character device
922 * @wdd: watchdog device
923 * @devno: character device number
924 *
925 * Register a watchdog character device including handling the
legacy
926 * /dev/watchdog node. /dev/watchdog is actually a miscdevice and
927 * thus we set it up like that.
928 */
929
930 static int watchdog_cdev_register(struct watchdog_device *wdd, dev_t
devno)
931 {
932 struct watchdog_core_data *wd_data;
933 int err;
934
935 wd_data = kzalloc(sizeof(struct watchdog_core_data),
GFP_KERNEL);
936 if (!wd_data)
937 return -ENOMEM;
938 kref_init(_data->kref);
939 mutex_init(_data->lock);
940
941 wd_data->wdd = wdd;
942 wdd->wd_data = wd_data;
943
944 if (IS_ERR_OR_NULL(watchdog_kworker))
945 return -ENODEV;
946
947 kthread_init_work(_data->work, watchdog_ping_work);
> 948 hrtimer_init(_data->timer, CLOCK_MONOTONIC,
> HRTIMER_MODE_REL_HARD);
949 wd_data->timer.function = watchdog_timer_expired;
950
951 if (wdd->id == 0) {
952 old_wd_data = wd_data;
953 watchdog_miscdev.parent = wdd->parent;
954 err = misc_register(_miscdev);
955 if (err != 0) {
956 pr_err("%s: cannot register miscdev on minor=%d
(err=%d).\n",
957 wdd->info->identity, WATCHDOG_MINOR,
err);
958 if (err == -EBUSY)
959 pr_err("%s: a legacy watchdog module is
probably present.\n",
960 wdd->info->identity);
961 old_wd_data = NULL;
962 kfree(wd_data);
963 return err;
964 }
965 }
966
967 /* Fill in the data structures */
968 cdev_init(_data->cdev, _fops);
969 wd_data->cdev.owner = wdd->ops->owner;
970
971 /* Add the device */
972 err = cdev_add(_data->cdev, devno, 1);
973 if (err) {
974 pr_err("watchdog%d unable to add device %d:%d\n",
975 wdd->id, MAJOR(watchdog_devt), wdd->id);
976 if (wdd->id == 0) {
977 misc_deregister(_miscdev);
978 old_wd_data = NULL;
979 kref_put(_data->kref,
watchdog_core_data_release);
980 }
981 return err;
982 }
983
984 /* Record time of most recent heartbeat as 'just before now'. */
985 wd_data->last_hw_keepalive = ktime_sub(ktime_get(), 1);
986
987 /*
988 * If the watchdog is running, prevent its driver from being
unloaded,
989 * and schedule an immediate ping.
990 */
991 if (watchdog_hw_running(wdd)) {
992 __module_get(wdd->ops->owner);
993 kref_get(_data->kref);
994 if (handle_boot_enabled)
995 hrtimer_start(_data->timer, 0,
HRTIMER_MODE_REL);
996 else
997 pr_info("watchdog%d running and kernel based
pre-userspace handler disabled\n",
998 wdd->id);
999 }
1000
1001 return 0;
1002 }
1003
---
0-DAY kernel test infrastructureOpen Source Technology Center
Re: [PATCH RT 2/2] watchdog, rt: prevent deferral of watchdogd wakeup
Hi Julia,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.19-rc5 next-20180928]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Julia-Cartwright/kthread-convert-worker-lock-to-raw-spinlock/20180929-052522
config: i386-randconfig-x008-201838 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
drivers//watchdog/watchdog_dev.c: In function 'watchdog_cdev_register':
>> drivers//watchdog/watchdog_dev.c:948:49: error: 'HRTIMER_MODE_REL_HARD'
>> undeclared (first use in this function); did you mean
>> 'HRTIMER_MODE_REL_SOFT'?
hrtimer_init(_data->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_HARD);
^
HRTIMER_MODE_REL_SOFT
drivers//watchdog/watchdog_dev.c:948:49: note: each undeclared identifier is
reported only once for each function it appears in
vim +948 drivers//watchdog/watchdog_dev.c
919
920 /*
921 * watchdog_cdev_register: register watchdog character device
922 * @wdd: watchdog device
923 * @devno: character device number
924 *
925 * Register a watchdog character device including handling the
legacy
926 * /dev/watchdog node. /dev/watchdog is actually a miscdevice and
927 * thus we set it up like that.
928 */
929
930 static int watchdog_cdev_register(struct watchdog_device *wdd, dev_t
devno)
931 {
932 struct watchdog_core_data *wd_data;
933 int err;
934
935 wd_data = kzalloc(sizeof(struct watchdog_core_data),
GFP_KERNEL);
936 if (!wd_data)
937 return -ENOMEM;
938 kref_init(_data->kref);
939 mutex_init(_data->lock);
940
941 wd_data->wdd = wdd;
942 wdd->wd_data = wd_data;
943
944 if (IS_ERR_OR_NULL(watchdog_kworker))
945 return -ENODEV;
946
947 kthread_init_work(_data->work, watchdog_ping_work);
> 948 hrtimer_init(_data->timer, CLOCK_MONOTONIC,
> HRTIMER_MODE_REL_HARD);
949 wd_data->timer.function = watchdog_timer_expired;
950
951 if (wdd->id == 0) {
952 old_wd_data = wd_data;
953 watchdog_miscdev.parent = wdd->parent;
954 err = misc_register(_miscdev);
955 if (err != 0) {
956 pr_err("%s: cannot register miscdev on minor=%d
(err=%d).\n",
957 wdd->info->identity, WATCHDOG_MINOR,
err);
958 if (err == -EBUSY)
959 pr_err("%s: a legacy watchdog module is
probably present.\n",
960 wdd->info->identity);
961 old_wd_data = NULL;
962 kfree(wd_data);
963 return err;
964 }
965 }
966
967 /* Fill in the data structures */
968 cdev_init(_data->cdev, _fops);
969 wd_data->cdev.owner = wdd->ops->owner;
970
971 /* Add the device */
972 err = cdev_add(_data->cdev, devno, 1);
973 if (err) {
974 pr_err("watchdog%d unable to add device %d:%d\n",
975 wdd->id, MAJOR(watchdog_devt), wdd->id);
976 if (wdd->id == 0) {
977 misc_deregister(_miscdev);
978 old_wd_data = NULL;
979 kref_put(_data->kref,
watchdog_core_data_release);
980 }
981 return err;
982 }
983
984 /* Record time of most recent heartbeat as 'just before now'. */
985 wd_data->last_hw_keepalive = ktime_sub(ktime_get(), 1);
986
987 /*
988 * If the watchdog is running, prevent its driver from being
unloaded,
989 * and schedule an immediate ping.
990 */
991 if (watchdog_hw_running(wdd)) {
992 __module_get(wdd->ops->owner);
993 kref_get(_data->kref);
994 if (handle_boot_enabled)
995 hrtimer_start(_data->timer, 0,
HRTIMER_MODE_REL);
996 else
997 pr_info("watchdog%d running and kernel based
pre-userspace handler disabled\n",
998 wdd->id);
999 }
1000
1001 return 0;
1002 }
1003
---
Re: [PATCH RT 2/2] watchdog, rt: prevent deferral of watchdogd wakeup
Hi Julia,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.19-rc5 next-20180928]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Julia-Cartwright/kthread-convert-worker-lock-to-raw-spinlock/20180929-052522
config: i386-randconfig-x008-201838 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
drivers//watchdog/watchdog_dev.c: In function 'watchdog_cdev_register':
>> drivers//watchdog/watchdog_dev.c:948:49: error: 'HRTIMER_MODE_REL_HARD'
>> undeclared (first use in this function); did you mean
>> 'HRTIMER_MODE_REL_SOFT'?
hrtimer_init(_data->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_HARD);
^
HRTIMER_MODE_REL_SOFT
drivers//watchdog/watchdog_dev.c:948:49: note: each undeclared identifier is
reported only once for each function it appears in
vim +948 drivers//watchdog/watchdog_dev.c
919
920 /*
921 * watchdog_cdev_register: register watchdog character device
922 * @wdd: watchdog device
923 * @devno: character device number
924 *
925 * Register a watchdog character device including handling the
legacy
926 * /dev/watchdog node. /dev/watchdog is actually a miscdevice and
927 * thus we set it up like that.
928 */
929
930 static int watchdog_cdev_register(struct watchdog_device *wdd, dev_t
devno)
931 {
932 struct watchdog_core_data *wd_data;
933 int err;
934
935 wd_data = kzalloc(sizeof(struct watchdog_core_data),
GFP_KERNEL);
936 if (!wd_data)
937 return -ENOMEM;
938 kref_init(_data->kref);
939 mutex_init(_data->lock);
940
941 wd_data->wdd = wdd;
942 wdd->wd_data = wd_data;
943
944 if (IS_ERR_OR_NULL(watchdog_kworker))
945 return -ENODEV;
946
947 kthread_init_work(_data->work, watchdog_ping_work);
> 948 hrtimer_init(_data->timer, CLOCK_MONOTONIC,
> HRTIMER_MODE_REL_HARD);
949 wd_data->timer.function = watchdog_timer_expired;
950
951 if (wdd->id == 0) {
952 old_wd_data = wd_data;
953 watchdog_miscdev.parent = wdd->parent;
954 err = misc_register(_miscdev);
955 if (err != 0) {
956 pr_err("%s: cannot register miscdev on minor=%d
(err=%d).\n",
957 wdd->info->identity, WATCHDOG_MINOR,
err);
958 if (err == -EBUSY)
959 pr_err("%s: a legacy watchdog module is
probably present.\n",
960 wdd->info->identity);
961 old_wd_data = NULL;
962 kfree(wd_data);
963 return err;
964 }
965 }
966
967 /* Fill in the data structures */
968 cdev_init(_data->cdev, _fops);
969 wd_data->cdev.owner = wdd->ops->owner;
970
971 /* Add the device */
972 err = cdev_add(_data->cdev, devno, 1);
973 if (err) {
974 pr_err("watchdog%d unable to add device %d:%d\n",
975 wdd->id, MAJOR(watchdog_devt), wdd->id);
976 if (wdd->id == 0) {
977 misc_deregister(_miscdev);
978 old_wd_data = NULL;
979 kref_put(_data->kref,
watchdog_core_data_release);
980 }
981 return err;
982 }
983
984 /* Record time of most recent heartbeat as 'just before now'. */
985 wd_data->last_hw_keepalive = ktime_sub(ktime_get(), 1);
986
987 /*
988 * If the watchdog is running, prevent its driver from being
unloaded,
989 * and schedule an immediate ping.
990 */
991 if (watchdog_hw_running(wdd)) {
992 __module_get(wdd->ops->owner);
993 kref_get(_data->kref);
994 if (handle_boot_enabled)
995 hrtimer_start(_data->timer, 0,
HRTIMER_MODE_REL);
996 else
997 pr_info("watchdog%d running and kernel based
pre-userspace handler disabled\n",
998 wdd->id);
999 }
1000
1001 return 0;
1002 }
1003
---
[PATCH RT 2/2] watchdog, rt: prevent deferral of watchdogd wakeup
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
Reported-and-tested-by: Steffen Trumtrar
Reported-by: Tim Sander
Signed-off-by: Julia Cartwright
---
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(_data->work, watchdog_ping_work);
- hrtimer_init(_data->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+ hrtimer_init(_data->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_HARD);
wd_data->timer.function = watchdog_timer_expired;
if (wdd->id == 0) {
--
2.18.0
[PATCH RT 2/2] watchdog, rt: prevent deferral of watchdogd wakeup
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
Reported-and-tested-by: Steffen Trumtrar
Reported-by: Tim Sander
Signed-off-by: Julia Cartwright
---
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(_data->work, watchdog_ping_work);
- hrtimer_init(_data->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+ hrtimer_init(_data->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_HARD);
wd_data->timer.function = watchdog_timer_expired;
if (wdd->id == 0) {
--
2.18.0
