[PATCH] perf/powerpc: reset event hw state when adding it to the PMU

[PATCH] perf/powerpc: reset event hw state when adding it to the PMU

[Alexandru-Cezar Sardan]

When adding an event to the PMU with PERF_EF_START the STOPPED and UPTODATE
flags need to be cleared in the hw.event status variable because they are
preventing the update of the event count on overflow interrupt.

Signed-off-by: Alexandru-Cezar Sardan <alexandru.sardan@freescale.com>
---
 arch/powerpc/perf/core-fsl-emb.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/perf/core-fsl-emb.c b/arch/powerpc/perf/core-fsl-emb.c
index d35ae52..ef2ce48 100644
--- a/arch/powerpc/perf/core-fsl-emb.c
+++ b/arch/powerpc/perf/core-fsl-emb.c
@@ -330,9 +330,11 @@ static int fsl_emb_pmu_add(struct perf_event *event, int flags)
 	}
 	local64_set(&event->hw.prev_count, val);
 
-	if (!(flags & PERF_EF_START)) {
+	if (unlikely(!(flags & PERF_EF_START))) {
 		event->hw.state = PERF_HES_STOPPED | PERF_HES_UPTODATE;
 		val = 0;
+	} else {
+		event->hw.state &= ~(PERF_HES_STOPPED | PERF_HES_UPTODATE);
 	}
 
 	write_pmc(i, val);
-- 
1.7.9.5

Re: perf/powerpc: reset event hw state when adding it to the PMU

[Scott Wood]

On Thu, Jun 26, 2014 at 11:58:58AM +0300, Alexandru-Cezar Sardan wrote:
> When adding an event to the PMU with PERF_EF_START the STOPPED and UPTODATE
> flags need to be cleared in the hw.event status variable because they are
> preventing the update of the event count on overflow interrupt.
> 
> Signed-off-by: Alexandru-Cezar Sardan <alexandru.sardan@freescale.com>
> ---
>  arch/powerpc/perf/core-fsl-emb.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Sorry for the delay -- it wasn't CCed to me and the subject line didn't
indicate that it was fsl related.

> diff --git a/arch/powerpc/perf/core-fsl-emb.c b/arch/powerpc/perf/core-fsl-emb.c
> index d35ae52..ef2ce48 100644
> --- a/arch/powerpc/perf/core-fsl-emb.c
> +++ b/arch/powerpc/perf/core-fsl-emb.c
> @@ -330,9 +330,11 @@ static int fsl_emb_pmu_add(struct perf_event *event, int flags)
>  	}
>  	local64_set(&event->hw.prev_count, val);
>  
> -	if (!(flags & PERF_EF_START)) {
> +	if (unlikely(!(flags & PERF_EF_START))) {
>  		event->hw.state = PERF_HES_STOPPED | PERF_HES_UPTODATE;
>  		val = 0;
> +	} else {
> +		event->hw.state &= ~(PERF_HES_STOPPED | PERF_HES_UPTODATE);
>  	}

Why unlikely()?  None of the other perf drivers have that there.

Commit f53d168c does something similar for book3s.  It sets hw.state to
zero instead of clearing the flags.  Any reason why core-fsl-emb should
be different?

-Scott