From: Juri Lelli <juri.lelli@arm.com>
To: Michael Turquette <mturquette@baylibre.com>
Cc: Viresh Kumar <viresh.kumar@linaro.org>,
linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org,
peterz@infradead.org, rjw@rjwysocki.net, steve.muckle@linaro.org,
vincent.guittot@linaro.org, morten.rasmussen@arm.com,
dietmar.eggemann@arm.com
Subject: Re: [RFC PATCH 18/19] cpufreq: remove transition_lock
Date: Thu, 14 Jan 2016 09:44:31 +0000 [thread overview]
Message-ID: <20160114094431.GF18603@e106622-lin> (raw)
In-Reply-To: <20160113182131.1168.45753@quark.deferred.io>
Hi,
On 13/01/16 10:21, Michael Turquette wrote:
> Hi Viresh,
>
> Quoting Viresh Kumar (2016-01-12 22:31:48)
> > On 12-01-16, 16:54, Michael Turquette wrote:
> > > __cpufreq_driver_target should be using a per-policy lock.
> >
> > It doesn't :)
>
> It should.
>
> A less conceited response is that a per-policy lock should be held
> around calls to __cpufreq_driver_target. This can obviously be done by
> cpufreq_driver_target (no double underscore), but there are quite a few
> drivers that call __cpufreq_driver_target, and since we're touching the
> policy structure we need a lock around it.
>
Agree, we should enforce the rule that everything that touches policy
structure has to lock it before.
> Juri's cover letter did not explicitly state my original, full intention
> for the patches I was working on. I'll spell that out below and
> hopefully we can gather consensus around it before moving forward. Juri,
> I'm implicitly assuming that you agree with the stuff below, but please
> correct me if I am wrong.
Right. I decided to post with this RFC only a subset of the patches we
came up with because I needed to build some more confidence with the
subsystem I was going to propose changes for. Review comments received
are helping me on that front. I didn't mention at all next steps (RCU)
because I wanted to focus on understanding and documenting, and maybe
fixing where required, the current status, before we change it.
> The original idea for overhauling the locking
> in cpufreq is to use two locks:
>
> 1) per-policy lock (my patches were using a mutex), which is the only
> lock that needs to be touched during a frequency transition. We do not
> want any lock contention between policy's during freq transition. For
> read-side operation this locking scheme should utilize RCU so that the
> scheduler can safely access the values in struct cpufreq_policy within
> it's schedule() context. [a note on RCU below]
>
> 2) a single, framework-wide lock (my patches were using a mutex) that
> handles all of the other synchronization: governor events, driver events
> and anything else that does not happen on a per-policy basis. I don't
> think RCU is necessary for this. These operations are all slow-path ones
> so reducing the mess of 6-ish locks in cpufreq.c and friends down to a
> single mutex simplifies things greatly, eliminates the "drop the lock
> here for a few instructions" hacks and generally makes things more
> readable.
>
This is basically what I also have on top of this series. I actually
went for RCUs also for 2, but yes, that's maybe overkilling.
A comment on 1 above, and something on which I got stuck upon for some
time, is that, if we implement RCU logic as it is supposed to be, I
think we can generate a lot of copy-update operations when changing
frequency (as policy structure needs to be changed). Also, we might read
stale data. So, I'm not sure this will pay off. However, I tried to get
around this problem and I guess we will discuss if 1 is doable in the
next RFC :-).
> A quick note on RCU and the scheduler-driven DVFS stuff: RCU only helps
> us on read-side operations. For the purposes of sched-dvfs, this means
> that when we look at capacity utilization and want to normalize
> frequency based on that, we need to access the per-policy structure in a
> lockless way. RCU makes this possible.
>
> RCU is absolutely not a magic bullet or elixir that lets us kick off
> DVFS transitions from the schedule() context. The frequency transitions
> are write-side operations, as we invariably touch struct cpufreq_policy.
> This means that the read-side stuff can live in the schedule() context,
> but write-side needs to be kicked out to a thread.
>
Correct. We will still need the kthread machinery even after this
changes.
Thanks for clarifying things!
Best,
- Juri
next prev parent reply other threads:[~2016-01-14 9:45 UTC|newest]
Thread overview: 110+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-11 17:35 [RFC PATCH 00/19] cpufreq locking cleanups and documentation Juri Lelli
2016-01-11 17:35 ` [RFC PATCH 01/19] cpufreq: do not expose cpufreq_governor_lock Juri Lelli
2016-01-12 8:56 ` Viresh Kumar
2016-01-11 17:35 ` [RFC PATCH 02/19] cpufreq: merge governor lock and mutex Juri Lelli
2016-01-12 9:00 ` Viresh Kumar
2016-01-11 17:35 ` [RFC PATCH 03/19] cpufreq: kill for_each_policy Juri Lelli
2016-01-12 9:01 ` Viresh Kumar
2016-01-11 17:35 ` [RFC PATCH 04/19] cpufreq: bring data structures close to their locks Juri Lelli
2016-01-11 22:05 ` Peter Zijlstra
2016-01-11 23:03 ` Rafael J. Wysocki
2016-01-12 8:27 ` Peter Zijlstra
2016-01-12 10:43 ` Juri Lelli
2016-01-12 16:47 ` Rafael J. Wysocki
2016-01-11 22:07 ` Peter Zijlstra
2016-01-12 9:27 ` Viresh Kumar
2016-01-12 11:21 ` Juri Lelli
2016-01-12 11:58 ` Peter Zijlstra
2016-01-12 12:36 ` Juri Lelli
2016-01-12 15:26 ` Juri Lelli
2016-01-12 15:58 ` Peter Zijlstra
2016-01-12 9:10 ` Viresh Kumar
2016-01-11 17:35 ` [RFC PATCH 05/19] cpufreq: assert locking when accessing cpufreq_policy_list Juri Lelli
2016-01-12 9:34 ` Viresh Kumar
2016-01-12 11:44 ` Juri Lelli
2016-01-13 5:59 ` Viresh Kumar
2016-01-11 17:35 ` [RFC PATCH 06/19] cpufreq: always access cpufreq_policy_list while holding cpufreq_driver_lock Juri Lelli
2016-01-12 9:57 ` Viresh Kumar
2016-01-12 12:08 ` Juri Lelli
2016-01-13 6:01 ` Viresh Kumar
2016-01-11 17:35 ` [RFC PATCH 07/19] cpufreq: assert locking when accessing cpufreq_governor_list Juri Lelli
2016-01-12 10:01 ` Viresh Kumar
2016-01-12 15:33 ` Juri Lelli
2016-01-11 17:35 ` [RFC PATCH 08/19] cpufreq: fix warning for cpufreq_init_policy unlocked access to cpufreq_governor_list Juri Lelli
2016-01-12 10:09 ` Viresh Kumar
2016-01-12 15:52 ` Juri Lelli
2016-01-13 6:07 ` Viresh Kumar
2016-01-14 16:35 ` Juri Lelli
2016-01-18 5:23 ` Viresh Kumar
2016-01-18 15:19 ` Juri Lelli
2016-01-11 17:35 ` [RFC PATCH 09/19] cpufreq: fix warning for show_scaling_available_governors " Juri Lelli
2016-01-12 10:13 ` Viresh Kumar
2016-01-13 10:25 ` Juri Lelli
2016-01-13 10:32 ` Viresh Kumar
2016-01-11 17:35 ` [RFC PATCH 10/19] cpufreq: assert policy->rwsem is held in cpufreq_set_policy Juri Lelli
2016-01-12 10:15 ` Viresh Kumar
2016-01-11 17:35 ` [RFC PATCH 11/19] cpufreq: assert policy->rwsem is held in __cpufreq_governor Juri Lelli
2016-01-12 10:20 ` Viresh Kumar
2016-01-30 0:33 ` Saravana Kannan
2016-01-30 11:49 ` Rafael J. Wysocki
2016-02-01 6:09 ` Viresh Kumar
2016-02-01 10:22 ` Rafael J. Wysocki
2016-02-01 20:24 ` Saravana Kannan
2016-02-01 21:00 ` Rafael J. Wysocki
2016-02-02 6:36 ` Viresh Kumar
2016-02-02 21:38 ` Saravana Kannan
2016-02-02 6:34 ` Viresh Kumar
2016-02-02 21:37 ` Saravana Kannan
2016-02-03 2:13 ` Viresh Kumar
2016-02-03 4:04 ` Saravana Kannan
2016-02-03 5:02 ` Viresh Kumar
2016-02-03 5:06 ` Saravana Kannan
2016-02-03 6:59 ` Viresh Kumar
2016-01-11 17:35 ` [RFC PATCH 12/19] cpufreq: fix locking of policy->rwsem in cpufreq_init_policy Juri Lelli
2016-01-12 10:39 ` Viresh Kumar
2016-01-14 17:58 ` Juri Lelli
2016-01-11 17:35 ` [RFC PATCH 13/19] cpufreq: fix locking of policy->rwsem in cpufreq_offline_prepare Juri Lelli
2016-01-12 10:54 ` Viresh Kumar
2016-01-15 12:37 ` Juri Lelli
2016-01-11 17:35 ` [RFC PATCH 14/19] cpufreq: fix locking of policy->rwsem in cpufreq_offline_finish Juri Lelli
2016-01-12 11:02 ` Viresh Kumar
2016-01-11 17:35 ` [RFC PATCH 15/19] cpufreq: remove useless usage of cpufreq_governor_mutex in __cpufreq_governor Juri Lelli
2016-01-12 11:06 ` Viresh Kumar
2016-01-15 16:30 ` Juri Lelli
2016-01-18 5:50 ` Viresh Kumar
2016-01-19 16:49 ` Juri Lelli
2016-01-20 7:29 ` Viresh Kumar
2016-01-20 10:17 ` Juri Lelli
2016-01-20 10:18 ` Viresh Kumar
2016-01-20 10:27 ` Juri Lelli
2016-01-20 10:30 ` Viresh Kumar
2016-01-11 17:35 ` [RFC PATCH 16/19] cpufreq: hold policy->rwsem across CPUFREQ_GOV_POLICY_EXIT Juri Lelli
2016-01-12 11:09 ` Viresh Kumar
2016-01-11 17:35 ` [RFC PATCH 17/19] cpufreq: stop checking for cpufreq_driver being present in cpufreq_cpu_get Juri Lelli
2016-01-12 11:17 ` Viresh Kumar
2016-01-11 17:35 ` [RFC PATCH 18/19] cpufreq: remove transition_lock Juri Lelli
2016-01-12 11:24 ` Viresh Kumar
2016-01-13 0:54 ` Michael Turquette
2016-01-13 6:31 ` Viresh Kumar
[not found] ` <20160113182131.1168.45753@quark.deferred.io>
2016-01-14 9:44 ` Juri Lelli [this message]
2016-01-14 10:32 ` Viresh Kumar
2016-01-14 13:52 ` Juri Lelli
2016-01-18 5:09 ` Viresh Kumar
2016-01-19 14:00 ` Peter Zijlstra
2016-01-19 14:42 ` Juri Lelli
2016-01-19 15:30 ` Peter Zijlstra
2016-01-19 16:01 ` Juri Lelli
2016-01-19 19:17 ` Peter Zijlstra
2016-01-19 19:21 ` Peter Zijlstra
2016-01-19 21:52 ` Rafael J. Wysocki
2016-01-20 17:04 ` Peter Zijlstra
2016-01-20 22:12 ` Rafael J. Wysocki
2016-01-20 22:38 ` Peter Zijlstra
2016-01-20 23:33 ` Rafael J. Wysocki
2016-01-20 12:59 ` Juri Lelli
2016-01-11 17:36 ` [RFC PATCH 19/19] cpufreq: documentation: document locking scheme Juri Lelli
2016-01-11 22:45 ` [RFC PATCH 00/19] cpufreq locking cleanups and documentation Rafael J. Wysocki
2016-01-12 10:46 ` Juri Lelli
2016-01-30 0:57 ` Saravana Kannan
2016-02-01 6:02 ` Viresh Kumar
2016-02-01 12:06 ` Juri Lelli
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160114094431.GF18603@e106622-lin \
--to=juri.lelli@arm.com \
--cc=dietmar.eggemann@arm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=morten.rasmussen@arm.com \
--cc=mturquette@baylibre.com \
--cc=peterz@infradead.org \
--cc=rjw@rjwysocki.net \
--cc=steve.muckle@linaro.org \
--cc=vincent.guittot@linaro.org \
--cc=viresh.kumar@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).