linux-kernel - Re: [RFC][PATCH] sched: Optimize cpufreq_update

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: <20210319073751.qz2ytpxl2ikrt2b7@vireshk-i7>
Date:   Fri, 19 Mar 2021 13:07:51 +0530
From:   Viresh Kumar <viresh.kumar@...aro.org>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     "Rafael J. Wysocki" <rjw@...ysocki.net>, mingo@...hat.com,
        vincent.guittot@...aro.org, dietmar.eggemann@....com,
        rostedt@...dmis.org, linux-kernel@...r.kernel.org,
        Josh Poimboeuf <jpoimboe@...hat.com>
Subject: Re: [RFC][PATCH] sched: Optimize cpufreq_update_util

On 18-03-21, 22:28, Peter Zijlstra wrote:
> Also, is there a lock order comment in cpufreq somewhere?

I don't think so.

> I tried
> following it, but eventually gave up and figured 'asking' lockdep was
> far simpler.

This will get called from CPU's online/offline path at worst, nothing more.

> +static void cpufreq_update_optimize(void)
> +{
> +	struct update_util_data *data;
> +	cpu_util_update_f func = NULL, dfunc;
> +	int cpu;
> +
> +	for_each_online_cpu(cpu) {
> +		data = per_cpu(cpufreq_update_util_data, cpu);
> +		dfunc = data ? READ_ONCE(data->func) : NULL;
> +
> +		if (dfunc) {
> +			if (!func)
> +				func = dfunc;
> +			else if (func != dfunc)
> +				return;
> +		} else if (func)
> +			return;
> +	}

So there is nothing cpufreq specific IIRC that can help make this better, this
is basically per policy.

For example, on an ARM platform we have two cpufreq policies with one policy
covering 4 CPUs, while the other one covering only 1 (maybe because we didn't
add those CPUs in DT or something else), then also we will end up separate
routines.

Or if we take all CPUs of a policy offline and then bring them up one by one, I
think for the first CPU online event in that policy we will end up using the
sugov_update_single_freq() variant for some time, until the time more CPUs come
up.

So traversing the way you did this is probably something that will work properly
in all corner cases.

-- 
viresh