linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ARM: perf: add PMU hotplug notifier
@ 2012-02-23 15:41 Will Deacon
  0 siblings, 0 replies; 6+ messages in thread
From: Will Deacon @ 2012-02-23 15:41 UTC (permalink / raw)
  To: linux-arm-kernel

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>
---
 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;
+}
+
+static struct notifier_block __cpuinitdata pmu_cpu_notifier = {
+	.notifier_call = pmu_cpu_notify,
+};
+
+/*
  * CPU PMU identification and registration.
  */
 static int __init
@@ -733,6 +755,7 @@ init_hw_perf_events(void)
 		pr_info("enabled with %s PMU driver, %d counters available\n",
 			cpu_pmu->name, cpu_pmu->num_events);
 		cpu_pmu_init(cpu_pmu);
+		register_cpu_notifier(&pmu_cpu_notifier);
 		armpmu_register(cpu_pmu, "cpu", PERF_TYPE_RAW);
 	} else {
 		pr_info("no hardware support available\n");
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] ARM: perf: add PMU hotplug notifier
       [not found] <CAKHPGBaY1OFiVubic+f1vxxzNeKjmzibxraBg8eNxZnJbhYi+A@mail.gmail.com>
@ 2012-02-23 15:57 ` Ashwin Chaugule
  2012-02-23 16:00   ` Ashwin Chaugule
  0 siblings, 1 reply; 6+ messages in thread
From: Ashwin Chaugule @ 2012-02-23 15:57 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] ARM: perf: add PMU hotplug notifier
  2012-02-23 15:57 ` [PATCH] ARM: perf: add PMU hotplug notifier Ashwin Chaugule
@ 2012-02-23 16:00   ` Ashwin Chaugule
  2012-02-23 16:04     ` Will Deacon
  0 siblings, 1 reply; 6+ messages in thread
From: Ashwin Chaugule @ 2012-02-23 16:00 UTC (permalink / raw)
  To: linux-arm-kernel

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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] ARM: perf: add PMU hotplug notifier
  2012-02-23 16:00   ` Ashwin Chaugule
@ 2012-02-23 16:04     ` Will Deacon
  2012-02-23 16:12       ` Ashwin Chaugule
  0 siblings, 1 reply; 6+ messages in thread
From: Will Deacon @ 2012-02-23 16:04 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] ARM: perf: add PMU hotplug notifier
  2012-02-23 16:04     ` Will Deacon
@ 2012-02-23 16:12       ` Ashwin Chaugule
  2012-02-23 16:31         ` [PATCH] ARM: perf: add PMU hotplug notifiery Lorenzo Pieralisi
  0 siblings, 1 reply; 6+ messages in thread
From: Ashwin Chaugule @ 2012-02-23 16:12 UTC (permalink / raw)
  To: linux-arm-kernel

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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] ARM: perf: add PMU hotplug notifiery
  2012-02-23 16:12       ` Ashwin Chaugule
@ 2012-02-23 16:31         ` Lorenzo Pieralisi
  0 siblings, 0 replies; 6+ messages in thread
From: Lorenzo Pieralisi @ 2012-02-23 16:31 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-02-23 16:31 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <CAKHPGBaY1OFiVubic+f1vxxzNeKjmzibxraBg8eNxZnJbhYi+A@mail.gmail.com>
2012-02-23 15:57 ` [PATCH] ARM: perf: add PMU hotplug notifier Ashwin Chaugule
2012-02-23 16:00   ` Ashwin Chaugule
2012-02-23 16:04     ` Will Deacon
2012-02-23 16:12       ` Ashwin Chaugule
2012-02-23 16:31         ` [PATCH] ARM: perf: add PMU hotplug notifiery Lorenzo Pieralisi
2012-02-23 15:41 [PATCH] ARM: perf: add PMU hotplug notifier Will Deacon

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).