2020-08-20 02:43:01

by Wang Wensheng

Subject: [PATCH] watchdog: Add interface to config timeout and pretimeout in sysfs

Those interfaces exist already in sysfs of watchdog driver core, but
they are readonly. This patch add write hook so we can config timeout
and pretimeout by writing those files.

Signed-off-by: Wang Wensheng <[email protected]>
---
drivers/watchdog/watchdog_dev.c | 48 +++++++++++++++++++++++++++++++--
1 file changed, 46 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 10b2090f3e5e..bb8ddc71d4ea 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -485,7 +485,29 @@ static ssize_t timeout_show(struct device *dev, struct device_attribute *attr,

return sprintf(buf, "%u\n", wdd->timeout);
}
-static DEVICE_ATTR_RO(timeout);
+
+static ssize_t timeout_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int ret;
+ unsigned int val;
+ struct watchdog_device *wdd = dev_get_drvdata(dev);
+ struct watchdog_core_data *wd_data = wdd->wd_data;
+
+ ret = kstrtouint(buf, 0, &val);
+ if (ret)
+ return ret;
+
+ mutex_lock(&wd_data->lock);
+ ret = watchdog_set_timeout(wdd, val);
+ mutex_unlock(&wd_data->lock);
+
+ if (!ret)
+ ret = count;
+
+ return ret;
+}
+static DEVICE_ATTR_RW(timeout);

static ssize_t pretimeout_show(struct device *dev,
struct device_attribute *attr, char *buf)
@@ -494,7 +516,29 @@ static ssize_t pretimeout_show(struct device *dev,

return sprintf(buf, "%u\n", wdd->pretimeout);
}
-static DEVICE_ATTR_RO(pretimeout);
+
+static ssize_t pretimeout_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ int ret;
+ unsigned int val;
+ struct watchdog_device *wdd = dev_get_drvdata(dev);
+ struct watchdog_core_data *wd_data = wdd->wd_data;
+
+ ret = kstrtouint(buf, 0, &val);
+ if (ret)
+ return ret;
+
+ mutex_lock(&wd_data->lock);
+ ret = watchdog_set_pretimeout(wdd, val);
+ mutex_unlock(&wd_data->lock);
+
+ if (!ret)
+ ret = count;
+
+ return ret;
+}
+static DEVICE_ATTR_RW(pretimeout);

static ssize_t identity_show(struct device *dev, struct device_attribute *attr,
char *buf)
--
2.25.0

2020-08-20 21:52:47

by Guenter Roeck

Subject: Re: [PATCH] watchdog: Add interface to config timeout and pretimeout in sysfs

On Thu, Aug 20, 2020 at 02:38:58AM +0000, Wang Wensheng wrote:
> Those interfaces exist already in sysfs of watchdog driver core, but
> they are readonly. This patch add write hook so we can config timeout
> and pretimeout by writing those files.
>
> Signed-off-by: Wang Wensheng <[email protected]>

This is problematic. For example, if a user changes the timeout on
a running watchdog, the application pinging the watchdog would not
know about it. Set the timeout to 1 second using the sysfs attribute,
and the system will trigger the watchdog almost immediately.

The only somewhat "safe" means to use this attribute would be to set
the timeout before a userspace application opens the watchdog device.
But then this application could just as well update the timeout
after opening the device. What is the use case ?

Thanks,
Guenter

> ---
> drivers/watchdog/watchdog_dev.c | 48 +++++++++++++++++++++++++++++++--
> 1 file changed, 46 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
> index 10b2090f3e5e..bb8ddc71d4ea 100644
> --- a/drivers/watchdog/watchdog_dev.c
> +++ b/drivers/watchdog/watchdog_dev.c
> @@ -485,7 +485,29 @@ static ssize_t timeout_show(struct device *dev, struct device_attribute *attr,
>
> return sprintf(buf, "%u\n", wdd->timeout);
> }
> -static DEVICE_ATTR_RO(timeout);
> +
> +static ssize_t timeout_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + int ret;
> + unsigned int val;
> + struct watchdog_device *wdd = dev_get_drvdata(dev);
> + struct watchdog_core_data *wd_data = wdd->wd_data;
> +
> + ret = kstrtouint(buf, 0, &val);
> + if (ret)
> + return ret;
> +
> + mutex_lock(&wd_data->lock);
> + ret = watchdog_set_timeout(wdd, val);
> + mutex_unlock(&wd_data->lock);
> +
> + if (!ret)
> + ret = count;
> +
> + return ret;
> +}
> +static DEVICE_ATTR_RW(timeout);
>
> static ssize_t pretimeout_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> @@ -494,7 +516,29 @@ static ssize_t pretimeout_show(struct device *dev,
>
> return sprintf(buf, "%u\n", wdd->pretimeout);
> }
> -static DEVICE_ATTR_RO(pretimeout);
> +
> +static ssize_t pretimeout_store(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t count)
> +{
> + int ret;
> + unsigned int val;
> + struct watchdog_device *wdd = dev_get_drvdata(dev);
> + struct watchdog_core_data *wd_data = wdd->wd_data;
> +
> + ret = kstrtouint(buf, 0, &val);
> + if (ret)
> + return ret;
> +
> + mutex_lock(&wd_data->lock);
> + ret = watchdog_set_pretimeout(wdd, val);
> + mutex_unlock(&wd_data->lock);
> +
> + if (!ret)
> + ret = count;
> +
> + return ret;
> +}
> +static DEVICE_ATTR_RW(pretimeout);
>
> static ssize_t identity_show(struct device *dev, struct device_attribute *attr,
> char *buf)
> --
> 2.25.0
>