All of lore.kernel.org
 help / color / mirror / Atom feed
* [v4.14.y] 3cce50dfec4a arm64: perf: Disable PMU while processing counter overflows
@ 2021-06-16 11:09 Aman Priyadarshi
  2021-06-16 14:55 ` Greg Kroah-Hartman
  0 siblings, 1 reply; 7+ messages in thread
From: Aman Priyadarshi @ 2021-06-16 11:09 UTC (permalink / raw)
  To: Greg Kroah-Hartman, stable; +Cc: Marc Zyngier, Alexander Graf, Ali Saidi

Hi Greg,

Following the conversation with Marc, we discovered that an
important fix for ARM PMU is not backported to 4.14.y tree, it affects
counter value and give out nonsensical result.

Can you please include the following commit?
```
commit 3cce50dfec4a5b0414c974190940f47dd32c6dee
Author: Suzuki K Poulose <suzuki.poulose@arm.com>
Date:   Tue Jul 10 09:58:03 2018 +0100

    arm64: perf: Disable PMU while processing counter overflows
    
    The arm64 PMU updates the event counters and reprograms the
    counters in the overflow IRQ handler without disabling the
    PMU. This could potentially cause skews in for group counters,
    where the overflowed counters may potentially loose some event
    counts, while they are reprogrammed. To prevent this, disable
    the PMU while we process the counter overflows and enable it
    right back when we are done.
    
    This patch also moves the PMU stop/start routines to avoid a
    forward declaration.
    
    Suggested-by: Mark Rutland <mark.rutland@arm.com>
    Cc: Will Deacon <will.deacon@arm.com>
    Acked-by: Mark Rutland <mark.rutland@arm.com>
    Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
    Signed-off-by: Will Deacon <will.deacon@arm.com>
```

For more details:

https://lists.cs.columbia.edu/pipermail/kvmarm/2021-June/047471.html

Regards,
Aman Priyadarshi







Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879



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

* Re: [v4.14.y] 3cce50dfec4a arm64: perf: Disable PMU while processing counter overflows
  2021-06-16 11:09 [v4.14.y] 3cce50dfec4a arm64: perf: Disable PMU while processing counter overflows Aman Priyadarshi
@ 2021-06-16 14:55 ` Greg Kroah-Hartman
  2021-06-16 19:28   ` [PATCH] " Aman Priyadarshi
  0 siblings, 1 reply; 7+ messages in thread
From: Greg Kroah-Hartman @ 2021-06-16 14:55 UTC (permalink / raw)
  To: Aman Priyadarshi; +Cc: stable, Marc Zyngier, Alexander Graf, Ali Saidi

On Wed, Jun 16, 2021 at 01:09:49PM +0200, Aman Priyadarshi wrote:
> Hi Greg,
> 
> Following the conversation with Marc, we discovered that an
> important fix for ARM PMU is not backported to 4.14.y tree, it affects
> counter value and give out nonsensical result.
> 
> Can you please include the following commit?
> ```
> commit 3cce50dfec4a5b0414c974190940f47dd32c6dee
> Author: Suzuki K Poulose <suzuki.poulose@arm.com>
> Date:   Tue Jul 10 09:58:03 2018 +0100
> 
>     arm64: perf: Disable PMU while processing counter overflows
>     
>     The arm64 PMU updates the event counters and reprograms the
>     counters in the overflow IRQ handler without disabling the
>     PMU. This could potentially cause skews in for group counters,
>     where the overflowed counters may potentially loose some event
>     counts, while they are reprogrammed. To prevent this, disable
>     the PMU while we process the counter overflows and enable it
>     right back when we are done.
>     
>     This patch also moves the PMU stop/start routines to avoid a
>     forward declaration.
>     
>     Suggested-by: Mark Rutland <mark.rutland@arm.com>
>     Cc: Will Deacon <will.deacon@arm.com>
>     Acked-by: Mark Rutland <mark.rutland@arm.com>
>     Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>     Signed-off-by: Will Deacon <will.deacon@arm.com>
> ```

It does not apply cleanly so you will have to provide a working backport
for us.

> For more details:
> 
> https://lists.cs.columbia.edu/pipermail/kvmarm/2021-June/047471.html


lore.kernel.org is your friend :)

thanks,

greg k-h

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

* [PATCH] arm64: perf: Disable PMU while processing counter overflows
  2021-06-16 14:55 ` Greg Kroah-Hartman
@ 2021-06-16 19:28   ` Aman Priyadarshi
  2021-06-17  4:51     ` Greg Kroah-Hartman
  0 siblings, 1 reply; 7+ messages in thread
From: Aman Priyadarshi @ 2021-06-16 19:28 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Marc Zyngier, Will Deacon, Alexander Graf, Mark Rutland, stable,
	Ali Saidi

From: Suzuki K Poulose <suzuki.poulose@arm.com>

[ Upstream commit 3cce50dfec4a5b0414c974190940f47dd32c6dee ]

The arm64 PMU updates the event counters and reprograms the
counters in the overflow IRQ handler without disabling the
PMU. This could potentially cause skews in for group counters,
where the overflowed counters may potentially loose some event
counts, while they are reprogrammed. To prevent this, disable
the PMU while we process the counter overflows and enable it
right back when we are done.

This patch also moves the PMU stop/start routines to avoid a
forward declaration.

Suggested-by: Mark Rutland <mark.rutland@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Aman Priyadarshi <apeureka@amazon.de>
Cc: stable@vger.kernel.org
---
 arch/arm64/kernel/perf_event.c | 50 +++++++++++++++++++---------------
 1 file changed, 28 insertions(+), 22 deletions(-)

diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index 53df84b2a07f..4ee1228d29eb 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -670,6 +670,28 @@ static void armv8pmu_disable_event(struct perf_event *event)
 	raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
 }
 
+static void armv8pmu_start(struct arm_pmu *cpu_pmu)
+{
+	unsigned long flags;
+	struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
+
+	raw_spin_lock_irqsave(&events->pmu_lock, flags);
+	/* Enable all counters */
+	armv8pmu_pmcr_write(armv8pmu_pmcr_read() | ARMV8_PMU_PMCR_E);
+	raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
+}
+
+static void armv8pmu_stop(struct arm_pmu *cpu_pmu)
+{
+	unsigned long flags;
+	struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
+
+	raw_spin_lock_irqsave(&events->pmu_lock, flags);
+	/* Disable all counters */
+	armv8pmu_pmcr_write(armv8pmu_pmcr_read() & ~ARMV8_PMU_PMCR_E);
+	raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
+}
+
 static irqreturn_t armv8pmu_handle_irq(int irq_num, void *dev)
 {
 	u32 pmovsr;
@@ -695,6 +717,11 @@ static irqreturn_t armv8pmu_handle_irq(int irq_num, void *dev)
 	 */
 	regs = get_irq_regs();
 
+	/*
+	 * Stop the PMU while processing the counter overflows
+	 * to prevent skews in group events.
+	 */
+	armv8pmu_stop(cpu_pmu);
 	for (idx = 0; idx < cpu_pmu->num_events; ++idx) {
 		struct perf_event *event = cpuc->events[idx];
 		struct hw_perf_event *hwc;
@@ -719,6 +746,7 @@ static irqreturn_t armv8pmu_handle_irq(int irq_num, void *dev)
 		if (perf_event_overflow(event, &data, regs))
 			cpu_pmu->disable(event);
 	}
+	armv8pmu_start(cpu_pmu);
 
 	/*
 	 * Handle the pending perf events.
@@ -732,28 +760,6 @@ static irqreturn_t armv8pmu_handle_irq(int irq_num, void *dev)
 	return IRQ_HANDLED;
 }
 
-static void armv8pmu_start(struct arm_pmu *cpu_pmu)
-{
-	unsigned long flags;
-	struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
-
-	raw_spin_lock_irqsave(&events->pmu_lock, flags);
-	/* Enable all counters */
-	armv8pmu_pmcr_write(armv8pmu_pmcr_read() | ARMV8_PMU_PMCR_E);
-	raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
-}
-
-static void armv8pmu_stop(struct arm_pmu *cpu_pmu)
-{
-	unsigned long flags;
-	struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events);
-
-	raw_spin_lock_irqsave(&events->pmu_lock, flags);
-	/* Disable all counters */
-	armv8pmu_pmcr_write(armv8pmu_pmcr_read() & ~ARMV8_PMU_PMCR_E);
-	raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
-}
-
 static int armv8pmu_get_event_idx(struct pmu_hw_events *cpuc,
 				  struct perf_event *event)
 {
-- 
2.17.1




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879




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

* Re: [PATCH] arm64: perf: Disable PMU while processing counter overflows
  2021-06-16 19:28   ` [PATCH] " Aman Priyadarshi
@ 2021-06-17  4:51     ` Greg Kroah-Hartman
  2021-06-17  7:34       ` Marc Zyngier
  0 siblings, 1 reply; 7+ messages in thread
From: Greg Kroah-Hartman @ 2021-06-17  4:51 UTC (permalink / raw)
  To: Aman Priyadarshi
  Cc: Marc Zyngier, Will Deacon, Alexander Graf, Mark Rutland, stable,
	Ali Saidi

On Wed, Jun 16, 2021 at 09:28:59PM +0200, Aman Priyadarshi wrote:
> From: Suzuki K Poulose <suzuki.poulose@arm.com>
> 
> [ Upstream commit 3cce50dfec4a5b0414c974190940f47dd32c6dee ]
> 
> The arm64 PMU updates the event counters and reprograms the
> counters in the overflow IRQ handler without disabling the
> PMU. This could potentially cause skews in for group counters,
> where the overflowed counters may potentially loose some event
> counts, while they are reprogrammed. To prevent this, disable
> the PMU while we process the counter overflows and enable it
> right back when we are done.
> 
> This patch also moves the PMU stop/start routines to avoid a
> forward declaration.
> 
> Suggested-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Acked-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Aman Priyadarshi <apeureka@amazon.de>
> Cc: stable@vger.kernel.org
> ---
>  arch/arm64/kernel/perf_event.c | 50 +++++++++++++++++++---------------
>  1 file changed, 28 insertions(+), 22 deletions(-)

What stable tree(s) do you want this applied to?

thanks,

greg k-h

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

* Re: [PATCH] arm64: perf: Disable PMU while processing counter overflows
  2021-06-17  4:51     ` Greg Kroah-Hartman
@ 2021-06-17  7:34       ` Marc Zyngier
  2021-06-17  8:57         ` Aman Priyadarshi
  0 siblings, 1 reply; 7+ messages in thread
From: Marc Zyngier @ 2021-06-17  7:34 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Aman Priyadarshi, Will Deacon, Alexander Graf, Mark Rutland,
	stable, Ali Saidi

On Thu, 17 Jun 2021 05:51:03 +0100,
Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> 
> On Wed, Jun 16, 2021 at 09:28:59PM +0200, Aman Priyadarshi wrote:
> > From: Suzuki K Poulose <suzuki.poulose@arm.com>
> > 
> > [ Upstream commit 3cce50dfec4a5b0414c974190940f47dd32c6dee ]
> > 
> > The arm64 PMU updates the event counters and reprograms the
> > counters in the overflow IRQ handler without disabling the
> > PMU. This could potentially cause skews in for group counters,
> > where the overflowed counters may potentially loose some event
> > counts, while they are reprogrammed. To prevent this, disable
> > the PMU while we process the counter overflows and enable it
> > right back when we are done.
> > 
> > This patch also moves the PMU stop/start routines to avoid a
> > forward declaration.
> > 
> > Suggested-by: Mark Rutland <mark.rutland@arm.com>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Acked-by: Mark Rutland <mark.rutland@arm.com>
> > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > Signed-off-by: Aman Priyadarshi <apeureka@amazon.de>
> > Cc: stable@vger.kernel.org
> > ---
> >  arch/arm64/kernel/perf_event.c | 50 +++++++++++++++++++---------------
> >  1 file changed, 28 insertions(+), 22 deletions(-)
> 
> What stable tree(s) do you want this applied to?

I guess that'd be 4.14 and previous stables if the patch actually
applies.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

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

* Re: [PATCH] arm64: perf: Disable PMU while processing counter overflows
  2021-06-17  7:34       ` Marc Zyngier
@ 2021-06-17  8:57         ` Aman Priyadarshi
  2021-06-25 10:22           ` Greg Kroah-Hartman
  0 siblings, 1 reply; 7+ messages in thread
From: Aman Priyadarshi @ 2021-06-17  8:57 UTC (permalink / raw)
  To: Marc Zyngier, Greg Kroah-Hartman
  Cc: Will Deacon, Alexander Graf, Mark Rutland, stable, Ali Saidi

On Thu, 2021-06-17 at 08:34 +0100, Marc Zyngier wrote:
> CAUTION: This email originated from outside of the organization. Do not
> click links or open attachments unless you can confirm the sender and
> know the content is safe.
> 
> 
> 
> On Thu, 17 Jun 2021 05:51:03 +0100,
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> > 
> > On Wed, Jun 16, 2021 at 09:28:59PM +0200, Aman Priyadarshi wrote:
> > > From: Suzuki K Poulose <suzuki.poulose@arm.com>
> > > 
> > > [ Upstream commit 3cce50dfec4a5b0414c974190940f47dd32c6dee ]
> > > 
> > > The arm64 PMU updates the event counters and reprograms the
> > > counters in the overflow IRQ handler without disabling the
> > > PMU. This could potentially cause skews in for group counters,
> > > where the overflowed counters may potentially loose some event
> > > counts, while they are reprogrammed. To prevent this, disable
> > > the PMU while we process the counter overflows and enable it
> > > right back when we are done.
> > > 
> > > This patch also moves the PMU stop/start routines to avoid a
> > > forward declaration.
> > > 
> > > Suggested-by: Mark Rutland <mark.rutland@arm.com>
> > > Cc: Will Deacon <will.deacon@arm.com>
> > > Acked-by: Mark Rutland <mark.rutland@arm.com>
> > > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> > > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > > Signed-off-by: Aman Priyadarshi <apeureka@amazon.de>
> > > Cc: stable@vger.kernel.org
> > > ---
> > >  arch/arm64/kernel/perf_event.c | 50 +++++++++++++++++++-------------
> > > --
> > >  1 file changed, 28 insertions(+), 22 deletions(-)
> > 
> > What stable tree(s) do you want this applied to?
> 
> I guess that'd be 4.14 and previous stables if the patch actually
> applies.
> 

Correct. I have tested the patch on 4.14.y, can confirm that it applies
cleanly on 4.9.y as well.

Thanks,
Aman Priyadarshi




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879



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

* Re: [PATCH] arm64: perf: Disable PMU while processing counter overflows
  2021-06-17  8:57         ` Aman Priyadarshi
@ 2021-06-25 10:22           ` Greg Kroah-Hartman
  0 siblings, 0 replies; 7+ messages in thread
From: Greg Kroah-Hartman @ 2021-06-25 10:22 UTC (permalink / raw)
  To: Aman Priyadarshi
  Cc: Marc Zyngier, Will Deacon, Alexander Graf, Mark Rutland, stable,
	Ali Saidi

On Thu, Jun 17, 2021 at 10:57:00AM +0200, Aman Priyadarshi wrote:
> On Thu, 2021-06-17 at 08:34 +0100, Marc Zyngier wrote:
> > CAUTION: This email originated from outside of the organization. Do not
> > click links or open attachments unless you can confirm the sender and
> > know the content is safe.
> > 
> > 
> > 
> > On Thu, 17 Jun 2021 05:51:03 +0100,
> > Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> > > 
> > > On Wed, Jun 16, 2021 at 09:28:59PM +0200, Aman Priyadarshi wrote:
> > > > From: Suzuki K Poulose <suzuki.poulose@arm.com>
> > > > 
> > > > [ Upstream commit 3cce50dfec4a5b0414c974190940f47dd32c6dee ]
> > > > 
> > > > The arm64 PMU updates the event counters and reprograms the
> > > > counters in the overflow IRQ handler without disabling the
> > > > PMU. This could potentially cause skews in for group counters,
> > > > where the overflowed counters may potentially loose some event
> > > > counts, while they are reprogrammed. To prevent this, disable
> > > > the PMU while we process the counter overflows and enable it
> > > > right back when we are done.
> > > > 
> > > > This patch also moves the PMU stop/start routines to avoid a
> > > > forward declaration.
> > > > 
> > > > Suggested-by: Mark Rutland <mark.rutland@arm.com>
> > > > Cc: Will Deacon <will.deacon@arm.com>
> > > > Acked-by: Mark Rutland <mark.rutland@arm.com>
> > > > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> > > > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > > > Signed-off-by: Aman Priyadarshi <apeureka@amazon.de>
> > > > Cc: stable@vger.kernel.org
> > > > ---
> > > >  arch/arm64/kernel/perf_event.c | 50 +++++++++++++++++++-------------
> > > > --
> > > >  1 file changed, 28 insertions(+), 22 deletions(-)
> > > 
> > > What stable tree(s) do you want this applied to?
> > 
> > I guess that'd be 4.14 and previous stables if the patch actually
> > applies.
> > 
> 
> Correct. I have tested the patch on 4.14.y, can confirm that it applies
> cleanly on 4.9.y as well.

Now queued up, thanks.

greg k-h

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

end of thread, other threads:[~2021-06-25 10:22 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-16 11:09 [v4.14.y] 3cce50dfec4a arm64: perf: Disable PMU while processing counter overflows Aman Priyadarshi
2021-06-16 14:55 ` Greg Kroah-Hartman
2021-06-16 19:28   ` [PATCH] " Aman Priyadarshi
2021-06-17  4:51     ` Greg Kroah-Hartman
2021-06-17  7:34       ` Marc Zyngier
2021-06-17  8:57         ` Aman Priyadarshi
2021-06-25 10:22           ` Greg Kroah-Hartman

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.