All of lore.kernel.org
 help / color / mirror / Atom feed
From: Steve Muckle <steve.muckle@linaro.org>
To: Michael Turquette <mturquette@baylibre.com>
Cc: Vincent Guittot <vincent.guittot@linaro.org>,
	Juri Lelli <Juri.Lelli@arm.com>,
	Patrick Bellasi <patrick.bellasi@arm.com>,
	Morten Rasmussen <morten.rasmussen@arm.com>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Viresh Kumar <viresh.kumar@linaro.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>
Subject: Re: sched-freq locking
Date: Tue, 19 Jan 2016 17:24:49 -0800	[thread overview]
Message-ID: <569EE1E1.3050407@linaro.org> (raw)
In-Reply-To: <CAEG3pNBgpsdX8Lk7_3nHd31_bsq2sLB_f+=4_xiFFbiAWiKsxg@mail.gmail.com>

On 01/19/2016 03:40 PM, Michael Turquette wrote:
> Right, this was _the_ original impetus behind the design decision to
> muck around with struct cpufreq_policy in the hot path which goes al
> the way back to v1.
> 
> An alternative thought is that we can make copies of the relevant bits
> of struct cpufreq_policy that we do not expect too change often. These
> will not require any locks as they are mostly read-only data on the
> scheduler side of the interface. Or we could even go all in and just
> make local copies of the struct directly, during the GOV_START
> perhaps, with:

I believe this is a good first step as it avoids reworking a huge amount
of locking and can get us to something functionally correct. It is what
I had proposed earlier, copying the enabled CPUs and freq table in
during the governor start callback. Unless there are objections to it
I'll add it to the next schedfreq RFC.

> 
...
> 
> Well if we're going to try an optimize out every single false-positive
> wakeup then I think that the cleanest long term solution would be
> rework the per-policy locking around struct cpufreq_policy to use a
> raw spinlock.

It would be nice if the policy lock was a spinlock but I don't know how
easy that is. From a quick look at cpufreq there's a blocking notifier
chain that's called with rwsem held, so it looks messy. Potentially long
term indeed.

>> Also it'd be good I think to avoid building in an assumption that we'll
>> never want to run solely in the fast (atomic) path. Perhaps ARM won't,
>> and x86 may never use this, but it's reasonable to think another
>> platform might come along which uses cpufreq and has the capability to
>> kick off cpufreq transitions swiftly and without sleeping. Maybe ARM
>> platforms will evolve to have that capability.
> 
> The current design of the cpufreq subsystem and its interfaces have
> made this choice for us. sched-freq is just another consumer of
> cpufreq, and until cpufreq's own locking scheme is improved then we
> have no choice.

I did not word that very well - I should have said, we should avoid
building in an assumption that we never want to try and run in the fast
path.

AFAICS, once we've calculated that a frequency change is required we can
down_write_trylock(&policy->rwsem) in the fast path and go ahead with
the transition, if the trylock succeeds and the driver supports fast
path transitions. We can fall back to the slow path (waking up the
kthread) if that fails.

> This discussion is pretty useful. Should we Cc lkml to this thread?

Done (added linux-pm, PeterZ and Rafael as well).

thanks,
Steve

       reply	other threads:[~2016-01-20  1:24 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <56984C30.8040402@linaro.org>
     [not found] ` <20160115104051.GP18603@e106622-lin>
     [not found]   ` <569D568D.5000500@linaro.org>
     [not found]     ` <CAEG3pNDoDHpgO7WdC8n7C8Ri+KNKZqOi9rKHMkTV0c5dkyevOw@mail.gmail.com>
     [not found]       ` <CAKfTPtCYUWBm_=GQ-NLLZ01xJqG+83vy4uapxRLjqMx_61HSew@mail.gmail.com>
     [not found]         ` <569E90CF.9050503@linaro.org>
     [not found]           ` <CAEG3pND5kHqJhrvwVjsnCnfT-nSrV8SrUaNk2p=ZfEx0MmmB=A@mail.gmail.com>
     [not found]             ` <569EB225.4040707@linaro.org>
     [not found]               ` <CAEG3pNBgpsdX8Lk7_3nHd31_bsq2sLB_f+=4_xiFFbiAWiKsxg@mail.gmail.com>
2016-01-20  1:24                 ` Steve Muckle [this message]
2016-01-20 12:18                   ` sched-freq locking Juri Lelli
2016-01-20 14:50                     ` Steve Muckle
2016-01-20 15:58                       ` Juri Lelli
2016-01-20 16:39                         ` Punit Agrawal
2016-01-20 16:39                           ` Punit Agrawal
2016-01-20 16:46                         ` Punit Agrawal
2016-01-20 16:46                           ` Punit Agrawal
2016-01-20 20:46                         ` Steve Muckle
2016-01-21  9:45                           ` Juri Lelli
2016-01-21 19:29                             ` Steve Muckle
2016-01-21  1:22                   ` Rafael J. Wysocki
2016-01-21  1:39                     ` Steve Muckle
2016-01-21  1:46                       ` Rafael J. Wysocki
2016-01-21 10:49                         ` Juri Lelli
2016-01-22  1:21                           ` Rafael J. Wysocki
2016-01-27  1:54                             ` Steve Muckle
2016-01-27  8:03                               ` Rafael J. Wysocki

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=569EE1E1.3050407@linaro.org \
    --to=steve.muckle@linaro.org \
    --cc=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=patrick.bellasi@arm.com \
    --cc=peterz@infradead.org \
    --cc=rjw@rjwysocki.net \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.