Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 15 Jan 2019 19:27:31 -0700
From:   Jerry Hoemann <jerry.hoemann@hpe.com>
To:     Ivan Mironov <mironov.ivan@gmail.com>
Cc:     linux-watchdog@vger.kernel.org, linux-kernel@vger.kernel.org,
        Wim Van Sebroeck <wim@linux-watchdog.org>,
        Guenter Roeck <linux@roeck-us.net>
Subject: Re: [RFC PATCH 1/4] watchdog: hpwdt: Don't disable watchdog on NMI
Message-ID: <20190116022731.GD18342@anatevka>
Reply-To: Jerry.Hoemann@hpe.com
References: <20190114023617.10656-1-mironov.ivan@gmail.com>
 <20190114023617.10656-2-mironov.ivan@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20190114023617.10656-2-mironov.ivan@gmail.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, Jan 14, 2019 at 07:36:14AM +0500, Ivan Mironov wrote:
> Existing code disables watchdog on NMI right before completely hanging
> the system.
> 
> There are two problems here:
> 
>  * First, watchdog is expected to reset the system in a case of such
>    failure, no matter what.

Documentation/watchdog/watchdog-api.txt

explicitly allows for pretimeout NMI and generation of kernel crash dumps.

By removing hpwdt_stop the system will likely fail to crash dump
as there is only 9 seconds between receipt of a NMI and the iLO
resetting the system.

Unfortunately, kdump is not without issues and can also be difficult
to properly configure either of which can result in failure to dump
and reset.

Customers who value availability over kdump collection, the pretimeout
NMI can be disabled and hardware will not issue the pretimeout NMI
and will only do reset.

A middle ground for those who want tombstones but not kdump, would
be to leave the pretimeout NMI enabled and add "panic=N" to the
Linux command line.  That way after the panic, the tombstone is
printed and the system resets after N seconds.


>  * Second, this code has no effect if there are more than one watchdog.

That is correct.  Hpwdt will not turn off any other WDT.

I don't see a current method of notifying other watchdogs
that a given watchdog is going to take the system down.

The closest I hook see is watchdog_notify_pretimeout, but I don't
see that notifying other WDT.  Its not clear to me that it should.
(e.g. the second WDT could be of longer duration and protect against
kdump hanging. This would need to be thought through.)


> 
> Signed-off-by: Ivan Mironov <mironov.ivan@gmail.com>
> ---
>  drivers/watchdog/hpwdt.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
> index ef30c7e9728d..2467e6bc25c2 100644
> --- a/drivers/watchdog/hpwdt.c
> +++ b/drivers/watchdog/hpwdt.c
> @@ -170,8 +170,6 @@ static int hpwdt_pretimeout(unsigned int ulReason, struct pt_regs *regs)
>  	if (ilo5 && !pretimeout && !mynmi)
>  		return NMI_DONE;
>  
> -	hpwdt_stop();
> -
>  	hex_byte_pack(panic_msg, mynmi);
>  	nmi_panic(regs, panic_msg);
>  
> -- 
> 2.20.1

-- 

-----------------------------------------------------------------------------
Jerry Hoemann                  Software Engineer   Hewlett Packard Enterprise
-----------------------------------------------------------------------------