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


Groups > linux.kernel > #1201004 > unrolled thread

Re: [PATCH 0/8] watchdog: Add support for keepalives triggered by infrastructure

Started byDavid Teigland <teigland@redhat.com>
First post2015-08-05 19:20 +0200
Last post2015-08-05 22:30 +0200
Articles 6 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH 0/8] watchdog: Add support for keepalives triggered by  infrastructure David Teigland <teigland@redhat.com> - 2015-08-05 19:20 +0200
    Re: [PATCH 0/8] watchdog: Add support for keepalives triggered by  infrastructure Guenter Roeck <linux@roeck-us.net> - 2015-08-05 19:50 +0200
      Re: [PATCH 0/8] watchdog: Add support for keepalives triggered by  infrastructure David Teigland <teigland@redhat.com> - 2015-08-05 20:00 +0200
        Re: [PATCH 0/8] watchdog: Add support for keepalives triggered by  infrastructure Guenter Roeck <linux@roeck-us.net> - 2015-08-05 21:10 +0200
          Re: [PATCH 0/8] watchdog: Add support for keepalives triggered by  infrastructure David Teigland <teigland@redhat.com> - 2015-08-05 22:00 +0200
            Re: [PATCH 0/8] watchdog: Add support for keepalives triggered by  infrastructure Guenter Roeck <linux@roeck-us.net> - 2015-08-05 22:30 +0200

#1201004 — Re: [PATCH 0/8] watchdog: Add support for keepalives triggered by infrastructure

FromDavid Teigland <teigland@redhat.com>
Date2015-08-05 19:20 +0200
SubjectRe: [PATCH 0/8] watchdog: Add support for keepalives triggered by infrastructure
Message-ID<pUawa-7PL-27@gated-at.bofh.it>
On Mon, Aug 03, 2015 at 07:13:26PM -0700, Guenter Roeck wrote:
> - 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.

> - 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.

> Patch #2 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 #3 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.

These sound concerning because it seems that heartbeats could be generated
outside of the direct control of userspace.  I have a program that depends
on having direct control over whether heartbeats are generated (or more
specifically, *not* generated.)  If these new features introduce a new way
for heartbeats to be generated, is there a way I can detect or disable
that behavior from userspace?  Unwanted heartbeats could break my program
and may lead to data corruption.

A related issue from some years ago is the unfortunate fact that closing
the watchdog device also generates a heartbeat.  I'd like to disable that
also, and submitted a patch for it here:
http://www.spinics.net/lists/linux-watchdog/msg01477.html

(Without the patch, I have to work around it by closing the device
prematurely as a way to generate the potentially final heartbeat, and then
reopen it again if I want to continue the heartbeats.)

Dave
--
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]


#1201024

FromGuenter Roeck <linux@roeck-us.net>
Date2015-08-05 19:50 +0200
Message-ID<pUaZc-8nv-15@gated-at.bofh.it>
In reply to#1201004
Hi David,

On 08/05/2015 10:13 AM, David Teigland wrote:
> On Mon, Aug 03, 2015 at 07:13:26PM -0700, Guenter Roeck wrote:
>> - 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.
>
>> - 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.
>
>> Patch #2 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 #3 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.
>
> These sound concerning because it seems that heartbeats could be generated
> outside of the direct control of userspace.  I have a program that depends
> on having direct control over whether heartbeats are generated (or more
> specifically, *not* generated.)  If these new features introduce a new way
> for heartbeats to be generated, is there a way I can detect or disable
> that behavior from userspace?  Unwanted heartbeats could break my program
> and may lead to data corruption.
>

Not really. The heartbeats will be generated such that the watchdog expires
no later that <last heartbeat from userspace + configured timeout>. I discussed
this already with Uwe; he had the same concern. This isn't in the current
version of the patch set, but it will be in the next version. That means
that nothing will change from user space perspective.

> A related issue from some years ago is the unfortunate fact that closing
> the watchdog device also generates a heartbeat.  I'd like to disable that
> also, and submitted a patch for it here:
> http://www.spinics.net/lists/linux-watchdog/msg01477.html
>

That is a different issue, though, and unrelated to this patch set.
Wim had a good point there: Presumably the problem you are trying to solve
applies to the entire system, not to a specific watchdog. What you are looking
for looks more like a system parameter, not like something to set with an ioctl
message. The reason here is that you'd still want to be able to use standard
applications such as systemd or watchdogd to trigger heartbeats, and not depend
on your own.

Thanks,
Guenter

--
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]


#1201028

FromDavid Teigland <teigland@redhat.com>
Date2015-08-05 20:00 +0200
Message-ID<pUb8S-7b-11@gated-at.bofh.it>
In reply to#1201024
On Wed, Aug 05, 2015 at 10:41:51AM -0700, Guenter Roeck wrote:
> Not really. The heartbeats will be generated such that the watchdog expires
> no later that <last heartbeat from userspace + configured timeout>. I discussed
> this already with Uwe; he had the same concern. This isn't in the current
> version of the patch set, but it will be in the next version. That means
> that nothing will change from user space perspective.

Sounds good, thanks.

> >A related issue from some years ago is the unfortunate fact that closing
> >the watchdog device also generates a heartbeat.  I'd like to disable that
> >also, and submitted a patch for it here:
> >http://www.spinics.net/lists/linux-watchdog/msg01477.html
> >
> 
> That is a different issue, though, and unrelated to this patch set.
> Wim had a good point there: Presumably the problem you are trying to solve
> applies to the entire system, not to a specific watchdog. What you are looking
> for looks more like a system parameter, not like something to set with an ioctl
> message. The reason here is that you'd still want to be able to use standard
> applications such as systemd or watchdogd to trigger heartbeats, and not depend
> on your own.

I'd need this behavior when the system is running my program (sanlock with
wdmd), which uses /dev/watchdog.  No other programs (systemd or watchdogd)
could be using /dev/watchdog at the same time.

Dave
--
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]


#1201067

FromGuenter Roeck <linux@roeck-us.net>
Date2015-08-05 21:10 +0200
Message-ID<pUceB-1SZ-3@gated-at.bofh.it>
In reply to#1201028
Hi David,

On 08/05/2015 10:51 AM, David Teigland wrote:
> On Wed, Aug 05, 2015 at 10:41:51AM -0700, Guenter Roeck wrote:
>> Not really. The heartbeats will be generated such that the watchdog expires
>> no later that <last heartbeat from userspace + configured timeout>. I discussed
>> this already with Uwe; he had the same concern. This isn't in the current
>> version of the patch set, but it will be in the next version. That means
>> that nothing will change from user space perspective.
>
> Sounds good, thanks.
>
>>> A related issue from some years ago is the unfortunate fact that closing
>>> the watchdog device also generates a heartbeat.  I'd like to disable that
>>> also, and submitted a patch for it here:
>>> http://www.spinics.net/lists/linux-watchdog/msg01477.html
>>>
>>
>> That is a different issue, though, and unrelated to this patch set.
>> Wim had a good point there: Presumably the problem you are trying to solve
>> applies to the entire system, not to a specific watchdog. What you are looking
>> for looks more like a system parameter, not like something to set with an ioctl
>> message. The reason here is that you'd still want to be able to use standard
>> applications such as systemd or watchdogd to trigger heartbeats, and not depend
>> on your own.
>
> I'd need this behavior when the system is running my program (sanlock with
> wdmd), which uses /dev/watchdog.  No other programs (systemd or watchdogd)
> could be using /dev/watchdog at the same time.
>

I think I can understand why Wim was reluctant to accept your patch;
I must admit I don't understand your use case either.

I wonder if you are actually mis-using the watchdog subsystem to generate
hard resets. After all, you could avoid the unexpected close situation with
an exit handler in your application. That handler could catch anything but
SIGKILL, but anyone using SIGKILL doesn't really deserve better. If the intent
is to reset the system after the application closes, executing "/sbin/restart -f"
might be a safer approach than just killing the watchdog.

In addition to that, I don't think it is a good idea to rely on the assumption
that the watchdog will expire exactly after the configured timeout.
Many watchdog drivers implement a soft timeout on top of the hardware timeout,
and thus already implement the internal heartbeat. Most of those drivers
will stop sending internal heartbeats if user space did not send a heartbeat
within the configured timeout period. The actual reset will then occur later,
after the actual hardware watchdog timed out. This can be as much as the
hardware timeout period, which may be substantial.

Thanks,
Guenter

--
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]


#1201086

FromDavid Teigland <teigland@redhat.com>
Date2015-08-05 22:00 +0200
Message-ID<pUd10-2OG-15@gated-at.bofh.it>
In reply to#1201067
On Wed, Aug 05, 2015 at 12:01:38PM -0700, Guenter Roeck wrote:
> I think I can understand why Wim was reluctant to accept your patch;
> I must admit I don't understand your use case either.

Very breifly, sanlock is a shared storage based lease manager, and the
expiration of a lease is tied to the expiration of the watchdog.  I have
to ensure that the watchdog expires at or before the time that the lease
expires.  This means that I cannot allow a watchdog heartbeat apart from a
corresponding lease renewal on the shared storage.  Otherwise, the
calculation by other hosts of the time of the hard reset will be wrong,
and the data on shared storage could be corrupted.

> I wonder if you are actually mis-using the watchdog subsystem to generate
> hard resets.

I am indeed using it to generate hard resets.

> After all, you could avoid the unexpected close situation with
> an exit handler in your application. That handler could catch anything but
> SIGKILL, but anyone using SIGKILL doesn't really deserve better.

I avoid the unexpected close situation by prematurely closing the device
to generate the heartbeat from close, and then reopening if needed.  That
covers the SIGKILL case.  So, I have a work around, but the patch would
still be nice.

> If the intent is to reset the system after the application closes,
> executing "/sbin/restart -f" might be a safer approach than just killing
> the watchdog.

I need to reset the system if the application crashes, or if the
application is running but can't renew its lease.  In the former case,
executing something doesn't work.  In the later case, I have done similar
(with /proc/sysrq-trigger), but it doesn't always apply and I'd still want
the hardware reset as redundancy.

> In addition to that, I don't think it is a good idea to rely on the assumption
> that the watchdog will expire exactly after the configured timeout.
> Many watchdog drivers implement a soft timeout on top of the hardware timeout,
> and thus already implement the internal heartbeat. Most of those drivers
> will stop sending internal heartbeats if user space did not send a heartbeat
> within the configured timeout period. The actual reset will then occur later,
> after the actual hardware watchdog timed out. This can be as much as the
> hardware timeout period, which may be substantial.

OK, thanks, I'll look into this in more detail.  Is there a way I can
identify which cases these are, or do you know an example I can look at?
In the worst case I'd have to extend the lease expiration time by a full
timeout period when the dubious drivers are used.

Dave
--
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]


#1201125

FromGuenter Roeck <linux@roeck-us.net>
Date2015-08-05 22:30 +0200
Message-ID<pUdu2-3Ci-3@gated-at.bofh.it>
In reply to#1201086
Hi David,

On 08/05/2015 12:51 PM, David Teigland wrote:
> On Wed, Aug 05, 2015 at 12:01:38PM -0700, Guenter Roeck wrote:
>> I think I can understand why Wim was reluctant to accept your patch;
>> I must admit I don't understand your use case either.
>
> Very breifly, sanlock is a shared storage based lease manager, and the
> expiration of a lease is tied to the expiration of the watchdog.  I have
> to ensure that the watchdog expires at or before the time that the lease
> expires.  This means that I cannot allow a watchdog heartbeat apart from a
> corresponding lease renewal on the shared storage.  Otherwise, the
> calculation by other hosts of the time of the hard reset will be wrong,
> and the data on shared storage could be corrupted.
>
>> I wonder if you are actually mis-using the watchdog subsystem to generate
>> hard resets.
>
> I am indeed using it to generate hard resets.
>
So there is no concern that the hard reset may corrupt some data ? Interesting.
Hope you don't use any SSDs - some of those don't like that.

>> After all, you could avoid the unexpected close situation with
>> an exit handler in your application. That handler could catch anything but
>> SIGKILL, but anyone using SIGKILL doesn't really deserve better.
>
> I avoid the unexpected close situation by prematurely closing the device
> to generate the heartbeat from close, and then reopening if needed.  That
> covers the SIGKILL case.  So, I have a work around, but the patch would
> still be nice.
>
Sounds messy....

>> If the intent is to reset the system after the application closes,
>> executing "/sbin/restart -f" might be a safer approach than just killing
>> the watchdog.
>
> I need to reset the system if the application crashes, or if the
> application is running but can't renew its lease.  In the former case,
> executing something doesn't work.  In the later case, I have done similar

Maybe you could have the system monitor (systemd or whatever it is) the
application and run /sbin/restart if it crashes. Essentially monitor
the application from the outside.

> (with /proc/sysrq-trigger), but it doesn't always apply and I'd still want
> the hardware reset as redundancy.
>
>> In addition to that, I don't think it is a good idea to rely on the assumption
>> that the watchdog will expire exactly after the configured timeout.
>> Many watchdog drivers implement a soft timeout on top of the hardware timeout,
>> and thus already implement the internal heartbeat. Most of those drivers
>> will stop sending internal heartbeats if user space did not send a heartbeat
>> within the configured timeout period. The actual reset will then occur later,
>> after the actual hardware watchdog timed out. This can be as much as the
>> hardware timeout period, which may be substantial.
>
> OK, thanks, I'll look into this in more detail.  Is there a way I can
> identify which cases these are, or do you know an example I can look at?
> In the worst case I'd have to extend the lease expiration time by a full
> timeout period when the dubious drivers are used.
>

git grep mod_timer drivers/watchdog | cut -f1 -d: | sort -u

gives you the following list:

alim7101_wdt.c
at91sam9_wdt.c
bcm47xx_wdt.c
bcm63xx_wdt.c
cpu5wdt.c
dw_wdt.c
ep93xx_wdt.c
gpio_wdt.c
imx2_wdt.c
machzwd.c
mixcomwd.c
mpc8xxx_wdt.c
mtx-1_wdt.c
nuc900_wdt.c
pcwd.c
pika_wdt.c
rdc321x_wdt.c
sbc60xxwdt.c
sc520_wdt.c
shwdt.c
softdog.c
via_wdt.c
w83877f_wdt.c

Those would be the immediate candidates to look out for. Note that this situation
will actually improve with my patch set, since it tries to tie the actual expiry
to the configured timeout. This will only work if the driver(s) are converted
to use the new infrastructure, of course.

Still, the ABI guarantees that "the hardware watchdog will reset the system
(causing a reboot) after the timeout occurs", but that doesn't mean that it
will reset the system immediately. I think the only safe guarantee is that
it won't reset the system as long as the timeout did _not_ occur. Extending
the lease expiration in your application by a timeout period will help,
but there is still no _guarantee_ that the reset will occur within
"expiration time + timeout".

I am not even sure if all watchdog drivers which don't implement a soft timer
will always timeout exactly after "timeout" seconds. It will be "at least
timeout" seconds, but I would not bet that it is always the exact time.

Guenter

--
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