Messages in this thread | | | Date | Fri, 26 May 2023 05:53:51 -0700 | Subject | Re: [PATCH] watchdog: delete old declarations for watchdog_soft,hardlockup_user_enabled + make static | From | Tom Rix <> |
| |
On 5/25/23 4:28 PM, Douglas Anderson wrote: > From: Tom Rix <trix@redhat.com> > > smatch reports > kernel/watchdog.c:40:19: warning: symbol > 'watchdog_hardlockup_user_enabled' was not declared. Should it be static? > kernel/watchdog.c:41:19: warning: symbol > 'watchdog_softlockup_user_enabled' was not declared. Should it be static? > > These variables are only used in their defining file, so they should > be static. > > This problem showed up after the patch ("watchdog/hardlockup: rename > some "NMI watchdog" constants/function") because that rename missed > the header file. That didn't cause any compile-time errors because, > since commit dd0693fdf054 ("watchdog: move watchdog sysctl interface > to watchdog.c"), nobody outside of "watchdog.c" was actually referring > to them. Thus, not only should we make these variables static but we > should remove the old declarations in the header file that we missed > renaming. > > Fixes: 4b95b620dcd5 ("watchdog/hardlockup: rename some "NMI watchdog" constants/function") > Signed-off-by: Tom Rix <trix@redhat.com> > [dianders: updated subject + commit message; squashed in Petr's suggestion] > Suggested-by: Petr Mladek <pmladek@suse.com> > Signed-off-by: Douglas Anderson <dianders@chromium.org> > --- > This is a squash of two patches that were posted to mailing lists, one > official patch posted by Tom [1] and one that was posted in reply to > my previous patch by Petr [2]. > > IMO it makes sense to put these two things into one patch since > they're basically dealing with the same issue. As promised [3] I'm > posting the squash of the two patches. > > I have no idea how to really tag this and set authorship. I've chosen > to leave author/Signed-off-by from Tom. Peter didn't officially > include his Singed-off-by on his patch (as is common when posting > suggestions in reply to another patch), so I didn't add it but added a > Suggested-by from him. Hopefully this is OK. I dropped Mukesh's > Reviewed-by just because it felt like things changed enough with the > addition of Petr's stuff that it should be re-added. > > I've tagged this as "Fixes" based on the git hash in the current > linuxnext. > > [1] https://lore.kernel.org/r/20230523122324.1668396-1-trix@redhat.com > [2] https://lore.kernel.org/r/ZG4TW--j-DdSsUO6@alley/ > [3] https://lore.kernel.org/all/CAD=FV=V_i5wR4oNy+xarA9e=VcgpH6i3U1uxFKtsaOe5AQX=Zw@mail.gmail.com/ > > include/linux/nmi.h | 6 ++---- > kernel/watchdog.c | 4 ++-- > 2 files changed, 4 insertions(+), 6 deletions(-)
Looks good to me.
Reviewed-by: Tom Rix <trix@redhat.com>
> > diff --git a/include/linux/nmi.h b/include/linux/nmi.h > index d23902a2fd49..333465e235e1 100644 > --- a/include/linux/nmi.h > +++ b/include/linux/nmi.h > @@ -18,8 +18,6 @@ void lockup_detector_soft_poweroff(void); > void lockup_detector_cleanup(void); > > extern int watchdog_user_enabled; > -extern int nmi_watchdog_user_enabled; > -extern int soft_watchdog_user_enabled; > extern int watchdog_thresh; > extern unsigned long watchdog_enabled; > > @@ -70,8 +68,8 @@ static inline void reset_hung_task_detector(void) { } > * 'watchdog_enabled' variable. Each lockup detector has its dedicated bit - > * bit 0 for the hard lockup detector and bit 1 for the soft lockup detector. > * > - * 'watchdog_user_enabled', 'nmi_watchdog_user_enabled' and > - * 'soft_watchdog_user_enabled' are variables that are only used as an > + * 'watchdog_user_enabled', 'watchdog_hardlockup_user_enabled' and > + * 'watchdog_softlockup_user_enabled' are variables that are only used as an > * 'interface' between the parameters in /proc/sys/kernel and the internal > * state bits in 'watchdog_enabled'. The 'watchdog_thresh' variable is > * handled differently because its value is not boolean, and the lockup > diff --git a/kernel/watchdog.c b/kernel/watchdog.c > index 877d8670f26e..237990e8d345 100644 > --- a/kernel/watchdog.c > +++ b/kernel/watchdog.c > @@ -37,8 +37,8 @@ static DEFINE_MUTEX(watchdog_mutex); > > unsigned long __read_mostly watchdog_enabled; > int __read_mostly watchdog_user_enabled = 1; > -int __read_mostly watchdog_hardlockup_user_enabled = WATCHDOG_HARDLOCKUP_DEFAULT; > -int __read_mostly watchdog_softlockup_user_enabled = 1; > +static int __read_mostly watchdog_hardlockup_user_enabled = WATCHDOG_HARDLOCKUP_DEFAULT; > +static int __read_mostly watchdog_softlockup_user_enabled = 1; > int __read_mostly watchdog_thresh = 10; > static int __read_mostly watchdog_hardlockup_available; >
| |