On Mon 15-10-12 13:41:20, Viresh Kumar wrote:
> Multiple cpufreq governers have defined similar get_cpu_idle_time_***()
> routines. These routines must be moved to some common place, so that all
> governors can use them.
>
> So moving them to tick-sched.c, which seems to be a better place for keeping
> these routines.
I do agree that this code duplication should be removed but tick-sched.c
is not a right place IMO. Who, apart from governors, should use those
"common" functions?
Having a generic get_cpu_idle_time, which in fact includes iowait time
as well is definitely not good. It is confusing and it doesn't match
get_cpu_idle_time_us.
I would suggest moving the common functionality into drivers/cpufreq/
(e.g. cpufreq_common.c).
> Signed-off-by: Viresh Kumar <viresh.kumar@xxxxxxxxxx>
> ---
> drivers/cpufreq/cpufreq_conservative.c | 34 ---------------------------------
> drivers/cpufreq/cpufreq_ondemand.c | 34 ---------------------------------
> include/linux/tick.h | 3 +++
> kernel/time/tick-sched.c | 35 ++++++++++++++++++++++++++++++++++
> 4 files changed, 38 insertions(+), 68 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c
> index a152af7..181abad 100644
> --- a/drivers/cpufreq/cpufreq_conservative.c
> +++ b/drivers/cpufreq/cpufreq_conservative.c
> @@ -95,40 +95,6 @@ static struct dbs_tuners {
> .freq_step = 5,
> };
>
> -static inline u64 get_cpu_idle_time_jiffy(unsigned int cpu, u64 *wall)
> -{
> - u64 idle_time;
> - u64 cur_wall_time;
> - u64 busy_time;
> -
> - cur_wall_time = jiffies64_to_cputime64(get_jiffies_64());
> -
> - busy_time = kcpustat_cpu(cpu).cpustat[CPUTIME_USER];
> - busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_SYSTEM];
> - busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_IRQ];
> - busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_SOFTIRQ];
> - busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_STEAL];
> - busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_NICE];
> -
> - idle_time = cur_wall_time - busy_time;
> - if (wall)
> - *wall = jiffies_to_usecs(cur_wall_time);
> -
> - return jiffies_to_usecs(idle_time);
> -}
> -
> -static inline cputime64_t get_cpu_idle_time(unsigned int cpu, cputime64_t *wall)
> -{
> - u64 idle_time = get_cpu_idle_time_us(cpu, NULL);
> -
> - if (idle_time == -1ULL)
> - return get_cpu_idle_time_jiffy(cpu, wall);
> - else
> - idle_time += get_cpu_iowait_time_us(cpu, wall);
> -
> - return idle_time;
> -}
> -
> /* keep track of frequency transitions */
> static int
> dbs_cpufreq_notifier(struct notifier_block *nb, unsigned long val,
> diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c
> index 396322f..d7f774b 100644
> --- a/drivers/cpufreq/cpufreq_ondemand.c
> +++ b/drivers/cpufreq/cpufreq_ondemand.c
> @@ -119,40 +119,6 @@ static struct dbs_tuners {
> .powersave_bias = 0,
> };
>
> -static inline u64 get_cpu_idle_time_jiffy(unsigned int cpu, u64 *wall)
> -{
> - u64 idle_time;
> - u64 cur_wall_time;
> - u64 busy_time;
> -
> - cur_wall_time = jiffies64_to_cputime64(get_jiffies_64());
> -
> - busy_time = kcpustat_cpu(cpu).cpustat[CPUTIME_USER];
> - busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_SYSTEM];
> - busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_IRQ];
> - busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_SOFTIRQ];
> - busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_STEAL];
> - busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_NICE];
> -
> - idle_time = cur_wall_time - busy_time;
> - if (wall)
> - *wall = jiffies_to_usecs(cur_wall_time);
> -
> - return jiffies_to_usecs(idle_time);
> -}
> -
> -static inline cputime64_t get_cpu_idle_time(unsigned int cpu, cputime64_t *wall)
> -{
> - u64 idle_time = get_cpu_idle_time_us(cpu, NULL);
> -
> - if (idle_time == -1ULL)
> - return get_cpu_idle_time_jiffy(cpu, wall);
> - else
> - idle_time += get_cpu_iowait_time_us(cpu, wall);
> -
> - return idle_time;
> -}
> -
> static inline cputime64_t get_cpu_iowait_time(unsigned int cpu, cputime64_t *wall)
> {
> u64 iowait_time = get_cpu_iowait_time_us(cpu, wall);
> diff --git a/include/linux/tick.h b/include/linux/tick.h
> index f37fceb..79ca824 100644
> --- a/include/linux/tick.h
> +++ b/include/linux/tick.h
> @@ -6,6 +6,7 @@
> #ifndef _LINUX_TICK_H
> #define _LINUX_TICK_H
>
> +#include <asm/cputime.h>
> #include <linux/clockchips.h>
> #include <linux/irqflags.h>
>
> @@ -142,4 +143,6 @@ 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 /* !NO_HZ */
>
> +cputime64_t get_cpu_idle_time(unsigned int cpu, cputime64_t *wall);
> +
> #endif
> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> index a402608..b458402 100644
> --- a/kernel/time/tick-sched.c
> +++ b/kernel/time/tick-sched.c
> @@ -271,6 +271,41 @@ u64 get_cpu_iowait_time_us(int cpu, u64 *last_update_time)
> }
> EXPORT_SYMBOL_GPL(get_cpu_iowait_time_us);
>
> +static inline u64 get_cpu_idle_time_jiffy(unsigned int cpu, u64 *wall)
> +{
> + u64 idle_time;
> + u64 cur_wall_time;
> + u64 busy_time;
> +
> + cur_wall_time = jiffies64_to_cputime64(get_jiffies_64());
> +
> + busy_time = kcpustat_cpu(cpu).cpustat[CPUTIME_USER];
> + busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_SYSTEM];
> + busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_IRQ];
> + busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_SOFTIRQ];
> + busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_STEAL];
> + busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_NICE];
> +
> + idle_time = cur_wall_time - busy_time;
> + if (wall)
> + *wall = jiffies_to_usecs(cur_wall_time);
> +
> + return jiffies_to_usecs(idle_time);
> +}
> +
> +cputime64_t get_cpu_idle_time(unsigned int cpu, cputime64_t *wall)
> +{
> + u64 idle_time = get_cpu_idle_time_us(cpu, NULL);
> +
> + if (idle_time == -1ULL)
> + return get_cpu_idle_time_jiffy(cpu, wall);
> + else
> + idle_time += get_cpu_iowait_time_us(cpu, wall);
> +
> + return idle_time;
> +}
> +EXPORT_SYMBOL_GPL(get_cpu_idle_time);
> +
> static ktime_t tick_nohz_stop_sched_tick(struct tick_sched *ts,
> ktime_t now, int cpu)
> {
> --
> 1.7.12.rc2.18.g61b472e
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe cpufreq" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
