* [PATCH] [RFC]Watchdog:core: constant pinging until userspace timesout when delay very less @ 2013-06-02 10:13 anish kumar 2013-06-02 10:26 ` anish singh ` (2 more replies) 0 siblings, 3 replies; 13+ messages in thread From: anish kumar @ 2013-06-02 10:13 UTC (permalink / raw) To: wim, linux; +Cc: linux-watchdog, linux-kernel, anish kumar Certain watchdog drivers use a timer to keep kicking the watchdog at a rate of 0.5s (HZ/2) untill userspace times out.They do this as we can't guarantee that watchdog will be pinged fast enough for all system loads, especially if timeout is configured for less than or equal to 1 second(basically small values). As suggested by Wim Van Sebroeck & Guenter Roeck we should add this functionality of individual watchdog drivers in the core watchdog core. Signed-off-by: anish kumar <anish198519851985@gmail.com> --- drivers/watchdog/watchdog_dev.c | 34 +++++++++++++++++++++++++++++----- include/linux/watchdog.h | 1 + 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c index faf4e18..0305803 100644 --- a/drivers/watchdog/watchdog_dev.c +++ b/drivers/watchdog/watchdog_dev.c @@ -41,9 +41,14 @@ #include <linux/miscdevice.h> /* For handling misc devices */ #include <linux/init.h> /* For __init/__exit/... */ #include <linux/uaccess.h> /* For copy_to_user/put_user/... */ +#include <linux/timer.h> +#include <linux/jiffies.h> #include "watchdog_core.h" +/* Timer heartbeat (500ms) */ +#define WDT_TIMEOUT (HZ/2) /* should this be sysfs? */ + /* the dev_t structure to store the dynamically allocated watchdog devices */ static dev_t watchdog_devt; /* the watchdog device behind /dev/watchdog */ @@ -73,16 +78,33 @@ static int watchdog_ping(struct watchdog_device *wddev) if (!watchdog_active(wddev)) goto out_ping; - if (wddev->ops->ping) - err = wddev->ops->ping(wddev); /* ping the watchdog */ - else - err = wddev->ops->start(wddev); /* restart watchdog */ + /* should we check ping interval value i.e. timeout value + if it is less than certain threshold then only we + should add this logic of periodic pinging? */ + if (time_before(jiffies, (unsigned long)wddev->timeout)) { + if (wddev->ops->ping) + err = wddev->ops->ping(wddev);/* ping the watchdog */ + else + err = wddev->ops->start(wddev);/* restart watchdog */ + mod_timer(&wddev->timer, jiffies + WDT_TIMEOUT); + } else { + /* + *what we should when we find out that userspace + *has timed out? + **/ + } out_ping: mutex_unlock(&wddev->lock); return err; } +static void watchdog_ping_wrapper(unsigned long priv) +{ + struct watchdog_device *wdd = (void *)priv; + watchdog_ping(wdd); +} + /* * watchdog_start: wrapper to start the watchdog. * @wddev: the watchdog device to start @@ -109,7 +131,8 @@ static int watchdog_start(struct watchdog_device *wddev) err = wddev->ops->start(wddev); if (err == 0) set_bit(WDOG_ACTIVE, &wddev->status); - + + mod_timer(&wddev->timer, jiffies + WDT_TIMEOUT); out_start: mutex_unlock(&wddev->lock); return err; @@ -552,6 +575,7 @@ int watchdog_dev_register(struct watchdog_device *watchdog) old_wdd = NULL; } } + setup_timer(&watchdog->timer, 0, (long)watchdog_ping_wrapper); return err; } diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h index 2a3038e..e5f18f7 100644 --- a/include/linux/watchdog.h +++ b/include/linux/watchdog.h @@ -84,6 +84,7 @@ struct watchdog_device { const struct watchdog_ops *ops; unsigned int bootstatus; unsigned int timeout; + struct timer_list timer; unsigned int min_timeout; unsigned int max_timeout; void *driver_data; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] [RFC]Watchdog:core: constant pinging until userspace timesout when delay very less 2013-06-02 10:13 [PATCH] [RFC]Watchdog:core: constant pinging until userspace timesout when delay very less anish kumar @ 2013-06-02 10:26 ` anish singh 2013-06-03 14:38 ` anish singh 2013-06-03 15:27 ` Guenter Roeck 2013-06-27 19:31 ` Wim Van Sebroeck 2 siblings, 1 reply; 13+ messages in thread From: anish singh @ 2013-06-02 10:26 UTC (permalink / raw) To: Wim Van Sebroeck, Guenter Roeck Cc: linux-watchdog, linux-kernel-mail, anish kumar On Sun, Jun 2, 2013 at 3:43 PM, anish kumar <anish198519851985@gmail.com> wrote: > Certain watchdog drivers use a timer to keep kicking the watchdog at > a rate of 0.5s (HZ/2) untill userspace times out.They do this as > we can't guarantee that watchdog will be pinged fast enough > for all system loads, especially if timeout is configured for > less than or equal to 1 second(basically small values). > > As suggested by Wim Van Sebroeck & Guenter Roeck we should > add this functionality of individual watchdog drivers in the core > watchdog core. > > Signed-off-by: anish kumar <anish198519851985@gmail.com> > --- > drivers/watchdog/watchdog_dev.c | 34 +++++++++++++++++++++++++++++----- > include/linux/watchdog.h | 1 + > 2 files changed, 30 insertions(+), 5 deletions(-) > > diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c > index faf4e18..0305803 100644 > --- a/drivers/watchdog/watchdog_dev.c > +++ b/drivers/watchdog/watchdog_dev.c > @@ -41,9 +41,14 @@ > #include <linux/miscdevice.h> /* For handling misc devices */ > #include <linux/init.h> /* For __init/__exit/... */ > #include <linux/uaccess.h> /* For copy_to_user/put_user/... */ > +#include <linux/timer.h> > +#include <linux/jiffies.h> > > #include "watchdog_core.h" > > +/* Timer heartbeat (500ms) */ > +#define WDT_TIMEOUT (HZ/2) /* should this be sysfs? */ > + > /* the dev_t structure to store the dynamically allocated watchdog devices */ > static dev_t watchdog_devt; > /* the watchdog device behind /dev/watchdog */ > @@ -73,16 +78,33 @@ static int watchdog_ping(struct watchdog_device *wddev) I think watchdog_ping function prototype should be changed as nowhere we use the return value. > if (!watchdog_active(wddev)) > goto out_ping; > > - if (wddev->ops->ping) > - err = wddev->ops->ping(wddev); /* ping the watchdog */ > - else > - err = wddev->ops->start(wddev); /* restart watchdog */ > + /* should we check ping interval value i.e. timeout value > + if it is less than certain threshold then only we > + should add this logic of periodic pinging? */ > + if (time_before(jiffies, (unsigned long)wddev->timeout)) { > + if (wddev->ops->ping) > + err = wddev->ops->ping(wddev);/* ping the watchdog */ > + else > + err = wddev->ops->start(wddev);/* restart watchdog */ > + mod_timer(&wddev->timer, jiffies + WDT_TIMEOUT); > + } else { > + /* > + *what we should when we find out that userspace > + *has timed out? > + **/ > + } > > out_ping: > mutex_unlock(&wddev->lock); > return err; > } > > +static void watchdog_ping_wrapper(unsigned long priv) if we change the watchdog_ping api then we don't need this wrapper. > +{ > + struct watchdog_device *wdd = (void *)priv; > + watchdog_ping(wdd); > +} > + > /* > * watchdog_start: wrapper to start the watchdog. > * @wddev: the watchdog device to start > @@ -109,7 +131,8 @@ static int watchdog_start(struct watchdog_device *wddev) > err = wddev->ops->start(wddev); > if (err == 0) > set_bit(WDOG_ACTIVE, &wddev->status); > - > + > + mod_timer(&wddev->timer, jiffies + WDT_TIMEOUT); > out_start: > mutex_unlock(&wddev->lock); > return err; > @@ -552,6 +575,7 @@ int watchdog_dev_register(struct watchdog_device *watchdog) > old_wdd = NULL; > } > } > + setup_timer(&watchdog->timer, 0, (long)watchdog_ping_wrapper); > return err; > } > > diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h > index 2a3038e..e5f18f7 100644 > --- a/include/linux/watchdog.h > +++ b/include/linux/watchdog.h > @@ -84,6 +84,7 @@ struct watchdog_device { > const struct watchdog_ops *ops; > unsigned int bootstatus; > unsigned int timeout; > + struct timer_list timer; should we use hrtimer? or timer would do? > unsigned int min_timeout; > unsigned int max_timeout; > void *driver_data; > -- > 1.7.10.4 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] [RFC]Watchdog:core: constant pinging until userspace timesout when delay very less 2013-06-02 10:26 ` anish singh @ 2013-06-03 14:38 ` anish singh 0 siblings, 0 replies; 13+ messages in thread From: anish singh @ 2013-06-03 14:38 UTC (permalink / raw) To: Wim Van Sebroeck, Guenter Roeck Cc: linux-watchdog, linux-kernel-mail, anish kumar any inputs? On Sun, Jun 2, 2013 at 3:56 PM, anish singh <anish198519851985@gmail.com> wrote: > On Sun, Jun 2, 2013 at 3:43 PM, anish kumar <anish198519851985@gmail.com> wrote: >> Certain watchdog drivers use a timer to keep kicking the watchdog at >> a rate of 0.5s (HZ/2) untill userspace times out.They do this as >> we can't guarantee that watchdog will be pinged fast enough >> for all system loads, especially if timeout is configured for >> less than or equal to 1 second(basically small values). >> >> As suggested by Wim Van Sebroeck & Guenter Roeck we should >> add this functionality of individual watchdog drivers in the core >> watchdog core. >> >> Signed-off-by: anish kumar <anish198519851985@gmail.com> >> --- >> drivers/watchdog/watchdog_dev.c | 34 +++++++++++++++++++++++++++++----- >> include/linux/watchdog.h | 1 + >> 2 files changed, 30 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c >> index faf4e18..0305803 100644 >> --- a/drivers/watchdog/watchdog_dev.c >> +++ b/drivers/watchdog/watchdog_dev.c >> @@ -41,9 +41,14 @@ >> #include <linux/miscdevice.h> /* For handling misc devices */ >> #include <linux/init.h> /* For __init/__exit/... */ >> #include <linux/uaccess.h> /* For copy_to_user/put_user/... */ >> +#include <linux/timer.h> >> +#include <linux/jiffies.h> >> >> #include "watchdog_core.h" >> >> +/* Timer heartbeat (500ms) */ >> +#define WDT_TIMEOUT (HZ/2) /* should this be sysfs? */ >> + >> /* the dev_t structure to store the dynamically allocated watchdog devices */ >> static dev_t watchdog_devt; >> /* the watchdog device behind /dev/watchdog */ >> @@ -73,16 +78,33 @@ static int watchdog_ping(struct watchdog_device *wddev) > I think watchdog_ping function prototype should be changed as nowhere > we use the return value. >> if (!watchdog_active(wddev)) >> goto out_ping; >> >> - if (wddev->ops->ping) >> - err = wddev->ops->ping(wddev); /* ping the watchdog */ >> - else >> - err = wddev->ops->start(wddev); /* restart watchdog */ >> + /* should we check ping interval value i.e. timeout value >> + if it is less than certain threshold then only we >> + should add this logic of periodic pinging? */ >> + if (time_before(jiffies, (unsigned long)wddev->timeout)) { >> + if (wddev->ops->ping) >> + err = wddev->ops->ping(wddev);/* ping the watchdog */ >> + else >> + err = wddev->ops->start(wddev);/* restart watchdog */ >> + mod_timer(&wddev->timer, jiffies + WDT_TIMEOUT); >> + } else { >> + /* >> + *what we should when we find out that userspace >> + *has timed out? >> + **/ >> + } >> >> out_ping: >> mutex_unlock(&wddev->lock); >> return err; >> } >> >> +static void watchdog_ping_wrapper(unsigned long priv) > if we change the watchdog_ping api then we don't need this wrapper. >> +{ >> + struct watchdog_device *wdd = (void *)priv; >> + watchdog_ping(wdd); >> +} >> + >> /* >> * watchdog_start: wrapper to start the watchdog. >> * @wddev: the watchdog device to start >> @@ -109,7 +131,8 @@ static int watchdog_start(struct watchdog_device *wddev) >> err = wddev->ops->start(wddev); >> if (err == 0) >> set_bit(WDOG_ACTIVE, &wddev->status); >> - >> + >> + mod_timer(&wddev->timer, jiffies + WDT_TIMEOUT); >> out_start: >> mutex_unlock(&wddev->lock); >> return err; >> @@ -552,6 +575,7 @@ int watchdog_dev_register(struct watchdog_device *watchdog) >> old_wdd = NULL; >> } >> } >> + setup_timer(&watchdog->timer, 0, (long)watchdog_ping_wrapper); >> return err; >> } >> >> diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h >> index 2a3038e..e5f18f7 100644 >> --- a/include/linux/watchdog.h >> +++ b/include/linux/watchdog.h >> @@ -84,6 +84,7 @@ struct watchdog_device { >> const struct watchdog_ops *ops; >> unsigned int bootstatus; >> unsigned int timeout; >> + struct timer_list timer; > should we use hrtimer? or timer would do? >> unsigned int min_timeout; >> unsigned int max_timeout; >> void *driver_data; >> -- >> 1.7.10.4 >> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] [RFC]Watchdog:core: constant pinging until userspace timesout when delay very less 2013-06-02 10:13 [PATCH] [RFC]Watchdog:core: constant pinging until userspace timesout when delay very less anish kumar 2013-06-02 10:26 ` anish singh @ 2013-06-03 15:27 ` Guenter Roeck 2013-06-03 16:53 ` anish singh 2013-06-27 19:31 ` Wim Van Sebroeck 2 siblings, 1 reply; 13+ messages in thread From: Guenter Roeck @ 2013-06-03 15:27 UTC (permalink / raw) To: anish kumar; +Cc: wim, linux-watchdog, linux-kernel On Sun, Jun 02, 2013 at 03:43:07PM +0530, anish kumar wrote: > Certain watchdog drivers use a timer to keep kicking the watchdog at > a rate of 0.5s (HZ/2) untill userspace times out.They do this as > we can't guarantee that watchdog will be pinged fast enough > for all system loads, especially if timeout is configured for > less than or equal to 1 second(basically small values). > > As suggested by Wim Van Sebroeck & Guenter Roeck we should > add this functionality of individual watchdog drivers in the core > watchdog core. > > Signed-off-by: anish kumar <anish198519851985@gmail.com> Not exactly what I had in mind. My idea was to enable the softdog only if the hardware watchdog's maximum timeout was low (say, less than a couple of minutes), and if a timeout larger than its maximum value was configured. In that case, I would have set the hardware watchdog to its maximum value and use the softdog to ping it at a rate of, say, 50% of this maximum. If userspace would not ping the watchdog within its configured value, I would stop pinging the hardware watchdog and let it time out. Guenter > --- > drivers/watchdog/watchdog_dev.c | 34 +++++++++++++++++++++++++++++----- > include/linux/watchdog.h | 1 + > 2 files changed, 30 insertions(+), 5 deletions(-) > > diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c > index faf4e18..0305803 100644 > --- a/drivers/watchdog/watchdog_dev.c > +++ b/drivers/watchdog/watchdog_dev.c > @@ -41,9 +41,14 @@ > #include <linux/miscdevice.h> /* For handling misc devices */ > #include <linux/init.h> /* For __init/__exit/... */ > #include <linux/uaccess.h> /* For copy_to_user/put_user/... */ > +#include <linux/timer.h> > +#include <linux/jiffies.h> > > #include "watchdog_core.h" > > +/* Timer heartbeat (500ms) */ > +#define WDT_TIMEOUT (HZ/2) /* should this be sysfs? */ > + > /* the dev_t structure to store the dynamically allocated watchdog devices */ > static dev_t watchdog_devt; > /* the watchdog device behind /dev/watchdog */ > @@ -73,16 +78,33 @@ static int watchdog_ping(struct watchdog_device *wddev) > if (!watchdog_active(wddev)) > goto out_ping; > > - if (wddev->ops->ping) > - err = wddev->ops->ping(wddev); /* ping the watchdog */ > - else > - err = wddev->ops->start(wddev); /* restart watchdog */ > + /* should we check ping interval value i.e. timeout value > + if it is less than certain threshold then only we > + should add this logic of periodic pinging? */ > + if (time_before(jiffies, (unsigned long)wddev->timeout)) { > + if (wddev->ops->ping) > + err = wddev->ops->ping(wddev);/* ping the watchdog */ > + else > + err = wddev->ops->start(wddev);/* restart watchdog */ > + mod_timer(&wddev->timer, jiffies + WDT_TIMEOUT); > + } else { > + /* > + *what we should when we find out that userspace > + *has timed out? > + **/ > + } > > out_ping: > mutex_unlock(&wddev->lock); > return err; > } > > +static void watchdog_ping_wrapper(unsigned long priv) > +{ > + struct watchdog_device *wdd = (void *)priv; > + watchdog_ping(wdd); > +} > + > /* > * watchdog_start: wrapper to start the watchdog. > * @wddev: the watchdog device to start > @@ -109,7 +131,8 @@ static int watchdog_start(struct watchdog_device *wddev) > err = wddev->ops->start(wddev); > if (err == 0) > set_bit(WDOG_ACTIVE, &wddev->status); > - > + > + mod_timer(&wddev->timer, jiffies + WDT_TIMEOUT); > out_start: > mutex_unlock(&wddev->lock); > return err; > @@ -552,6 +575,7 @@ int watchdog_dev_register(struct watchdog_device *watchdog) > old_wdd = NULL; > } > } > + setup_timer(&watchdog->timer, 0, (long)watchdog_ping_wrapper); > return err; > } > > diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h > index 2a3038e..e5f18f7 100644 > --- a/include/linux/watchdog.h > +++ b/include/linux/watchdog.h > @@ -84,6 +84,7 @@ struct watchdog_device { > const struct watchdog_ops *ops; > unsigned int bootstatus; > unsigned int timeout; > + struct timer_list timer; > unsigned int min_timeout; > unsigned int max_timeout; > void *driver_data; > -- > 1.7.10.4 > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] [RFC]Watchdog:core: constant pinging until userspace timesout when delay very less 2013-06-03 15:27 ` Guenter Roeck @ 2013-06-03 16:53 ` anish singh 2013-06-03 22:25 ` Guenter Roeck 0 siblings, 1 reply; 13+ messages in thread From: anish singh @ 2013-06-03 16:53 UTC (permalink / raw) To: Guenter Roeck; +Cc: Wim Van Sebroeck, linux-watchdog, linux-kernel-mail On Mon, Jun 3, 2013 at 8:57 PM, Guenter Roeck <linux@roeck-us.net> wrote: > On Sun, Jun 02, 2013 at 03:43:07PM +0530, anish kumar wrote: >> Certain watchdog drivers use a timer to keep kicking the watchdog at >> a rate of 0.5s (HZ/2) untill userspace times out.They do this as >> we can't guarantee that watchdog will be pinged fast enough >> for all system loads, especially if timeout is configured for >> less than or equal to 1 second(basically small values). >> >> As suggested by Wim Van Sebroeck & Guenter Roeck we should >> add this functionality of individual watchdog drivers in the core >> watchdog core. >> >> Signed-off-by: anish kumar <anish198519851985@gmail.com> > > Not exactly what I had in mind. My idea was to enable the softdog only if > the hardware watchdog's maximum timeout was low (say, less than a couple > of minutes), and if a timeout larger than its maximum value was configured. watchdog_timeout_invalid wouldn't this check will fail if the user space tries to set maximum timeout more that what driver can support?It would work for pika_wdt.c as it is old watchdog driver and doesn't register with watchdog framwork but new drivers has to pass this api. OR Do you want to remove this check and go as explained by you?I would favour this approach though. > In that case, I would have set the hardware watchdog to its maximum value > and use the softdog to ping it at a rate of, say, 50% of this maximum. > > If userspace would not ping the watchdog within its configured value, > I would stop pinging the hardware watchdog and let it time out. One more question.Why is the return value of watchdog_ping int? Anyway we discard it. > > Guenter > >> --- >> drivers/watchdog/watchdog_dev.c | 34 +++++++++++++++++++++++++++++----- >> include/linux/watchdog.h | 1 + >> 2 files changed, 30 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c >> index faf4e18..0305803 100644 >> --- a/drivers/watchdog/watchdog_dev.c >> +++ b/drivers/watchdog/watchdog_dev.c >> @@ -41,9 +41,14 @@ >> #include <linux/miscdevice.h> /* For handling misc devices */ >> #include <linux/init.h> /* For __init/__exit/... */ >> #include <linux/uaccess.h> /* For copy_to_user/put_user/... */ >> +#include <linux/timer.h> >> +#include <linux/jiffies.h> >> >> #include "watchdog_core.h" >> >> +/* Timer heartbeat (500ms) */ >> +#define WDT_TIMEOUT (HZ/2) /* should this be sysfs? */ >> + >> /* the dev_t structure to store the dynamically allocated watchdog devices */ >> static dev_t watchdog_devt; >> /* the watchdog device behind /dev/watchdog */ >> @@ -73,16 +78,33 @@ static int watchdog_ping(struct watchdog_device *wddev) >> if (!watchdog_active(wddev)) >> goto out_ping; >> >> - if (wddev->ops->ping) >> - err = wddev->ops->ping(wddev); /* ping the watchdog */ >> - else >> - err = wddev->ops->start(wddev); /* restart watchdog */ >> + /* should we check ping interval value i.e. timeout value >> + if it is less than certain threshold then only we >> + should add this logic of periodic pinging? */ >> + if (time_before(jiffies, (unsigned long)wddev->timeout)) { >> + if (wddev->ops->ping) >> + err = wddev->ops->ping(wddev);/* ping the watchdog */ >> + else >> + err = wddev->ops->start(wddev);/* restart watchdog */ >> + mod_timer(&wddev->timer, jiffies + WDT_TIMEOUT); >> + } else { >> + /* >> + *what we should when we find out that userspace >> + *has timed out? >> + **/ >> + } >> >> out_ping: >> mutex_unlock(&wddev->lock); >> return err; >> } >> >> +static void watchdog_ping_wrapper(unsigned long priv) >> +{ >> + struct watchdog_device *wdd = (void *)priv; >> + watchdog_ping(wdd); >> +} >> + >> /* >> * watchdog_start: wrapper to start the watchdog. >> * @wddev: the watchdog device to start >> @@ -109,7 +131,8 @@ static int watchdog_start(struct watchdog_device *wddev) >> err = wddev->ops->start(wddev); >> if (err == 0) >> set_bit(WDOG_ACTIVE, &wddev->status); >> - >> + >> + mod_timer(&wddev->timer, jiffies + WDT_TIMEOUT); >> out_start: >> mutex_unlock(&wddev->lock); >> return err; >> @@ -552,6 +575,7 @@ int watchdog_dev_register(struct watchdog_device *watchdog) >> old_wdd = NULL; >> } >> } >> + setup_timer(&watchdog->timer, 0, (long)watchdog_ping_wrapper); >> return err; >> } >> >> diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h >> index 2a3038e..e5f18f7 100644 >> --- a/include/linux/watchdog.h >> +++ b/include/linux/watchdog.h >> @@ -84,6 +84,7 @@ struct watchdog_device { >> const struct watchdog_ops *ops; >> unsigned int bootstatus; >> unsigned int timeout; >> + struct timer_list timer; >> unsigned int min_timeout; >> unsigned int max_timeout; >> void *driver_data; >> -- >> 1.7.10.4 >> >> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] [RFC]Watchdog:core: constant pinging until userspace timesout when delay very less 2013-06-03 16:53 ` anish singh @ 2013-06-03 22:25 ` Guenter Roeck 2013-06-04 3:09 ` anish singh 0 siblings, 1 reply; 13+ messages in thread From: Guenter Roeck @ 2013-06-03 22:25 UTC (permalink / raw) To: anish singh; +Cc: Wim Van Sebroeck, linux-watchdog, linux-kernel-mail On Mon, Jun 03, 2013 at 10:23:04PM +0530, anish singh wrote: > On Mon, Jun 3, 2013 at 8:57 PM, Guenter Roeck <linux@roeck-us.net> wrote: > > On Sun, Jun 02, 2013 at 03:43:07PM +0530, anish kumar wrote: > >> Certain watchdog drivers use a timer to keep kicking the watchdog at > >> a rate of 0.5s (HZ/2) untill userspace times out.They do this as > >> we can't guarantee that watchdog will be pinged fast enough > >> for all system loads, especially if timeout is configured for > >> less than or equal to 1 second(basically small values). > >> > >> As suggested by Wim Van Sebroeck & Guenter Roeck we should > >> add this functionality of individual watchdog drivers in the core > >> watchdog core. > >> > >> Signed-off-by: anish kumar <anish198519851985@gmail.com> > > > > Not exactly what I had in mind. My idea was to enable the softdog only if > > the hardware watchdog's maximum timeout was low (say, less than a couple > > of minutes), and if a timeout larger than its maximum value was configured. > > watchdog_timeout_invalid wouldn't this check will fail if the user space tries > to set maximum timeout more that what driver can support?It would work > for pika_wdt.c as it is old watchdog driver and doesn't register with watchdog > framwork but new drivers has to pass this api. > > OR > > Do you want to remove this check and go as explained by you?I would > favour this approach though. > One would still have a check, but the enforced limits would no longer be the driver limits, but larger limits implemented in the watchdog core. > > In that case, I would have set the hardware watchdog to its maximum value > > and use the softdog to ping it at a rate of, say, 50% of this maximum. > > > > If userspace would not ping the watchdog within its configured value, > > I would stop pinging the hardware watchdog and let it time out. > > One more question.Why is the return value of watchdog_ping int? Anyway > we discard it. I can not answer that question. Guenter ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] [RFC]Watchdog:core: constant pinging until userspace timesout when delay very less 2013-06-03 22:25 ` Guenter Roeck @ 2013-06-04 3:09 ` anish singh 2013-06-05 3:09 ` anish singh 0 siblings, 1 reply; 13+ messages in thread From: anish singh @ 2013-06-04 3:09 UTC (permalink / raw) To: Guenter Roeck, randy.dunlap, alan Cc: Wim Van Sebroeck, linux-watchdog, linux-kernel-mail On Tue, Jun 4, 2013 at 3:55 AM, Guenter Roeck <linux@roeck-us.net> wrote: > On Mon, Jun 03, 2013 at 10:23:04PM +0530, anish singh wrote: >> On Mon, Jun 3, 2013 at 8:57 PM, Guenter Roeck <linux@roeck-us.net> wrote: >> > On Sun, Jun 02, 2013 at 03:43:07PM +0530, anish kumar wrote: >> >> Certain watchdog drivers use a timer to keep kicking the watchdog at >> >> a rate of 0.5s (HZ/2) untill userspace times out.They do this as >> >> we can't guarantee that watchdog will be pinged fast enough >> >> for all system loads, especially if timeout is configured for >> >> less than or equal to 1 second(basically small values). >> >> >> >> As suggested by Wim Van Sebroeck & Guenter Roeck we should >> >> add this functionality of individual watchdog drivers in the core >> >> watchdog core. >> >> >> >> Signed-off-by: anish kumar <anish198519851985@gmail.com> >> > >> > Not exactly what I had in mind. My idea was to enable the softdog only if >> > the hardware watchdog's maximum timeout was low (say, less than a couple >> > of minutes), and if a timeout larger than its maximum value was configured. >> >> watchdog_timeout_invalid wouldn't this check will fail if the user space tries >> to set maximum timeout more that what driver can support?It would work >> for pika_wdt.c as it is old watchdog driver and doesn't register with watchdog >> framwork but new drivers has to pass this api. >> >> OR >> >> Do you want to remove this check and go as explained by you?I would >> favour this approach though. >> > One would still have a check, but the enforced limits would no longer be > the driver limits, but larger limits implemented in the watchdog core. How much larger would be the big question here?Should it be configurable property(sysfs?) or some hardcoding based on existing drivers? Before going for next patch, it would be better for me to wait for some more comments. > >> > In that case, I would have set the hardware watchdog to its maximum value >> > and use the softdog to ping it at a rate of, say, 50% of this maximum. >> > >> > If userspace would not ping the watchdog within its configured value, >> > I would stop pinging the hardware watchdog and let it time out. >> >> One more question.Why is the return value of watchdog_ping int? Anyway >> we discard it. > > I can not answer that question. > > Guenter ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] [RFC]Watchdog:core: constant pinging until userspace timesout when delay very less 2013-06-04 3:09 ` anish singh @ 2013-06-05 3:09 ` anish singh 2013-06-06 3:00 ` anish singh 0 siblings, 1 reply; 13+ messages in thread From: anish singh @ 2013-06-05 3:09 UTC (permalink / raw) To: Guenter Roeck, randy.dunlap, alan Cc: Wim Van Sebroeck, linux-watchdog, linux-kernel-mail Hello Wim Van Sabroeck, Can I get your inputs on this? On Tue, Jun 4, 2013 at 8:39 AM, anish singh <anish198519851985@gmail.com> wrote: > On Tue, Jun 4, 2013 at 3:55 AM, Guenter Roeck <linux@roeck-us.net> wrote: >> On Mon, Jun 03, 2013 at 10:23:04PM +0530, anish singh wrote: >>> On Mon, Jun 3, 2013 at 8:57 PM, Guenter Roeck <linux@roeck-us.net> wrote: >>> > On Sun, Jun 02, 2013 at 03:43:07PM +0530, anish kumar wrote: >>> >> Certain watchdog drivers use a timer to keep kicking the watchdog at >>> >> a rate of 0.5s (HZ/2) untill userspace times out.They do this as >>> >> we can't guarantee that watchdog will be pinged fast enough >>> >> for all system loads, especially if timeout is configured for >>> >> less than or equal to 1 second(basically small values). >>> >> >>> >> As suggested by Wim Van Sebroeck & Guenter Roeck we should >>> >> add this functionality of individual watchdog drivers in the core >>> >> watchdog core. >>> >> >>> >> Signed-off-by: anish kumar <anish198519851985@gmail.com> >>> > >>> > Not exactly what I had in mind. My idea was to enable the softdog only if >>> > the hardware watchdog's maximum timeout was low (say, less than a couple >>> > of minutes), and if a timeout larger than its maximum value was configured. >>> >>> watchdog_timeout_invalid wouldn't this check will fail if the user space tries >>> to set maximum timeout more that what driver can support?It would work >>> for pika_wdt.c as it is old watchdog driver and doesn't register with watchdog >>> framwork but new drivers has to pass this api. >>> >>> OR >>> >>> Do you want to remove this check and go as explained by you?I would >>> favour this approach though. >>> >> One would still have a check, but the enforced limits would no longer be >> the driver limits, but larger limits implemented in the watchdog core. > How much larger would be the big question here?Should it be configurable > property(sysfs?) or some hardcoding based on existing drivers? > > Before going for next patch, it would be better for me to wait for some > more comments. >> >>> > In that case, I would have set the hardware watchdog to its maximum value >>> > and use the softdog to ping it at a rate of, say, 50% of this maximum. >>> > >>> > If userspace would not ping the watchdog within its configured value, >>> > I would stop pinging the hardware watchdog and let it time out. >>> >>> One more question.Why is the return value of watchdog_ping int? Anyway >>> we discard it. >> >> I can not answer that question. >> >> Guenter ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] [RFC]Watchdog:core: constant pinging until userspace timesout when delay very less 2013-06-05 3:09 ` anish singh @ 2013-06-06 3:00 ` anish singh 2013-06-06 4:41 ` Guenter Roeck 0 siblings, 1 reply; 13+ messages in thread From: anish singh @ 2013-06-06 3:00 UTC (permalink / raw) To: Guenter Roeck, randy.dunlap, alan, Wim Van Sebroeck Cc: linux-watchdog, linux-kernel-mail Hello Wim Van, Can you look into below? On Wed, Jun 5, 2013 at 8:39 AM, anish singh <anish198519851985@gmail.com> wrote: > Hello Wim Van Sabroeck, > Can I get your inputs on this? > > On Tue, Jun 4, 2013 at 8:39 AM, anish singh <anish198519851985@gmail.com> wrote: >> On Tue, Jun 4, 2013 at 3:55 AM, Guenter Roeck <linux@roeck-us.net> wrote: >>> On Mon, Jun 03, 2013 at 10:23:04PM +0530, anish singh wrote: >>>> On Mon, Jun 3, 2013 at 8:57 PM, Guenter Roeck <linux@roeck-us.net> wrote: >>>> > On Sun, Jun 02, 2013 at 03:43:07PM +0530, anish kumar wrote: >>>> >> Certain watchdog drivers use a timer to keep kicking the watchdog at >>>> >> a rate of 0.5s (HZ/2) untill userspace times out.They do this as >>>> >> we can't guarantee that watchdog will be pinged fast enough >>>> >> for all system loads, especially if timeout is configured for >>>> >> less than or equal to 1 second(basically small values). >>>> >> >>>> >> As suggested by Wim Van Sebroeck & Guenter Roeck we should >>>> >> add this functionality of individual watchdog drivers in the core >>>> >> watchdog core. >>>> >> >>>> >> Signed-off-by: anish kumar <anish198519851985@gmail.com> >>>> > >>>> > Not exactly what I had in mind. My idea was to enable the softdog only if >>>> > the hardware watchdog's maximum timeout was low (say, less than a couple >>>> > of minutes), and if a timeout larger than its maximum value was configured. >>>> >>>> watchdog_timeout_invalid wouldn't this check will fail if the user space tries >>>> to set maximum timeout more that what driver can support?It would work >>>> for pika_wdt.c as it is old watchdog driver and doesn't register with watchdog >>>> framwork but new drivers has to pass this api. >>>> >>>> OR >>>> >>>> Do you want to remove this check and go as explained by you?I would >>>> favour this approach though. >>>> >>> One would still have a check, but the enforced limits would no longer be >>> the driver limits, but larger limits implemented in the watchdog core. >> How much larger would be the big question here?Should it be configurable >> property(sysfs?) or some hardcoding based on existing drivers? >> >> Before going for next patch, it would be better for me to wait for some >> more comments. >>> >>>> > In that case, I would have set the hardware watchdog to its maximum value >>>> > and use the softdog to ping it at a rate of, say, 50% of this maximum. >>>> > >>>> > If userspace would not ping the watchdog within its configured value, >>>> > I would stop pinging the hardware watchdog and let it time out. >>>> >>>> One more question.Why is the return value of watchdog_ping int? Anyway >>>> we discard it. >>> >>> I can not answer that question. >>> >>> Guenter ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] [RFC]Watchdog:core: constant pinging until userspace timesout when delay very less 2013-06-06 3:00 ` anish singh @ 2013-06-06 4:41 ` Guenter Roeck 2013-06-08 13:14 ` anish singh 0 siblings, 1 reply; 13+ messages in thread From: Guenter Roeck @ 2013-06-06 4:41 UTC (permalink / raw) To: anish singh Cc: randy.dunlap, alan, Wim Van Sebroeck, linux-watchdog, linux-kernel-mail On Thu, Jun 06, 2013 at 08:30:01AM +0530, anish singh wrote: > Hello Wim Van, > Can you look into below? > Please be patient. Wim tends to be busy. Guenter > On Wed, Jun 5, 2013 at 8:39 AM, anish singh <anish198519851985@gmail.com> wrote: > > Hello Wim Van Sabroeck, > > Can I get your inputs on this? > > > > On Tue, Jun 4, 2013 at 8:39 AM, anish singh <anish198519851985@gmail.com> wrote: > >> On Tue, Jun 4, 2013 at 3:55 AM, Guenter Roeck <linux@roeck-us.net> wrote: > >>> On Mon, Jun 03, 2013 at 10:23:04PM +0530, anish singh wrote: > >>>> On Mon, Jun 3, 2013 at 8:57 PM, Guenter Roeck <linux@roeck-us.net> wrote: > >>>> > On Sun, Jun 02, 2013 at 03:43:07PM +0530, anish kumar wrote: > >>>> >> Certain watchdog drivers use a timer to keep kicking the watchdog at > >>>> >> a rate of 0.5s (HZ/2) untill userspace times out.They do this as > >>>> >> we can't guarantee that watchdog will be pinged fast enough > >>>> >> for all system loads, especially if timeout is configured for > >>>> >> less than or equal to 1 second(basically small values). > >>>> >> > >>>> >> As suggested by Wim Van Sebroeck & Guenter Roeck we should > >>>> >> add this functionality of individual watchdog drivers in the core > >>>> >> watchdog core. > >>>> >> > >>>> >> Signed-off-by: anish kumar <anish198519851985@gmail.com> > >>>> > > >>>> > Not exactly what I had in mind. My idea was to enable the softdog only if > >>>> > the hardware watchdog's maximum timeout was low (say, less than a couple > >>>> > of minutes), and if a timeout larger than its maximum value was configured. > >>>> > >>>> watchdog_timeout_invalid wouldn't this check will fail if the user space tries > >>>> to set maximum timeout more that what driver can support?It would work > >>>> for pika_wdt.c as it is old watchdog driver and doesn't register with watchdog > >>>> framwork but new drivers has to pass this api. > >>>> > >>>> OR > >>>> > >>>> Do you want to remove this check and go as explained by you?I would > >>>> favour this approach though. > >>>> > >>> One would still have a check, but the enforced limits would no longer be > >>> the driver limits, but larger limits implemented in the watchdog core. > >> How much larger would be the big question here?Should it be configurable > >> property(sysfs?) or some hardcoding based on existing drivers? > >> > >> Before going for next patch, it would be better for me to wait for some > >> more comments. > >>> > >>>> > In that case, I would have set the hardware watchdog to its maximum value > >>>> > and use the softdog to ping it at a rate of, say, 50% of this maximum. > >>>> > > >>>> > If userspace would not ping the watchdog within its configured value, > >>>> > I would stop pinging the hardware watchdog and let it time out. > >>>> > >>>> One more question.Why is the return value of watchdog_ping int? Anyway > >>>> we discard it. > >>> > >>> I can not answer that question. > >>> > >>> Guenter > -- > To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] [RFC]Watchdog:core: constant pinging until userspace timesout when delay very less 2013-06-06 4:41 ` Guenter Roeck @ 2013-06-08 13:14 ` anish singh 0 siblings, 0 replies; 13+ messages in thread From: anish singh @ 2013-06-08 13:14 UTC (permalink / raw) To: Guenter Roeck Cc: randy.dunlap, alan, Wim Van Sebroeck, linux-watchdog, linux-kernel-mail On Thu, Jun 6, 2013 at 10:11 AM, Guenter Roeck <linux@roeck-us.net> wrote: > On Thu, Jun 06, 2013 at 08:30:01AM +0530, anish singh wrote: >> Hello Wim Van, >> Can you look into below? >> > Please be patient. Wim tends to be busy. Sorry, I will wait. > > Guenter > >> On Wed, Jun 5, 2013 at 8:39 AM, anish singh <anish198519851985@gmail.com> wrote: >> > Hello Wim Van Sabroeck, >> > Can I get your inputs on this? >> > >> > On Tue, Jun 4, 2013 at 8:39 AM, anish singh <anish198519851985@gmail.com> wrote: >> >> On Tue, Jun 4, 2013 at 3:55 AM, Guenter Roeck <linux@roeck-us.net> wrote: >> >>> On Mon, Jun 03, 2013 at 10:23:04PM +0530, anish singh wrote: >> >>>> On Mon, Jun 3, 2013 at 8:57 PM, Guenter Roeck <linux@roeck-us.net> wrote: >> >>>> > On Sun, Jun 02, 2013 at 03:43:07PM +0530, anish kumar wrote: >> >>>> >> Certain watchdog drivers use a timer to keep kicking the watchdog at >> >>>> >> a rate of 0.5s (HZ/2) untill userspace times out.They do this as >> >>>> >> we can't guarantee that watchdog will be pinged fast enough >> >>>> >> for all system loads, especially if timeout is configured for >> >>>> >> less than or equal to 1 second(basically small values). >> >>>> >> >> >>>> >> As suggested by Wim Van Sebroeck & Guenter Roeck we should >> >>>> >> add this functionality of individual watchdog drivers in the core >> >>>> >> watchdog core. >> >>>> >> >> >>>> >> Signed-off-by: anish kumar <anish198519851985@gmail.com> >> >>>> > >> >>>> > Not exactly what I had in mind. My idea was to enable the softdog only if >> >>>> > the hardware watchdog's maximum timeout was low (say, less than a couple >> >>>> > of minutes), and if a timeout larger than its maximum value was configured. >> >>>> >> >>>> watchdog_timeout_invalid wouldn't this check will fail if the user space tries >> >>>> to set maximum timeout more that what driver can support?It would work >> >>>> for pika_wdt.c as it is old watchdog driver and doesn't register with watchdog >> >>>> framwork but new drivers has to pass this api. >> >>>> >> >>>> OR >> >>>> >> >>>> Do you want to remove this check and go as explained by you?I would >> >>>> favour this approach though. >> >>>> >> >>> One would still have a check, but the enforced limits would no longer be >> >>> the driver limits, but larger limits implemented in the watchdog core. >> >> How much larger would be the big question here?Should it be configurable >> >> property(sysfs?) or some hardcoding based on existing drivers? >> >> >> >> Before going for next patch, it would be better for me to wait for some >> >> more comments. >> >>> >> >>>> > In that case, I would have set the hardware watchdog to its maximum value >> >>>> > and use the softdog to ping it at a rate of, say, 50% of this maximum. >> >>>> > >> >>>> > If userspace would not ping the watchdog within its configured value, >> >>>> > I would stop pinging the hardware watchdog and let it time out. >> >>>> >> >>>> One more question.Why is the return value of watchdog_ping int? Anyway >> >>>> we discard it. >> >>> >> >>> I can not answer that question. >> >>> >> >>> Guenter >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] [RFC]Watchdog:core: constant pinging until userspace timesout when delay very less 2013-06-02 10:13 [PATCH] [RFC]Watchdog:core: constant pinging until userspace timesout when delay very less anish kumar 2013-06-02 10:26 ` anish singh 2013-06-03 15:27 ` Guenter Roeck @ 2013-06-27 19:31 ` Wim Van Sebroeck 2013-06-28 3:31 ` anish singh 2 siblings, 1 reply; 13+ messages in thread From: Wim Van Sebroeck @ 2013-06-27 19:31 UTC (permalink / raw) To: anish kumar; +Cc: linux, linux-watchdog, linux-kernel Hi Anish, > Certain watchdog drivers use a timer to keep kicking the watchdog at > a rate of 0.5s (HZ/2) untill userspace times out.They do this as > we can't guarantee that watchdog will be pinged fast enough > for all system loads, especially if timeout is configured for > less than or equal to 1 second(basically small values). > > As suggested by Wim Van Sebroeck & Guenter Roeck we should > add this functionality of individual watchdog drivers in the core > watchdog core. Have you considered the effect this change has on all watchdog drivers that don't need a timer? Kind regards, Wim. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] [RFC]Watchdog:core: constant pinging until userspace timesout when delay very less 2013-06-27 19:31 ` Wim Van Sebroeck @ 2013-06-28 3:31 ` anish singh 0 siblings, 0 replies; 13+ messages in thread From: anish singh @ 2013-06-28 3:31 UTC (permalink / raw) To: Wim Van Sebroeck; +Cc: Guenter Roeck, linux-watchdog, linux-kernel-mail On Fri, Jun 28, 2013 at 1:01 AM, Wim Van Sebroeck <wim@iguana.be> wrote: > Hi Anish, > >> Certain watchdog drivers use a timer to keep kicking the watchdog at >> a rate of 0.5s (HZ/2) untill userspace times out.They do this as >> we can't guarantee that watchdog will be pinged fast enough >> for all system loads, especially if timeout is configured for >> less than or equal to 1 second(basically small values). >> >> As suggested by Wim Van Sebroeck & Guenter Roeck we should >> add this functionality of individual watchdog drivers in the core >> watchdog core. > > Have you considered the effect this change has on all watchdog drivers > that don't need a timer? No,I will drop this patch. > > Kind regards, > Wim. > ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2013-06-28 3:31 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-06-02 10:13 [PATCH] [RFC]Watchdog:core: constant pinging until userspace timesout when delay very less anish kumar 2013-06-02 10:26 ` anish singh 2013-06-03 14:38 ` anish singh 2013-06-03 15:27 ` Guenter Roeck 2013-06-03 16:53 ` anish singh 2013-06-03 22:25 ` Guenter Roeck 2013-06-04 3:09 ` anish singh 2013-06-05 3:09 ` anish singh 2013-06-06 3:00 ` anish singh 2013-06-06 4:41 ` Guenter Roeck 2013-06-08 13:14 ` anish singh 2013-06-27 19:31 ` Wim Van Sebroeck 2013-06-28 3:31 ` anish singh
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).