All of lore.kernel.org
 help / color / mirror / Atom feed
From: Don Zickus <dzickus@redhat.com>
To: Babu Moger <babu.moger@oracle.com>
Cc: mingo@kernel.org, akpm@linux-foundation.org, ak@linux.intel.com,
	jkosina@suse.cz, baiyaowei@cmss.chinamobile.com,
	atomlin@redhat.com, uobergfe@redhat.com, tj@kernel.org,
	hidehiro.kawai.ez@hitachi.com, johunt@akamai.com,
	davem@davemloft.net, sparclinux@vger.kernel.org,
	linux-kernel@vger.kernel.org, sam@ravnborg.org
Subject: Re: [PATCH v2 0/2] Introduce arch specific nmi enable, disable handlers
Date: Mon, 17 Oct 2016 10:25:01 -0400	[thread overview]
Message-ID: <20161017142501.GJ98438@redhat.com> (raw)
In-Reply-To: <1476391082-77928-1-git-send-email-babu.moger@oracle.com>

On Thu, Oct 13, 2016 at 01:38:00PM -0700, Babu Moger wrote:
> During our testing we noticed that nmi watchdogs in sparc could not be disabled or
> enabled dynamically using sysctl/proc interface. Sparc uses its own arch specific
> nmi watchdogs. There is a sysctl and proc interface(proc/sys/kernel/nmi_watchdog)
> to enable/disable nmi watchdogs. However, that is not working for sparc. There
> is no interface to feed this parameter to arch specific nmi watchdogs.
> 
> These patches extend the same sysctl/proc interface to enable or disable
> these arch specific nmi watchdogs dynamically. Introduced new functions
> arch_watchdog_nmi_enable and arch_watchdog_nmi_disable which can be implemented
> in arch specific handlers.
> If you think there is a better way to do this. Please advice.
> 
> Tested on sparc. Compile tested on x86.

Thanks Babu!  Looking through the code it seems appropriate with what I
suggested.  I am running it through my testsuite and things are failing.
But it is most likely my system is misconfigured.  Give me another day or so
to straighten that out.

Cheers,
Don

> 
> v2:
>   a)Sam Ravnborg's comments about making the definitions visible.
>   With the new approach we dont need those definitions((NMI_WATCHDOG_ENABLED,
>   SOFT_WATCHDOG_ENABLED etc..) outside watchdog.c. So no action.
> 
>   b) Made changes per Don Zickus comments.
>   Don, I could not use your patches as is. Reason is sparc does not define
>   CONFIG_HARDLOCKUP_DETECTOR. So, defining default __weak function did not
>   work for me. However, I have used your idea to define __weak functions
>   arch_watchdog_nmi_enable and arch_watchdog_nmi_disable when CONFIG_HARDLOCKUP_DETECTOR
>   is not defined. I feel this should have very less impact on the races you are
>   concerned about. Please take a look. Feel free to suggest.
> 
>   Patch2 changes: I had to introduce new variable nmi_init_done to synchronize
>   watchdog thread and kernel init thread.
> 
> v1:
>  Initial version. Discussion thread here
>  http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1245427.html
> 
> Babu Moger (2):
>   watchdog: Introduce arch_watchdog_nmi_enable and
>     arch_watchdog_nmi_disable
>   sparc: Implement arch_watchdog_nmi_enable and
>     arch_watchdog_nmi_disable
> 
>  arch/sparc/kernel/nmi.c |   41 +++++++++++++++++++++++++++++-
>  kernel/watchdog.c       |   65 +++++++++++++++++++++++++++++++---------------
>  2 files changed, 84 insertions(+), 22 deletions(-)
> 

WARNING: multiple messages have this Message-ID (diff)
From: Don Zickus <dzickus@redhat.com>
To: Babu Moger <babu.moger@oracle.com>
Cc: mingo@kernel.org, akpm@linux-foundation.org, ak@linux.intel.com,
	jkosina@suse.cz, baiyaowei@cmss.chinamobile.com,
	atomlin@redhat.com, uobergfe@redhat.com, tj@kernel.org,
	hidehiro.kawai.ez@hitachi.com, johunt@akamai.com,
	davem@davemloft.net, sparclinux@vger.kernel.org,
	linux-kernel@vger.kernel.org, sam@ravnborg.org
Subject: Re: [PATCH v2 0/2] Introduce arch specific nmi enable, disable handlers
Date: Mon, 17 Oct 2016 14:25:01 +0000	[thread overview]
Message-ID: <20161017142501.GJ98438@redhat.com> (raw)
In-Reply-To: <1476391082-77928-1-git-send-email-babu.moger@oracle.com>

On Thu, Oct 13, 2016 at 01:38:00PM -0700, Babu Moger wrote:
> During our testing we noticed that nmi watchdogs in sparc could not be disabled or
> enabled dynamically using sysctl/proc interface. Sparc uses its own arch specific
> nmi watchdogs. There is a sysctl and proc interface(proc/sys/kernel/nmi_watchdog)
> to enable/disable nmi watchdogs. However, that is not working for sparc. There
> is no interface to feed this parameter to arch specific nmi watchdogs.
> 
> These patches extend the same sysctl/proc interface to enable or disable
> these arch specific nmi watchdogs dynamically. Introduced new functions
> arch_watchdog_nmi_enable and arch_watchdog_nmi_disable which can be implemented
> in arch specific handlers.
> If you think there is a better way to do this. Please advice.
> 
> Tested on sparc. Compile tested on x86.

Thanks Babu!  Looking through the code it seems appropriate with what I
suggested.  I am running it through my testsuite and things are failing.
But it is most likely my system is misconfigured.  Give me another day or so
to straighten that out.

Cheers,
Don

> 
> v2:
>   a)Sam Ravnborg's comments about making the definitions visible.
>   With the new approach we dont need those definitions((NMI_WATCHDOG_ENABLED,
>   SOFT_WATCHDOG_ENABLED etc..) outside watchdog.c. So no action.
> 
>   b) Made changes per Don Zickus comments.
>   Don, I could not use your patches as is. Reason is sparc does not define
>   CONFIG_HARDLOCKUP_DETECTOR. So, defining default __weak function did not
>   work for me. However, I have used your idea to define __weak functions
>   arch_watchdog_nmi_enable and arch_watchdog_nmi_disable when CONFIG_HARDLOCKUP_DETECTOR
>   is not defined. I feel this should have very less impact on the races you are
>   concerned about. Please take a look. Feel free to suggest.
> 
>   Patch2 changes: I had to introduce new variable nmi_init_done to synchronize
>   watchdog thread and kernel init thread.
> 
> v1:
>  Initial version. Discussion thread here
>  http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1245427.html
> 
> Babu Moger (2):
>   watchdog: Introduce arch_watchdog_nmi_enable and
>     arch_watchdog_nmi_disable
>   sparc: Implement arch_watchdog_nmi_enable and
>     arch_watchdog_nmi_disable
> 
>  arch/sparc/kernel/nmi.c |   41 +++++++++++++++++++++++++++++-
>  kernel/watchdog.c       |   65 +++++++++++++++++++++++++++++++---------------
>  2 files changed, 84 insertions(+), 22 deletions(-)
> 

  parent reply	other threads:[~2016-10-17 14:25 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-13 20:38 [PATCH v2 0/2] Introduce arch specific nmi enable, disable handlers Babu Moger
2016-10-13 20:38 ` Babu Moger
2016-10-13 20:38 ` [PATCH v2 1/2] watchdog: Introduce arch_watchdog_nmi_enable and arch_watchdog_nmi_disable Babu Moger
2016-10-13 20:38   ` Babu Moger
2016-10-17 17:31   ` Don Zickus
2016-10-17 17:31     ` Don Zickus
2016-10-18  2:46     ` Babu Moger
2016-10-18  2:46       ` Babu Moger
2016-10-20  0:00   ` Andrew Morton
2016-10-20  0:00     ` Andrew Morton
2016-10-20 16:14     ` Don Zickus
2016-10-20 16:14       ` Don Zickus
2016-10-21  3:25       ` Andrew Morton
2016-10-21  3:25         ` Andrew Morton
2016-10-21 15:11         ` Don Zickus
2016-10-21 15:11           ` Don Zickus
2016-10-21 19:19           ` Andrew Morton
2016-10-21 19:19             ` Andrew Morton
2016-10-21 21:50             ` Babu Moger
2016-10-21 21:50               ` Babu Moger
2016-10-24 15:19               ` Don Zickus
2016-10-24 15:19                 ` Don Zickus
2016-10-25  0:55                 ` Babu Moger
2016-10-25  0:55                   ` Babu Moger
2016-10-13 20:38 ` [PATCH v2 2/2] sparc: Implement " Babu Moger
2016-10-13 20:38   ` Babu Moger
2016-10-17 14:25 ` Don Zickus [this message]
2016-10-17 14:25   ` [PATCH v2 0/2] Introduce arch specific nmi enable, disable handlers Don Zickus

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161017142501.GJ98438@redhat.com \
    --to=dzickus@redhat.com \
    --cc=ak@linux.intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=atomlin@redhat.com \
    --cc=babu.moger@oracle.com \
    --cc=baiyaowei@cmss.chinamobile.com \
    --cc=davem@davemloft.net \
    --cc=hidehiro.kawai.ez@hitachi.com \
    --cc=jkosina@suse.cz \
    --cc=johunt@akamai.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=sam@ravnborg.org \
    --cc=sparclinux@vger.kernel.org \
    --cc=tj@kernel.org \
    --cc=uobergfe@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.