| Message ID | 20230519101840.v5.6.I4e47cbfa1bb2ebbcdb5ca16817aa2887f15dc82c@changeid (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | watchdog/hardlockup: Add the buddy hardlockup detector | expand |
On Fri 2023-05-19 10:18:30, Douglas Anderson wrote: > In preparation for the buddy hardlockup detector, add comments to > touch_nmi_watchdog() to make it obvious that it touches the configured > hardlockup detector regardless of whether it's backed by an NMI. Also > note that arch_touch_nmi_watchdog() may not be architecture-specific. > > Ideally, we'd like to rename these functions but that is a fairly > disruptive change touching a lot of drivers. After discussion [1] the > plan is to defer this until a good time. > > [1] https://lore.kernel.org/r/ZFy0TX1tfhlH8gxj@alley > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > --- > > Changes in v5: > - No longer rename touch_nmi_watchdog(), just add comments. > > Changes in v4: > - ("Rename touch_nmi_watchdog() to ...") new for v4. > > include/linux/nmi.h | 23 +++++++++++++++++++---- > 1 file changed, 19 insertions(+), 4 deletions(-) > > diff --git a/include/linux/nmi.h b/include/linux/nmi.h > index 454fe99c4874..fafab128f37e 100644 > --- a/include/linux/nmi.h > +++ b/include/linux/nmi.h > @@ -125,15 +125,30 @@ void watchdog_nmi_disable(unsigned int cpu); > void lockup_detector_reconfigure(void); > > /** > - * touch_nmi_watchdog - restart NMI watchdog timeout. > + * touch_nmi_watchdog - manually pet the hardlockup watchdog. > * > - * If the architecture supports the NMI watchdog, touch_nmi_watchdog() > - * may be used to reset the timeout - for code which intentionally > - * disables interrupts for a long time. This call is stateless. > + * If we support detecting hardlockups, touch_nmi_watchdog() may be > + * used to pet the watchdog (reset the timeout) - for code which Nit: I personally prefer "reset the timeout" over "pet the watchdog". "pet" is just another ambiguous name as "touch" ;-) > + * intentionally disables interrupts for a long time. This call is stateless. > + * > + * Though this function has "nmi" in the name, the hardlockup watchdog might > + * not be backed by NMIs. This function will likely be renamed to > + * touch_hardlockup_watchdog() in the future. > */ > static inline void touch_nmi_watchdog(void) > { > + /* > + * Pass on to the hardlockup detector selected via CONFIG_. Note that > + * the hardlockup detector may not be arch-specific nor using NMIs > + * and the arch_touch_nmi_watchdog() function will likely be renamed > + * in the future. > + */ > arch_touch_nmi_watchdog(); > + > + /* > + * Touching the hardlock detector implcitily pets the > + * softlockup detector too > + */ s/implcitily/implicitly/ That said, I would remove this comment completely. It describes what is clear from the code. A more useful information would be why it is done. But it is probably clear as well. CPU could not schedule when interrupts are disabled. > touch_softlockup_watchdog(); > } With the removed comment above touch_softlockup_watchdog(): Reviewed-by: Petr Mladek <pmladek@suse.com> Best Regards, Petr
diff --git a/include/linux/nmi.h b/include/linux/nmi.h index 454fe99c4874..fafab128f37e 100644 --- a/include/linux/nmi.h +++ b/include/linux/nmi.h @@ -125,15 +125,30 @@ void watchdog_nmi_disable(unsigned int cpu); void lockup_detector_reconfigure(void); /** - * touch_nmi_watchdog - restart NMI watchdog timeout. + * touch_nmi_watchdog - manually pet the hardlockup watchdog. * - * If the architecture supports the NMI watchdog, touch_nmi_watchdog() - * may be used to reset the timeout - for code which intentionally - * disables interrupts for a long time. This call is stateless. + * If we support detecting hardlockups, touch_nmi_watchdog() may be + * used to pet the watchdog (reset the timeout) - for code which + * intentionally disables interrupts for a long time. This call is stateless. + * + * Though this function has "nmi" in the name, the hardlockup watchdog might + * not be backed by NMIs. This function will likely be renamed to + * touch_hardlockup_watchdog() in the future. */ static inline void touch_nmi_watchdog(void) { + /* + * Pass on to the hardlockup detector selected via CONFIG_. Note that + * the hardlockup detector may not be arch-specific nor using NMIs + * and the arch_touch_nmi_watchdog() function will likely be renamed + * in the future. + */ arch_touch_nmi_watchdog(); + + /* + * Touching the hardlock detector implcitily pets the + * softlockup detector too + */ touch_softlockup_watchdog(); }
In preparation for the buddy hardlockup detector, add comments to touch_nmi_watchdog() to make it obvious that it touches the configured hardlockup detector regardless of whether it's backed by an NMI. Also note that arch_touch_nmi_watchdog() may not be architecture-specific. Ideally, we'd like to rename these functions but that is a fairly disruptive change touching a lot of drivers. After discussion [1] the plan is to defer this until a good time. [1] https://lore.kernel.org/r/ZFy0TX1tfhlH8gxj@alley Signed-off-by: Douglas Anderson <dianders@chromium.org> --- Changes in v5: - No longer rename touch_nmi_watchdog(), just add comments. Changes in v4: - ("Rename touch_nmi_watchdog() to ...") new for v4. include/linux/nmi.h | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-)