[Ccing Guenter as he reviewed the patches]
On Mon, Nov 23, 2015 at 04:02:52PM +0100, Harald Geyer wrote:
> Hi,
>
> sorry for breaking the threading. Only stumbled upon this message in the
> webarchive of the Mailinglist and couldn't reconstruct full headers ...
>
> I think there is an issue with your patch:
>
> +static int watchdog_reboot_notifier(struct notifier_block *nb,
> + unsigned long code, void *data)
> +{
> + struct watchdog_device *wdd = container_of(nb, struct watchdog_device,
> + reboot_nb);
> +
> + if (code == SYS_DOWN || code == SYS_HALT) {
>
> I think you want this to be (code == SYS_POWER_OFF || code == SYS_HALT)
> AFAIK SYS_DOWN is the code for a reboot, where the system should come
> back up immediatly, so probably we shouldn't disable the watchdog in
> this case, for the system might crash during going down.
Well, most of the drivers (all of them but gpio) that I changed stopped
on SYS_HALT and SYS_DOWN, so they explicitely wanted to stop the
watchdog on reboot. I just factorized that in watchdog core.
Maybe they should not stop on reboot in the first place, but this serie
does not introduce a new behaviour. Also, note that the watchdog will
be stopped only if it registers for that by calling
watchdog_stop_on_reboot, so there is no side effect for watchdogs that
should not be stopped.
>
> More importantly however we should stop the watchdog on SYS_POWER_OFF
> I think.
>
My understanding here is that if the system is powered off, the watchdog
will be powered off too, so there is no need to stop it.
Damien
> Otherwise your series looks good to me and I look forward to rebase
> my own patches on it.
>
> HTH,
> Harald
>
> + if (watchdog_active(wdd)) {
> + int ret;
> +
> + ret = wdd->ops->stop(wdd);
> + if (ret)
> + return NOTIFY_BAD;
> + }
> + }
> +
> + return NOTIFY_DONE;
> +}
> --
> If you want to support my work:
> see http://friends.ccbib.org/harald/supporting/
> or donate via bitcoin to 1FUtd8T9jRN1rFz63vZz7s2fDtB6d6A7aS
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
