All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] [Watchdog][Trivial] Added comments to explain watchdog_disabled variable
@ 2013-02-03 16:01 anish kumar
  2013-02-13  7:17 ` anish singh
  0 siblings, 1 reply; 16+ messages in thread
From: anish kumar @ 2013-02-03 16:01 UTC (permalink / raw)
  To: akpm, dzickus, tglx, mingo, chuansheng.liu, linux-kernel; +Cc: anish kumar

From: anish kumar <anish198519851985@gmail.com>

This watchdog_disabled flag is bit of cryptic.Howerver it's usefullnes is multifold.
Uses are:
1. Check if smpboot_register_percpu_thread function passed.
2. Makes sure that user enables and disables the watchdog in sequence
   i.e. enable watchdog->disable watchdog->enable watchdog
   Unlike enable watchdog->enable watchdog which is wrong.

Signed-off-by: anish kumar <anish198519851985@gmail.com>
---
 kernel/watchdog.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index 75a2ab3..87a19aa 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -518,6 +518,11 @@ int proc_dowatchdog(struct ctl_table *table, int write,
 		return ret;
 
 	set_sample_period();
+	/*
+	 * We shouldn't enable watchdog threads if it is
+	 * disabled.This is done by watchdog_disabled
+	 * variable check in watchdog_*_all_cpus function.
+	 */
 	if (watchdog_enabled && watchdog_thresh)
 		watchdog_enable_all_cpus();
 	else
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] [Watchdog][Trivial] Added comments to explain watchdog_disabled variable
  2013-02-03 16:01 [PATCH] [Watchdog][Trivial] Added comments to explain watchdog_disabled variable anish kumar
@ 2013-02-13  7:17 ` anish singh
  2013-02-13  9:21   ` Ingo Molnar
  0 siblings, 1 reply; 16+ messages in thread
From: anish singh @ 2013-02-13  7:17 UTC (permalink / raw)
  To: akpm, dzickus, tglx, mingo, chuansheng.liu, linux-kernel; +Cc: anish kumar

Is the below patch picked up?

On Sun, Feb 3, 2013 at 9:31 PM, anish kumar <anish198519851985@gmail.com> wrote:
> From: anish kumar <anish198519851985@gmail.com>
>
> This watchdog_disabled flag is bit of cryptic.Howerver it's usefullnes is multifold.
> Uses are:
> 1. Check if smpboot_register_percpu_thread function passed.
> 2. Makes sure that user enables and disables the watchdog in sequence
>    i.e. enable watchdog->disable watchdog->enable watchdog
>    Unlike enable watchdog->enable watchdog which is wrong.
>
> Signed-off-by: anish kumar <anish198519851985@gmail.com>
> ---
>  kernel/watchdog.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/kernel/watchdog.c b/kernel/watchdog.c
> index 75a2ab3..87a19aa 100644
> --- a/kernel/watchdog.c
> +++ b/kernel/watchdog.c
> @@ -518,6 +518,11 @@ int proc_dowatchdog(struct ctl_table *table, int write,
>                 return ret;
>
>         set_sample_period();
> +       /*
> +        * We shouldn't enable watchdog threads if it is
> +        * disabled.This is done by watchdog_disabled
> +        * variable check in watchdog_*_all_cpus function.
> +        */
>         if (watchdog_enabled && watchdog_thresh)
>                 watchdog_enable_all_cpus();
>         else
> --
> 1.7.1
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] [Watchdog][Trivial] Added comments to explain watchdog_disabled variable
  2013-02-13  7:17 ` anish singh
@ 2013-02-13  9:21   ` Ingo Molnar
  2013-02-13  9:58     ` anish singh
  0 siblings, 1 reply; 16+ messages in thread
From: Ingo Molnar @ 2013-02-13  9:21 UTC (permalink / raw)
  To: anish singh; +Cc: akpm, dzickus, tglx, chuansheng.liu, linux-kernel


* anish singh <anish198519851985@gmail.com> wrote:

> Is the below patch picked up?
> 
> On Sun, Feb 3, 2013 at 9:31 PM, anish kumar <anish198519851985@gmail.com> wrote:
> > From: anish kumar <anish198519851985@gmail.com>
> >
> > This watchdog_disabled flag is bit of cryptic.Howerver it's usefullnes is multifold.
> > Uses are:
> > 1. Check if smpboot_register_percpu_thread function passed.
> > 2. Makes sure that user enables and disables the watchdog in sequence
> >    i.e. enable watchdog->disable watchdog->enable watchdog
> >    Unlike enable watchdog->enable watchdog which is wrong.
> >
> > Signed-off-by: anish kumar <anish198519851985@gmail.com>
> > ---
> >  kernel/watchdog.c |    5 +++++
> >  1 files changed, 5 insertions(+), 0 deletions(-)
> >
> > diff --git a/kernel/watchdog.c b/kernel/watchdog.c
> > index 75a2ab3..87a19aa 100644
> > --- a/kernel/watchdog.c
> > +++ b/kernel/watchdog.c
> > @@ -518,6 +518,11 @@ int proc_dowatchdog(struct ctl_table *table, int write,
> >                 return ret;
> >
> >         set_sample_period();
> > +       /*
> > +        * We shouldn't enable watchdog threads if it is
> > +        * disabled.This is done by watchdog_disabled
> > +        * variable check in watchdog_*_all_cpus function.

It has two grammatic and a stylistic error in it, plus misses 
the convention that function names are mentioned with a '()'.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] [Watchdog][Trivial] Added comments to explain watchdog_disabled variable
  2013-02-13  9:21   ` Ingo Molnar
@ 2013-02-13  9:58     ` anish singh
  2013-02-15 16:29       ` Don Zickus
  0 siblings, 1 reply; 16+ messages in thread
From: anish singh @ 2013-02-13  9:58 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: akpm, dzickus, tglx, chuansheng.liu, linux-kernel

On Wed, Feb 13, 2013 at 2:51 PM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * anish singh <anish198519851985@gmail.com> wrote:
>
>> Is the below patch picked up?
>>
>> On Sun, Feb 3, 2013 at 9:31 PM, anish kumar <anish198519851985@gmail.com> wrote:
>> > From: anish kumar <anish198519851985@gmail.com>
>> >
>> > This watchdog_disabled flag is bit of cryptic.Howerver it's usefullnes is multifold.
>> > Uses are:
>> > 1. Check if smpboot_register_percpu_thread function passed.
>> > 2. Makes sure that user enables and disables the watchdog in sequence
>> >    i.e. enable watchdog->disable watchdog->enable watchdog
>> >    Unlike enable watchdog->enable watchdog which is wrong.
>> >
>> > Signed-off-by: anish kumar <anish198519851985@gmail.com>
>> > ---
>> >  kernel/watchdog.c |    5 +++++
>> >  1 files changed, 5 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/kernel/watchdog.c b/kernel/watchdog.c
>> > index 75a2ab3..87a19aa 100644
>> > --- a/kernel/watchdog.c
>> > +++ b/kernel/watchdog.c
>> > @@ -518,6 +518,11 @@ int proc_dowatchdog(struct ctl_table *table, int write,
>> >                 return ret;
>> >
>> >         set_sample_period();
>> > +       /*
>> > +        * We shouldn't enable watchdog threads if it is
>> > +        * disabled.This is done by watchdog_disabled
>> > +        * variable check in watchdog_*_all_cpus function.
>
> It has two grammatic and a stylistic error in it, plus misses
Would you mind pointing it out to me the grammatical mistakes
as I am not that good with grammar.
I thought I followed the conventions as below:
/*
 *
 *
 */
> the convention that function names are mentioned with a '()'.
>
> Thanks,
>
>         Ingo

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] [Watchdog][Trivial] Added comments to explain watchdog_disabled variable
  2013-02-13  9:58     ` anish singh
@ 2013-02-15 16:29       ` Don Zickus
  2013-02-16  8:42         ` Ingo Molnar
  0 siblings, 1 reply; 16+ messages in thread
From: Don Zickus @ 2013-02-15 16:29 UTC (permalink / raw)
  To: anish singh; +Cc: Ingo Molnar, akpm, tglx, chuansheng.liu, linux-kernel

On Wed, Feb 13, 2013 at 03:28:00PM +0530, anish singh wrote:
> On Wed, Feb 13, 2013 at 2:51 PM, Ingo Molnar <mingo@kernel.org> wrote:
> >
> > * anish singh <anish198519851985@gmail.com> wrote:
> >
> >> Is the below patch picked up?
> >>
> >> On Sun, Feb 3, 2013 at 9:31 PM, anish kumar <anish198519851985@gmail.com> wrote:
> >> > From: anish kumar <anish198519851985@gmail.com>
> >> >
> >> > This watchdog_disabled flag is bit of cryptic.Howerver it's usefullnes is multifold.
> >> > Uses are:
> >> > 1. Check if smpboot_register_percpu_thread function passed.
> >> > 2. Makes sure that user enables and disables the watchdog in sequence
> >> >    i.e. enable watchdog->disable watchdog->enable watchdog
> >> >    Unlike enable watchdog->enable watchdog which is wrong.
> >> >
> >> > Signed-off-by: anish kumar <anish198519851985@gmail.com>
> >> > ---
> >> >  kernel/watchdog.c |    5 +++++
> >> >  1 files changed, 5 insertions(+), 0 deletions(-)
> >> >
> >> > diff --git a/kernel/watchdog.c b/kernel/watchdog.c
> >> > index 75a2ab3..87a19aa 100644
> >> > --- a/kernel/watchdog.c
> >> > +++ b/kernel/watchdog.c
> >> > @@ -518,6 +518,11 @@ int proc_dowatchdog(struct ctl_table *table, int write,
> >> >                 return ret;
> >> >
> >> >         set_sample_period();
> >> > +       /*
> >> > +        * We shouldn't enable watchdog threads if it is
> >> > +        * disabled.This is done by watchdog_disabled
> >> > +        * variable check in watchdog_*_all_cpus function.
> >
> > It has two grammatic and a stylistic error in it, plus misses
> Would you mind pointing it out to me the grammatical mistakes
> as I am not that good with grammar.

I am not entirely sure which ones Ingo is referring to, but what I see are

'disabled.This' needs a space after period
'This is done by watchdog_disabled' needs a 'the' after 'by'
'variable check in watchdog..' needs a 'the' after 'in'.

in addition to the missing ()'s after 'watchdog_*_all_cpus.

Unless I am missing something else, I think that is it.

Cheers,
Don

> I thought I followed the conventions as below:
> /*
>  *
>  *
>  */
> > the convention that function names are mentioned with a '()'.
> >
> > Thanks,
> >
> >         Ingo

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] [Watchdog][Trivial] Added comments to explain watchdog_disabled variable
  2013-02-15 16:29       ` Don Zickus
@ 2013-02-16  8:42         ` Ingo Molnar
  2013-02-16 11:38           ` anish kumar
  0 siblings, 1 reply; 16+ messages in thread
From: Ingo Molnar @ 2013-02-16  8:42 UTC (permalink / raw)
  To: Don Zickus; +Cc: anish singh, akpm, tglx, chuansheng.liu, linux-kernel


* Don Zickus <dzickus@redhat.com> wrote:

> > >> > +       /*
> > >> > +        * We shouldn't enable watchdog threads if it is
> > >> > +        * disabled.This is done by watchdog_disabled
> > >> > +        * variable check in watchdog_*_all_cpus function.
> > >
> > > It has two grammatic and a stylistic error in it, plus misses
> > Would you mind pointing it out to me the grammatical mistakes
> > as I am not that good with grammar.
> 
> I am not entirely sure which ones Ingo is referring to, but what I see are
> 
> 'disabled.This' needs a space after period
> 'This is done by watchdog_disabled' needs a 'the' after 'by'
> 'variable check in watchdog..' needs a 'the' after 'in'.
> 
> in addition to the missing ()'s after 'watchdog_*_all_cpus.

There's also a plural/singular mismatch between 'watchdog 
threads' and 'if it is disabled'.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] [Watchdog][Trivial] Added comments to explain watchdog_disabled variable
  2013-02-16  8:42         ` Ingo Molnar
@ 2013-02-16 11:38           ` anish kumar
  0 siblings, 0 replies; 16+ messages in thread
From: anish kumar @ 2013-02-16 11:38 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: Don Zickus, akpm, tglx, chuansheng.liu, linux-kernel

On Sat, 2013-02-16 at 09:42 +0100, Ingo Molnar wrote:
> * Don Zickus <dzickus@redhat.com> wrote:
> 
> > > >> > +       /*
> > > >> > +        * We shouldn't enable watchdog threads if it is
> > > >> > +        * disabled.This is done by watchdog_disabled
> > > >> > +        * variable check in watchdog_*_all_cpus function.
> > > >
> > > > It has two grammatic and a stylistic error in it, plus misses
> > > Would you mind pointing it out to me the grammatical mistakes
> > > as I am not that good with grammar.
> > 
> > I am not entirely sure which ones Ingo is referring to, but what I see are
> > 
> > 'disabled.This' needs a space after period
> > 'This is done by watchdog_disabled' needs a 'the' after 'by'
> > 'variable check in watchdog..' needs a 'the' after 'in'.
> > 
> > in addition to the missing ()'s after 'watchdog_*_all_cpus.
> 
> There's also a plural/singular mismatch between 'watchdog 
> threads' and 'if it is disabled'.
Thanks, I will update the same in the next patch.
> 
> Thanks,
> 
> 	Ingo



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] [Watchdog][Trivial] Added comments to explain watchdog_disabled variable
  2013-03-06 11:47   ` anish singh
@ 2013-03-07 15:40     ` Don Zickus
  0 siblings, 0 replies; 16+ messages in thread
From: Don Zickus @ 2013-03-07 15:40 UTC (permalink / raw)
  To: anish singh; +Cc: akpm, mingo, chuansheng.liu, paulmck, linux-kernel

On Wed, Mar 06, 2013 at 05:17:04PM +0530, anish singh wrote:
> On Tue, Feb 19, 2013 at 2:18 AM, Don Zickus <dzickus@redhat.com> wrote:
> > On Sat, Feb 16, 2013 at 05:44:09PM +0530, anish kumar wrote:
> >> From: anish kumar <anish198519851985@gmail.com>
> >>
> >> This watchdog_disabled flag is bit of cryptic.Howerver it's usefullnes is multifold.
> >> Uses are:
> >> 1. Check if smpboot_register_percpu_thread function passed.
> >> 2. Makes sure that user enables and disables the watchdog in sequence
> >>    i.e. enable watchdog->disable watchdog->enable watchdog
> >>    Unlike enable watchdog->enable watchdog which is wrong.
> >>
> >> Signed-off-by: anish kumar <anish198519851985@gmail.com>
> >> ---
> >>  kernel/watchdog.c |    5 +++++
> >>  1 files changed, 5 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/kernel/watchdog.c b/kernel/watchdog.c
> >> index 75a2ab3..8a20ebe 100644
> >> --- a/kernel/watchdog.c
> >> +++ b/kernel/watchdog.c
> >> @@ -518,6 +518,11 @@ int proc_dowatchdog(struct ctl_table *table, int write,
> >>               return ret;
> >>
> >>       set_sample_period();
> >> +     /*
> >> +      * Watchdog threads shouldn't be enabled if they are
> >> +      * disabled.'watchdog_disabled' variable check in
> >
> > Missing a 'The'  ^^^
> >
> > Other than that,
> >
> > Acked-by: Don Zickus <dzickus@redhat.com>
> Is this patch picked up?

Andrew,

Do you want me to respin the patch with that change? Or how should I move
this patch forward?

Cheers,
Don

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] [Watchdog][Trivial] Added comments to explain watchdog_disabled variable
  2013-02-18 20:48 ` Don Zickus
@ 2013-03-06 11:47   ` anish singh
  2013-03-07 15:40     ` Don Zickus
  0 siblings, 1 reply; 16+ messages in thread
From: anish singh @ 2013-03-06 11:47 UTC (permalink / raw)
  To: Don Zickus; +Cc: akpm, mingo, chuansheng.liu, paulmck, linux-kernel

On Tue, Feb 19, 2013 at 2:18 AM, Don Zickus <dzickus@redhat.com> wrote:
> On Sat, Feb 16, 2013 at 05:44:09PM +0530, anish kumar wrote:
>> From: anish kumar <anish198519851985@gmail.com>
>>
>> This watchdog_disabled flag is bit of cryptic.Howerver it's usefullnes is multifold.
>> Uses are:
>> 1. Check if smpboot_register_percpu_thread function passed.
>> 2. Makes sure that user enables and disables the watchdog in sequence
>>    i.e. enable watchdog->disable watchdog->enable watchdog
>>    Unlike enable watchdog->enable watchdog which is wrong.
>>
>> Signed-off-by: anish kumar <anish198519851985@gmail.com>
>> ---
>>  kernel/watchdog.c |    5 +++++
>>  1 files changed, 5 insertions(+), 0 deletions(-)
>>
>> diff --git a/kernel/watchdog.c b/kernel/watchdog.c
>> index 75a2ab3..8a20ebe 100644
>> --- a/kernel/watchdog.c
>> +++ b/kernel/watchdog.c
>> @@ -518,6 +518,11 @@ int proc_dowatchdog(struct ctl_table *table, int write,
>>               return ret;
>>
>>       set_sample_period();
>> +     /*
>> +      * Watchdog threads shouldn't be enabled if they are
>> +      * disabled.'watchdog_disabled' variable check in
>
> Missing a 'The'  ^^^
>
> Other than that,
>
> Acked-by: Don Zickus <dzickus@redhat.com>
Is this patch picked up?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] [Watchdog][Trivial] Added comments to explain watchdog_disabled variable
  2013-02-16 12:14 anish kumar
@ 2013-02-18 20:48 ` Don Zickus
  2013-03-06 11:47   ` anish singh
  0 siblings, 1 reply; 16+ messages in thread
From: Don Zickus @ 2013-02-18 20:48 UTC (permalink / raw)
  To: anish kumar; +Cc: akpm, mingo, chuansheng.liu, paulmck, linux-kernel

On Sat, Feb 16, 2013 at 05:44:09PM +0530, anish kumar wrote:
> From: anish kumar <anish198519851985@gmail.com>
> 
> This watchdog_disabled flag is bit of cryptic.Howerver it's usefullnes is multifold.
> Uses are:
> 1. Check if smpboot_register_percpu_thread function passed.
> 2. Makes sure that user enables and disables the watchdog in sequence
>    i.e. enable watchdog->disable watchdog->enable watchdog
>    Unlike enable watchdog->enable watchdog which is wrong.
> 
> Signed-off-by: anish kumar <anish198519851985@gmail.com>
> ---
>  kernel/watchdog.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/kernel/watchdog.c b/kernel/watchdog.c
> index 75a2ab3..8a20ebe 100644
> --- a/kernel/watchdog.c
> +++ b/kernel/watchdog.c
> @@ -518,6 +518,11 @@ int proc_dowatchdog(struct ctl_table *table, int write,
>  		return ret;
>  
>  	set_sample_period();
> +	/*
> +	 * Watchdog threads shouldn't be enabled if they are
> +	 * disabled.'watchdog_disabled' variable check in

Missing a 'The'  ^^^

Other than that,

Acked-by: Don Zickus <dzickus@redhat.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] [Watchdog][Trivial] Added comments to explain watchdog_disabled variable
@ 2013-02-16 12:14 anish kumar
  2013-02-18 20:48 ` Don Zickus
  0 siblings, 1 reply; 16+ messages in thread
From: anish kumar @ 2013-02-16 12:14 UTC (permalink / raw)
  To: akpm, dzickus, mingo; +Cc: chuansheng.liu, paulmck, linux-kernel, anish kumar

From: anish kumar <anish198519851985@gmail.com>

This watchdog_disabled flag is bit of cryptic.Howerver it's usefullnes is multifold.
Uses are:
1. Check if smpboot_register_percpu_thread function passed.
2. Makes sure that user enables and disables the watchdog in sequence
   i.e. enable watchdog->disable watchdog->enable watchdog
   Unlike enable watchdog->enable watchdog which is wrong.

Signed-off-by: anish kumar <anish198519851985@gmail.com>
---
 kernel/watchdog.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index 75a2ab3..8a20ebe 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -518,6 +518,11 @@ int proc_dowatchdog(struct ctl_table *table, int write,
 		return ret;
 
 	set_sample_period();
+	/*
+	 * Watchdog threads shouldn't be enabled if they are
+	 * disabled.'watchdog_disabled' variable check in
+	 * watchdog_*_all_cpus() function takes care of this.
+	 */
 	if (watchdog_enabled && watchdog_thresh)
 		watchdog_enable_all_cpus();
 	else
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] [Watchdog][Trivial] Added comments to explain watchdog_disabled variable
  2013-02-01 14:59 ` Don Zickus
@ 2013-02-02  3:16   ` anish kumar
  0 siblings, 0 replies; 16+ messages in thread
From: anish kumar @ 2013-02-02  3:16 UTC (permalink / raw)
  To: akpm; +Cc: tglx, mingo, chuansheng.liu, linux-kernel, Don Zickus

On Fri, 2013-02-01 at 09:59 -0500, Don Zickus wrote:
> On Fri, Feb 01, 2013 at 07:19:07PM +0530, anish kumar wrote:
> > From: anish kumar <anish198519851985@gmail.com>
> > 
> > This watchdog_disabled flag is bit of cryptic.Howerver it's usefullnes is multifold.
> > Uses are:
> > 1. Check if smpboot_register_percpu_thread function passed.
> > 2. Makes sure that user enables and disables the watchdog in sequence
> >    i.e. enable watchdog->disable watchdog->enable watchdog
> >    Unlike enable watchdog->enable watchdog which is wrong.
> > 
> > Signed-off-by: anish kumar <anish198519851985@gmail.com>
> > ---
> >  kernel/watchdog.c |    5 +++++
> >  1 files changed, 5 insertions(+), 0 deletions(-)
> > 
> > diff --git a/kernel/watchdog.c b/kernel/watchdog.c
> > index 6ef638b..dfd843a 100644
> > --- a/kernel/watchdog.c
> > +++ b/kernel/watchdog.c
> > @@ -519,6 +519,11 @@ int proc_dowatchdog(struct ctl_table *table, int write,
> >  		return ret;
> >  
> >  	set_sample_period();
> > +	/*
> > +	 * We shouldn't enable watchdog threads if it is not
> 							^^^
> the 'not' is not needed I believe.  Other than that, if it helps
> to understand the code better.  I am fine with it.
> 
> Acked-by: Don Zickus <dzickus@redhat.com>
Should I wait for some more acked-by's?
> 
> > +	 * disabled.This is done by watchdog_disabled
> > +	 * variable check in watchdog_*_all_cpus function.
> > +	 */
> >  	if (watchdog_enabled && watchdog_thresh)
> >  		watchdog_enable_all_cpus();
> >  	else
> > -- 
> > 1.7.1
> > 



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] [Watchdog][Trivial] Added comments to explain watchdog_disabled variable
  2013-02-01 13:49 anish kumar
@ 2013-02-01 14:59 ` Don Zickus
  2013-02-02  3:16   ` anish kumar
  0 siblings, 1 reply; 16+ messages in thread
From: Don Zickus @ 2013-02-01 14:59 UTC (permalink / raw)
  To: anish kumar; +Cc: akpm, tglx, mingo, chuansheng.liu, linux-kernel

On Fri, Feb 01, 2013 at 07:19:07PM +0530, anish kumar wrote:
> From: anish kumar <anish198519851985@gmail.com>
> 
> This watchdog_disabled flag is bit of cryptic.Howerver it's usefullnes is multifold.
> Uses are:
> 1. Check if smpboot_register_percpu_thread function passed.
> 2. Makes sure that user enables and disables the watchdog in sequence
>    i.e. enable watchdog->disable watchdog->enable watchdog
>    Unlike enable watchdog->enable watchdog which is wrong.
> 
> Signed-off-by: anish kumar <anish198519851985@gmail.com>
> ---
>  kernel/watchdog.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/kernel/watchdog.c b/kernel/watchdog.c
> index 6ef638b..dfd843a 100644
> --- a/kernel/watchdog.c
> +++ b/kernel/watchdog.c
> @@ -519,6 +519,11 @@ int proc_dowatchdog(struct ctl_table *table, int write,
>  		return ret;
>  
>  	set_sample_period();
> +	/*
> +	 * We shouldn't enable watchdog threads if it is not
							^^^
the 'not' is not needed I believe.  Other than that, if it helps
to understand the code better.  I am fine with it.

Acked-by: Don Zickus <dzickus@redhat.com>

> +	 * disabled.This is done by watchdog_disabled
> +	 * variable check in watchdog_*_all_cpus function.
> +	 */
>  	if (watchdog_enabled && watchdog_thresh)
>  		watchdog_enable_all_cpus();
>  	else
> -- 
> 1.7.1
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] [Watchdog][Trivial] Added comments to explain watchdog_disabled variable
  2013-02-01 13:33 anish kumar
@ 2013-02-01 13:50 ` anish kumar
  0 siblings, 0 replies; 16+ messages in thread
From: anish kumar @ 2013-02-01 13:50 UTC (permalink / raw)
  To: akpm; +Cc: dzickus, tglx, mingo, chuansheng.liu, linux-kernel

Please ignore this patch.
On Fri, 2013-02-01 at 19:03 +0530, anish kumar wrote:
> From: anish kumar <anish198519851985@gmail.com>
> 
> This watchdog_disabled flag is bit of cryptic.Howerver it's usefullnes is multifold.
> Uses are:
> 1. Check if smpboot_register_percpu_thread function passed.
> 2. Makes sure that user enables and disables the watchdog in sequence
>    i.e. enable watchdog->disable watchdog->enable watchdog
>    Unlike enable watchdog->enable watchdog which is wrong.
> 
> Signed-off-by: anish kumar <anish198519851985@gmail.com>
> ---
>  kernel/watchdog.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/kernel/watchdog.c b/kernel/watchdog.c
> index 75a2ab3..6ef638b 100644
> --- a/kernel/watchdog.c
> +++ b/kernel/watchdog.c
> @@ -155,6 +155,7 @@ void touch_all_softlockup_watchdogs(void)
>  	for_each_online_cpu(cpu)
>  		per_cpu(watchdog_touch_ts, cpu) = 0;
>  }
> +EXPORT_SYMBOL(touch_all_softlockup_watchdogs);
>  
>  #ifdef CONFIG_HARDLOCKUP_DETECTOR
>  void touch_nmi_watchdog(void)



^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] [Watchdog][Trivial] Added comments to explain watchdog_disabled variable
@ 2013-02-01 13:49 anish kumar
  2013-02-01 14:59 ` Don Zickus
  0 siblings, 1 reply; 16+ messages in thread
From: anish kumar @ 2013-02-01 13:49 UTC (permalink / raw)
  To: akpm, dzickus, tglx, mingo, chuansheng.liu, linux-kernel; +Cc: anish kumar

From: anish kumar <anish198519851985@gmail.com>

This watchdog_disabled flag is bit of cryptic.Howerver it's usefullnes is multifold.
Uses are:
1. Check if smpboot_register_percpu_thread function passed.
2. Makes sure that user enables and disables the watchdog in sequence
   i.e. enable watchdog->disable watchdog->enable watchdog
   Unlike enable watchdog->enable watchdog which is wrong.

Signed-off-by: anish kumar <anish198519851985@gmail.com>
---
 kernel/watchdog.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index 6ef638b..dfd843a 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -519,6 +519,11 @@ int proc_dowatchdog(struct ctl_table *table, int write,
 		return ret;
 
 	set_sample_period();
+	/*
+	 * We shouldn't enable watchdog threads if it is not
+	 * disabled.This is done by watchdog_disabled
+	 * variable check in watchdog_*_all_cpus function.
+	 */
 	if (watchdog_enabled && watchdog_thresh)
 		watchdog_enable_all_cpus();
 	else
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH] [Watchdog][Trivial] Added comments to explain watchdog_disabled variable
@ 2013-02-01 13:33 anish kumar
  2013-02-01 13:50 ` anish kumar
  0 siblings, 1 reply; 16+ messages in thread
From: anish kumar @ 2013-02-01 13:33 UTC (permalink / raw)
  To: akpm, dzickus, tglx, mingo, chuansheng.liu, linux-kernel; +Cc: anish kumar

From: anish kumar <anish198519851985@gmail.com>

This watchdog_disabled flag is bit of cryptic.Howerver it's usefullnes is multifold.
Uses are:
1. Check if smpboot_register_percpu_thread function passed.
2. Makes sure that user enables and disables the watchdog in sequence
   i.e. enable watchdog->disable watchdog->enable watchdog
   Unlike enable watchdog->enable watchdog which is wrong.

Signed-off-by: anish kumar <anish198519851985@gmail.com>
---
 kernel/watchdog.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index 75a2ab3..6ef638b 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -155,6 +155,7 @@ void touch_all_softlockup_watchdogs(void)
 	for_each_online_cpu(cpu)
 		per_cpu(watchdog_touch_ts, cpu) = 0;
 }
+EXPORT_SYMBOL(touch_all_softlockup_watchdogs);
 
 #ifdef CONFIG_HARDLOCKUP_DETECTOR
 void touch_nmi_watchdog(void)
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2013-03-07 15:40 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-03 16:01 [PATCH] [Watchdog][Trivial] Added comments to explain watchdog_disabled variable anish kumar
2013-02-13  7:17 ` anish singh
2013-02-13  9:21   ` Ingo Molnar
2013-02-13  9:58     ` anish singh
2013-02-15 16:29       ` Don Zickus
2013-02-16  8:42         ` Ingo Molnar
2013-02-16 11:38           ` anish kumar
  -- strict thread matches above, loose matches on Subject: below --
2013-02-16 12:14 anish kumar
2013-02-18 20:48 ` Don Zickus
2013-03-06 11:47   ` anish singh
2013-03-07 15:40     ` Don Zickus
2013-02-01 13:49 anish kumar
2013-02-01 14:59 ` Don Zickus
2013-02-02  3:16   ` anish kumar
2013-02-01 13:33 anish kumar
2013-02-01 13:50 ` anish kumar

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.