Re: [PATCH 1/2] watchdog: iTCO_wdt: optionally leave watchdog enabled during restart

Re: [PATCH 1/2] watchdog: iTCO_wdt: optionally leave watchdog enabled during restart

[Valdis.Kletnieks]

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

On Wed, 06 Jul 2011 10:09:36 MDT, Bjorn Helgaas said:

> If we reboot via BIOS, BIOS should disable the watchdog itself, so this
> shouldn't cause unintended resets, even if the user interrupts the boot.

Yes, but didn't Linus say something about BIOS code authors being
crack-addicted monkeys? :)

[-- Attachment #2: Type: application/pgp-signature, Size: 227 bytes --]

Re: [PATCH 1/2] watchdog: iTCO_wdt: optionally leave watchdog enabled during restart

[Pádraig Brady]

On 07/07/11 16:39, Valdis.Kletnieks@vt.edu wrote:
> On Wed, 06 Jul 2011 10:09:36 MDT, Bjorn Helgaas said:
> 
>> If we reboot via BIOS, BIOS should disable the watchdog itself, so this
>> shouldn't cause unintended resets, even if the user interrupts the boot.
> 
> Yes, but didn't Linus say something about BIOS code authors being
> crack-addicted monkeys? :)

Yes as I said in a round about way in another mail,
one can't depend on that at all.
Some reset, some don't, some behave weirdly,
iTCO is unusual as kernel resets early at boot, ...

If using this, one would have to set the timeout large enough,
to encompass a full reboot

Re: [PATCH 1/2] watchdog: iTCO_wdt: optionally leave watchdog enabled during restart

[Bjorn Helgaas]

On Thu, Jul 7, 2011 at 9:53 AM, Pádraig Brady <P@draigbrady.com> wrote:
> On 07/07/11 16:39, Valdis.Kletnieks@vt.edu wrote:
>> On Wed, 06 Jul 2011 10:09:36 MDT, Bjorn Helgaas said:
>>
>>> If we reboot via BIOS, BIOS should disable the watchdog itself, so this
>>> shouldn't cause unintended resets, even if the user interrupts the boot.
>>
>> Yes, but didn't Linus say something about BIOS code authors being
>> crack-addicted monkeys? :)

I shouldn't have written anything about what BIOS "should" do.  That's
not very useful because, as you suggest, there is room for variation
there.

The risk I was alluding to was this:
  - User boots with "reboot_timeout=X"
  - User reboots normally (non-kexec)
  - BIOS does some reinitialization
  - Machine doesn't autoboot, e.g., because user interrupted boot
  - Watchdog resets machine -- this may be unexpected by the user

On the machines I tested, the unexpected reset doesn't happen because
the BIOS reinit includes disabling the watchdog.  But obviously, that
depends on BIOS details, so there's no guarantee.

I should have just written something along the lines of:

  The reboot_timeout option is intended for kexec reboots, which do
not involve BIOS.
  In this case, the reboot_timeout covers the interval between shutdown of the
  watchdog driver in the old kernel and startup of the driver in the new kernel.

  For normal reboots (via the BIOS), the behavior depends on the BIOS
implementation.
  Some BIOSes disable the watchdog timer, so the reboot_timeout only covers the
  interval until the BIOS disable.  Others leave the timer running, so
the reboot_timeout
  may cause a reset if the machine doesn't autoboot, e.g., if the user
interrupts the boot.

I think the *option* of using a reboot_timeout is still useful,
especially in clusters of unattended machines where it's expensive to
deal with boot failures.

> Yes as I said in a round about way in another mail,
> one can't depend on that at all.
> Some reset, some don't, some behave weirdly,
> iTCO is unusual as kernel resets early at boot, ...

You mentioned an unexplained iTCO reset in your other mail.  That
sounds like a kernel or iTCO_wdt bug, but I think it's unrelated to
this patch.

> If using this, one would have to set the timeout large enough,
> to encompass a full reboot

Right.  In the case of iTCO, I think the range is up to about 10
minutes, which is enough in my case (things like fsck may take longer,
but that's OK as long as the watchdog driver is built in statically).

Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] [RFC] watchdog reboot timeout

[Bjorn Helgaas]

On Thu, Jul 7, 2011 at 7:51 AM, Pádraig Brady <P@draigbrady.com> wrote:
> On 06/07/11 17:09, Bjorn Helgaas wrote:
>> I'm looking for comments on these patches.  I'm not really happy with
>> them because they add basically identical code in each driver.  I also
>> don't like the fact that the parameter is not generic, so I have to know
>> which driver will be used on my machine.
>
> Hmm, so this is protecting the window between watchdog_stop and machine_reset.

That's part of it, but I'm really interested in reboots using kexec,
and in that case, there is no machine reset, and BIOS isn't involved
at all.  If the kexec fails or the new kernel hangs for some reason,
I'd like the watchdog to reset the machine.

Currently, we call all the driver .shutdown methods() on the way down,
and the watchdog shutdown methods turn off the watchdog, so we have no
protection after that.

> It would be nice to have this as generic code that did WDIOC_SETTIMEOUT
> immediately after (or instead of) shutdown.

Yes.  I'd prefer this to be more generic.  I've skimmed the new
generic watchdog code a bit, hoping for a nice way to do this, but the
actual .shutdown() method is still in each driver, so making it
generic doesn't seem trivial.  So consider this a plea for future
development.

> Leaving a watchdog enabled across boot does have issues on some systems.
> It was OK for winboond based watchdogs I tested, but for some IBM
> server systems, "weird stuff" happened if the watchdog was left enabled
> across boot.

Point taken: it's unwise to assume consistency across all BIOSes, so
it was a mistake to write the last paragraph ("If we reboot via BIOS,
BIOS should disable the watchdog ...").

My reboot_timeout is disabled by default and is only enabled if the
user specifies the module option.  I don't think it's possible to
enable something like this by default, because in general we don't
know anything about the new kernel.  It may not have a watchdog driver
at all.

But in many cases, we *do* know the machine doesn't do "weird stuff,"
the new kernel has a driver, and we can bound the expected boot time.
In those cases, I think this option provides useful protection.

> Just to mention the other watchdog boot scenario I often use
> (which does _not_ protect your window above), which is to enable the
> watchdog in the BIOS.  This is more general protection for the
> boot _up_ process.

Yes, that makes good sense.  In my case, I'm interested in kexec, so
the BIOS isn't involved, so enabling it in the BIOS wouldn't help.

> A related issue I noticed with iTCO_wdt is that something
> else in the kernel disables the iTCO watchdog (which my BIOS had started).
> This is undesirable for obvious reasons.

This sounds related to iTCO, but unrelated to my patch.  Right?

Thanks a lot for your comments!

Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] [RFC] watchdog reboot timeout

[Pádraig Brady]

On 12/07/11 17:25, Bjorn Helgaas wrote:
> On Thu, Jul 7, 2011 at 7:51 AM, Pádraig Brady <P@draigbrady.com> wrote:
>> A related issue I noticed with iTCO_wdt is that something
>> else in the kernel disables the iTCO watchdog (which my BIOS had started).
>> This is undesirable for obvious reasons.
> 
> This sounds related to iTCO, but unrelated to my patch.  Right?

Right, unrelated to your patch but it could affect you.
As the kernel comes up it may reinit the iTCO, disabling
the watchdog until the iTCO_wdt module is loaded and patted.

cheers,
Pádraig.
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html