[PATCH] watchdog: Fix watchdog_nmi_enable_all()

[PATCH] watchdog: Fix watchdog_nmi_enable_all()

[Don Zickus]

From: Ulrich Obergfell <uobergfe@redhat.com>

The 'watchdog_user_enabled' variable is only used as an 'interface'
to the /proc/sys/kernel/watchdog parameter. The actual state of the
watchdog is tracked by bits in the 'watchdog_enabled' variable. So,
watchdog_nmi_enable_all() should check the NMI_WATCHDOG_ENABLED bit
in 'watchdog_enabled'.

Signed-off-by: Ulrich Obergfell <uobergfe@redhat.com>
Signed-off-by: Don Zickus <dzickus@redhat.com>
---
 kernel/watchdog.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index 2316f50..cba2110 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -608,7 +608,7 @@ void watchdog_nmi_enable_all(void)
 {
 	int cpu;
 
-	if (!watchdog_user_enabled)
+	if (!(watchdog_enabled & NMI_WATCHDOG_ENABLED))
 		return;
 
 	get_online_cpus();
-- 
2.1.0

Re: [PATCH] watchdog: Fix watchdog_nmi_enable_all()

[Andrew Morton]

On Wed, 22 Apr 2015 10:47:49 -0400 Don Zickus <dzickus@redhat.com> wrote:

> From: Ulrich Obergfell <uobergfe@redhat.com>
> 
> The 'watchdog_user_enabled' variable is only used as an 'interface'
> to the /proc/sys/kernel/watchdog parameter. The actual state of the
> watchdog is tracked by bits in the 'watchdog_enabled' variable. So,
> watchdog_nmi_enable_all() should check the NMI_WATCHDOG_ENABLED bit
> in 'watchdog_enabled'.

What are the user-visible effects of the patch?  Please always include
this information when fixing a bug.

Re: [PATCH] watchdog: Fix watchdog_nmi_enable_all()

[Ulrich Obergfell]

> ----- Original Message -----
> From: "Andrew Morton" <akpm@linux-foundation.org>
> To: "Don Zickus" <dzickus@redhat.com>
> Cc: "LKML" <linux-kernel@vger.kernel.org>, "Ulrich Obergfell" <uobergfe@redhat.com>
> Sent: Wednesday, April 22, 2015 10:12:01 PM
> Subject: Re: [PATCH] watchdog: Fix watchdog_nmi_enable_all()
>
> On Wed, 22 Apr 2015 10:47:49 -0400 Don Zickus <dzickus@redhat.com> wrote:
>
>> From: Ulrich Obergfell <uobergfe@redhat.com>
>> 
>> The 'watchdog_user_enabled' variable is only used as an 'interface'
>> to the /proc/sys/kernel/watchdog parameter. The actual state of the
>> watchdog is tracked by bits in the 'watchdog_enabled' variable. So,
>> watchdog_nmi_enable_all() should check the NMI_WATCHDOG_ENABLED bit
>> in 'watchdog_enabled'.
>
> What are the user-visible effects of the patch?  Please always include
> this information when fixing a bug.

Andrew,

this patch has no user-visible effects. The issue that is addressed by this
patch came up during the recent merges. It is related to:

1) https://lkml.org/lkml/2015/2/5/624
2) http://lkml.kernel.org/r/1416251225-17721-12-git-send-email-eranian@google.com

Patch series 1) changes the way how the code in kernel/watchdog.c uses certain
variables such as 'watchdog_user_enabled' and it also introduces a new variable
'watchdog_enabled' as described in:

   https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/kernel/watchdog.c?id=84d56e66b9b4a646f04ec30696ca1aeea5e654d5

This patch adjusts patch 2) which introduces watchdog_nmi_enable_all() to the
new way how the code in kernel/watchdog.c should use 'watchdog_user_enabled'
and 'watchdog_enabled'. Patch 2) is here:

  https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/kernel/watchdog.c?id=b3738d29323344da3017a91010530cf3a58590fc


Regards,

Uli

Re: [PATCH] watchdog: Fix watchdog_nmi_enable_all()

[Don Zickus]

On Thu, Apr 23, 2015 at 04:30:22AM -0400, Ulrich Obergfell wrote:
> > ----- Original Message -----
> > From: "Andrew Morton" <akpm@linux-foundation.org>
> > To: "Don Zickus" <dzickus@redhat.com>
> > Cc: "LKML" <linux-kernel@vger.kernel.org>, "Ulrich Obergfell" <uobergfe@redhat.com>
> > Sent: Wednesday, April 22, 2015 10:12:01 PM
> > Subject: Re: [PATCH] watchdog: Fix watchdog_nmi_enable_all()
> >
> > On Wed, 22 Apr 2015 10:47:49 -0400 Don Zickus <dzickus@redhat.com> wrote:
> >
> >> From: Ulrich Obergfell <uobergfe@redhat.com>
> >> 
> >> The 'watchdog_user_enabled' variable is only used as an 'interface'
> >> to the /proc/sys/kernel/watchdog parameter. The actual state of the
> >> watchdog is tracked by bits in the 'watchdog_enabled' variable. So,
> >> watchdog_nmi_enable_all() should check the NMI_WATCHDOG_ENABLED bit
> >> in 'watchdog_enabled'.
> >
> > What are the user-visible effects of the patch?  Please always include
> > this information when fixing a bug.
> 
> Andrew,
> 
> this patch has no user-visible effects. The issue that is addressed by this
> patch came up during the recent merges. It is related to:
> 
> 1) https://lkml.org/lkml/2015/2/5/624
> 2) http://lkml.kernel.org/r/1416251225-17721-12-git-send-email-eranian@google.com
> 
> Patch series 1) changes the way how the code in kernel/watchdog.c uses certain
> variables such as 'watchdog_user_enabled' and it also introduces a new variable
> 'watchdog_enabled' as described in:
> 
>    https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/kernel/watchdog.c?id=84d56e66b9b4a646f04ec30696ca1aeea5e654d5
> 
> This patch adjusts patch 2) which introduces watchdog_nmi_enable_all() to the
> new way how the code in kernel/watchdog.c should use 'watchdog_user_enabled'
> and 'watchdog_enabled'. Patch 2) is here:
> 
>   https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/kernel/watchdog.c?id=b3738d29323344da3017a91010530cf3a58590fc

Hi Andrew,

Sorry about that.  As Uli describes above, he pushed some patches to change
how the variable was used.  From the time his patches were posted to the
time they were committed, another patch popped in that was not updated to
use the new mechanism he patches created.

This patch just modifies Stephane's patch to reflect the changes Uli posted.

You are right, I did a poor job explaining that.  I hope things are clearer.

Do you want me to repost with an updated explaination?

Cheers,
Don