linux-kernel - Re: [PATCH v2 3/3] PM / EM: Skip inefficient OPPs

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: <f54a1dd5-fe1f-1d2e-3fd9-6ba019157e4a@arm.com>
Date:   Fri, 28 May 2021 10:00:59 +0100
From:   Lukasz Luba <lukasz.luba@....com>
To:     Viresh Kumar <viresh.kumar@...aro.org>
Cc:     Vincent Donnefort <vincent.donnefort@....com>,
        peterz@...radead.org, rjw@...ysocki.net,
        vincent.guittot@...aro.org, qperret@...gle.com,
        linux-kernel@...r.kernel.org, ionela.voinescu@....com,
        dietmar.eggemann@....com
Subject: Re: [PATCH v2 3/3] PM / EM: Skip inefficient OPPs



On 5/28/21 6:04 AM, Viresh Kumar wrote:
> On 21-05-21, 17:54, Vincent Donnefort wrote:
>> diff --git a/include/linux/energy_model.h b/include/linux/energy_model.h
>> +static inline
>> +struct em_perf_state *em_pd_get_efficient_state(struct em_perf_domain *pd,
>> +						unsigned long freq)
>> +{
>> +	struct em_perf_state *ps;
>> +	int i;
>> +
>> +	for (i = 0; i < pd->nr_perf_states; i++) {
>> +		ps = &pd->table[i];
>> +		if (ps->flags & EM_PERF_STATE_INEFFICIENT)
>> +			continue;
>> +		if (ps->frequency >= freq)
>> +			break;
> 
> I believe it may be more optimal if we change the sequence of these two 'if'
> blocks here. We only need to check for inefficient frequencies if it is >= freq.

Make sense (especially in context of your example below).

> 
>> +	}
>> +
>> +	return ps;
>> +}
> 
> 
>> diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
>> @@ -153,6 +154,9 @@ static unsigned int get_next_freq(struct sugov_policy *sg_policy,
>>   
>>   	freq = map_util_freq(util, freq, max);
>>   
>> +	/* Avoid inefficient performance states */
>> +	freq = em_pd_get_efficient_freq(em_cpu_get(policy->cpu), freq);
>> +
>>   	if (freq == sg_policy->cached_raw_freq && !sg_policy->need_freq_update)
>>   		return sg_policy->next_freq;
> 
> Assume this freq-table (E=efficient, IE=inefficient): 800M (E), 1G (E), 1.2G (IE), 1.4G (IE), 1.6G (E).
> Thermal limits max to 1.4G
> 
> Freq returned by map_util_freq() is 1.01G.
> 
> Will we not end up selecting 1.4G here ? Inefficient as well as much higher than
> what we requested for ?
> 

Correct, it will be 1.4G, instead of selecting 1.2G, in this example.
The two or more consecutive inefficient OPPs in the table ruins the
proper behavior. We have to reevaluate this.

Thanks Viresh for pointing this out.

Regards,
Lukasz