Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1681273

Re: [PATCH v2 2/6] cpufreq: schedutil: reset sg_cpus's flags at IDLE enter

From Viresh Kumar <viresh.kumar@linaro.org>
Newsgroups linux.kernel
Subject Re: [PATCH v2 2/6] cpufreq: schedutil: reset sg_cpus's flags at IDLE enter
Date 2017-07-05 07:00 +0200
Message-ID <tZKzT-1us-3@gated-at.bofh.it> (permalink)
References <tZzXQ-2ZJ-7@gated-at.bofh.it> <tZzXQ-2ZJ-25@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


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

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH v2 0/6] cpufreq: schedutil: fixes for flags updates Patrick Bellasi <patrick.bellasi@arm.com> - 2017-07-04 19:40 +0200
  [PATCH v2 2/6] cpufreq: schedutil: reset sg_cpus's flags at IDLE enter Patrick Bellasi <patrick.bellasi@arm.com> - 2017-07-04 19:40 +0200
    Re: [PATCH v2 2/6] cpufreq: schedutil: reset sg_cpus's flags at IDLE  enter Viresh Kumar <viresh.kumar@linaro.org> - 2017-07-05 07:00 +0200
      Re: [PATCH v2 2/6] cpufreq: schedutil: reset sg_cpus's flags at IDLE  enter Patrick Bellasi <patrick.bellasi@arm.com> - 2017-07-05 15:10 +0200
        Re: [PATCH v2 2/6] cpufreq: schedutil: reset sg_cpus's flags at IDLE  enter Viresh Kumar <viresh.kumar@linaro.org> - 2017-07-06 07:50 +0200
    Re: [PATCH v2 2/6] cpufreq: schedutil: reset sg_cpus's flags at IDLE enter Joel Fernandes <joelaf@google.com> - 2017-07-07 06:50 +0200
      Re: [PATCH v2 2/6] cpufreq: schedutil: reset sg_cpus's flags at IDLE  enter Juri Lelli <juri.lelli@arm.com> - 2017-07-07 12:20 +0200
        Re: [PATCH v2 2/6] cpufreq: schedutil: reset sg_cpus's flags at IDLE  enter Saravana Kannan <skannan@codeaurora.org> - 2017-07-11 21:20 +0200
  [PATCH v2 5/6] sched/rt: fast switch to maximum frequency when RT tasks are scheduled Patrick Bellasi <patrick.bellasi@arm.com> - 2017-07-04 19:40 +0200
  [PATCH v2 6/6] cpufreq: schedutil: relax rate-limiting while running RT/DL tasks Patrick Bellasi <patrick.bellasi@arm.com> - 2017-07-04 19:40 +0200
  Re: [PATCH v2 0/6] cpufreq: schedutil: fixes for flags updates "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-07-07 00:40 +0200

csiph-web