linux-kernel - Re: [PATCH RFC v4] cpufreq: schedutil: Make iowait boost more energy efficient

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 11 Jul 2017 15:44:32 +0530
From:   Viresh Kumar <viresh.kumar@...aro.org>
To:     Joel Fernandes <joelaf@...gle.com>
Cc:     linux-kernel@...r.kernel.org, patrick.bellasi@....com,
        juri.lelli@....com, andresoportus@...gle.com,
        dietmar.eggemann@....com,
        Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
        Len Brown <lenb@...nel.org>,
        "Rafael J . Wysocki" <rjw@...ysocki.net>,
        Ingo Molnar <mingo@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH RFC v4] cpufreq: schedutil: Make iowait boost more energy
 efficient

On 09-07-17, 10:08, Joel Fernandes wrote:
> diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
> index 622eed1b7658..4d9e8b96bed1 100644
> --- a/kernel/sched/cpufreq_schedutil.c
> +++ b/kernel/sched/cpufreq_schedutil.c
> @@ -53,7 +53,9 @@ struct sugov_cpu {
>  	struct update_util_data update_util;
>  	struct sugov_policy *sg_policy;
>  
> +	bool prev_iowait_boost;
>  	unsigned long iowait_boost;
> +	unsigned long iowait_boost_min;
>  	unsigned long iowait_boost_max;
>  	u64 last_update;
>  
> @@ -168,22 +170,47 @@ static void sugov_get_util(unsigned long *util, unsigned long *max)
>  	*max = cfs_max;
>  }
>  
> +static void sugov_decay_iowait_boost(struct sugov_cpu *sg_cpu)
> +{
> +	sg_cpu->iowait_boost >>= 1;
> +
> +	if (sg_cpu->iowait_boost < sg_cpu->iowait_boost_min)
> +		sg_cpu->iowait_boost = 0;
> +}
> +
>  static void sugov_set_iowait_boost(struct sugov_cpu *sg_cpu, u64 time,
>  				   unsigned int flags)
>  {
>  	if (flags & SCHED_CPUFREQ_IOWAIT) {
> -		sg_cpu->iowait_boost = sg_cpu->iowait_boost_max;
> +		/* Remember for next time that we did an iowait boost */
> +		sg_cpu->prev_iowait_boost = true;
> +		if (sg_cpu->iowait_boost) {
> +			sg_cpu->iowait_boost <<= 1;
> +			sg_cpu->iowait_boost = min(sg_cpu->iowait_boost,
> +						   sg_cpu->iowait_boost_max);
> +		} else {
> +			sg_cpu->iowait_boost = sg_cpu->iowait_boost_min;

I am not sure if boost should start from the min frequency, as the current
frequency will at least be equal to that. Which means that with no boost
initially, we will never increase the frequency for the first IOWAIT flag event.

> +		}
>  	} else if (sg_cpu->iowait_boost) {
>  		s64 delta_ns = time - sg_cpu->last_update;
>  
>  		/* Clear iowait_boost if the CPU apprears to have been idle. */
>  		if (delta_ns > TICK_NSEC)
>  			sg_cpu->iowait_boost = 0;
> +
> +		/*
> +		 * Since we don't decay iowait_boost when its consumed during
> +		 * the previous SCHED_CPUFREQ_IOWAIT update, decay it now.
> +		 */
> +		if (sg_cpu->prev_iowait_boost) {

SCHED_CPUFREQ_IOWAIT flag is set only by CFS from the enqueue_task() and in many
cases we call the util hook twice from the same enqueue_task() instance before
returning (2nd one after updating util). And in such cases we will set
iowait_boost as 0 on the second call.

Have you ever seen two consecutive calls to sugov_set_iowait_boost() with IOWAIT
flag set ? Can we get the ratio of that against the other case where we have
IOWAIT flag set in first call, followed by one or more non-IOWAIT calls and then
IOWAIT again ?

I am asking because if the calls with IOWAIT flag aren't consecutive then we
will make iowait_boost as 0 in the next non-IOWAIT call.

-- 
viresh