Message-ID: <20230525162822.1.I0fb41d138d158c9230573eaa37dc56afa2fb14ee@changeid>
Date:   Thu, 25 May 2023 16:28:32 -0700
From:   Douglas Anderson <dianders@...omium.org>
To:     Andrew Morton <akpm@...ux-foundation.org>
Cc:     Tom Rix <trix@...hat.com>, Petr Mladek <pmladek@...e.com>,
        Mukesh Ojha <quic_mojha@...cinc.com>,
        Douglas Anderson <dianders@...omium.org>,
        Laurent Dufour <ldufour@...ux.ibm.com>,
        Lecopzer Chen <lecopzer.chen@...iatek.com>,
        "Paul E. McKenney" <paulmck@...nel.org>,
        Pingfan Liu <kernelfans@...il.com>,
        linux-kernel@...r.kernel.org
Subject: [PATCH] watchdog: delete old declarations for watchdog_soft,hardlockup_user_enabled + make static

From: Tom Rix <trix@...hat.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@...hat.com>
[dianders: updated subject + commit message; squashed in Petr's suggestion]
Suggested-by: Petr Mladek <pmladek@...e.com>
Signed-off-by: Douglas Anderson <dianders@...omium.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(-)

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;
 
-- 
2.41.0.rc0.172.g3f132b7071-goog

Powered by blists - more mailing lists