On 04-07-17, 18:34, Patrick Bellasi wrote:
> diff --git a/include/linux/sched/cpufreq.h b/include/linux/sched/cpufreq.h
> index d2be2cc..36ac8d2 100644
> --- a/include/linux/sched/cpufreq.h
> +++ b/include/linux/sched/cpufreq.h
> @@ -10,6 +10,7 @@
>  #define SCHED_CPUFREQ_RT     (1U << 0)
>  #define SCHED_CPUFREQ_DL     (1U << 1)
>  #define SCHED_CPUFREQ_IOWAIT (1U << 2)
> +#define SCHED_CPUFREQ_IDLE   (1U << 3)
>  
>  #define SCHED_CPUFREQ_RT_DL  (SCHED_CPUFREQ_RT | SCHED_CPUFREQ_DL)
>  
> diff --git a/kernel/sched/cpufreq_schedutil.c 
> b/kernel/sched/cpufreq_schedutil.c
> index eaba6d6..004ae18 100644
> --- a/kernel/sched/cpufreq_schedutil.c
> +++ b/kernel/sched/cpufreq_schedutil.c
> @@ -304,6 +304,12 @@ static void sugov_update_shared(struct update_util_data 
> *hook, u64 time,
>  
>       sg_cpu->util = util;
>       sg_cpu->max = max;
> +
> +     /* CPU is entering IDLE, reset flags without triggering an update */
> +     if (unlikely(flags & SCHED_CPUFREQ_IDLE)) {
> +             sg_cpu->flags = 0;
> +             goto done;
> +     }

Why is it important to have the above diff at all ? For example we aren't doing
similar stuff in sugov_update_single() and that will go on and try to change the
frequency if rate_limit_us time is over since last update.

And also why is it important to write 0 to sg_cpu->flags ? What wouldn't work if
we set sg_cpu->flags to SCHED_CPUFREQ_IDLE in this case ? i.e. Just the below
statement should be good for us.

>       sg_cpu->flags = flags;
>  
>       sugov_set_iowait_boost(sg_cpu, time, flags);
> @@ -318,6 +324,7 @@ static void sugov_update_shared(struct update_util_data 
> *hook, u64 time,
>               sugov_update_commit(sg_policy, time, next_f);
>       }
>  
> +done:
>       raw_spin_unlock(&sg_policy->update_lock);
>  }
>  
> diff --git a/kernel/sched/idle_task.c b/kernel/sched/idle_task.c
> index 0c00172..a844c91 100644
> --- a/kernel/sched/idle_task.c
> +++ b/kernel/sched/idle_task.c
> @@ -29,6 +29,10 @@ pick_next_task_idle(struct rq *rq, struct task_struct 
> *prev, struct rq_flags *rf
>       put_prev_task(rq, prev);
>       update_idle_core(rq);
>       schedstat_inc(rq->sched_goidle);
> +
> +     /* kick cpufreq (see the comment in kernel/sched/sched.h). */
> +     cpufreq_update_this_cpu(rq, SCHED_CPUFREQ_IDLE);
> +

This looks correct.

Can we completely avoid the utilization contribution of the CPUs which have gone
idle? Right now we avoid them with help of (delta_ns > TICK_NSEC). Can we
instead check this SCHED_CPUFREQ_IDLE flag ?

-- 
viresh

Reply via email to