[PATCH] watchdog: move nmi_watchdog sysctl into .rodata

[PATCH] watchdog: move nmi_watchdog sysctl into .rodata

[Joel Granados]

Move nmi_watchdog into the watchdog_sysctls array to prevent it from
unnecessary modification. This move effectively moves it inside the
.rodata section.

Initially moved out into its own non-const array in commit 9ec272c586b0
("watchdog/hardlockup: keep kernel.nmi_watchdog sysctl as 0444 if probe
fails"), which made it writable only when watchdog_hardlockup_available
was true. Moving it back to watchdog_sysctl keeps this behavior as
writing to nmi_watchdog still fails when watchdog_hardlockup_available
is false.

Signed-off-by: Joel Granados <joel.granados@kernel.org>
---
This patch is a followup on the discussion/question I had in [1]. I
would prefer this to be const as it locks things in .rodata and prevents
inadvertent modifications [2].

[1] https://lore.kernel.org/all/588ec9ab-b38a-40b3-8db5-575a09e9a126@meta.com/
[2] https://lore.kernel.org/all/20250109-jag-ctl_table_const-v1-1-622aea7230cf@kernel.org/
---
 kernel/watchdog.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index 80b56c002c7f123c49f65465fe7af9c74de4f46a..1e4e3eccb065a5ce95486f24fa07387e8218cd62 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -1213,14 +1213,11 @@ static const struct ctl_table watchdog_sysctls[] = {
 	},
 #endif /* CONFIG_SMP */
 #endif
-};
-
-static struct ctl_table watchdog_hardlockup_sysctl[] = {
 	{
 		.procname       = "nmi_watchdog",
 		.data		= &watchdog_hardlockup_user_enabled,
 		.maxlen		= sizeof(int),
-		.mode		= 0444,
+		.mode		= 0644,
 		.proc_handler   = proc_nmi_watchdog,
 		.extra1		= SYSCTL_ZERO,
 		.extra2		= SYSCTL_ONE,
@@ -1230,10 +1227,6 @@ static struct ctl_table watchdog_hardlockup_sysctl[] = {
 static void __init watchdog_sysctl_init(void)
 {
 	register_sysctl_init("kernel", watchdog_sysctls);
-
-	if (watchdog_hardlockup_available)
-		watchdog_hardlockup_sysctl[0].mode = 0644;
-	register_sysctl_init("kernel", watchdog_hardlockup_sysctl);
 }
 
 #else

---
base-commit: e5f0a698b34ed76002dc5cff3804a61c80233a7a
change-id: 20250929-jag-nmiwd_const-d7423ab25496

Best regards,
-- 
Joel Granados <joel.granados@kernel.org>

Re: [PATCH] watchdog: move nmi_watchdog sysctl into .rodata

[Petr Mladek]

On Mon 2025-09-29 17:55:07, Joel Granados wrote:
> Move nmi_watchdog into the watchdog_sysctls array to prevent it from
> unnecessary modification. This move effectively moves it inside the
> .rodata section.
> 
> Initially moved out into its own non-const array in commit 9ec272c586b0
> ("watchdog/hardlockup: keep kernel.nmi_watchdog sysctl as 0444 if probe
> fails"), which made it writable only when watchdog_hardlockup_available
> was true. Moving it back to watchdog_sysctl keeps this behavior as
> writing to nmi_watchdog still fails when watchdog_hardlockup_available
> is false.
> 
> Signed-off-by: Joel Granados <joel.granados@kernel.org>

The patch looks good to me. Updating the access rights was nice to
have. But it does not look not worth complicating the constification.
And proc_nmi_watchdog() works correctly even when the access rights
are always 0644.

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

Best Regards,
Petr

Re: [PATCH] watchdog: move nmi_watchdog sysctl into .rodata

[Joel Granados]

[-- Attachment #1: Type: text/plain, Size: 1216 bytes --]

On Mon, Oct 06, 2025 at 05:06:25PM +0200, Petr Mladek wrote:
> On Mon 2025-09-29 17:55:07, Joel Granados wrote:
> > Move nmi_watchdog into the watchdog_sysctls array to prevent it from
> > unnecessary modification. This move effectively moves it inside the
> > .rodata section.
> > 
> > Initially moved out into its own non-const array in commit 9ec272c586b0
> > ("watchdog/hardlockup: keep kernel.nmi_watchdog sysctl as 0444 if probe
> > fails"), which made it writable only when watchdog_hardlockup_available
> > was true. Moving it back to watchdog_sysctl keeps this behavior as
> > writing to nmi_watchdog still fails when watchdog_hardlockup_available
> > is false.
> > 
> > Signed-off-by: Joel Granados <joel.granados@kernel.org>
> 
> The patch looks good to me. Updating the access rights was nice to
> have. But it does not look not worth complicating the constification.
> And proc_nmi_watchdog() works correctly even when the access rights
> are always 0644.
> 
> Reviewed-by: Petr Mladek <pmladek@suse.com>
> 
> Best Regards,
> Petr
Thx for the review.

I'll push this through the sysctl-next branch unless you want to handle
it somewhere else.

Best

-- 

Joel Granados

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] watchdog: move nmi_watchdog sysctl into .rodata

[Petr Mladek]

On Thu 2025-10-09 16:20:48, Joel Granados wrote:
> On Mon, Oct 06, 2025 at 05:06:25PM +0200, Petr Mladek wrote:
> > On Mon 2025-09-29 17:55:07, Joel Granados wrote:
> > > Move nmi_watchdog into the watchdog_sysctls array to prevent it from
> > > unnecessary modification. This move effectively moves it inside the
> > > .rodata section.
> > > 
> > > Initially moved out into its own non-const array in commit 9ec272c586b0
> > > ("watchdog/hardlockup: keep kernel.nmi_watchdog sysctl as 0444 if probe
> > > fails"), which made it writable only when watchdog_hardlockup_available
> > > was true. Moving it back to watchdog_sysctl keeps this behavior as
> > > writing to nmi_watchdog still fails when watchdog_hardlockup_available
> > > is false.
> > > 
> > > Signed-off-by: Joel Granados <joel.granados@kernel.org>
> > 
> > The patch looks good to me. Updating the access rights was nice to
> > have. But it does not look not worth complicating the constification.
> > And proc_nmi_watchdog() works correctly even when the access rights
> > are always 0644.
> > 
> > Reviewed-by: Petr Mladek <pmladek@suse.com>
> > 
> > Best Regards,
> > Petr
> Thx for the review.
> 
> I'll push this through the sysctl-next branch unless you want to handle
> it somewhere else.

Feel free to push it via the sysctl-next branch.

Best Regards,
Petr