From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from metis.whiteo.stw.pengutronix.de (metis.whiteo.stw.pengutronix.de [185.203.201.7]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E50FC242939 for ; Wed, 5 Mar 2025 12:24:47 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.203.201.7 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741177489; cv=none; b=TDcvU+ZG75dEb42vD1KaiYFf5Z7fvvijxxV6w3/4oPi4K9KgZnk/YbRyDFiKHDSAXToBQLzInq5dY09kfY7kT0ZW2a1z836rMKE4tcJagVTs/HOVmvETOsrxaLIWgy9ELWRzy65nsRcL3rARiZX5RMo4LwfCZ9odAaTpKT3BG4o= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741177489; c=relaxed/simple; bh=bpULnFJHbcm6cClhnUJ0bpMib8w3hmogj9PdLruqLWk=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=S39eXxAIEBCgt5ScnfRniL2xxVXBOpdZBnQQPYwi31N0rjicuj5G0j973my+C1a8IJmmwzG3i9jGpXZ421Ts0b5FN4b1IU2NcGj93/SmocHZpfYkuyw7MKkvKTA7gP57zxEuwUpwqSUFhox+XhBd6C0NI2ys19D3ZALaC3mu3ec= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pengutronix.de; spf=pass smtp.mailfrom=pengutronix.de; arc=none smtp.client-ip=185.203.201.7 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pengutronix.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pengutronix.de Received: from ptz.office.stw.pengutronix.de ([2a0a:edc0:0:900:1d::77] helo=[127.0.0.1]) by metis.whiteo.stw.pengutronix.de with esmtp (Exim 4.92) (envelope-from ) id 1tpnn2-00021N-8w; Wed, 05 Mar 2025 13:23:56 +0100 Message-ID: Date: Wed, 5 Mar 2025 13:23:46 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v4 0/2] Add stop_on_panic support for watchdog To: George Cherian , "linux@roeck-us.net" , "wim@linux-watchdog.org" , "jwerner@chromium.org" , "evanbenn@chromium.org" , "kabel@kernel.org" , "krzk@kernel.org" , "mazziesaccount@gmail.com" , "thomas.richard@bootlin.com" , "lma@chromium.org" , "bleung@chromium.org" , "support.opensource@diasemi.com" , "shawnguo@kernel.org" , "s.hauer@pengutronix.de" , "kernel@pengutronix.de" , "festevam@gmail.com" , "andy@kernel.org" , "paul@crapouillou.net" , "alexander.usyskin@intel.com" , "andreas.werner@men.de" , "daniel@thingy.jp" , "romain.perier@gmail.com" , "avifishman70@gmail.com" , "tmaimon77@gmail.com" , "tali.perry1@gmail.com" , "venture@google.com" , "yuenn@google.com" , "benjaminfair@google.com" , "maddy@linux.ibm.com" , "mpe@ellerman.id.au" , "npiggin@gmail.com" , "christophe.leroy@csgroup.eu" , "naveen@kernel.org" , "mwalle@kernel.org" , "xingyu.wu@starfivetech.com" , "ziv.xu@starfivetech.com" , "hayashi.kunihiko@socionext.com" , "mhiramat@kernel.org" Cc: "chrome-platform@lists.linux.dev" , "linux-watchdog@vger.kernel.org" , "imx@lists.linux.dev" , "patches@opensource.cirrus.com" , "linux-mips@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linuxppc-dev@lists.ozlabs.org" , "linux-arm-kernel@lists.infradead.org" References: <20250305101025.2279951-1-george.cherian@marvell.com> <43fb0965-04b7-41dc-ae3f-54676eefdbb5@pengutronix.de> <28d0ea70-db7a-40e7-aac9-86808320f252@pengutronix.de> Content-Language: en-US From: Ahmad Fatoum In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SA-Exim-Connect-IP: 2a0a:edc0:0:900:1d::77 X-SA-Exim-Mail-From: a.fatoum@pengutronix.de X-SA-Exim-Scanned: No (on metis.whiteo.stw.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-kernel@vger.kernel.org Hello George, On 05.03.25 13:15, George Cherian wrote: >> On 05.03.25 12:28, George Cherian wrote: >>>> that can't be disabled and would protect against system lock up: >>>> Consider a memory-corruption bug (perhaps externally via DMA), which partially >>>> overwrites both main and kdump kernel. With a disabled watchdog, the system >>>> may not be able to recover on its own. >>> >>> Yes, that is the reason why the kernel command-line is optional and by default it is set to zero. >>> So that in cases if you have a corrupted kdump kernel then watchdog kicks in. >> >> The existing option isn't enough for the kdump kernel use case. >> If we (i.e. you) are going to do something about it, wouldn't it be >> better to have a solution that's applicable to a wider number of >> watchdog devices? > > I need a slight clarification here. > 1. reset_on_panic takes the number of seconds to be reloaded to watchdog HW, so that it initiates a > watchdog reset after the specified timeout, if kdump kernel fails to boot or hung while booting. Yes. > 2. in case reset_on_panic = 0 then it behaves like stop_on_panic=1. > Is this what you meant? Alternatively, reset_on_panic = 0 could also mean stopping the watchdog as you do now. I haven't thought though yet what would make the most sense. > I would let Guenter comment on this approach. +1. >> If you are serious with the watchdog use, you'll want to use the watchdog to >> monitor kernel startup as well. If the bootloader can set a watchdog timeout >> just before starting the kernel and it doesn't expire before the kernel watchdog >> driver takes over, why can't we do the same just before starting the dumpkernel? > > Yes, in an ideal world with ideal HW. But there are HW with issues which cannot have large > enough Watchdog time. Such HW would boot from FW to kernel without watchdog enabled. > And stop_on_panic does the similar for kdump kernel too. Yes, but there is likely more kinds of watchdog devices that can not be disabled, so it makes sense to have a solution that is more broadly applicable from the get-go. Cheers, Ahmad > > -George >> >> Thanks, >> Ahmad >> >> >>> >>> Thanks, >>> Ahmad >>> >>>> >>>> >>> Changelog: >>> v1 -> v2 >>> - Remove the per driver flag setting option >>> - Take the parameter via kernel command-line parameter to watchdog_core. >>> >>> v2 -> v3 >>> - Remove the helper function watchdog_stop_on_panic() from watchdog.h. >>> - There are no users for this. >>> >>> v3 -> v4 >>> - Since the panic notifier is in atomic context, watchdog functions >>> which sleep can't be called. >>> - Add an options flag WDIOF_STOP_MAYSLEEP to indicate whether stop >>> function sleeps. >>> - Simplify the stop_on_panic kernel command line parsing. >>> - Enable the panic notiffier only if the watchdog stop function doesn't >>> sleep >>> >>> George Cherian (2): >>> watchdog: Add a new flag WDIOF_STOP_MAYSLEEP >>> drivers: watchdog: Add support for panic notifier callback >> >> - George > > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |