linux-kernel - Re: [RFC PATCH v3 5/6] sched/cpufreq: Boost schedutil frequency ramp up

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]

Message-ID: <4235a01d-26ee-418c-b78a-6c6109deda1d@arm.com>
Date:   Thu, 17 Oct 2019 11:21:21 +0200
From:   Dietmar Eggemann <dietmar.eggemann@....com>
To:     Douglas RAILLARD <douglas.raillard@....com>,
        linux-kernel@...r.kernel.org
Cc:     linux-pm@...r.kernel.org, mingo@...hat.com, peterz@...radead.org,
        rjw@...ysocki.net, viresh.kumar@...aro.org, juri.lelli@...hat.com,
        vincent.guittot@...aro.org, qperret@...rret.net,
        patrick.bellasi@...bug.net, dh.han@...sung.com
Subject: Re: [RFC PATCH v3 5/6] sched/cpufreq: Boost schedutil frequency ramp
 up

On 11/10/2019 15:44, Douglas RAILLARD wrote:

[...]

> @@ -539,6 +543,7 @@ static void sugov_update_single(struct update_util_data *hook, u64 time,
>  	unsigned long util, max;
>  	unsigned int next_f;
>  	bool busy;
> +	unsigned long ramp_boost = 0;

Shouldn't always order local variable declarations from longest to
shortest line?

>  	sugov_iowait_boost(sg_cpu, time, flags);
>  	sg_cpu->last_update = time;
> @@ -552,10 +557,10 @@ static void sugov_update_single(struct update_util_data *hook, u64 time,
>  	busy = !sg_policy->need_freq_update && sugov_cpu_is_busy(sg_cpu);
>  
>  	util = sugov_get_util(sg_cpu);
> -	sugov_cpu_ramp_boost_update(sg_cpu);
> +	ramp_boost = sugov_cpu_ramp_boost_update(sg_cpu);
>  	max = sg_cpu->max;
>  	util = sugov_iowait_apply(sg_cpu, time, util, max);
> -	next_f = get_next_freq(sg_policy, util, max);
> +	next_f = get_next_freq(sg_policy, util, max, ramp_boost);
>  	/*
>  	 * Do not reduce the frequency if the CPU has not been idle
>  	 * recently, as the reduction is likely to be premature then.
> @@ -587,6 +592,8 @@ static unsigned int sugov_next_freq_shared(struct sugov_cpu *sg_cpu, u64 time)
>  	struct cpufreq_policy *policy = sg_policy->policy;
>  	unsigned long util = 0, max = 1;
>  	unsigned int j;
> +	unsigned long ramp_boost = 0;
> +	unsigned long j_ramp_boost = 0;

Shouldn't always order local variable declarations from longest to
shortest line?

>  	for_each_cpu(j, policy->cpus) {
>  		struct sugov_cpu *j_sg_cpu = &per_cpu(sugov_cpu, j);
> @@ -594,7 +601,11 @@ static unsigned int sugov_next_freq_shared(struct sugov_cpu *sg_cpu, u64 time)
>  
>  		j_util = sugov_get_util(j_sg_cpu);
>  		if (j_sg_cpu == sg_cpu)
> -			sugov_cpu_ramp_boost_update(sg_cpu);
> +			j_ramp_boost = sugov_cpu_ramp_boost_update(sg_cpu);
> +		else
> +			j_ramp_boost = sugov_cpu_ramp_boost(j_sg_cpu);
> +		ramp_boost = max(ramp_boost, j_ramp_boost);

Ah, that's why you call sugov_cpu_ramp_boost_update() from
sugov_next_freq_shared() in 4/6. So sugov_cpu_ramp_boost_update() is
more a sugov_cpu_ramp_boost(..., int update)? Minor detail though.

[...]