linux-kernel - Re: [PATCH v2 1/10] cpufreq: Reduce 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: <CAJZ5v0gt3tmJ1ZunvkLKfeUSgZv7yiO9vBDtDmXSuC=tUUpPLg@mail.gmail.com>
Date:	Wed, 9 Mar 2016 15:17:48 +0100
From:	"Rafael J. Wysocki" <rafael@...nel.org>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Linux PM list <linux-pm@...r.kernel.org>,
	Juri Lelli <juri.lelli@....com>,
	Steve Muckle <steve.muckle@...aro.org>,
	ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
	Viresh Kumar <viresh.kumar@...aro.org>,
	Vincent Guittot <vincent.guittot@...aro.org>,
	Michael Turquette <mturquette@...libre.com>,
	Ingo Molnar <mingo@...nel.org>
Subject: Re: [PATCH v2 1/10] cpufreq: Reduce cpufreq_update_util() overhead a bit

On Wed, Mar 9, 2016 at 1:39 PM, Peter Zijlstra <peterz@...radead.org> wrote:
> On Fri, Mar 04, 2016 at 03:58:22AM +0100, Rafael J. Wysocki wrote:
>> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
>>
>> Use the observation that cpufreq_update_util() is only called
>> by the scheduler with rq->lock held, so the callers of
>> cpufreq_set_update_util_data() can use synchronize_sched()
>> instead of synchronize_rcu() to wait for cpufreq_update_util()
>> to complete.  Moreover, if they are updated to do that,
>> rcu_read_(un)lock() calls in cpufreq_update_util() might be
>> replaced with rcu_read_(un)lock_sched(), respectively, but
>> those aren't really necessary, because the scheduler calls
>> that function from RCU-sched read-side critical sections
>> already.
>>
>> In addition to that, if cpufreq_set_update_util_data() checks
>> the func field in the struct update_util_data before setting
>> the per-CPU pointer to it, the data->func check may be dropped
>> from cpufreq_update_util() as well.
>>
>> Make the above changes to reduce the overhead from
>> cpufreq_update_util() in the scheduler paths invoking it
>> and to make the cleanup after removing its callbacks less
>> heavy-weight somewhat.
>>
>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
>> Acked-by: Viresh Kumar <viresh.kumar@...aro.org>
>> ---
>>
>> Changes from the previous version:
>> - Use rcu_dereference_sched() in cpufreq_update_util().
>
> Which I think also shows the WARN_ON I insisted upon is redundant.
>
> In any case, I cannot object to reducing overhead, esp. as this whole
> patch was suggested by me in the first place, so:
>
> Acked-by: Peter Zijlstra (Intel) <peterz@...radead.org>

Thanks!

> That said, how about the below? It avoids a function call.

That is fine by me.

What about taking it a bit further, though, and moving the definition
of cpufreq_update_util_data to somewhere under kernel/sched/ (like
kernel/sched/cpufreq.c maybe)?

Then, the whole static inline void cpufreq_update_util() definition
can go into kernel/sched/sched.h (it doesn't have to be visible
anywhere beyond kernel/sched/) and the only thing that needs to be
exported to cpufreq will be a helper (or two), to set/clear the
cpufreq_update_util_data pointers.

I'll try to cut a patch doing that later today for illustration.