[PATCH 0/7] watchdog: add pretimeout support

[PATCH 0/7] watchdog: add pretimeout support

[Wolfram Sang]

Okay, here is my approach of adding pretimeout support to the watchdog core. It
is based on work of Vladimir Zapolskiy and Robin Gong (Thanks!) yet rebased
(quite some core changes since then) and reworked significantly. Check the
patch descriptions for changes. I also added pretimeout support to the softdog
because the board in question did not have hardware support for pretimeouts.

Note that in this series the userspace governor is not included. I will send
out an RFC series with it after this one. I expect more discussion on my
approach there while I think these patches could be ready to go. They work fine
here, so the rest may done incrementally?

Here is a q'n'd patch for the busybox 'watchdog' command for easier testing:

Index: b/miscutils/watchdog.c
===================================================================
--- a/miscutils/watchdog.c
+++ b/miscutils/watchdog.c
@@ -10,11 +10,12 @@
  */
 
 //usage:#define watchdog_trivial_usage
-//usage:       "[-t N[ms]] [-T N[ms]] [-F] DEV"
+//usage:       "[-t N[ms]] [-T N[ms]] [-P N[ms]] [-F] DEV"
 //usage:#define watchdog_full_usage "\n\n"
 //usage:       "Periodically write to watchdog device DEV\n"
 //usage:     "\n	-T N	Reboot after N seconds if not reset (default 60)"
 //usage:     "\n	-t N	Reset every N seconds (default 30)"
+//usage:     "\n	-P N	Pretimeout warning N seconds before reboot (default 0)"
 //usage:     "\n	-F	Run in foreground"
 //usage:     "\n"
 //usage:     "\nUse 500ms to specify period in milliseconds"
@@ -26,6 +27,7 @@
 #define OPT_FOREGROUND  (1 << 0)
 #define OPT_STIMER      (1 << 1)
 #define OPT_HTIMER      (1 << 2)
+#define OPT_PTIMER      (1 << 3)
 
 static void watchdog_shutdown(int sig UNUSED_PARAM)
 {
@@ -50,11 +52,13 @@ int watchdog_main(int argc, char **argv)
 	unsigned opts;
 	unsigned stimer_duration; /* how often to restart */
 	unsigned htimer_duration = 60000; /* reboots after N ms if not restarted */
+	unsigned ptimer_duration = 0; /* pre-timeout notification N ms before reboot */
 	char *st_arg;
 	char *ht_arg;
+	char *pt_arg;
 
 	opt_complementary = "=1"; /* must have exactly 1 argument */
-	opts = getopt32(argv, "Ft:T:", &st_arg, &ht_arg);
+	opts = getopt32(argv, "Ft:T:P:", &st_arg, &ht_arg, &pt_arg);
 
 	/* We need to daemonize *before* opening the watchdog as many drivers
 	 * will only allow one process at a time to do so.  Since daemonizing
@@ -88,6 +92,10 @@ int watchdog_main(int argc, char **argv)
 	}
 # endif
 	ioctl_or_warn(3, WDIOC_SETTIMEOUT, &htimer_duration);
+	if (opts & OPT_PTIMER) {
+		ptimer_duration = xatou_sfx(pt_arg, suffixes) / 1000;
+		ioctl_or_warn(3, WDIOC_SETPRETIMEOUT, &ptimer_duration);
+	}
 #endif
 
 #if 0



Please test, review, comment, apply...

Thanks,

   Wolfram

Vladimir Zapolskiy (2):
  watchdog: pretimeout: add panic pretimeout governor
  watchdog: pretimeout: add noop pretimeout governor

Wolfram Sang (5):
  watchdog: add watchdog pretimeout framework
  watchdog: documentation: squash paragraphs about 'no set_timeout'
  watchdog: add WDIOC_SETPRETIMEOUT and WDIOC_GETPRETIMEOUT
  fs: compat_ioctl: add pretimeout functions for watchdogs
  watchdog: softdog: implement pretimeout support

 Documentation/watchdog/watchdog-kernel-api.txt |  24 ++-
 drivers/watchdog/Kconfig                       |  52 +++++
 drivers/watchdog/Makefile                      |   9 +-
 drivers/watchdog/pretimeout_noop.c             |  47 +++++
 drivers/watchdog/pretimeout_panic.c            |  47 +++++
 drivers/watchdog/softdog.c                     |  20 +-
 drivers/watchdog/watchdog_dev.c                |  61 +++++-
 drivers/watchdog/watchdog_pretimeout.c         | 269 +++++++++++++++++++++++++
 drivers/watchdog/watchdog_pretimeout.h         |  41 ++++
 fs/compat_ioctl.c                              |   2 +
 include/linux/watchdog.h                       |  21 ++
 11 files changed, 586 insertions(+), 7 deletions(-)
 create mode 100644 drivers/watchdog/pretimeout_noop.c
 create mode 100644 drivers/watchdog/pretimeout_panic.c
 create mode 100644 drivers/watchdog/watchdog_pretimeout.c
 create mode 100644 drivers/watchdog/watchdog_pretimeout.h

-- 
2.8.1

[PATCH 1/7] watchdog: add watchdog pretimeout framework

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

The change adds a simple watchdog pretimeout framework infrastructure,
its purpose is to allow users to select a desired handling of watchdog
pretimeout events, which may be generated by a watchdog driver.

By design every watchdog pretimeout governor may be compiled as a
kernel module, a user selects a default watchdog pretimeout
governor during compilation stage and can select another governor in
runtime.

Watchdogs with WDIOF_PRETIMEOUT capability now have two device
attributes in sysfs: read/write pretimeout_governor attribute and read
only pretimeout_available_governors attribute.

Watchdogs with no WDIOF_PRETIMEOUT capability has no changes in
sysfs.

Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

Changes since Vladimir's last version:

* rebased
  adapt to the internal data reorganization, especially the now private
  struct device *dev
* dropped can_sleep support!
  The additional lock, list, and workqueue made the code quite complex. The
  only user was the userspace governor which can be reworked to let the
  watchdog device code do the bottom half. In addition, I am not fully
  convinced sending a uevent is the proper thing to do, but this needs to
  be discussed in another thread. Removing this support makes the code much
  easier to follow (locking!), saves 30% of LoC + a list + a workqueue.
* moved pretimeout registration from watchdog_core to watchdog_dev
  Let's handle it exactly where the device is created, so we have access to
  the now private device pointer for adding the sysfs files.
* don't export watchdog_(un)register_pretimeout since they are linked to the
  core anyhow
* whitespace cleanups

 drivers/watchdog/Kconfig               |   8 +
 drivers/watchdog/Makefile              |   6 +-
 drivers/watchdog/watchdog_dev.c        |   8 +
 drivers/watchdog/watchdog_pretimeout.c | 269 +++++++++++++++++++++++++++++++++
 drivers/watchdog/watchdog_pretimeout.h |  35 +++++
 include/linux/watchdog.h               |  10 ++
 6 files changed, 334 insertions(+), 2 deletions(-)
 create mode 100644 drivers/watchdog/watchdog_pretimeout.c
 create mode 100644 drivers/watchdog/watchdog_pretimeout.h

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 3902c9ca7f099d..909d1021de5cbc 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1774,4 +1774,12 @@ config USBPCWATCHDOG
 
 	  Most people will say N.
 
+comment "Watchdog Pretimeout Governors"
+
+config WATCHDOG_PRETIMEOUT_GOV
+	bool "Enable watchdog pretimeout governors"
+	default n
+	help
+	  The option allows to select watchdog pretimeout governors.
+
 endif # WATCHDOG
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 2cbc9709852d0e..820860cf3e8d62 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -3,8 +3,10 @@
 #
 
 # The WatchDog Timer Driver Core.
-watchdog-objs	+= watchdog_core.o watchdog_dev.o
-obj-$(CONFIG_WATCHDOG_CORE)	+= watchdog.o
+obj-$(CONFIG_WATCHDOG_CORE) += watchdog.o
+
+watchdog-y += watchdog_core.o watchdog_dev.o
+watchdog-$(CONFIG_WATCHDOG_PRETIMEOUT_GOV) += watchdog_pretimeout.o
 
 # Only one watchdog can succeed. We probe the ISA/PCI/USB based
 # watchdog-cards first, then the architecture specific watchdog
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 3595cffa24ea49..5d028f94a90743 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -49,6 +49,7 @@
 #include <linux/uaccess.h>	/* For copy_to_user/put_user/... */
 
 #include "watchdog_core.h"
+#include "watchdog_pretimeout.h"
 
 /*
  * struct watchdog_core_data - watchdog core internal data
@@ -911,6 +912,12 @@ int watchdog_dev_register(struct watchdog_device *wdd)
 		return PTR_ERR(dev);
 	}
 
+	ret = watchdog_register_pretimeout(wdd, dev);
+	if (ret) {
+		device_destroy(&watchdog_class, devno);
+		watchdog_cdev_unregister(wdd);
+	}
+
 	return ret;
 }
 
@@ -924,6 +931,7 @@ int watchdog_dev_register(struct watchdog_device *wdd)
 
 void watchdog_dev_unregister(struct watchdog_device *wdd)
 {
+	watchdog_unregister_pretimeout(wdd);
 	device_destroy(&watchdog_class, wdd->wd_data->cdev.dev);
 	watchdog_cdev_unregister(wdd);
 }
diff --git a/drivers/watchdog/watchdog_pretimeout.c b/drivers/watchdog/watchdog_pretimeout.c
new file mode 100644
index 00000000000000..87a10ebeaacc7e
--- /dev/null
+++ b/drivers/watchdog/watchdog_pretimeout.c
@@ -0,0 +1,269 @@
+/*
+ * Watchdog pretimout governor framework
+ *
+ * Copyright (C) 2015 Mentor Graphics
+ * Copyright (C) 2016 Renesas Electronics Corporation
+ * Copyright (C) 2016 Sang Engineering, Wolfram Sang
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/watchdog.h>
+
+#include "watchdog_pretimeout.h"
+
+/* The mutex protects governor list and serializes external interfaces */
+static DEFINE_MUTEX(governor_lock);
+
+/* List of registered watchdog pretimeout governors */
+static LIST_HEAD(governor_list);
+
+/* The spinlock protects wdd->gov */
+static DEFINE_SPINLOCK(pretimeout_lock);
+
+struct governor_priv {
+	struct watchdog_governor *gov;
+	bool is_default;
+	struct list_head entry;
+};
+
+static struct governor_priv *find_governor_by_name(const char *gov_name)
+{
+	struct governor_priv *priv;
+
+	list_for_each_entry(priv, &governor_list, entry)
+		if (!strncmp(gov_name, priv->gov->name, WATCHDOG_GOV_NAME_LEN))
+			return priv;
+
+	return NULL;
+}
+
+static struct watchdog_governor *find_default_governor(void)
+{
+	struct governor_priv *priv;
+
+	list_for_each_entry(priv, &governor_list, entry)
+		if (priv->is_default)
+			return priv->gov;
+
+	return NULL;
+}
+
+void watchdog_notify_pretimeout(struct watchdog_device *wdd)
+{
+	unsigned long flags;
+
+	if (!wdd)
+		return;
+
+	spin_lock_irqsave(&pretimeout_lock, flags);
+
+	if (wdd->gov)
+		wdd->gov->pretimeout(wdd);
+
+	spin_unlock_irqrestore(&pretimeout_lock, flags);
+}
+EXPORT_SYMBOL_GPL(watchdog_notify_pretimeout);
+
+int watchdog_register_governor(struct watchdog_governor *gov)
+{
+	struct governor_priv	*priv;
+
+	if (!gov || !gov->pretimeout || strlen(gov->name) >= WATCHDOG_GOV_NAME_LEN)
+		return -EINVAL;
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	mutex_lock(&governor_lock);
+
+	if (find_governor_by_name(gov->name)) {
+		kfree(priv);
+		mutex_unlock(&governor_lock);
+		return -EBUSY;
+	}
+
+	priv->gov = gov;
+
+	if (!strncmp(WATCHDOG_PRETIMEOUT_DEFAULT_GOV,
+		     gov->name, WATCHDOG_GOV_NAME_LEN)) {
+		priv->is_default = true;
+	}
+
+	list_add(&priv->entry, &governor_list);
+
+	mutex_unlock(&governor_lock);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(watchdog_register_governor);
+
+void watchdog_unregister_governor(struct watchdog_governor *gov)
+{
+	struct governor_priv *priv;
+
+	if (!gov)
+		return;
+
+	mutex_lock(&governor_lock);
+
+	list_for_each_entry(priv, &governor_list, entry) {
+		if (priv->gov == gov) {
+			list_del(&priv->entry);
+			kfree(priv);
+			break;
+		}
+	}
+
+	mutex_unlock(&governor_lock);
+}
+EXPORT_SYMBOL_GPL(watchdog_unregister_governor);
+
+static ssize_t pretimeout_governor_store(struct device *dev,
+					 struct device_attribute *attr,
+					 const char *buf, size_t count)
+{
+	struct watchdog_device *wdd = dev_get_drvdata(dev);
+	struct governor_priv *priv;
+
+	mutex_lock(&governor_lock);
+
+	priv = find_governor_by_name(buf);
+	if (!priv) {
+		mutex_unlock(&governor_lock);
+		return -EINVAL;
+	}
+
+	if (!try_module_get(priv->gov->owner)) {
+		mutex_unlock(&governor_lock);
+		return -ENODEV;
+	}
+
+	spin_lock_irq(&pretimeout_lock);
+	if (wdd->gov)
+		module_put(wdd->gov->owner);
+	wdd->gov = priv->gov;
+	spin_unlock_irq(&pretimeout_lock);
+
+	mutex_unlock(&governor_lock);
+
+	return count;
+}
+
+static ssize_t pretimeout_governor_show(struct device *dev,
+				struct device_attribute *devattr, char *buf)
+{
+	struct watchdog_device *wdd = dev_get_drvdata(dev);
+	char gov_name[WATCHDOG_GOV_NAME_LEN] = { 0 };
+	int count;
+
+	mutex_lock(&governor_lock);
+
+	spin_lock_irq(&pretimeout_lock);
+	if (wdd->gov)
+		strncpy(gov_name, wdd->gov->name, WATCHDOG_GOV_NAME_LEN);
+	spin_unlock_irq(&pretimeout_lock);
+
+	count = sprintf(buf, "%s\n", gov_name);
+
+	mutex_unlock(&governor_lock);
+
+	return count;
+}
+static DEVICE_ATTR_RW(pretimeout_governor);
+
+static ssize_t pretimeout_available_governors_show(struct device *dev,
+				struct device_attribute *devattr, char *buf)
+{
+	struct governor_priv *priv;
+	int count = 0;
+
+	mutex_lock(&governor_lock);
+
+	list_for_each_entry(priv, &governor_list, entry)
+		count += sprintf(buf + count, "%s\n", priv->gov->name);
+
+	mutex_unlock(&governor_lock);
+
+	return count;
+}
+static DEVICE_ATTR_RO(pretimeout_available_governors);
+
+static struct attribute *wdd_pretimeout_attrs[] = {
+	&dev_attr_pretimeout_governor.attr,
+	&dev_attr_pretimeout_available_governors.attr,
+	NULL,
+};
+
+static umode_t wdd_pretimeout_attr_is_visible(struct kobject *kobj,
+				   struct attribute *attr, int n)
+{
+	struct device *dev = container_of(kobj, struct device, kobj);
+	struct watchdog_device *wdd = dev_get_drvdata(dev);
+
+	if (wdd->info->options & WDIOF_PRETIMEOUT)
+		return attr->mode;
+
+	return 0;
+}
+
+static const struct attribute_group wdd_pretimeout_group = {
+	.is_visible = wdd_pretimeout_attr_is_visible,
+	.attrs = wdd_pretimeout_attrs,
+};
+
+static const struct attribute_group *wdd_pretimeout_groups[] = {
+	&wdd_pretimeout_group,
+	NULL,
+};
+
+int watchdog_register_pretimeout(struct watchdog_device *wdd, struct device *dev)
+{
+	struct watchdog_governor *gov;
+	int ret;
+
+	if (!(wdd->info->options & WDIOF_PRETIMEOUT))
+		return 0;
+
+	ret = sysfs_create_groups(&dev->kobj, wdd_pretimeout_groups);
+	if (ret)
+		return ret;
+
+	mutex_lock(&governor_lock);
+	gov = find_default_governor();
+	if (gov && !try_module_get(gov->owner)) {
+		mutex_unlock(&governor_lock);
+		return -ENODEV;
+	}
+
+	spin_lock_irq(&pretimeout_lock);
+	wdd->gov = gov;
+	spin_unlock_irq(&pretimeout_lock);
+
+	mutex_unlock(&governor_lock);
+
+	return 0;
+}
+
+void watchdog_unregister_pretimeout(struct watchdog_device *wdd)
+{
+	if (!(wdd->info->options & WDIOF_PRETIMEOUT))
+		return;
+
+	spin_lock_irq(&pretimeout_lock);
+	if (wdd->gov)
+		module_put(wdd->gov->owner);
+	wdd->gov = NULL;
+	spin_unlock_irq(&pretimeout_lock);
+}
+
+MODULE_AUTHOR("Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>");
+MODULE_DESCRIPTION("Watchdog pretimeout governor framework");
+MODULE_LICENSE("GPL");
diff --git a/drivers/watchdog/watchdog_pretimeout.h b/drivers/watchdog/watchdog_pretimeout.h
new file mode 100644
index 00000000000000..38965cdd23bebe
--- /dev/null
+++ b/drivers/watchdog/watchdog_pretimeout.h
@@ -0,0 +1,35 @@
+#ifndef __WATCHDOG_PRETIMEOUT_H
+#define __WATCHDOG_PRETIMEOUT_H
+
+#define WATCHDOG_GOV_NAME_LEN	20
+
+struct module;
+struct watchdog_device;
+
+struct watchdog_governor {
+	const char		name[WATCHDOG_GOV_NAME_LEN];
+	void			(*pretimeout)(struct watchdog_device *wdd);
+	struct module		*owner;
+};
+
+/* Interfaces to watchdog pretimeout governors */
+int watchdog_register_governor(struct watchdog_governor *gov);
+void watchdog_unregister_governor(struct watchdog_governor *gov);
+
+/* Interfaces to watchdog_core.c */
+#ifdef CONFIG_WATCHDOG_PRETIMEOUT_GOV
+
+#define WATCHDOG_PRETIMEOUT_DEFAULT_GOV	"none"
+
+int watchdog_register_pretimeout(struct watchdog_device *wdd, struct device *dev);
+void watchdog_unregister_pretimeout(struct watchdog_device *wdd);
+#else
+static inline int watchdog_register_pretimeout(struct watchdog_device *wdd,
+					       struct device *dev)
+{
+	return 0;
+}
+static inline void watchdog_unregister_pretimeout(struct watchdog_device *wdd) {}
+#endif
+
+#endif
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index 51732d6c9555de..4b2fa45370fceb 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -19,6 +19,7 @@
 struct watchdog_ops;
 struct watchdog_device;
 struct watchdog_core_data;
+struct watchdog_governor;
 
 /** struct watchdog_ops - The watchdog-devices operations
  *
@@ -59,6 +60,7 @@ struct watchdog_ops {
  *		watchdog device.
  * @info:	Pointer to a watchdog_info structure.
  * @ops:	Pointer to the list of watchdog operations.
+ * @gov:	Pointer to watchdog pretimeout governor.
  * @bootstatus:	Status of the watchdog device at boot.
  * @timeout:	The watchdog devices timeout value (in seconds).
  * @min_timeout:The watchdog devices minimum timeout value (in seconds).
@@ -93,6 +95,7 @@ struct watchdog_device {
 	const struct attribute_group **groups;
 	const struct watchdog_info *info;
 	const struct watchdog_ops *ops;
+	struct watchdog_governor *gov;
 	unsigned int bootstatus;
 	unsigned int timeout;
 	unsigned int min_timeout;
@@ -180,4 +183,11 @@ extern int watchdog_init_timeout(struct watchdog_device *wdd,
 extern int watchdog_register_device(struct watchdog_device *);
 extern void watchdog_unregister_device(struct watchdog_device *);
 
+/* drivers/watchdog/watchdog_pretimeout.c */
+#ifdef CONFIG_WATCHDOG_PRETIMEOUT_GOV
+void watchdog_notify_pretimeout(struct watchdog_device *wdd);
+#else
+static inline void watchdog_notify_pretimeout(struct watchdog_device *wdd) {}
+#endif
+
 #endif  /* ifndef _LINUX_WATCHDOG_H */
-- 
2.8.1

[PATCH 2/7] watchdog: pretimeout: add panic pretimeout governor

[Wolfram Sang]

From: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>

Panic watchdog pretimeout governor, on watchdog pretimeout event the
kernel shall panic.

Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
Changes since Vladimir's last version:

* rebased
* shortened the panic message a little (removed redundancy)

 drivers/watchdog/Kconfig               | 29 +++++++++++++++++++++
 drivers/watchdog/Makefile              |  2 ++
 drivers/watchdog/pretimeout_panic.c    | 47 ++++++++++++++++++++++++++++++++++
 drivers/watchdog/watchdog_pretimeout.h |  6 ++++-
 4 files changed, 83 insertions(+), 1 deletion(-)
 create mode 100644 drivers/watchdog/pretimeout_panic.c

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 909d1021de5cbc..36b7d1f83b3c51 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1782,4 +1782,33 @@ config WATCHDOG_PRETIMEOUT_GOV
 	help
 	  The option allows to select watchdog pretimeout governors.
 
+if WATCHDOG_PRETIMEOUT_GOV
+
+choice
+	prompt "Default Watchdog Pretimeout Governor"
+	default WATCHDOG_PRETIMEOUT_DEFAULT_GOV_PANIC
+	help
+	  This option selects a default watchdog pretimeout governor.
+	  The governor takes its action, if a watchdog is capable
+	  to report a pretimeout event.
+
+config WATCHDOG_PRETIMEOUT_DEFAULT_GOV_PANIC
+	bool "panic"
+	select WATCHDOG_PRETIMEOUT_GOV_PANIC
+	help
+	  Use panic watchdog pretimeout governor by default, if
+	  a watchdog pretimeout event happens, consider that
+	  a watchdog feeder is dead and reboot is unavoidable.
+
+endchoice
+
+config WATCHDOG_PRETIMEOUT_GOV_PANIC
+	tristate "Panic watchdog pretimeout governor"
+	help
+	  Select this option to enable panic watchdog pretimeout
+	  governor, on a watchdog pretimeout event the kernel shall
+	  panic before an expected system reboot.
+
+endif # WATCHDOG_PRETIMEOUT_GOV
+
 endif # WATCHDOG
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 820860cf3e8d62..56dfadc0cee2a9 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -8,6 +8,8 @@ obj-$(CONFIG_WATCHDOG_CORE) += watchdog.o
 watchdog-y += watchdog_core.o watchdog_dev.o
 watchdog-$(CONFIG_WATCHDOG_PRETIMEOUT_GOV) += watchdog_pretimeout.o
 
+obj-$(CONFIG_WATCHDOG_PRETIMEOUT_GOV_PANIC) += pretimeout_panic.o
+
 # Only one watchdog can succeed. We probe the ISA/PCI/USB based
 # watchdog-cards first, then the architecture specific watchdog
 # drivers and then the architecture independent "softdog" driver.
diff --git a/drivers/watchdog/pretimeout_panic.c b/drivers/watchdog/pretimeout_panic.c
new file mode 100644
index 00000000000000..f8c2410d993f02
--- /dev/null
+++ b/drivers/watchdog/pretimeout_panic.c
@@ -0,0 +1,47 @@
+/*
+ * Copyright (C) 2015 Mentor Graphics
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/watchdog.h>
+
+#include "watchdog_pretimeout.h"
+
+/**
+ * pretimeout_panic - Panic on watchdog pretimeout event
+ * @wdd - watchdog_device
+ *
+ * Panic, watchdog has not been fed till pretimeout event.
+ */
+static void pretimeout_panic(struct watchdog_device *wdd)
+{
+	panic("watchdog pretimeout event");
+}
+
+static struct watchdog_governor watchdog_gov_panic = {
+	.name		= "panic",
+	.pretimeout	= pretimeout_panic,
+	.owner		= THIS_MODULE,
+};
+
+static int __init watchdog_gov_panic_register(void)
+{
+	return watchdog_register_governor(&watchdog_gov_panic);
+}
+module_init(watchdog_gov_panic_register);
+
+static void __exit watchdog_gov_panic_unregister(void)
+{
+	watchdog_unregister_governor(&watchdog_gov_panic);
+}
+module_exit(watchdog_gov_panic_unregister);
+
+MODULE_AUTHOR("Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>");
+MODULE_DESCRIPTION("Panic watchdog pretimeout governor");
+MODULE_LICENSE("GPL");
diff --git a/drivers/watchdog/watchdog_pretimeout.h b/drivers/watchdog/watchdog_pretimeout.h
index 38965cdd23bebe..8b03fd480cb533 100644
--- a/drivers/watchdog/watchdog_pretimeout.h
+++ b/drivers/watchdog/watchdog_pretimeout.h
@@ -19,7 +19,11 @@ void watchdog_unregister_governor(struct watchdog_governor *gov);
 /* Interfaces to watchdog_core.c */
 #ifdef CONFIG_WATCHDOG_PRETIMEOUT_GOV
 
-#define WATCHDOG_PRETIMEOUT_DEFAULT_GOV	"none"
+#if IS_ENABLED(CONFIG_WATCHDOG_PRETIMEOUT_DEFAULT_GOV_PANIC)
+#define WATCHDOG_PRETIMEOUT_DEFAULT_GOV	"panic"
+#else
+#error "Default watchdog pretimeout governor is not set."
+#endif
 
 int watchdog_register_pretimeout(struct watchdog_device *wdd, struct device *dev);
 void watchdog_unregister_pretimeout(struct watchdog_device *wdd);
-- 
2.8.1

[PATCH 3/7] watchdog: pretimeout: add noop pretimeout governor

[Wolfram Sang]

From: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>

Noop watchdog pretimeout governor, only an informational message is
added to the kernel log buffer.

Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
Changes since Vladimir's last version:

* rebased
* use pr_* now since device pointer became private

 drivers/watchdog/Kconfig               | 15 +++++++++++
 drivers/watchdog/Makefile              |  1 +
 drivers/watchdog/pretimeout_noop.c     | 47 ++++++++++++++++++++++++++++++++++
 drivers/watchdog/watchdog_pretimeout.h |  2 ++
 4 files changed, 65 insertions(+)
 create mode 100644 drivers/watchdog/pretimeout_noop.c

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 36b7d1f83b3c51..a3ef2bcd94f10c 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1800,6 +1800,14 @@ config WATCHDOG_PRETIMEOUT_DEFAULT_GOV_PANIC
 	  a watchdog pretimeout event happens, consider that
 	  a watchdog feeder is dead and reboot is unavoidable.
 
+config WATCHDOG_PRETIMEOUT_DEFAULT_GOV_NOOP
+	bool "noop"
+	select WATCHDOG_PRETIMEOUT_GOV_NOOP
+	help
+	  Use noop watchdog pretimeout governor by default. If noop
+	  governor is selected by a user, write a short message to
+	  the kernel log buffer and don't do any system changes.
+
 endchoice
 
 config WATCHDOG_PRETIMEOUT_GOV_PANIC
@@ -1809,6 +1817,13 @@ config WATCHDOG_PRETIMEOUT_GOV_PANIC
 	  governor, on a watchdog pretimeout event the kernel shall
 	  panic before an expected system reboot.
 
+config WATCHDOG_PRETIMEOUT_GOV_NOOP
+	tristate "Noop watchdog pretimeout governor"
+	help
+	  Select this option to enable noop watchdog pretimeout
+	  governor, only an informational message is added to the
+	  kernel log buffer, if watchdog pretimeout is happened.
+
 endif # WATCHDOG_PRETIMEOUT_GOV
 
 endif # WATCHDOG
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 56dfadc0cee2a9..3ad68029916e00 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -9,6 +9,7 @@ watchdog-y += watchdog_core.o watchdog_dev.o
 watchdog-$(CONFIG_WATCHDOG_PRETIMEOUT_GOV) += watchdog_pretimeout.o
 
 obj-$(CONFIG_WATCHDOG_PRETIMEOUT_GOV_PANIC) += pretimeout_panic.o
+obj-$(CONFIG_WATCHDOG_PRETIMEOUT_GOV_NOOP) += pretimeout_noop.o
 
 # Only one watchdog can succeed. We probe the ISA/PCI/USB based
 # watchdog-cards first, then the architecture specific watchdog
diff --git a/drivers/watchdog/pretimeout_noop.c b/drivers/watchdog/pretimeout_noop.c
new file mode 100644
index 00000000000000..91f36aa533b398
--- /dev/null
+++ b/drivers/watchdog/pretimeout_noop.c
@@ -0,0 +1,47 @@
+/*
+ * Copyright (C) 2015 Mentor Graphics
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/watchdog.h>
+
+#include "watchdog_pretimeout.h"
+
+/**
+ * pretimeout_noop - No operation on watchdog pretimeout event
+ * @wdd - watchdog_device
+ *
+ * This function prints a message about pretimeout to kernel log.
+ */
+static void pretimeout_noop(struct watchdog_device *wdd)
+{
+	pr_info("watchdog%d: pretimeout event\n", wdd->id);
+}
+
+static struct watchdog_governor watchdog_gov_noop = {
+	.name		= "noop",
+	.pretimeout	= pretimeout_noop,
+	.owner		= THIS_MODULE,
+};
+
+static int __init watchdog_gov_noop_register(void)
+{
+	return watchdog_register_governor(&watchdog_gov_noop);
+}
+module_init(watchdog_gov_noop_register);
+
+static void __exit watchdog_gov_noop_unregister(void)
+{
+	watchdog_unregister_governor(&watchdog_gov_noop);
+}
+module_exit(watchdog_gov_noop_unregister);
+
+MODULE_AUTHOR("Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>");
+MODULE_DESCRIPTION("Noop watchdog pretimeout governor");
+MODULE_LICENSE("GPL");
diff --git a/drivers/watchdog/watchdog_pretimeout.h b/drivers/watchdog/watchdog_pretimeout.h
index 8b03fd480cb533..2e60450a2be64e 100644
--- a/drivers/watchdog/watchdog_pretimeout.h
+++ b/drivers/watchdog/watchdog_pretimeout.h
@@ -21,6 +21,8 @@ void watchdog_unregister_governor(struct watchdog_governor *gov);
 
 #if IS_ENABLED(CONFIG_WATCHDOG_PRETIMEOUT_DEFAULT_GOV_PANIC)
 #define WATCHDOG_PRETIMEOUT_DEFAULT_GOV	"panic"
+#elif IS_ENABLED(CONFIG_WATCHDOG_PRETIMEOUT_DEFAULT_GOV_NOOP)
+#define WATCHDOG_PRETIMEOUT_DEFAULT_GOV	"noop"
 #else
 #error "Default watchdog pretimeout governor is not set."
 #endif
-- 
2.8.1

[PATCH 4/7] watchdog: documentation: squash paragraphs about 'no set_timeout'

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Those paragraphs deal with the same topic, so tie them together for
easier reading.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 Documentation/watchdog/watchdog-kernel-api.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/watchdog/watchdog-kernel-api.txt b/Documentation/watchdog/watchdog-kernel-api.txt
index 917eeeabfa5eae..a9a65f8c0de9f0 100644
--- a/Documentation/watchdog/watchdog-kernel-api.txt
+++ b/Documentation/watchdog/watchdog-kernel-api.txt
@@ -179,8 +179,8 @@ they are supported. These optional routines/operations are:
   (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
+  watchdog_device.timeout, this callback can be omitted. That means 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.
-- 
2.8.1

[PATCH 5/7] watchdog: add pretimeout support to the core

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Since the watchdog framework centrializes the IOCTL interfaces of device
drivers now, SETPRETIMEOUT and GETPRETIMEOUT need to be added in the
common code.

Signed-off-by: Robin Gong <b38343@freescale.com>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
Changes since Robin's last version:

* rebased
* 0 is valid and means disable pretimeout
  add code and docs for that
* add docs that drivers must update pretimeout when a new timeout is set
* reworded some documentation
* core handles wdd->pretimeout if set_pretimeout() is not populated
* add pretimeout to sysfs

 Documentation/watchdog/watchdog-kernel-api.txt | 20 ++++++++++
 drivers/watchdog/watchdog_dev.c                | 53 +++++++++++++++++++++++++-
 include/linux/watchdog.h                       | 11 ++++++
 3 files changed, 82 insertions(+), 2 deletions(-)

diff --git a/Documentation/watchdog/watchdog-kernel-api.txt b/Documentation/watchdog/watchdog-kernel-api.txt
index a9a65f8c0de9f0..4d4e0db6750c5f 100644
--- a/Documentation/watchdog/watchdog-kernel-api.txt
+++ b/Documentation/watchdog/watchdog-kernel-api.txt
@@ -50,6 +50,7 @@ struct watchdog_device {
 	const struct watchdog_ops *ops;
 	unsigned int bootstatus;
 	unsigned int timeout;
+	unsigned int pretimeout;
 	unsigned int min_timeout;
 	unsigned int max_timeout;
 	unsigned int min_hw_heartbeat_ms;
@@ -77,6 +78,7 @@ It contains following fields:
 * timeout: the watchdog timer's timeout value (in seconds).
   This is the time after which the system will reboot if user space does
   not send a heartbeat request if WDOG_ACTIVE is set.
+* pretimeout: the watchdog timer's pretimeout value (in seconds).
 * min_timeout: the watchdog timer's minimum timeout value (in seconds).
   If set, the minimum configurable value for 'timeout'.
 * max_timeout: the watchdog timer's maximum timeout value (in seconds),
@@ -120,6 +122,7 @@ struct watchdog_ops {
 	int (*ping)(struct watchdog_device *);
 	unsigned int (*status)(struct watchdog_device *);
 	int (*set_timeout)(struct watchdog_device *, unsigned int);
+	int (*set_pretimeout)(struct watchdog_device *, unsigned int);
 	unsigned int (*get_timeleft)(struct watchdog_device *);
 	int (*restart)(struct watchdog_device *);
 	void (*ref)(struct watchdog_device *) __deprecated;
@@ -183,6 +186,23 @@ they are supported. These optional routines/operations are:
   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.
+  If the pretimeout feature is used (WDIOF_PRETIMEOUT), then set_timeout must
+  also take care of checking if pretimeout is still valid and set up the timer
+  accordingly. This can't be done in the core without races, so it is the
+  duty of the driver.
+* set_pretimeout: this routine checks and changes the pretimeout value of
+  the watchdog. It is optional because not all watchdogs support pretimeout
+  notification. The timeout value is not an absolute time, but the number of
+  seconds before the actual timeout would happen. It returns 0 on success,
+  -EINVAL for "parameter out of range" and -EIO for "could not write value to
+  the watchdog". A value of 0 disables pretimeout notification.
+  (Note: the WDIOF_PRETIMEOUT 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.pretimeout, this callback can be omitted. That means if
+  set_pretimeout is not provided but WDIOF_PRETIMEOUT is set, the watchdog
+  infrastructure updates the pretimeout value of the watchdog_device internally
+  to the requested value.
 * get_timeleft: this routines returns the time that's left before a reset.
 * restart: this routine restarts the machine. It returns 0 on success or a
   negative errno code for failure.
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 5d028f94a90743..ad9f4032f02d18 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -308,10 +308,14 @@ static int watchdog_set_timeout(struct watchdog_device *wdd,
 	if (watchdog_timeout_invalid(wdd, timeout))
 		return -EINVAL;
 
-	if (wdd->ops->set_timeout)
+	if (wdd->ops->set_timeout) {
 		err = wdd->ops->set_timeout(wdd, timeout);
-	else
+	} else {
 		wdd->timeout = timeout;
+		/* Disable pretimeout if it doesn't fit the new timeout */
+		if (wdd->pretimeout > wdd->timeout)
+			wdd->pretimeout = 0;
+	}
 
 	watchdog_update_worker(wdd);
 
@@ -319,6 +323,31 @@ static int watchdog_set_timeout(struct watchdog_device *wdd,
 }
 
 /*
+ *	watchdog_set_pretimeout: set the watchdog timer pretimeout
+ *	@wdd: the watchdog device to set the timeout for
+ *	@timeout: pretimeout to set in seconds
+ */
+
+static int watchdog_set_pretimeout(struct watchdog_device *wdd,
+				   unsigned int timeout)
+{
+	int err = 0;
+
+	if (!(wdd->info->options & WDIOF_PRETIMEOUT))
+		return -EOPNOTSUPP;
+
+	if (watchdog_pretimeout_invalid(wdd, timeout))
+		return -EINVAL;
+
+	if (wdd->ops->set_pretimeout)
+		err = wdd->ops->set_pretimeout(wdd, timeout);
+	else
+		wdd->pretimeout = timeout;
+
+	return err;
+}
+
+/*
  *	watchdog_get_timeleft: wrapper to get the time left before a reboot
  *	@wdd: the watchdog device to get the remaining time from
  *	@timeleft: the time that's left
@@ -402,6 +431,15 @@ static ssize_t timeout_show(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR_RO(timeout);
 
+static ssize_t pretimeout_show(struct device *dev, struct device_attribute *attr,
+				char *buf)
+{
+	struct watchdog_device *wdd = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%u\n", wdd->pretimeout);
+}
+static DEVICE_ATTR_RO(pretimeout);
+
 static ssize_t identity_show(struct device *dev, struct device_attribute *attr,
 				char *buf)
 {
@@ -441,6 +479,7 @@ static struct attribute *wdt_attrs[] = {
 	&dev_attr_state.attr,
 	&dev_attr_identity.attr,
 	&dev_attr_timeout.attr,
+	&dev_attr_pretimeout.attr,
 	&dev_attr_timeleft.attr,
 	&dev_attr_bootstatus.attr,
 	&dev_attr_status.attr,
@@ -621,6 +660,16 @@ static long watchdog_ioctl(struct file *file, unsigned int cmd,
 			break;
 		err = put_user(val, p);
 		break;
+	case WDIOC_SETPRETIMEOUT:
+		if (get_user(val, p)) {
+			err = -EFAULT;
+			break;
+		}
+		err = watchdog_set_pretimeout(wdd, val);
+		break;
+	case WDIOC_GETPRETIMEOUT:
+		err = put_user(wdd->pretimeout, p);
+		break;
 	default:
 		err = -ENOTTY;
 		break;
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index 4b2fa45370fceb..496e52e5b91fc7 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -29,6 +29,7 @@ struct watchdog_governor;
  * @ping:	The routine that sends a keepalive ping to the watchdog device.
  * @status:	The routine that shows the status of the watchdog device.
  * @set_timeout:The routine for setting the watchdog devices timeout value (in seconds).
+ * @set_pretimeout:The routine for setting the watchdog devices pretimeout.
  * @get_timeleft:The routine that gets the time left before a reset (in seconds).
  * @restart:	The routine for restarting the machine.
  * @ioctl:	The routines that handles extra ioctl calls.
@@ -47,6 +48,7 @@ struct watchdog_ops {
 	int (*ping)(struct watchdog_device *);
 	unsigned int (*status)(struct watchdog_device *);
 	int (*set_timeout)(struct watchdog_device *, unsigned int);
+	int (*set_pretimeout)(struct watchdog_device *, unsigned int);
 	unsigned int (*get_timeleft)(struct watchdog_device *);
 	int (*restart)(struct watchdog_device *, unsigned long, void *);
 	long (*ioctl)(struct watchdog_device *, unsigned int, unsigned long);
@@ -63,6 +65,7 @@ struct watchdog_ops {
  * @gov:	Pointer to watchdog pretimeout governor.
  * @bootstatus:	Status of the watchdog device at boot.
  * @timeout:	The watchdog devices timeout value (in seconds).
+ * @pretimeout: The watchdog devices pre_timeout value.
  * @min_timeout:The watchdog devices minimum timeout value (in seconds).
  * @max_timeout:The watchdog devices maximum timeout value (in seconds)
  *		as configurable from user space. Only relevant if
@@ -98,6 +101,7 @@ struct watchdog_device {
 	struct watchdog_governor *gov;
 	unsigned int bootstatus;
 	unsigned int timeout;
+	unsigned int pretimeout;
 	unsigned int min_timeout;
 	unsigned int max_timeout;
 	unsigned int min_hw_heartbeat_ms;
@@ -165,6 +169,13 @@ static inline bool watchdog_timeout_invalid(struct watchdog_device *wdd, unsigne
 		 t > wdd->max_timeout);
 }
 
+/* Use the following function to check if a pretimeout value is invalid */
+static inline bool watchdog_pretimeout_invalid(struct watchdog_device *wdd,
+					       unsigned int t)
+{
+	return t && wdd->timeout && t >= wdd->timeout;
+}
+
 /* Use the following functions to manipulate watchdog driver specific data */
 static inline void watchdog_set_drvdata(struct watchdog_device *wdd, void *data)
 {
-- 
2.8.1

[PATCH 6/7] fs: compat_ioctl: add pretimeout functions for watchdogs

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Watchdog core now handles those ioctls centrally, so we want 64 bit
support, too.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: linux-fsdevel@vger.kernel.org
---

I think it would make sense to let this go in via the watchdog tree with the
rest of the series moving the IOCTL support into the core?

 fs/compat_ioctl.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index bd01b92aad98eb..914bfc5f3d4bd3 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -1208,6 +1208,8 @@ COMPATIBLE_IOCTL(WDIOC_SETOPTIONS)
 COMPATIBLE_IOCTL(WDIOC_KEEPALIVE)
 COMPATIBLE_IOCTL(WDIOC_SETTIMEOUT)
 COMPATIBLE_IOCTL(WDIOC_GETTIMEOUT)
+COMPATIBLE_IOCTL(WDIOC_SETPRETIMEOUT)
+COMPATIBLE_IOCTL(WDIOC_GETPRETIMEOUT)
 /* Big R */
 COMPATIBLE_IOCTL(RNDGETENTCNT)
 COMPATIBLE_IOCTL(RNDADDTOENTCNT)
-- 
2.8.1

[PATCH 7/7] watchdog: softdog: implement pretimeout support

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Give devices which do not have hardware support for pretimeout at least a
software version of it.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/watchdog/softdog.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/softdog.c b/drivers/watchdog/softdog.c
index b067edf246dff2..adfb3899296f0f 100644
--- a/drivers/watchdog/softdog.c
+++ b/drivers/watchdog/softdog.c
@@ -72,10 +72,25 @@ static void softdog_fire(unsigned long data)
 static struct timer_list softdog_ticktock =
 		TIMER_INITIALIZER(softdog_fire, 0, 0);
 
+static struct watchdog_device softdog_dev;
+
+static void softdog_pretimeout(unsigned long data)
+{
+	watchdog_notify_pretimeout(&softdog_dev);
+}
+
+static struct timer_list softdog_preticktock =
+		TIMER_INITIALIZER(softdog_pretimeout, 0, 0);
+
 static int softdog_ping(struct watchdog_device *w)
 {
 	if (!mod_timer(&softdog_ticktock, jiffies + (w->timeout * HZ)))
 		__module_get(THIS_MODULE);
+
+	if (w->pretimeout)
+		mod_timer(&softdog_preticktock, jiffies +
+		          (w->timeout - w->pretimeout) * HZ);
+
 	return 0;
 }
 
@@ -84,12 +99,15 @@ static int softdog_stop(struct watchdog_device *w)
 	if (del_timer(&softdog_ticktock))
 		module_put(THIS_MODULE);
 
+	del_timer(&softdog_preticktock);
+
 	return 0;
 }
 
 static struct watchdog_info softdog_info = {
 	.identity = "Software Watchdog",
-	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
+	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE |
+		   WDIOF_PRETIMEOUT,
 };
 
 static struct watchdog_ops softdog_ops = {
-- 
2.8.1

Re: [PATCH 1/7] watchdog: add watchdog pretimeout framework

[Vladimir Zapolskiy]

Hi Wolfram,

On 25.05.2016 16:32, Wolfram Sang wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> The change adds a simple watchdog pretimeout framework infrastructure,
> its purpose is to allow users to select a desired handling of watchdog
> pretimeout events, which may be generated by a watchdog driver.
> 
> By design every watchdog pretimeout governor may be compiled as a
> kernel module, a user selects a default watchdog pretimeout
> governor during compilation stage and can select another governor in
> runtime.
> 
> Watchdogs with WDIOF_PRETIMEOUT capability now have two device
> attributes in sysfs: read/write pretimeout_governor attribute and read
> only pretimeout_available_governors attribute.
> 
> Watchdogs with no WDIOF_PRETIMEOUT capability has no changes in
> sysfs.
> 
> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
> 
> Changes since Vladimir's last version:
> 
> * rebased
>   adapt to the internal data reorganization, especially the now private
>   struct device *dev
> * dropped can_sleep support!
>   The additional lock, list, and workqueue made the code quite complex. The
>   only user was the userspace governor which can be reworked to let the
>   watchdog device code do the bottom half. In addition, I am not fully
>   convinced sending a uevent is the proper thing to do, but this needs to
>   be discussed in another thread. Removing this support makes the code much
>   easier to follow (locking!), saves 30% of LoC + a list + a workqueue.

The thing is that I'm particularly interested in

1) sleeping governors,
2) userspace notification of any appropriate kind, but preferably not by
   adding a clumsy .poll callback, uevent is the best IMHO.

The userspace sleeping governor is the only one proposed for a mainline,
however the whole idea of having a framework is to allow users to write
their own private governors, and that's exactly what we need and use.

So the original complexity has its state-of-the-art grounds, and for
sake of getting a solid picture for reviewers and users it is better to
introduce sleeping functionality right from the beginning. I know it
is quite complex, probably it might be better to add it to the series
as a separate patch?

> * moved pretimeout registration from watchdog_core to watchdog_dev
>   Let's handle it exactly where the device is created, so we have access to
>   the now private device pointer for adding the sysfs files.
> * don't export watchdog_(un)register_pretimeout since they are linked to the
>   core anyhow
> * whitespace cleanups
> 

Thanks for pushing it, but do you think that the authorship of the
code can be preserved?

Feel free to ask me to rebase the change and so on, patch review procedure
is well established and I'm pretty sure I can cope with it.

I believe the main problem with the original code since the time when
rebase was not required is that it didn't receive any formal technical
review from Guenter or Wim, but I'm glad to know that someone else is
interested in it.

Merge window is closing, so it's good time for me to rebase the change
and resend it, do you have any objections?

>  drivers/watchdog/Kconfig               |   8 +
>  drivers/watchdog/Makefile              |   6 +-
>  drivers/watchdog/watchdog_dev.c        |   8 +
>  drivers/watchdog/watchdog_pretimeout.c | 269 +++++++++++++++++++++++++++++++++
>  drivers/watchdog/watchdog_pretimeout.h |  35 +++++
>  include/linux/watchdog.h               |  10 ++
>  6 files changed, 334 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/watchdog/watchdog_pretimeout.c
>  create mode 100644 drivers/watchdog/watchdog_pretimeout.h
> 

--
With best wishes,
Vladimir

Re: [PATCH 1/7] watchdog: add watchdog pretimeout framework

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 2870 bytes --]

Hi Vladimir,

great to see you still have capacity for this series :)

> The thing is that I'm particularly interested in
> 
> 1) sleeping governors,
> 2) userspace notification of any appropriate kind, but preferably not by
>    adding a clumsy .poll callback, uevent is the best IMHO.

I am totally open that poll might not be a good idea, but why do you
think uevent is best? (Disclaimer: I don't do much userspace code)

> The userspace sleeping governor is the only one proposed for a mainline,
> however the whole idea of having a framework is to allow users to write
> their own private governors, and that's exactly what we need and use.

One reason I decided to drop 'can_sleep' is that I guessed 98% of users
will be happy with the panic, noop, and userspace governers. 2% might
need custom governors from which maybe not even all need to sleep.
Chances are high IMO that these govenors will be out-of-tree code, so
having all this additional complexity for some out-of-tree govenors was
questionable to me. I wondered if it would make sense to let those
govenors do the bottom half handling themselves.

There was also a technical reason: The dev pointer was first moved to
watchdog_device private data before it was ultimately removed. So, while
trying to fix this, the code got more and more complicated which led me
to the decision to go the other way around: make the code simpler so it
will be easier maintainable in the future.

> So the original complexity has its state-of-the-art grounds, and for
> sake of getting a solid picture for reviewers and users it is better to
> introduce sleeping functionality right from the beginning.

I still wonder if bottom half handling shouldn't be put to the governors
which need that.

> I know it is quite complex, probably it might be better to add it to
> the series as a separate patch?

That might help the initial review.

> Thanks for pushing it, but do you think that the authorship of the
> code can be preserved?

I changed the authorship because I did one fundamental change to your
original design. Not knowing if you'd approve of that, I didn't want to
put your sticker on something you might not even like.

> Feel free to ask me to rebase the change and so on, patch review procedure
> is well established and I'm pretty sure I can cope with it.

No doubt about that. I had some ideas and thought it is easier to talk
over code. If you want to rebase it, too, I'd be happy to check what you
came up with to solve the problems. I might still argue that I prefer
the less-code approach, but it will be Guenter's / Wim's decision, of
course.

And I apologize for not contacting you beforehand which would have been
friendly. I got a rush on hacking it and wanted to show what I came up
with. No offence, sorry!

Thanks,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 1/7] watchdog: add watchdog pretimeout framework

[Guenter Roeck]

On Thu, May 26, 2016 at 06:41:36PM +0200, Wolfram Sang wrote:
[ ... ]
> 
> No doubt about that. I had some ideas and thought it is easier to talk
> over code. If you want to rebase it, too, I'd be happy to check what you
> came up with to solve the problems. I might still argue that I prefer
> the less-code approach, but it will be Guenter's / Wim's decision, of
> course.
> 
I have a large back-log of patches to review. Simple patches with less code
will get preferential treating. The more complex, the higher the likelyhood
that the patches get pushed to the end of the queue.

Giving a quick glance, I liked Wolfram's patches because they seemed to
be simple and straightforward. I hope the next version won't add too much
additional complexity.

Guenter

Re: [PATCH 1/7] watchdog: add watchdog pretimeout framework

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1144 bytes --]

On Thu, May 26, 2016 at 03:37:17PM -0700, Guenter Roeck wrote:
> On Thu, May 26, 2016 at 06:41:36PM +0200, Wolfram Sang wrote:
> [ ... ]
> > 
> > No doubt about that. I had some ideas and thought it is easier to talk
> > over code. If you want to rebase it, too, I'd be happy to check what you
> > came up with to solve the problems. I might still argue that I prefer
> > the less-code approach, but it will be Guenter's / Wim's decision, of
> > course.
> > 
> I have a large back-log of patches to review. Simple patches with less code
> will get preferential treating. The more complex, the higher the likelyhood
> that the patches get pushed to the end of the queue.
> 
> Giving a quick glance, I liked Wolfram's patches because they seemed to
> be simple and straightforward. I hope the next version won't add too much
> additional complexity.

Vladimir, did you have time to look into this?

I can offer to resend this series on-top of v4.7-rc1 with the core patch
re-attributed to Vladimir. Then we can review the basic stuff now, and
can discuss/deal with the bottom half handling incrementally.

Sounds good?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 1/7] watchdog: add watchdog pretimeout framework

[Vladimir Zapolskiy]

Hi Wolfram,

On 05.06.2016 12:48, Wolfram Sang wrote:
> On Thu, May 26, 2016 at 03:37:17PM -0700, Guenter Roeck wrote:
>> On Thu, May 26, 2016 at 06:41:36PM +0200, Wolfram Sang wrote:
>> [ ... ]
>>>
>>> No doubt about that. I had some ideas and thought it is easier to talk
>>> over code. If you want to rebase it, too, I'd be happy to check what you
>>> came up with to solve the problems. I might still argue that I prefer
>>> the less-code approach, but it will be Guenter's / Wim's decision, of
>>> course.
>>>
>> I have a large back-log of patches to review. Simple patches with less code
>> will get preferential treating. The more complex, the higher the likelyhood
>> that the patches get pushed to the end of the queue.
>>
>> Giving a quick glance, I liked Wolfram's patches because they seemed to
>> be simple and straightforward. I hope the next version won't add too much
>> additional complexity.
> 
> Vladimir, did you have time to look into this?

yes, I managed to rebase and split my change to smaller pieces to encourage
reviewers.

My plan is to do thorough testing of the change and send it for review
tomorrow or on Tuesday, please participate in review.

> I can offer to resend this series on-top of v4.7-rc1 with the core patch
> re-attributed to Vladimir. Then we can review the basic stuff now, and
> can discuss/deal with the bottom half handling incrementally.
> 
> Sounds good?
> 

--
Best wishes,
Vladimir