watchdog: delete old declarations for watchdog_soft,hardlockup_user_enabled + make static

  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(-)
  

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;
>
  

On Thu 2023-05-25 16:28:32, 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")

This would need to be a hash which exists or is going to be merged
into Linus' tree. I am afraid that this is not the case here.
Andrew maintains branches in quilt and the patches have different
time in linux-next each time. Also the patchset might still need
a rebase even when it was maintained in git.

> 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.

It looks OK. Or maybe something like:

Signed-off-by: Tom Rix <trix@redhat.com>
[pmladek@suse.com: changes in nmi.h]
[dianders@chromium.org: merged changes, updated subject and commit message]
Signed-off-by: Douglas Anderson <dianders@chromium.org>

Also feel free to add:

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr