public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
From: lorenzo.pieralisi@arm.com (Lorenzo Pieralisi)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] drivers: perf: arm: implement CPU_PM notifier
Date: Thu, 25 Feb 2016 18:46:32 +0000	[thread overview]
Message-ID: <20160225184632.GB5922@red-moon> (raw)
In-Reply-To: <20160225164354.GE16546@arm.com>

On Thu, Feb 25, 2016 at 04:43:54PM +0000, Will Deacon wrote:
> On Thu, Feb 25, 2016 at 09:32:17AM -0700, Mathieu Poirier wrote:
> > On 25 February 2016 at 02:44, Lorenzo Pieralisi
> > <lorenzo.pieralisi@arm.com> wrote:
> > > On Wed, Feb 24, 2016 at 05:10:20PM -0800, Kevin Hilman wrote:
> > >> Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> writes:
> > >>
> > >> > When a CPU is suspended (either through suspend-to-RAM or CPUidle),
> > >> > its PMU registers content can be lost, which means that counters
> > >> > registers values that were initialized on power down entry have to be
> > >> > reprogrammed on power-up to make sure the counters set-up is preserved
> > >>
> > >> This assumes that the PMUs are always sharing a power rail with a
> > >> specific CPU, not the cluster.  Is that a reliable assumption?
> > >
> > > As reliable as assuming the GIC HW state needs save/restore on idle-state
> > > entry, if we have no power domains information linked to CPU PM
> > > notifiers saving/restoring everything every time an idle state deeper
> > > than wfi is entered is the best we can do.
> > >
> > > As far as this patch is concerned, if the PMU is on a different power
> > > domain than the CPU, I agree that stopping/resetting/restoring the
> > > PMU registers is a waste of cycles (I will test it also by triggering
> > > the notifiers on wfi, to make sure the reset is not disruptive on
> > > a cpu that is not powered off), I see no alternative other than disabling
> > > idle to carry out profiling sanely (or implement CPU PM notifiers with
> > > runtime PM).
> > 
> > I totally understand - I'm faced with the same problem on Coresight.
> > To me the ultimate solution is still to integrate CPU power domain
> > management with runtime PM (like we talked about many times before).
> > I know Lina is working on something that will get us close to that but
> > it's not finished yet.
> 
> I've been waiting for that code for *years* now...
> 
> Ultimately, Juno-r2 loses its PMU state over idle if you run mainline
> on it and, worse still, you end up getting junk numbers back to userspace,
> so it's not even obvious what happened.

I hacked the idle driver to trigger notifiers on wfi (so PMU state is
kept intact on idle exit - to mimic idle states that do not destroy PMU
context) and still get some reasonable perf stats, this needs more testing
but it means that save/restore still work on idle states that do not destroy
PMU context (ok, we add a profile black out period between CPU PM entry and
exit, that can't be avoided).

> I'm not aware of any flags that indicate loss of state.

kvm notifier had to add a check (in hyp_init_cpu_pm_notifier()), since
restoring KVM hooks if the cpu was not reset triggers kernel errors,
point is, we need to have a hook (this is generic arm perf core, it has
to work on all arm pmus) that allows us detecting if a reset happened,
question is if we should bother (and if it is doable), given the test
above, I am happy to implement it.

I definitely agree it is time to fix this, it has been hanging in
the balance for too long.

Lorenzo

  reply	other threads:[~2016-02-25 18:46 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-23 18:22 [PATCH] drivers: perf: arm: implement CPU_PM notifier Lorenzo Pieralisi
2016-02-24 16:20 ` Mathieu Poirier
2016-02-24 17:35   ` Lorenzo Pieralisi
2016-02-24 19:53     ` Ashwin Chaugule
2016-02-24 22:31       ` Lorenzo Pieralisi
2016-02-26  0:22         ` Ashwin Chaugule
2016-02-25 16:37     ` Mathieu Poirier
2016-02-25  1:10 ` Kevin Hilman
2016-02-25  9:44   ` Lorenzo Pieralisi
2016-02-25 16:32     ` Mathieu Poirier
2016-02-25 16:43       ` Will Deacon
2016-02-25 18:46         ` Lorenzo Pieralisi [this message]
2016-02-25 21:55         ` Kevin Hilman

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=20160225184632.GB5922@red-moon \
    --to=lorenzo.pieralisi@arm.com \
    --cc=linux-arm-kernel@lists.infradead.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