[PATCH] ARM: perf: add PMU hotplug notifier

[PATCH] ARM: perf: add PMU hotplug notifier

[Ashwin Chaugule]

Hi Will,

On Thu, Feb 23, 2012 at 10:41 AM, Will Deacon <will.deacon@arm.com> wrote:
> From: Lorenzo Pieralisi <Lorenzo.Pieralisi@arm.com>
>
> When a CPU is taken out of reset, either cold booted or hotplugged in,
> some of its PMU registers can contain UNKNOWN values.
>
> This patch adds a hotplug notifier to ARM core perf code so that upon
> CPU restart the PMU unit is reset and becomes ready to use again.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>

I was playing around with this stuff recently, but used the CPU PM code from

http://lists.infradead.org/pipermail/linux-arm-kernel/2011-June/052829.html

> ---
>  arch/arm/kernel/perf_event.c |   23 +++++++++++++++++++++++
>  1 files changed, 23 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/kernel/perf_event.c b/arch/arm/kernel/perf_event.c
> index ab59c3b..fa30ff9 100644
> --- a/arch/arm/kernel/perf_event.c
> +++ b/arch/arm/kernel/perf_event.c
> @@ -680,6 +680,28 @@ static void __init cpu_pmu_init(struct arm_pmu *armpmu)
>  }
>
>  /*
> + * PMU hardware loses all context when a CPU goes offline.
> + * When a CPU is hotplugged back in, since some hardware registers are
> + * UNKNOWN at reset, the PMU must be explicitly reset to avoid reading
> + * junk values out of them.
> + */
> +static int __cpuinit pmu_cpu_notify(struct notifier_block *b,
> +                                       unsigned long action, void *hcpu)
> +{
> +       if (action != CPU_UP_PREPARE)
> +               return NOTIFY_DONE;
> +
> +       if (cpu_pmu && cpu_pmu->reset)
> +               cpu_pmu->reset(NULL);
> +
> +       return NOTIFY_OK;
> +}
> +

I realized I needed to save the counters before going to sleep. In my
case, the notifiers were being called from the idle thread, so I
didn't know which "sleep state" I was going to go into. In the deepest
idle state, the counters are completely reset. When we come out of
sleep, unless there had been an explicit "read" earlier, we will have
lost the cached value(event->count) of the counters. This manifests as
counts reduced by an order of magnitude.

I'll send a link to my code once the git server is up. It's a bit flaky today.


Cheers,
Ashwin

[PATCH] ARM: perf: add PMU hotplug notifier

[Ashwin Chaugule]

Gah. Messed up email addr.

On 2/23/2012 10:57 AM, Ashwin Chaugule wrote:
> Hi Will,
> 
> On Thu, Feb 23, 2012 at 10:41 AM, Will Deacon <will.deacon@arm.com> wrote:
>> From: Lorenzo Pieralisi <Lorenzo.Pieralisi@arm.com>
>>
>> When a CPU is taken out of reset, either cold booted or hotplugged in,
>> some of its PMU registers can contain UNKNOWN values.
>>
>> This patch adds a hotplug notifier to ARM core perf code so that upon
>> CPU restart the PMU unit is reset and becomes ready to use again.
>>
>> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> Signed-off-by: Will Deacon <will.deacon@arm.com>
> 
> I was playing around with this stuff recently, but used the CPU PM code from
> 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2011-June/052829.html
> 
>> ---
>>  arch/arm/kernel/perf_event.c |   23 +++++++++++++++++++++++
>>  1 files changed, 23 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/kernel/perf_event.c b/arch/arm/kernel/perf_event.c
>> index ab59c3b..fa30ff9 100644
>> --- a/arch/arm/kernel/perf_event.c
>> +++ b/arch/arm/kernel/perf_event.c
>> @@ -680,6 +680,28 @@ static void __init cpu_pmu_init(struct arm_pmu *armpmu)
>>  }
>>
>>  /*
>> + * PMU hardware loses all context when a CPU goes offline.
>> + * When a CPU is hotplugged back in, since some hardware registers are
>> + * UNKNOWN at reset, the PMU must be explicitly reset to avoid reading
>> + * junk values out of them.
>> + */
>> +static int __cpuinit pmu_cpu_notify(struct notifier_block *b,
>> +                                       unsigned long action, void *hcpu)
>> +{
>> +       if (action != CPU_UP_PREPARE)
>> +               return NOTIFY_DONE;
>> +
>> +       if (cpu_pmu && cpu_pmu->reset)
>> +               cpu_pmu->reset(NULL);
>> +
>> +       return NOTIFY_OK;
>> +}
>> +
> 
> I realized I needed to save the counters before going to sleep. In my
> case, the notifiers were being called from the idle thread, so I
> didn't know which "sleep state" I was going to go into. In the deepest
> idle state, the counters are completely reset. When we come out of
> sleep, unless there had been an explicit "read" earlier, we will have
> lost the cached value(event->count) of the counters. This manifests as
> counts reduced by an order of magnitude.
> 
> I'll send a link to my code once the git server is up. It's a bit flaky today.
> 
> 
> Cheers,
> Ashwin
> 


-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH] ARM: perf: add PMU hotplug notifier

[Will Deacon]

Hi Ashwin,

On Thu, Feb 23, 2012 at 04:00:45PM +0000, Ashwin Chaugule wrote:
> >> diff --git a/arch/arm/kernel/perf_event.c b/arch/arm/kernel/perf_event.c
> >> index ab59c3b..fa30ff9 100644
> >> --- a/arch/arm/kernel/perf_event.c
> >> +++ b/arch/arm/kernel/perf_event.c
> >> @@ -680,6 +680,28 @@ static void __init cpu_pmu_init(struct arm_pmu *armpmu)
> >>  }
> >>
> >>  /*
> >> + * PMU hardware loses all context when a CPU goes offline.
> >> + * When a CPU is hotplugged back in, since some hardware registers are
> >> + * UNKNOWN at reset, the PMU must be explicitly reset to avoid reading
> >> + * junk values out of them.
> >> + */
> >> +static int __cpuinit pmu_cpu_notify(struct notifier_block *b,
> >> +                                       unsigned long action, void *hcpu)
> >> +{
> >> +       if (action != CPU_UP_PREPARE)
> >> +               return NOTIFY_DONE;
> >> +
> >> +       if (cpu_pmu && cpu_pmu->reset)
> >> +               cpu_pmu->reset(NULL);
> >> +
> >> +       return NOTIFY_OK;
> >> +}
> >> +
> > 
> > I realized I needed to save the counters before going to sleep. In my
> > case, the notifiers were being called from the idle thread, so I
> > didn't know which "sleep state" I was going to go into. In the deepest
> > idle state, the counters are completely reset. When we come out of
> > sleep, unless there had been an explicit "read" earlier, we will have
> > lost the cached value(event->count) of the counters. This manifests as
> > counts reduced by an order of magnitude.

Ok. Note that the perf core will actually destroy perf events on a hotplug
down notification, so profiling runs will be dead on that core even if it
comes back up.

This patch just ensures that we have a sane state in the control registers
given that their state is UNKNOWN out of reset. Adding full hotplug support
to perf (that is, supporting hotplug events during a run and representing
that in a meaningful way) is a larger issue.

> > I'll send a link to my code once the git server is up. It's a bit flaky today.

Ok, cool. Are you happy with me to proceed with the current code for the
time being?

Will

[PATCH] ARM: perf: add PMU hotplug notifier

[Ashwin Chaugule]

On 2/23/2012 11:04 AM, Will Deacon wrote:
> 
> Ok. Note that the perf core will actually destroy perf events on a hotplug
> down notification, so profiling runs will be dead on that core even if it
> comes back up.
> 
> This patch just ensures that we have a sane state in the control registers
> given that their state is UNKNOWN out of reset. Adding full hotplug support
> to perf (that is, supporting hotplug events during a run and representing
> that in a meaningful way) is a larger issue.
> 
>>> I'll send a link to my code once the git server is up. It's a bit flaky today.
> 
> Ok, cool. Are you happy with me to proceed with the current code for the
> time being?
> 

Sure. We'll need to be able to distinguish between a hotplug and a CPU
power-collapse / deep sleep / what-have-you state in this code at some
point since they all have a different effect on the PMU regs.

Cheers,
Ashwin

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH] ARM: perf: add PMU hotplug notifiery

[Lorenzo Pieralisi]

On Thu, Feb 23, 2012 at 04:12:59PM +0000, Ashwin Chaugule wrote:
> On 2/23/2012 11:04 AM, Will Deacon wrote:
> > 
> > Ok. Note that the perf core will actually destroy perf events on a hotplug
> > down notification, so profiling runs will be dead on that core even if it
> > comes back up.
> > 
> > This patch just ensures that we have a sane state in the control registers
> > given that their state is UNKNOWN out of reset. Adding full hotplug support
> > to perf (that is, supporting hotplug events during a run and representing
> > that in a meaningful way) is a larger issue.
> > 
> >>> I'll send a link to my code once the git server is up. It's a bit flaky today.
> > 
> > Ok, cool. Are you happy with me to proceed with the current code for the
> > time being?
> > 
> 
> Sure. We'll need to be able to distinguish between a hotplug and a CPU
> power-collapse / deep sleep / what-have-you state in this code at some
> point since they all have a different effect on the PMU regs.

I have a patch to save/restore PMU regs from idle, I will send it to
Will for review soon so that he can merge the bits that you both deem
worthwhile.

save/restore will be called from CPU PM notifiers on deep-sleep
entry/exit, it is up to the idle state enter function to trigger them when PMU
state is lost.

Thanks,
Lorenzo