Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1279404 > unrolled thread

[PATCH v6 0/9] watchdog: Add support for keepalives triggered by infrastructure

Back to article view | Back to linux.kernel

Contents

  [PATCH v6 0/9] watchdog: Add support for keepalives triggered by infrastructure Guenter Roeck <linux@roeck-us.net> - 2015-11-29 20:40 +0100
    [PATCH v6 3/9] watchdog: Make set_timeout function optional Guenter Roeck <linux@roeck-us.net> - 2015-11-29 20:40 +0100
    [PATCH v6 4/9] watchdog: Add support for minimum time between heartbeats Guenter Roeck <linux@roeck-us.net> - 2015-11-29 20:40 +0100

#1279404 — [PATCH v6 0/9] watchdog: Add support for keepalives triggered by infrastructure

The watchdog infrastructure is currently purely passive, meaning
it only passes information from user space to drivers and vice versa.

Since watchdog hardware tends to have its own quirks, this can result
in quite complex watchdog drivers. A number of scanarios are especially common.

- A watchdog is always active and can not be disabled, or can not be disabled
  once enabled. To support such hardware, watchdog drivers have to implement
  their own timers and use those timers to trigger watchdog keepalives while
  the watchdog device is not or not yet opened.
- A variant of this is the desire to enable a watchdog as soon as its driver
  has been instantiated, to protect the system while it is still booting up,
  but the watchdog daemon is not yet running.
- Some watchdogs have a very short maximum timeout, in the range of just a few
  seconds. Such low timeouts are difficult if not impossible to support from
  user space. Drivers supporting such watchdog hardware need to implement
  a timer function to augment heartbeats from user space.

This patch set solves the above problems while keeping changes to the
watchdog core minimal.

- A new status flag, WDOG_RUNNING, informs the watchdog subsystem that a
  watchdog is running, and that the watchdog subsystem needs to generate
  heartbeat requests while the associated watchdog device is closed.
- A new parameter in the watchdog data structure, max_hw_timeout_ms, informs
  the watchdog subsystem about a maximum hardware timeout. The watchdog
  subsystem uses this information together with the configured timeout
  and the maximum permitted timeout to determine if it needs to generate
  additional heartbeat requests.

As part of this patchset, the semantics of the 'timeout' variable and of
the WDOG_ACTIVE flag are changed slightly.

Per the current watchdog kernel API, the 'timeout' variable is supposed
to reflect the actual hardware watcdog timeout. WDOG_ACTIVE is supposed
to reflect if the hardware watchdog is running or not.

Unfortunately, this does not always reflect reality. In drivers which solve
the above mentioned problems internally, 'timeout' is the watchdog timeout
as seen from user space, and WDOG_ACTIVE reflects that user space is expected
to send keepalive requests to the watchdog driver.

After this patch set is applied, this so far inofficial interpretation
is the 'official' semantics for the timeout variable and the WDOG_ACTIVE
flag. In other words, both values no longer reflect the hardware watchdog
status, but its status as seen from user space.

Patch #1 adds timer functionality to the watchdog core. It solves the problem
of short maximum hardware timeouts by augmenting heartbeats triggered from
user space with internally triggered heartbeats.

Patch #2 adds functionality to generate heartbeats while the watchdog device is
closed. It handles situation where where the watchdog is running after
the driver has been instantiated, but the device is not yet opened,
and post-close situations necessary if a watchdog can not be stopped.

Patch #3 makes the set_timeout function optional. This is now possible since
timeout changes can now be completely handled in the watchdog core, for
example if the hardware watchdog timeout is fixed.

Patch #4 adds code to unconditionally ensure that the minimum timeout meets
constraints provided by the watchdog driver.

Patch #5 simplifies the watchdog_update_worker() function introduced with
patch #1 to only take a single argument, and always cancel any pending work
if a worker is not or no longer needed. This patch is kept as separate
patch on purpose, to enable dropping or reverting it easily if it causes
any problems. It should not cause any problems; this is just out of an
abundance of caution.

Patch #6 to #9 are example conversions of some watchdog drivers.
Those patches will require testing and are marked as RFT.

The patch set is also available in branch watchdog-timer of
git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git.
In this branch, the series is rebased on top of the watchdog-next branch
in the same repository. This was done since other pending changes in the
watchdog subsystem cause merge conflicts, and we want to be able to test
all pending changes together.

The patch set was inspired by an earlier patch set from Timo Kokonnen.

v6:
- Added patch #9, converting the dw_wdt driver.
  Set last_keepalive more accurately when starting the watchdog.
  Rebased to v4.4-rc2.
  Renamed WDOG_RUNNING to WDOG_HW_RUNNING and watchdog_running() to
  watchdog_hw_running().
v5:
- Patches #1 and #2 of the original patch series are now in mainline
  and have been dropped.
- Rebased to v4.4-rc1. 
- Added patch to simplify watchdog_update_worker().
v4:
- Rebased to v4.3-rc3
- Rearranged patch sequence
- Dropped gpio driver patch. The driver was changed since v4.2,
  and merging the changes turned out to be too difficult.
- Various other cleanups as listed in individual patches
v3:
- Rebased to v4.2-rc8
- Reworked and cleaned up some of the functions.
- No longer call the worker update function if all that is needed is to stop
  the worker.
- max_timeout will now be ignored if max_hw_timeout_ms is provided.
- Added patch 9/9.
v2:
- Rebased to v4.2-rc5
- Improved and hopefully clarified documentation.
- Rearranged variables in struct watchdog_device such that internal variables
  come last.
- The code now ensures that the watchdog times out <timeout> seconds after
  the most recent keepalive sent from user space.
- The internal keepalive now stops silently and no longer generates a
  warning message. Reason is that it will now stop early, while there
  may still be a substantial amount of time for keepalives from user space
  to arrive. If such keepalives arrive late (for example if user space
  is configured to send keepalives just a few seconds before the watchdog
  times out), the message would just be noise and not provide any value.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1279405 — [PATCH v6 3/9] watchdog: Make set_timeout function optional

For some watchdogs, the hardware timeout is fixed, and the
watchdog driver depends on the watchdog core to handle the
actual timeout. In this situation, the watchdog driver might
only set the 'timeout' variable but do nothing else.
This can as well be handled by the infrastructure, so make
the set_timeout callback optional. If WDIOF_SETTIMEOUT is
configured but the .set_timeout callback is not available,
update the timeout variable in the infrastructure code.

Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v6: Rebased to v4.4-rc2
v5: Rebased to v4.4-rc1
v4: No changes
v3: No changes
v2: No changes
---
 Documentation/watchdog/watchdog-kernel-api.txt | 5 +++++
 drivers/watchdog/watchdog_dev.c                | 9 ++++++---
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/Documentation/watchdog/watchdog-kernel-api.txt b/Documentation/watchdog/watchdog-kernel-api.txt
index 44787ddd59de..f2ee8226d320 100644
--- a/Documentation/watchdog/watchdog-kernel-api.txt
+++ b/Documentation/watchdog/watchdog-kernel-api.txt
@@ -184,6 +184,11 @@ they are supported. These optional routines/operations are:
   (if it is larger than hw_max_timeout_ms), or to the achieved timeout value.
   (Note: the WDIOF_SETTIMEOUT needs to be set in the options field of the
   watchdog's info structure).
+  If the watchdog driver does not have to perform any action but setting the
+  watchdog_device.timeout, this callback can be omitted.
+  If set_timeout is not provided but, WDIOF_SETTIMEOUT is set, the watchdog
+  infrastructure updates the timeout value of the watchdog_device internally
+  to the requested value.
 * get_timeleft: this routines returns the time that's left before a reset.
 * ref: the operation that calls kref_get on the kref of a dynamically
   allocated watchdog_device struct.
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 2c99c3bb2fcf..5dd7319c0169 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -284,9 +284,9 @@ out_status:
 static int watchdog_set_timeout(struct watchdog_device *wdd,
 							unsigned int timeout)
 {
-	int err;
+	int err = 0;
 
-	if (!wdd->ops->set_timeout || !(wdd->info->options & WDIOF_SETTIMEOUT))
+	if (!(wdd->info->options & WDIOF_SETTIMEOUT))
 		return -EOPNOTSUPP;
 
 	if (watchdog_timeout_invalid(wdd, timeout))
@@ -299,7 +299,10 @@ static int watchdog_set_timeout(struct watchdog_device *wdd,
 		goto out_timeout;
 	}
 
-	err = wdd->ops->set_timeout(wdd, timeout);
+	if (wdd->ops->set_timeout)
+		err = wdd->ops->set_timeout(wdd, timeout);
+	else
+		wdd->timeout = timeout;
 
 	watchdog_update_worker(wdd, true);
 
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1279406 — [PATCH v6 4/9] watchdog: Add support for minimum time between heartbeats

Some watchdogs require a minimum time between heartbeats.
Examples are the watchdogs in DA9062 and AT91SAM9x.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>

---
v6: Rebased to v4.4-rc2
v5: Rebased to v4.4-rc1
    Fixed typo in documentation.
v4: Added patch
---
 Documentation/watchdog/watchdog-kernel-api.txt |  4 ++++
 drivers/watchdog/watchdog_dev.c                | 12 ++++++++++++
 include/linux/watchdog.h                       |  6 ++++++
 3 files changed, 22 insertions(+)

diff --git a/Documentation/watchdog/watchdog-kernel-api.txt b/Documentation/watchdog/watchdog-kernel-api.txt
index f2ee8226d320..b417fdb6caac 100644
--- a/Documentation/watchdog/watchdog-kernel-api.txt
+++ b/Documentation/watchdog/watchdog-kernel-api.txt
@@ -53,11 +53,13 @@ struct watchdog_device {
 	unsigned int timeout;
 	unsigned int min_timeout;
 	unsigned int max_timeout;
+	unsigned int min_hw_heartbeat_ms;
 	unsigned int max_hw_timeout_ms;
 	void *driver_data;
 	unsigned long status;
 	struct mutex lock;
 	unsigned long last_keepalive;
+	unsigned long last_hw_keepalive;
 	struct delayed_work work;
 	struct list_head deferred;
 };
@@ -83,6 +85,8 @@ It contains following fields:
 * max_timeout: the watchdog timer's maximum timeout value (in seconds),
   as seen from userspace. If set, the maximum configurable value for
   'timeout'. Not used if max_hw_timeout_ms is non-zero.
+* min_hw_heartbeat_ms: Minimum time between heartbeats sent to the chip,
+  in milli-seconds.
 * max_hw_timeout_ms: Maximum hardware timeout, in milli-seconds.
   If set, the infrastructure will send heartbeats to the watchdog driver
   if 'timeout' is larger than max_hw_timeout, unless WDOG_ACTIVE
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 5dd7319c0169..82706d43e0c8 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -111,6 +111,8 @@ static inline void watchdog_update_worker(struct watchdog_device *wdd,
 
 static int _watchdog_ping(struct watchdog_device *wdd)
 {
+	unsigned long earliest_keepalive = wdd->last_hw_keepalive +
+				msecs_to_jiffies(wdd->min_hw_heartbeat_ms);
 	int err;
 
 	if (test_bit(WDOG_UNREGISTERED, &wdd->status))
@@ -119,6 +121,13 @@ static int _watchdog_ping(struct watchdog_device *wdd)
 	if (!watchdog_active(wdd) && !watchdog_hw_running(wdd))
 		return 0;
 
+	if (time_is_after_jiffies(earliest_keepalive)) {
+		mod_delayed_work(watchdog_wq, &wdd->work,
+				 earliest_keepalive - jiffies);
+		return 0;
+	}
+
+	wdd->last_hw_keepalive = jiffies;
 	if (wdd->ops->ping)
 		err = wdd->ops->ping(wdd);  /* ping the watchdog */
 	else
@@ -667,6 +676,9 @@ int watchdog_dev_register(struct watchdog_device *wdd)
 		return err;
 	}
 
+	/* Record time of most recent heartbeat as 'just before now'. */
+	wdd->last_hw_keepalive = jiffies - 1;
+
 	/*
 	 * If the watchdog is running, prevent its driver from being unloaded,
 	 * and schedule an immediate ping.
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index 98f79db93600..606dfd55927f 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -66,6 +66,8 @@ struct watchdog_ops {
  * @max_timeout:The watchdog devices maximum timeout value (in seconds)
  *		as configurable from user space. Only relevant if
  *		max_hw_timeout_ms is not provided.
+ * @min_hw_heartbeat_ms:
+ *		Minimum time between heartbeats, in milli-seconds.
  * @max_hw_timeout_ms:
  *		Hardware limit for maximum timeout, in milli-seconds.
  *		Replaces max_timeout if specified.
@@ -75,6 +77,8 @@ struct watchdog_ops {
  * @last_keepalive:
  *		Time of most recent keepalive triggered from user space,
  *		in jiffies (watchdog core internal).
+ * @last_hw_keepalive:
+ *		Time of most recent keepalive sent to the driver, in jiffies.
  * @work:	Data structure for worker function (watchdog core internal).
  * @deferred:	entry in wtd_deferred_reg_list which is used to
  *		register early initialized watchdogs.
@@ -99,6 +103,7 @@ struct watchdog_device {
 	unsigned int timeout;
 	unsigned int min_timeout;
 	unsigned int max_timeout;
+	unsigned int min_hw_heartbeat_ms;
 	unsigned int max_hw_timeout_ms;
 	void *driver_data;
 	unsigned long status;
@@ -112,6 +117,7 @@ struct watchdog_device {
 	/* the following variables are for internal use only */
 	struct mutex lock;
 	unsigned long last_keepalive;
+	unsigned long last_hw_keepalive;
 	struct delayed_work work;
 	struct list_head deferred;
 };
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web