Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 19 Mar 2021 13:07:51 +0530
From:   Viresh Kumar <viresh.kumar@linaro.org>
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     "Rafael J. Wysocki" <rjw@rjwysocki.net>, mingo@redhat.com,
        vincent.guittot@linaro.org, dietmar.eggemann@arm.com,
        rostedt@goodmis.org, linux-kernel@vger.kernel.org,
        Josh Poimboeuf <jpoimboe@redhat.com>
Subject: Re: [RFC][PATCH] sched: Optimize cpufreq_update_util
Message-ID: <20210319073751.qz2ytpxl2ikrt2b7@vireshk-i7>
References: <20210318212826.GW4746@worktop.programming.kicks-ass.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210318212826.GW4746@worktop.programming.kicks-ass.net>
User-Agent: NeoMutt/20180716-391-311a52
Precedence: bulk

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