Re: [PATCH 2/2] perf/imx_ddr: Add stop counter support for i.MX8MP

[Will Deacon <will@kernel.org>]

On Tue, Feb 25, 2020 at 08:56:44PM +0800, Joakim Zhang wrote:
> DDR perf driver now only supports free-running event counters
> (counter1/2/3), which means that event counters will continue counting
> even they are overflow.
> 
> However, the situation is changed on i.MX8MP, event counters are not
> free-running any more. Event counters would stop counting if they are
> overflow. So we need clear event counters when cycle counter overflow.
> 
> The patch adds stop counter support which would be compatible to
> free-running counter.

Hmm, are you saying that the hardware behaviour has changed here, so that
newer SoCs force the behaviour where all the counters stop when one
overflows? Is there any way to control that behaviour?

> @@ -566,6 +571,25 @@ static irqreturn_t ddr_perf_irq_handler(int irq, void *p)
>  			cycle_event = event;
>  	}
>  
> +	spin_lock(&pmu->lock);
> +
> +	for (i = 0; i < NUM_COUNTERS; i++) {
> +		if (!pmu->events[i])
> +			continue;
> +
> +		event = pmu->events[i];
> +
> +		if (event->hw.idx == EVENT_CYCLES_COUNTER)
> +			continue;
> +
> +		/* clear non-cycle counters */
> +		ddr_perf_counter_enable(pmu, event->attr.config, event->hw.idx, true);
> +
> +		local64_set(&event->hw.prev_count, 0);
> +	}
> +
> +	spin_unlock(&pmu->lock);

I'm suspicious of this locking, as it's /very/ fine-grained. What prevents
racing with a concurrent ddr_perf_counter_enable() call? Also, doesn't perf
core need to be aware that you're stopping counters here?

Finally, this looks like it's an unconditional change in user-visible
behaviour. Why doesn't it break existing usage of the perf tool?

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel