[PATCH V2] perf/imx_ddr: Add stop event counters support for i.MX8MP
Will Deacon
will at kernel.org
Mon Sep 21 16:56:42 EDT 2020
On Mon, Sep 21, 2020 at 07:36:02PM +0100, Will Deacon wrote:
> On Tue, Sep 08, 2020 at 06:47:34PM +0800, Joakim Zhang wrote:
> > +static void ddr_perf_event_update(struct perf_event *event)
> > +{
> > + struct ddr_pmu *pmu = to_ddr_pmu(event->pmu);
> > + struct hw_perf_event *hwc = &event->hw;
> > + u64 delta, prev_raw_count, new_raw_count;
> > + int counter = hwc->idx;
> > + int ret;
> > +
> > + if (counter == EVENT_CYCLES_COUNTER) {
> > + do {
> > + prev_raw_count = local64_read(&hwc->prev_count);
> > + new_raw_count = ddr_perf_read_counter(pmu, counter);
> > + } while (local64_cmpxchg(&hwc->prev_count, prev_raw_count,
> > + new_raw_count) != prev_raw_count);
> > +
> > + delta = (new_raw_count - prev_raw_count) & 0xFFFFFFFF;
> > +
> > + local64_add(delta, &event->count);
>
> Why do we treat the cycle counter so differently here?
Ah, please can you remind me: does the cycle counter stop on overflow, or
does it continue to count?
> > + } else {
> > + /*
> > + * For legacy SoCs: event counters continue counting when overflow,
> > + * no need to clear the counter.
> > + * For new SoCs: event counters stop counting when overflow, need
> > + * clear counter to let it count again.
> > + */
> > + ret = ddr_perf_counter_overflow(pmu, counter);
> > + if (ret)
> > + dev_warn(pmu->dev, "Event Counter%d overflow happened, data incorrect!!\n", counter);
>
> I don't understand this message: if the data is incorrect, why do we need
> to handle overflow at all/, rather than putting the event into an error
> state?
Also, now I remember how this all works (we use the cycle counter to stop
overflow of the event counter), then warning here is ok, but we should do
something like:
dev_warn_ratelimited(pmu->dev, "events lost due to counter overflow (config 0x%llx)\n", event->attr.config);
instead.
Will
More information about the linux-arm-kernel
mailing list