On Tue, Mar 17, 2015 at 05:24:18PM +0000, Will Deacon wrote:

On Fri, Mar 13, 2015 at 11:31:37PM +0000, Kevin Hilman wrote:

quoted

Lorenzo Pieralisi [off-list ref] writes:

quoted

On Thu, Mar 12, 2015 at 01:18:54PM +0000, Ashwin Chaugule wrote:

quoted

On 11 March 2015 at 12:02, Kevin Hilman [off-list ref] wrote:

quoted

Lorenzo Pieralisi [off-list ref] writes:

quoted

When a CPU is being profiled through PMU events and it enters suspend
or idle states, the PMU registers content can be lost, which means that
counters that were relied upon on power down entry are reset on power
up to values that are incosistent with the profile session.

This patch adds a CPU PM notifier to arm64 perf code, that detects
on entry if events are being monitored, and if so, it returns
failure to the CPU PM notification chain, causing the suspend
thread or the idle thread to abort power down, therefore preventing
registers content loss.

By triggering CPU PM notification failure this patch prevents
suspending a system if the suspend thread is being profiled and
it also prevents entering idle deep states on cores that have profile
events in use, somehow limiting power management capabilities when
there are active perf sessions.

I guess that's one choice.  Couldn't you also stop the PMU and
save/restore it's context in the notifiers? so that you wouldn't affect
PM capabilities?

That would imply that you lose the ability to profile after a certain
point in suspend/idle, but maybe that's a better trade off than having
profiling disable certain PM features?

I had something like that a few years ago on the Kraits and Scorpions [1].

That's another option, but the point is understanding how we want to
tackle the issue, by preventing power down or by restoring the
PMU registers.

Personally, I think the save/restore approach is preferred.  IMO, it's
more intuitive from the perspective of a user who doesn't understand all
the mechanics and also actually allows you to profile most of the
low-power paths and still actually hit the low power states.

I agree that save/restore is the nicest thing to do, but until we have a
generic description of the power-domains in device-tree I really don't want
PMU-specific hacks in the arch code to deal with this (since many platforms
do not lose PMU state over suspend).

What Lorenzo is proposing is a stop-gap to prevent perf silently losing its
state on platforms where the register contents is lost.

Yes, that's exactly the goal. I am fine either way, at the moment we are
not in a position to define whether a core loses the PMU context, that's
certain.

I am not sure to understand why people want the core to really go
to shutdown and save/restore perf context if perf is on (eg profiling a
cpu for cache stats, clearly powering down a core biases the results
and might even obfuscate them).

It is not an easy call. If we go for this patch approach I am happy to
rebase and retest the series again, for power domains I fear we have to
wait given the sheer amount of changes in ARM CPUidle world at the moment,
too much stuff going on there.

Lorenzo