On Tue, Apr 01, 2025 at 03:35:44PM +0100, Mike Leach wrote:
[...]
> > > -static void etm_event_pause(struct coresight_device *csdev,
> > > +static void etm_event_pause(struct perf_event *event,
> > > + struct coresight_device *csdev,
> > > struct etm_ctxt *ctxt)
> > > {
> > > + int cpu = smp_processor_id();
> > > + struct coresight_device *sink;
> > > + struct perf_output_handle *handle = &ctxt->handle;
> > > + struct coresight_path *path;
> > > + unsigned long size;
> > > +
> > > if (!ctxt->event_data)
> > > return;
> > >
> > > /* Stop tracer */
> > > coresight_pause_source(csdev);
> > > +
> > > + path = etm_event_cpu_path(ctxt->event_data, cpu);
> > > + sink = coresight_get_sink(path);
> > > + if (WARN_ON_ONCE(!sink))
> > > + return;
> > > +
> > > + /*
> > > + * The per CPU sink has own interrupt handling, it might have
> > > + * race condition with updating buffer on AUX trace pause if
> > > + * it is invoked from NMI. To avoid the race condition,
> > > + * disallows updating buffer for the per CPU sink case.
> > > + */
> > > + if (coresight_is_percpu_sink(sink))
> > > + return;
> > > +
> > > + if (WARN_ON_ONCE(handle->event != event))
> > > + return;
> > > +
> > > + if (!sink_ops(sink)->update_buffer)
> > > + return;
> > > +
> > > + size = sink_ops(sink)->update_buffer(sink, handle,
> > > + ctxt->event_data->snk_config);
> >
> > I believe we keep the sink disabled/stopped in update_buffer. We need to
> > turn it back ON after the "buffer update". May be we could use a flag
> > to update_buffer() to indicate this is "pause" triggered update.
Thanks for pointing out, Suzuki.
I will fix it in next spin.
> The sink is stopped to read data, but also important is the
> enable/disable refcount. The only time that "update_buffer" will read
> is if the sink has a refcount == 1. These are ultimately controlled by
> enable/disable path - which will not occur during pause/resume
> operations.
Hi Mike,
My understanding is: if a sink is shared by multiple CPUs, e.g. a Perf
session with CPU mode or system wide mode, then we cannot arbitrarily
disable sink and update buffer, otherwise, it might cause hardware
lockup issue.
For per-thread mode, the refcount == 1, we can take chance to update
buffer, right?
Thanks,
Leo
