Messages in this thread | | | From | Doug Anderson <> | Date | Wed, 24 May 2023 11:05:15 -0700 | Subject | Re: [PATCH] watchdog: set variables watchdog_soft,hardlockup_user_enabled storage-class-specifier to static |
| |
Hi,
On Tue, May 23, 2023 at 7:12 AM Doug Anderson <dianders@chromium.org> wrote: > > Hi, > > On Tue, May 23, 2023 at 5:23 AM Tom Rix <trix@redhat.com> wrote: > > > > 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 variabled are only used in their defining file, so it should be static. > > s/variabled/variables > > > > > Signed-off-by: Tom Rix <trix@redhat.com> > > --- > > kernel/watchdog.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > While your fix is valid (thanks!), it's only half the fix. > > I wondered why smatch would have suddenly noticed this since the > change that touched this variable recently was only a rename. When I > dug deeper, I realized that the old names actually _were_ referenced > outside this file and my rename missed them. The reason I missed them > is that the only reference is an "extern" reference in > `include/linux/nmi.h`. The references in `include/linux/nmi.h` > probably should have been removed in commit dd0693fdf054 ("watchdog: > move watchdog sysctl interface to watchdog.c") > > ...so a more complete fix would also remove references to the old > names (nmi_watchdog_user_enabled and soft_watchdog_user_enabled) in > `include/linux/nmi.h`.
FWIW, Petr has the other half of the fix at:
https://lore.kernel.org/r/ZG4TW--j-DdSsUO6@alley
Any chance you could send out a v2 and include that? If I don't see something by tomorrow morning I'll try to send out a v2 for you that squashes your and his changes.
-Doug
| |