Thread (21 messages) 21 messages, 7 authors, 2025-03-06

RE: [EXTERNAL] Re: [PATCH v4 1/2] watchdog: Add a new flag WDIOF_STOP_MAYSLEEP

From: George Cherian <gcherian@marvell.com>
Date: 2025-03-06 12:20:11
Also in: chrome-platform, imx, linux-arm-kernel, linux-mips, linux-watchdog, lkml, openbmc

Hi Guenter,

I am summarizing the topics we discussed in multiple threads here.
quoted
On 3/5/25 03:01, Ahmad Fatoum wrote:
Hi George,
Hi Guenter,

On 05.03.25 11:34, George Cherian wrote:
quoted
quoted
why is armada_37xx_wdt also here?
The stop function in that driver may not sleep.
Marek,

Thanks for reviewing.
Since the stop function has a regmap_write(), I thought it might sleep.
Now that you pointed it out, I assume that it is an MMIO based regmap being used for armada.
I will update the same in the next version
Failure to add WDIOF_STOP_MAYSLEEP when it's needed can lead to
kernel hanging. Failure to add an alternative WDIOF_STOP_ATOMIC
would lead to the kernel option being a no-op.

I think a no-op stop_on_panic (or reset_on_panic) is a saner default.
Agreed. Also, I like WDIOF_STOP_ATOMIC more than the WDIOF_STOP_NOSLEEP
I had suggested in my other response.
1. Instead of blacklisting drivers as WDIOF_STOP_MAYSLEEP, the option will an opt-in.
2. This may not be WDIOF_STOP_AOMIC, instead would be a generic flag not limited to STOP
    operation. May be WDIOF_OPS_ATOMIC (OPS include - .start, .stop, .set_timeout, .ping)
3. Remove the kernel command line option (stop_on_panic) and have a generic reset_on_panic.
4. reset_on_panic=60 (by default )  meaning on a panic the wdog timeout is updated to 60sec
     or the clamp_t(reset_on_panic, min, max_hw_heartbeat_ms).
5. if reset_on_panic=0, it means the watchdog is stopped on panic.
6. All of these to be done by default from panic handler incase of a configured kdump kernel.

-George
Thanks,
Guenter
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help