[PATCH 1/2] Tick: Introduce tick_nohz_check() to check nohz enable status

[PATCH 1/2] Tick: Introduce tick_nohz_check() to check nohz enable status

[Lan Tianyu]

In some cases, nohz enable status needs to be checked. E.G, in RCU and cpufreq
ondemand governor. So add tick_nohz_check() to return tick_nohz_enabled value
And use tick_nohz_check() instead of referencing tick_nohz_enabled in the rcutree_plugin.h.

Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
---
 include/linux/tick.h     | 2 ++
 kernel/rcutree_plugin.h  | 4 +---
 kernel/time/tick-sched.c | 8 +++++++-
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/include/linux/tick.h b/include/linux/tick.h
index 5128d33..a9c5374 100644
--- a/include/linux/tick.h
+++ b/include/linux/tick.h
@@ -136,6 +136,7 @@ static inline int tick_nohz_tick_stopped(void)
 extern void tick_nohz_idle_enter(void);
 extern void tick_nohz_idle_exit(void);
 extern void tick_nohz_irq_exit(void);
+extern int tick_nohz_check(void);
 extern ktime_t tick_nohz_get_sleep_length(void);
 extern u64 get_cpu_idle_time_us(int cpu, u64 *last_update_time);
 extern u64 get_cpu_iowait_time_us(int cpu, u64 *last_update_time);
@@ -155,6 +156,7 @@ static inline ktime_t tick_nohz_get_sleep_length(void)
 
 	return len;
 }
+static inline int tick_nohz_check(void) { return 0; }
 static inline u64 get_cpu_idle_time_us(int cpu, u64 *unused) { return -1; }
 static inline u64 get_cpu_iowait_time_us(int cpu, u64 *unused) { return -1; }
 # endif /* !CONFIG_NO_HZ_COMMON */
diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h
index 130c97b..af167ec 100644
--- a/kernel/rcutree_plugin.h
+++ b/kernel/rcutree_plugin.h
@@ -1627,8 +1627,6 @@ module_param(rcu_idle_gp_delay, int, 0644);
 static int rcu_idle_lazy_gp_delay = RCU_IDLE_LAZY_GP_DELAY;
 module_param(rcu_idle_lazy_gp_delay, int, 0644);
 
-extern int tick_nohz_enabled;
-
 /*
  * Try to advance callbacks for all flavors of RCU on the current CPU.
  * Afterwards, if there are any callbacks ready for immediate invocation,
@@ -1718,7 +1716,7 @@ static void rcu_prepare_for_idle(int cpu)
 	int tne;
 
 	/* Handle nohz enablement switches conservatively. */
-	tne = ACCESS_ONCE(tick_nohz_enabled);
+	tne = tick_nohz_check();
 	if (tne != rdtp->tick_nohz_enabled_snap) {
 		if (rcu_cpu_has_callbacks(cpu, NULL))
 			invoke_rcu_core(); /* force nohz to see update. */
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 3612fc7..d381a22 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -361,7 +361,13 @@ void __init tick_nohz_init(void)
 /*
  * NO HZ enabled ?
  */
-int tick_nohz_enabled __read_mostly  = 1;
+static int tick_nohz_enabled __read_mostly  = 1;
+
+int tick_nohz_check(void)
+{
+	return	tick_nohz_enabled;
+}
+EXPORT_SYMBOL_GPL(tick_nohz_check);
 
 /*
  * Enable / Disable tickless mode
-- 
1.8.4.rc0.1.g8f6a3e5.dirty

[PATCH 2/2] Cpufreq/gov: use tick_nohz_check() to check idle micro accouting support

[Lan Tianyu]

Current, od_init() uses get_cpu_idle_time_us() to check whether
idle micro accouting is supported. The get_cpu_idle_time_us()
returns -1 only when tick nohz is disabled. This patch is to use
tick_nohz_check() directly to check nohz enable status instead of
get_cpu_idle_time_us(). This can avoid unnecessary getting and
putting cpu.

Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
---
 drivers/cpufreq/cpufreq_ondemand.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c
index 32f26f6..79986a9 100644
--- a/drivers/cpufreq/cpufreq_ondemand.c
+++ b/drivers/cpufreq/cpufreq_ondemand.c
@@ -485,10 +485,7 @@ static int od_init(struct dbs_data *dbs_data)
 		return -ENOMEM;
 	}
 
-	cpu = get_cpu();
-	idle_time = get_cpu_idle_time_us(cpu, NULL);
-	put_cpu();
-	if (idle_time != -1ULL) {
+	if (tick_nohz_check()) {
 		/* Idle micro accounting is supported. Use finer thresholds */
 		tuners->up_threshold = MICRO_FREQUENCY_UP_THRESHOLD;
 		/*
-- 
1.8.4.rc0.1.g8f6a3e5.dirty

Re: [PATCH 1/2] Tick: Introduce tick_nohz_check() to check nohz enable status

[Paul E. McKenney]

On Tue, Oct 29, 2013 at 04:48:56PM +0800, Lan Tianyu wrote:
> In some cases, nohz enable status needs to be checked. E.G, in RCU and cpufreq
> ondemand governor. So add tick_nohz_check() to return tick_nohz_enabled value
> And use tick_nohz_check() instead of referencing tick_nohz_enabled in the rcutree_plugin.h.
> 
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>

NACK on the rcutree change unless you put the ACCESS_ONCE() in.

Or is there some reason that ACCESS_ONCE() is not needed?  If so, what
is that reason?

							Thanx, Paul

> ---
>  include/linux/tick.h     | 2 ++
>  kernel/rcutree_plugin.h  | 4 +---
>  kernel/time/tick-sched.c | 8 +++++++-
>  3 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/tick.h b/include/linux/tick.h
> index 5128d33..a9c5374 100644
> --- a/include/linux/tick.h
> +++ b/include/linux/tick.h
> @@ -136,6 +136,7 @@ static inline int tick_nohz_tick_stopped(void)
>  extern void tick_nohz_idle_enter(void);
>  extern void tick_nohz_idle_exit(void);
>  extern void tick_nohz_irq_exit(void);
> +extern int tick_nohz_check(void);
>  extern ktime_t tick_nohz_get_sleep_length(void);
>  extern u64 get_cpu_idle_time_us(int cpu, u64 *last_update_time);
>  extern u64 get_cpu_iowait_time_us(int cpu, u64 *last_update_time);
> @@ -155,6 +156,7 @@ static inline ktime_t tick_nohz_get_sleep_length(void)
> 
>  	return len;
>  }
> +static inline int tick_nohz_check(void) { return 0; }
>  static inline u64 get_cpu_idle_time_us(int cpu, u64 *unused) { return -1; }
>  static inline u64 get_cpu_iowait_time_us(int cpu, u64 *unused) { return -1; }
>  # endif /* !CONFIG_NO_HZ_COMMON */
> diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h
> index 130c97b..af167ec 100644
> --- a/kernel/rcutree_plugin.h
> +++ b/kernel/rcutree_plugin.h
> @@ -1627,8 +1627,6 @@ module_param(rcu_idle_gp_delay, int, 0644);
>  static int rcu_idle_lazy_gp_delay = RCU_IDLE_LAZY_GP_DELAY;
>  module_param(rcu_idle_lazy_gp_delay, int, 0644);
> 
> -extern int tick_nohz_enabled;
> -
>  /*
>   * Try to advance callbacks for all flavors of RCU on the current CPU.
>   * Afterwards, if there are any callbacks ready for immediate invocation,
> @@ -1718,7 +1716,7 @@ static void rcu_prepare_for_idle(int cpu)
>  	int tne;
> 
>  	/* Handle nohz enablement switches conservatively. */
> -	tne = ACCESS_ONCE(tick_nohz_enabled);
> +	tne = tick_nohz_check();
>  	if (tne != rdtp->tick_nohz_enabled_snap) {
>  		if (rcu_cpu_has_callbacks(cpu, NULL))
>  			invoke_rcu_core(); /* force nohz to see update. */
> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> index 3612fc7..d381a22 100644
> --- a/kernel/time/tick-sched.c
> +++ b/kernel/time/tick-sched.c
> @@ -361,7 +361,13 @@ void __init tick_nohz_init(void)
>  /*
>   * NO HZ enabled ?
>   */
> -int tick_nohz_enabled __read_mostly  = 1;
> +static int tick_nohz_enabled __read_mostly  = 1;
> +
> +int tick_nohz_check(void)
> +{
> +	return	tick_nohz_enabled;
> +}
> +EXPORT_SYMBOL_GPL(tick_nohz_check);
> 
>  /*
>   * Enable / Disable tickless mode
> -- 
> 1.8.4.rc0.1.g8f6a3e5.dirty
> 

Re: [PATCH 1/2] Tick: Introduce tick_nohz_check() to check nohz enable status

[Lan Tianyu]

On 10/29/2013 05:51 PM, Paul E. McKenney wrote:
> On Tue, Oct 29, 2013 at 04:48:56PM +0800, Lan Tianyu wrote:
>> In some cases, nohz enable status needs to be checked. E.G, in RCU and cpufreq
>> ondemand governor. So add tick_nohz_check() to return tick_nohz_enabled value
>> And use tick_nohz_check() instead of referencing tick_nohz_enabled in the rcutree_plugin.h.
>>
>> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
>
> NACK on the rcutree change unless you put the ACCESS_ONCE() in.
>
> Or is there some reason that ACCESS_ONCE() is not needed?  If so, what
> is that reason?

Hi Paul:

Thanks for review. When I change this code, I find the tick_nohz_enabled
isn't changed dynamically. It's only changed during parsing kernel
params when "nohz=off/on" is set. Except this, it will not be changed.
So I ignored ACCESS_ONCE(). If necessary, I can add it back.

>
> 							Thanx, Paul
>
>> ---
>>   include/linux/tick.h     | 2 ++
>>   kernel/rcutree_plugin.h  | 4 +---
>>   kernel/time/tick-sched.c | 8 +++++++-
>>   3 files changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/linux/tick.h b/include/linux/tick.h
>> index 5128d33..a9c5374 100644
>> --- a/include/linux/tick.h
>> +++ b/include/linux/tick.h
>> @@ -136,6 +136,7 @@ static inline int tick_nohz_tick_stopped(void)
>>   extern void tick_nohz_idle_enter(void);
>>   extern void tick_nohz_idle_exit(void);
>>   extern void tick_nohz_irq_exit(void);
>> +extern int tick_nohz_check(void);
>>   extern ktime_t tick_nohz_get_sleep_length(void);
>>   extern u64 get_cpu_idle_time_us(int cpu, u64 *last_update_time);
>>   extern u64 get_cpu_iowait_time_us(int cpu, u64 *last_update_time);
>> @@ -155,6 +156,7 @@ static inline ktime_t tick_nohz_get_sleep_length(void)
>>
>>   	return len;
>>   }
>> +static inline int tick_nohz_check(void) { return 0; }
>>   static inline u64 get_cpu_idle_time_us(int cpu, u64 *unused) { return -1; }
>>   static inline u64 get_cpu_iowait_time_us(int cpu, u64 *unused) { return -1; }
>>   # endif /* !CONFIG_NO_HZ_COMMON */
>> diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h
>> index 130c97b..af167ec 100644
>> --- a/kernel/rcutree_plugin.h
>> +++ b/kernel/rcutree_plugin.h
>> @@ -1627,8 +1627,6 @@ module_param(rcu_idle_gp_delay, int, 0644);
>>   static int rcu_idle_lazy_gp_delay = RCU_IDLE_LAZY_GP_DELAY;
>>   module_param(rcu_idle_lazy_gp_delay, int, 0644);
>>
>> -extern int tick_nohz_enabled;
>> -
>>   /*
>>    * Try to advance callbacks for all flavors of RCU on the current CPU.
>>    * Afterwards, if there are any callbacks ready for immediate invocation,
>> @@ -1718,7 +1716,7 @@ static void rcu_prepare_for_idle(int cpu)
>>   	int tne;
>>
>>   	/* Handle nohz enablement switches conservatively. */
>> -	tne = ACCESS_ONCE(tick_nohz_enabled);
>> +	tne = tick_nohz_check();
>>   	if (tne != rdtp->tick_nohz_enabled_snap) {
>>   		if (rcu_cpu_has_callbacks(cpu, NULL))
>>   			invoke_rcu_core(); /* force nohz to see update. */
>> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
>> index 3612fc7..d381a22 100644
>> --- a/kernel/time/tick-sched.c
>> +++ b/kernel/time/tick-sched.c
>> @@ -361,7 +361,13 @@ void __init tick_nohz_init(void)
>>   /*
>>    * NO HZ enabled ?
>>    */
>> -int tick_nohz_enabled __read_mostly  = 1;
>> +static int tick_nohz_enabled __read_mostly  = 1;
>> +
>> +int tick_nohz_check(void)
>> +{
>> +	return	tick_nohz_enabled;
>> +}
>> +EXPORT_SYMBOL_GPL(tick_nohz_check);
>>
>>   /*
>>    * Enable / Disable tickless mode
>> --
>> 1.8.4.rc0.1.g8f6a3e5.dirty
>>
>

Re: [PATCH 1/2] Tick: Introduce tick_nohz_check() to check nohz enable status

[Lan Tianyu]

On 2013年10月29日 18:29, Lan Tianyu wrote:
> On 10/29/2013 05:51 PM, Paul E. McKenney wrote:
>> On Tue, Oct 29, 2013 at 04:48:56PM +0800, Lan Tianyu wrote:
>>> In some cases, nohz enable status needs to be checked. E.G, in RCU
>>> and cpufreq
>>> ondemand governor. So add tick_nohz_check() to return
>>> tick_nohz_enabled value
>>> And use tick_nohz_check() instead of referencing tick_nohz_enabled in
>>> the rcutree_plugin.h.
>>>
>>> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
>>
>> NACK on the rcutree change unless you put the ACCESS_ONCE() in.
>>
>> Or is there some reason that ACCESS_ONCE() is not needed?  If so, what
>> is that reason?
> 
> Hi Paul:
> 
> Thanks for review. When I change this code, I find the tick_nohz_enabled
> isn't changed dynamically. It's only changed during parsing kernel
> params when "nohz=off/on" is set. Except this, it will not be changed.
> So I ignored ACCESS_ONCE(). If necessary, I can add it back.

Hi Paul:
	Does this reason make sense to you? Or you still prefer to add
ACCESS_ONCE() in the new tick_nohz_check()?

> 
>>
>>                             Thanx, Paul
>>
>>> ---
>>>   include/linux/tick.h     | 2 ++
>>>   kernel/rcutree_plugin.h  | 4 +---
>>>   kernel/time/tick-sched.c | 8 +++++++-
>>>   3 files changed, 10 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/include/linux/tick.h b/include/linux/tick.h
>>> index 5128d33..a9c5374 100644
>>> --- a/include/linux/tick.h
>>> +++ b/include/linux/tick.h
>>> @@ -136,6 +136,7 @@ static inline int tick_nohz_tick_stopped(void)
>>>   extern void tick_nohz_idle_enter(void);
>>>   extern void tick_nohz_idle_exit(void);
>>>   extern void tick_nohz_irq_exit(void);
>>> +extern int tick_nohz_check(void);
>>>   extern ktime_t tick_nohz_get_sleep_length(void);
>>>   extern u64 get_cpu_idle_time_us(int cpu, u64 *last_update_time);
>>>   extern u64 get_cpu_iowait_time_us(int cpu, u64 *last_update_time);
>>> @@ -155,6 +156,7 @@ static inline ktime_t
>>> tick_nohz_get_sleep_length(void)
>>>
>>>       return len;
>>>   }
>>> +static inline int tick_nohz_check(void) { return 0; }
>>>   static inline u64 get_cpu_idle_time_us(int cpu, u64 *unused) {
>>> return -1; }
>>>   static inline u64 get_cpu_iowait_time_us(int cpu, u64 *unused) {
>>> return -1; }
>>>   # endif /* !CONFIG_NO_HZ_COMMON */
>>> diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h
>>> index 130c97b..af167ec 100644
>>> --- a/kernel/rcutree_plugin.h
>>> +++ b/kernel/rcutree_plugin.h
>>> @@ -1627,8 +1627,6 @@ module_param(rcu_idle_gp_delay, int, 0644);
>>>   static int rcu_idle_lazy_gp_delay = RCU_IDLE_LAZY_GP_DELAY;
>>>   module_param(rcu_idle_lazy_gp_delay, int, 0644);
>>>
>>> -extern int tick_nohz_enabled;
>>> -
>>>   /*
>>>    * Try to advance callbacks for all flavors of RCU on the current CPU.
>>>    * Afterwards, if there are any callbacks ready for immediate
>>> invocation,
>>> @@ -1718,7 +1716,7 @@ static void rcu_prepare_for_idle(int cpu)
>>>       int tne;
>>>
>>>       /* Handle nohz enablement switches conservatively. */
>>> -    tne = ACCESS_ONCE(tick_nohz_enabled);
>>> +    tne = tick_nohz_check();
>>>       if (tne != rdtp->tick_nohz_enabled_snap) {
>>>           if (rcu_cpu_has_callbacks(cpu, NULL))
>>>               invoke_rcu_core(); /* force nohz to see update. */
>>> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
>>> index 3612fc7..d381a22 100644
>>> --- a/kernel/time/tick-sched.c
>>> +++ b/kernel/time/tick-sched.c
>>> @@ -361,7 +361,13 @@ void __init tick_nohz_init(void)
>>>   /*
>>>    * NO HZ enabled ?
>>>    */
>>> -int tick_nohz_enabled __read_mostly  = 1;
>>> +static int tick_nohz_enabled __read_mostly  = 1;
>>> +
>>> +int tick_nohz_check(void)
>>> +{
>>> +    return    tick_nohz_enabled;
>>> +}
>>> +EXPORT_SYMBOL_GPL(tick_nohz_check);
>>>
>>>   /*
>>>    * Enable / Disable tickless mode
>>> -- 
>>> 1.8.4.rc0.1.g8f6a3e5.dirty
>>>
>>
> 


-- 
Best regards
Tianyu Lan

Re: [PATCH 1/2] Tick: Introduce tick_nohz_check() to check nohz enable status

[Paul E. McKenney]

On Mon, Nov 04, 2013 at 11:13:12AM +0800, Lan Tianyu wrote:
> On 2013年10月29日 18:29, Lan Tianyu wrote:
> > On 10/29/2013 05:51 PM, Paul E. McKenney wrote:
> >> On Tue, Oct 29, 2013 at 04:48:56PM +0800, Lan Tianyu wrote:
> >>> In some cases, nohz enable status needs to be checked. E.G, in RCU
> >>> and cpufreq
> >>> ondemand governor. So add tick_nohz_check() to return
> >>> tick_nohz_enabled value
> >>> And use tick_nohz_check() instead of referencing tick_nohz_enabled in
> >>> the rcutree_plugin.h.
> >>>
> >>> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> >>
> >> NACK on the rcutree change unless you put the ACCESS_ONCE() in.
> >>
> >> Or is there some reason that ACCESS_ONCE() is not needed?  If so, what
> >> is that reason?
> > 
> > Hi Paul:
> > 
> > Thanks for review. When I change this code, I find the tick_nohz_enabled
> > isn't changed dynamically. It's only changed during parsing kernel
> > params when "nohz=off/on" is set. Except this, it will not be changed.
> > So I ignored ACCESS_ONCE(). If necessary, I can add it back.
> 
> Hi Paul:
> 	Does this reason make sense to you? Or you still prefer to add
> ACCESS_ONCE() in the new tick_nohz_check()?

I prefer the ACCESS_ONCE().  It adds little or no overhead, and Frederic
has been heard to say that he mkight allow tick_nohz_check() to change
in the future.

							Thanx, Paul

> >>> ---
> >>>   include/linux/tick.h     | 2 ++
> >>>   kernel/rcutree_plugin.h  | 4 +---
> >>>   kernel/time/tick-sched.c | 8 +++++++-
> >>>   3 files changed, 10 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/include/linux/tick.h b/include/linux/tick.h
> >>> index 5128d33..a9c5374 100644
> >>> --- a/include/linux/tick.h
> >>> +++ b/include/linux/tick.h
> >>> @@ -136,6 +136,7 @@ static inline int tick_nohz_tick_stopped(void)
> >>>   extern void tick_nohz_idle_enter(void);
> >>>   extern void tick_nohz_idle_exit(void);
> >>>   extern void tick_nohz_irq_exit(void);
> >>> +extern int tick_nohz_check(void);
> >>>   extern ktime_t tick_nohz_get_sleep_length(void);
> >>>   extern u64 get_cpu_idle_time_us(int cpu, u64 *last_update_time);
> >>>   extern u64 get_cpu_iowait_time_us(int cpu, u64 *last_update_time);
> >>> @@ -155,6 +156,7 @@ static inline ktime_t
> >>> tick_nohz_get_sleep_length(void)
> >>>
> >>>       return len;
> >>>   }
> >>> +static inline int tick_nohz_check(void) { return 0; }
> >>>   static inline u64 get_cpu_idle_time_us(int cpu, u64 *unused) {
> >>> return -1; }
> >>>   static inline u64 get_cpu_iowait_time_us(int cpu, u64 *unused) {
> >>> return -1; }
> >>>   # endif /* !CONFIG_NO_HZ_COMMON */
> >>> diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h
> >>> index 130c97b..af167ec 100644
> >>> --- a/kernel/rcutree_plugin.h
> >>> +++ b/kernel/rcutree_plugin.h
> >>> @@ -1627,8 +1627,6 @@ module_param(rcu_idle_gp_delay, int, 0644);
> >>>   static int rcu_idle_lazy_gp_delay = RCU_IDLE_LAZY_GP_DELAY;
> >>>   module_param(rcu_idle_lazy_gp_delay, int, 0644);
> >>>
> >>> -extern int tick_nohz_enabled;
> >>> -
> >>>   /*
> >>>    * Try to advance callbacks for all flavors of RCU on the current CPU.
> >>>    * Afterwards, if there are any callbacks ready for immediate
> >>> invocation,
> >>> @@ -1718,7 +1716,7 @@ static void rcu_prepare_for_idle(int cpu)
> >>>       int tne;
> >>>
> >>>       /* Handle nohz enablement switches conservatively. */
> >>> -    tne = ACCESS_ONCE(tick_nohz_enabled);
> >>> +    tne = tick_nohz_check();
> >>>       if (tne != rdtp->tick_nohz_enabled_snap) {
> >>>           if (rcu_cpu_has_callbacks(cpu, NULL))
> >>>               invoke_rcu_core(); /* force nohz to see update. */
> >>> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> >>> index 3612fc7..d381a22 100644
> >>> --- a/kernel/time/tick-sched.c
> >>> +++ b/kernel/time/tick-sched.c
> >>> @@ -361,7 +361,13 @@ void __init tick_nohz_init(void)
> >>>   /*
> >>>    * NO HZ enabled ?
> >>>    */
> >>> -int tick_nohz_enabled __read_mostly  = 1;
> >>> +static int tick_nohz_enabled __read_mostly  = 1;
> >>> +
> >>> +int tick_nohz_check(void)
> >>> +{
> >>> +    return    tick_nohz_enabled;
> >>> +}
> >>> +EXPORT_SYMBOL_GPL(tick_nohz_check);
> >>>
> >>>   /*
> >>>    * Enable / Disable tickless mode
> >>> -- 
> >>> 1.8.4.rc0.1.g8f6a3e5.dirty
> >>>
> >>
> > 
> 
> 
> -- 
> Best regards
> Tianyu Lan
> 

Re: [PATCH 1/2] Tick: Introduce tick_nohz_check() to check nohz enable status

[Lan Tianyu]

On 2013年11月04日 17:52, Paul E. McKenney wrote:
> On Mon, Nov 04, 2013 at 11:13:12AM +0800, Lan Tianyu wrote:
>> On 2013年10月29日 18:29, Lan Tianyu wrote:
>>> On 10/29/2013 05:51 PM, Paul E. McKenney wrote:
>>>> On Tue, Oct 29, 2013 at 04:48:56PM +0800, Lan Tianyu wrote:
>>>>> In some cases, nohz enable status needs to be checked. E.G, in RCU
>>>>> and cpufreq
>>>>> ondemand governor. So add tick_nohz_check() to return
>>>>> tick_nohz_enabled value
>>>>> And use tick_nohz_check() instead of referencing tick_nohz_enabled in
>>>>> the rcutree_plugin.h.
>>>>>
>>>>> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
>>>>
>>>> NACK on the rcutree change unless you put the ACCESS_ONCE() in.
>>>>
>>>> Or is there some reason that ACCESS_ONCE() is not needed?  If so, what
>>>> is that reason?
>>>
>>> Hi Paul:
>>>
>>> Thanks for review. When I change this code, I find the tick_nohz_enabled
>>> isn't changed dynamically. It's only changed during parsing kernel
>>> params when "nohz=off/on" is set. Except this, it will not be changed.
>>> So I ignored ACCESS_ONCE(). If necessary, I can add it back.
>>
>> Hi Paul:
>> 	Does this reason make sense to you? Or you still prefer to add
>> ACCESS_ONCE() in the new tick_nohz_check()?
> 
> I prefer the ACCESS_ONCE().  It adds little or no overhead, and Frederic
> has been heard to say that he mkight allow tick_nohz_check() to change
> in the future.
> 

Ok. I will update it.

> 							Thanx, Paul
> 
>>>>> ---
>>>>>   include/linux/tick.h     | 2 ++
>>>>>   kernel/rcutree_plugin.h  | 4 +---
>>>>>   kernel/time/tick-sched.c | 8 +++++++-
>>>>>   3 files changed, 10 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/include/linux/tick.h b/include/linux/tick.h
>>>>> index 5128d33..a9c5374 100644
>>>>> --- a/include/linux/tick.h
>>>>> +++ b/include/linux/tick.h
>>>>> @@ -136,6 +136,7 @@ static inline int tick_nohz_tick_stopped(void)
>>>>>   extern void tick_nohz_idle_enter(void);
>>>>>   extern void tick_nohz_idle_exit(void);
>>>>>   extern void tick_nohz_irq_exit(void);
>>>>> +extern int tick_nohz_check(void);
>>>>>   extern ktime_t tick_nohz_get_sleep_length(void);
>>>>>   extern u64 get_cpu_idle_time_us(int cpu, u64 *last_update_time);
>>>>>   extern u64 get_cpu_iowait_time_us(int cpu, u64 *last_update_time);
>>>>> @@ -155,6 +156,7 @@ static inline ktime_t
>>>>> tick_nohz_get_sleep_length(void)
>>>>>
>>>>>       return len;
>>>>>   }
>>>>> +static inline int tick_nohz_check(void) { return 0; }
>>>>>   static inline u64 get_cpu_idle_time_us(int cpu, u64 *unused) {
>>>>> return -1; }
>>>>>   static inline u64 get_cpu_iowait_time_us(int cpu, u64 *unused) {
>>>>> return -1; }
>>>>>   # endif /* !CONFIG_NO_HZ_COMMON */
>>>>> diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h
>>>>> index 130c97b..af167ec 100644
>>>>> --- a/kernel/rcutree_plugin.h
>>>>> +++ b/kernel/rcutree_plugin.h
>>>>> @@ -1627,8 +1627,6 @@ module_param(rcu_idle_gp_delay, int, 0644);
>>>>>   static int rcu_idle_lazy_gp_delay = RCU_IDLE_LAZY_GP_DELAY;
>>>>>   module_param(rcu_idle_lazy_gp_delay, int, 0644);
>>>>>
>>>>> -extern int tick_nohz_enabled;
>>>>> -
>>>>>   /*
>>>>>    * Try to advance callbacks for all flavors of RCU on the current CPU.
>>>>>    * Afterwards, if there are any callbacks ready for immediate
>>>>> invocation,
>>>>> @@ -1718,7 +1716,7 @@ static void rcu_prepare_for_idle(int cpu)
>>>>>       int tne;
>>>>>
>>>>>       /* Handle nohz enablement switches conservatively. */
>>>>> -    tne = ACCESS_ONCE(tick_nohz_enabled);
>>>>> +    tne = tick_nohz_check();
>>>>>       if (tne != rdtp->tick_nohz_enabled_snap) {
>>>>>           if (rcu_cpu_has_callbacks(cpu, NULL))
>>>>>               invoke_rcu_core(); /* force nohz to see update. */
>>>>> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
>>>>> index 3612fc7..d381a22 100644
>>>>> --- a/kernel/time/tick-sched.c
>>>>> +++ b/kernel/time/tick-sched.c
>>>>> @@ -361,7 +361,13 @@ void __init tick_nohz_init(void)
>>>>>   /*
>>>>>    * NO HZ enabled ?
>>>>>    */
>>>>> -int tick_nohz_enabled __read_mostly  = 1;
>>>>> +static int tick_nohz_enabled __read_mostly  = 1;
>>>>> +
>>>>> +int tick_nohz_check(void)
>>>>> +{
>>>>> +    return    tick_nohz_enabled;
>>>>> +}
>>>>> +EXPORT_SYMBOL_GPL(tick_nohz_check);
>>>>>
>>>>>   /*
>>>>>    * Enable / Disable tickless mode
>>>>> -- 
>>>>> 1.8.4.rc0.1.g8f6a3e5.dirty
>>>>>
>>>>
>>>
>>
>>
>> -- 
>> Best regards
>> Tianyu Lan
>>
> 


-- 
Best regards
Tianyu Lan

[PATCH V2 1/2] Tick: Introduce tick_nohz_check() to check nohz enable status

[Lan Tianyu]

In some cases, nohz enable status needs to be checked. E.G, in RCU and cpufreq
ondemand governor. So add tick_nohz_check() to return tick_nohz_enabled value
And use tick_nohz_check() instead of referencing tick_nohz_enabled in the rcutree_plugin.h.

Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
---

Change since V2:
	Add ACCESS_ONCE to tick_nohz_check().

 include/linux/tick.h     | 2 ++
 kernel/rcutree_plugin.h  | 4 +---
 kernel/time/tick-sched.c | 8 +++++++-
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/include/linux/tick.h b/include/linux/tick.h
index 5128d33..a9c5374 100644
--- a/include/linux/tick.h
+++ b/include/linux/tick.h
@@ -136,6 +136,7 @@ static inline int tick_nohz_tick_stopped(void)
 extern void tick_nohz_idle_enter(void);
 extern void tick_nohz_idle_exit(void);
 extern void tick_nohz_irq_exit(void);
+extern int tick_nohz_check(void);
 extern ktime_t tick_nohz_get_sleep_length(void);
 extern u64 get_cpu_idle_time_us(int cpu, u64 *last_update_time);
 extern u64 get_cpu_iowait_time_us(int cpu, u64 *last_update_time);
@@ -155,6 +156,7 @@ static inline ktime_t tick_nohz_get_sleep_length(void)
 
 	return len;
 }
+static inline int tick_nohz_check(void) { return 0; }
 static inline u64 get_cpu_idle_time_us(int cpu, u64 *unused) { return -1; }
 static inline u64 get_cpu_iowait_time_us(int cpu, u64 *unused) { return -1; }
 # endif /* !CONFIG_NO_HZ_COMMON */
diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h
index 130c97b..af167ec 100644
--- a/kernel/rcutree_plugin.h
+++ b/kernel/rcutree_plugin.h
@@ -1627,8 +1627,6 @@ module_param(rcu_idle_gp_delay, int, 0644);
 static int rcu_idle_lazy_gp_delay = RCU_IDLE_LAZY_GP_DELAY;
 module_param(rcu_idle_lazy_gp_delay, int, 0644);
 
-extern int tick_nohz_enabled;
-
 /*
  * Try to advance callbacks for all flavors of RCU on the current CPU.
  * Afterwards, if there are any callbacks ready for immediate invocation,
@@ -1718,7 +1716,7 @@ static void rcu_prepare_for_idle(int cpu)
 	int tne;
 
 	/* Handle nohz enablement switches conservatively. */
-	tne = ACCESS_ONCE(tick_nohz_enabled);
+	tne = tick_nohz_check();
 	if (tne != rdtp->tick_nohz_enabled_snap) {
 		if (rcu_cpu_has_callbacks(cpu, NULL))
 			invoke_rcu_core(); /* force nohz to see update. */
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 3612fc7..a568845 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -361,7 +361,13 @@ void __init tick_nohz_init(void)
 /*
  * NO HZ enabled ?
  */
-int tick_nohz_enabled __read_mostly  = 1;
+static int tick_nohz_enabled __read_mostly  = 1;
+
+int tick_nohz_check(void)
+{
+	return	ACCESS_ONCE(tick_nohz_enabled);
+}
+EXPORT_SYMBOL_GPL(tick_nohz_check);
 
 /*
  * Enable / Disable tickless mode
-- 
1.8.4.rc0.1.g8f6a3e5.dirty

[PATCH V2 2/2] Cpufreq/gov: use tick_nohz_check() to check idle micro accouting support

[Lan Tianyu]

Current, od_init() uses get_cpu_idle_time_us() to check whether
idle micro accouting is supported. The get_cpu_idle_time_us()
returns -1 only when tick nohz is disabled. This patch is to use
tick_nohz_check() directly to check nohz enable status instead of
get_cpu_idle_time_us(). This can avoid unnecessary getting and
putting cpu.

Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
---
 drivers/cpufreq/cpufreq_ondemand.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c
index 18d4091..7db2468 100644
--- a/drivers/cpufreq/cpufreq_ondemand.c
+++ b/drivers/cpufreq/cpufreq_ondemand.c
@@ -475,8 +475,6 @@ static struct attribute_group od_attr_group_gov_pol = {
 static int od_init(struct dbs_data *dbs_data)
 {
 	struct od_dbs_tuners *tuners;
-	u64 idle_time;
-	int cpu;
 
 	tuners = kzalloc(sizeof(*tuners), GFP_KERNEL);
 	if (!tuners) {
@@ -484,10 +482,7 @@ static int od_init(struct dbs_data *dbs_data)
 		return -ENOMEM;
 	}
 
-	cpu = get_cpu();
-	idle_time = get_cpu_idle_time_us(cpu, NULL);
-	put_cpu();
-	if (idle_time != -1ULL) {
+	if (tick_nohz_check()) {
 		/* Idle micro accounting is supported. Use finer thresholds */
 		tuners->up_threshold = MICRO_FREQUENCY_UP_THRESHOLD;
 		/*
-- 
1.8.4.rc0.1.g8f6a3e5.dirty

Re: [PATCH V2 2/2] Cpufreq/gov: use tick_nohz_check() to check idle micro accouting support

[Viresh Kumar]

On 15 November 2013 14:05, Lan Tianyu <tianyu.lan@intel.com> wrote:
> Current, od_init() uses get_cpu_idle_time_us() to check whether
> idle micro accouting is supported. The get_cpu_idle_time_us()
> returns -1 only when tick nohz is disabled. This patch is to use
> tick_nohz_check() directly to check nohz enable status instead of
> get_cpu_idle_time_us(). This can avoid unnecessary getting and
> putting cpu.
>
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> ---
>  drivers/cpufreq/cpufreq_ondemand.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

Re: [PATCH V2 1/2] Tick: Introduce tick_nohz_check() to check nohz enable status

[Viresh Kumar]

On 15 November 2013 14:05, Lan Tianyu <tianyu.lan@intel.com> wrote:
> In some cases, nohz enable status needs to be checked. E.G, in RCU and cpufreq
> ondemand governor. So add tick_nohz_check() to return tick_nohz_enabled value
> And use tick_nohz_check() instead of referencing tick_nohz_enabled in the rcutree_plugin.h.
>
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> ---
>
> Change since V2:
>         Add ACCESS_ONCE to tick_nohz_check().
>
>  include/linux/tick.h     | 2 ++
>  kernel/rcutree_plugin.h  | 4 +---
>  kernel/time/tick-sched.c | 8 +++++++-
>  3 files changed, 10 insertions(+), 4 deletions(-)

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

Re: [PATCH V2 1/2] Tick: Introduce tick_nohz_check() to check nohz enable status

[Thomas Gleixner]

On Fri, 15 Nov 2013, Lan Tianyu wrote:

> In some cases, nohz enable status needs to be checked. E.G, in RCU and cpufreq
> ondemand governor. So add tick_nohz_check() to return tick_nohz_enabled value
> And use tick_nohz_check() instead of referencing tick_nohz_enabled in the rcutree_plugin.h.

What's the point of this? 

Checking for tick_nohz_enabled is wrong to begin with.

See https://lkml.org/lkml/2013/11/13/424