* [PATCH 1/2] Tick: Introduce tick_nohz_check() to check nohz enable status
@ 2013-10-29 8:48 Lan Tianyu
2013-10-29 8:48 ` [PATCH 2/2] Cpufreq/gov: use tick_nohz_check() to check idle micro accouting support Lan Tianyu
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Lan Tianyu @ 2013-10-29 8:48 UTC (permalink / raw)
To: rjw, viresh.kumar, dipankar, paulmck, fweisbec, youquan.song, riel
Cc: Lan Tianyu, linux-kernel
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/2] Cpufreq/gov: use tick_nohz_check() to check idle micro accouting support
2013-10-29 8:48 [PATCH 1/2] Tick: Introduce tick_nohz_check() to check nohz enable status Lan Tianyu
@ 2013-10-29 8:48 ` Lan Tianyu
2013-10-29 9:51 ` [PATCH 1/2] Tick: Introduce tick_nohz_check() to check nohz enable status Paul E. McKenney
2013-11-15 8:35 ` [PATCH V2 " Lan Tianyu
2 siblings, 0 replies; 12+ messages in thread
From: Lan Tianyu @ 2013-10-29 8:48 UTC (permalink / raw)
To: rjw, viresh.kumar; +Cc: Lan Tianyu, cpufreq, linux-pm, linux-kernel
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] Tick: Introduce tick_nohz_check() to check nohz enable status
2013-10-29 8:48 [PATCH 1/2] Tick: Introduce tick_nohz_check() to check nohz enable status Lan Tianyu
2013-10-29 8:48 ` [PATCH 2/2] Cpufreq/gov: use tick_nohz_check() to check idle micro accouting support Lan Tianyu
@ 2013-10-29 9:51 ` Paul E. McKenney
2013-10-29 10:29 ` Lan Tianyu
2013-11-15 8:35 ` [PATCH V2 " Lan Tianyu
2 siblings, 1 reply; 12+ messages in thread
From: Paul E. McKenney @ 2013-10-29 9:51 UTC (permalink / raw)
To: Lan Tianyu
Cc: rjw, viresh.kumar, dipankar, fweisbec, youquan.song, riel, linux-kernel
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
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] Tick: Introduce tick_nohz_check() to check nohz enable status
2013-10-29 9:51 ` [PATCH 1/2] Tick: Introduce tick_nohz_check() to check nohz enable status Paul E. McKenney
@ 2013-10-29 10:29 ` Lan Tianyu
2013-11-04 3:13 ` Lan Tianyu
0 siblings, 1 reply; 12+ messages in thread
From: Lan Tianyu @ 2013-10-29 10:29 UTC (permalink / raw)
To: paulmck
Cc: rjw, viresh.kumar, dipankar, fweisbec, youquan.song, riel, linux-kernel
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
>>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] Tick: Introduce tick_nohz_check() to check nohz enable status
2013-10-29 10:29 ` Lan Tianyu
@ 2013-11-04 3:13 ` Lan Tianyu
2013-11-04 9:52 ` Paul E. McKenney
0 siblings, 1 reply; 12+ messages in thread
From: Lan Tianyu @ 2013-11-04 3:13 UTC (permalink / raw)
To: paulmck
Cc: rjw, viresh.kumar, dipankar, fweisbec, youquan.song, riel, linux-kernel
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] Tick: Introduce tick_nohz_check() to check nohz enable status
2013-11-04 3:13 ` Lan Tianyu
@ 2013-11-04 9:52 ` Paul E. McKenney
2013-11-05 0:42 ` Lan Tianyu
0 siblings, 1 reply; 12+ messages in thread
From: Paul E. McKenney @ 2013-11-04 9:52 UTC (permalink / raw)
To: Lan Tianyu
Cc: rjw, viresh.kumar, dipankar, fweisbec, youquan.song, riel, linux-kernel
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
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] Tick: Introduce tick_nohz_check() to check nohz enable status
2013-11-04 9:52 ` Paul E. McKenney
@ 2013-11-05 0:42 ` Lan Tianyu
0 siblings, 0 replies; 12+ messages in thread
From: Lan Tianyu @ 2013-11-05 0:42 UTC (permalink / raw)
To: paulmck
Cc: rjw, viresh.kumar, dipankar, fweisbec, youquan.song, riel, linux-kernel
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH V2 1/2] Tick: Introduce tick_nohz_check() to check nohz enable status
2013-10-29 8:48 [PATCH 1/2] Tick: Introduce tick_nohz_check() to check nohz enable status Lan Tianyu
2013-10-29 8:48 ` [PATCH 2/2] Cpufreq/gov: use tick_nohz_check() to check idle micro accouting support Lan Tianyu
2013-10-29 9:51 ` [PATCH 1/2] Tick: Introduce tick_nohz_check() to check nohz enable status Paul E. McKenney
@ 2013-11-15 8:35 ` Lan Tianyu
2013-11-15 8:35 ` [PATCH V2 2/2] Cpufreq/gov: use tick_nohz_check() to check idle micro accouting support Lan Tianyu
` (2 more replies)
2 siblings, 3 replies; 12+ messages in thread
From: Lan Tianyu @ 2013-11-15 8:35 UTC (permalink / raw)
To: dipankar, paulmck, tglx, fweisbec, rjw, viresh.kumar, rostedt
Cc: Lan Tianyu, cpufreq, linux-pm, linux-kernel
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH V2 2/2] Cpufreq/gov: use tick_nohz_check() to check idle micro accouting support
2013-11-15 8:35 ` [PATCH V2 " Lan Tianyu
@ 2013-11-15 8:35 ` Lan Tianyu
2013-11-15 10:36 ` Viresh Kumar
2013-11-15 10:35 ` [PATCH V2 1/2] Tick: Introduce tick_nohz_check() to check nohz enable status Viresh Kumar
2013-11-15 11:06 ` Thomas Gleixner
2 siblings, 1 reply; 12+ messages in thread
From: Lan Tianyu @ 2013-11-15 8:35 UTC (permalink / raw)
To: rjw, viresh.kumar; +Cc: Lan Tianyu, cpufreq, linux-pm, linux-kernel
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH V2 1/2] Tick: Introduce tick_nohz_check() to check nohz enable status
2013-11-15 8:35 ` [PATCH V2 " Lan Tianyu
2013-11-15 8:35 ` [PATCH V2 2/2] Cpufreq/gov: use tick_nohz_check() to check idle micro accouting support Lan Tianyu
@ 2013-11-15 10:35 ` Viresh Kumar
2013-11-15 11:06 ` Thomas Gleixner
2 siblings, 0 replies; 12+ messages in thread
From: Viresh Kumar @ 2013-11-15 10:35 UTC (permalink / raw)
To: Lan Tianyu
Cc: dipankar, Paul McKenney, Thomas Gleixner,
Frédéric Weisbecker, Rafael J. Wysocki, Steven Rostedt,
cpufreq, linux-pm, Linux Kernel Mailing List
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>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V2 2/2] Cpufreq/gov: use tick_nohz_check() to check idle micro accouting support
2013-11-15 8:35 ` [PATCH V2 2/2] Cpufreq/gov: use tick_nohz_check() to check idle micro accouting support Lan Tianyu
@ 2013-11-15 10:36 ` Viresh Kumar
0 siblings, 0 replies; 12+ messages in thread
From: Viresh Kumar @ 2013-11-15 10:36 UTC (permalink / raw)
To: Lan Tianyu
Cc: Rafael J. Wysocki, cpufreq, linux-pm, Linux Kernel Mailing List
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>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V2 1/2] Tick: Introduce tick_nohz_check() to check nohz enable status
2013-11-15 8:35 ` [PATCH V2 " Lan Tianyu
2013-11-15 8:35 ` [PATCH V2 2/2] Cpufreq/gov: use tick_nohz_check() to check idle micro accouting support Lan Tianyu
2013-11-15 10:35 ` [PATCH V2 1/2] Tick: Introduce tick_nohz_check() to check nohz enable status Viresh Kumar
@ 2013-11-15 11:06 ` Thomas Gleixner
2 siblings, 0 replies; 12+ messages in thread
From: Thomas Gleixner @ 2013-11-15 11:06 UTC (permalink / raw)
To: Lan Tianyu
Cc: dipankar, paulmck, fweisbec, rjw, viresh.kumar, rostedt, cpufreq,
linux-pm, linux-kernel
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
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2013-11-15 11:06 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-29 8:48 [PATCH 1/2] Tick: Introduce tick_nohz_check() to check nohz enable status Lan Tianyu
2013-10-29 8:48 ` [PATCH 2/2] Cpufreq/gov: use tick_nohz_check() to check idle micro accouting support Lan Tianyu
2013-10-29 9:51 ` [PATCH 1/2] Tick: Introduce tick_nohz_check() to check nohz enable status Paul E. McKenney
2013-10-29 10:29 ` Lan Tianyu
2013-11-04 3:13 ` Lan Tianyu
2013-11-04 9:52 ` Paul E. McKenney
2013-11-05 0:42 ` Lan Tianyu
2013-11-15 8:35 ` [PATCH V2 " Lan Tianyu
2013-11-15 8:35 ` [PATCH V2 2/2] Cpufreq/gov: use tick_nohz_check() to check idle micro accouting support Lan Tianyu
2013-11-15 10:36 ` Viresh Kumar
2013-11-15 10:35 ` [PATCH V2 1/2] Tick: Introduce tick_nohz_check() to check nohz enable status Viresh Kumar
2013-11-15 11:06 ` Thomas Gleixner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).