Re: [PATCHv2 2/4] coresight: tmc-etr: Track perf handler.

Mathieu Poirier
2021-08-25T19:12:00Z

On Wed, Aug 25, 2021 at 11:02:13AM -0600, Mathieu Poirier wrote:
> Hi Daniel,
>
> On Tue, Jul 13, 2021 at 02:15:30PM +0200, Daniel Kiss wrote:
> > Keep track of the perf handler that is registred by the first tracer.
> > This will be used by the update call from polling.
> >
> > Signed-off-by: Daniel Kiss <daniel.kiss@arm.com>
> > Signed-off-by: Branislav Rankov <Branislav.Rankov@arm.com>
> > ---
> > drivers/hwtracing/coresight/coresight-tmc-etr.c | 6 ++++--
> > drivers/hwtracing/coresight/coresight-tmc.h | 2 ++
> > 2 files changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> > index 589bb2d56e802..55c9b5fd9f832 100644
> > --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> > +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> > @@ -1503,8 +1503,8 @@ tmc_update_etr_buffer(struct coresight_device *csdev,
> >
> > spin_lock_irqsave(&drvdata->spinlock, flags);
> >
> > - /* Don't do anything if another tracer is using this sink */
> > - if (atomic_read(csdev->refcnt) != 1) {
> > + /* Serve only the tracer with the leading perf handler */
> > + if (drvdata->perf_handle != handle) {
>
> In CPU wide trace scenarios the first CPU to enable a sink is not
> guaranteed to be the same as the last CPU to use it. As far as I understand the
> above assumes the first and last CPUs to use a sink are the same.

I want to take a little more time to ponder about the rest of this patchset.
More comments to come tomorrow.

Thanks,
Mathieu

>
> > spin_unlock_irqrestore(&drvdata->spinlock, flags);
> > goto out;
> > }
> > @@ -1619,6 +1619,7 @@ static int tmc_enable_etr_sink_perf(struct coresight_device *csdev, void *data)
> > drvdata->pid = pid;
> > drvdata->mode = CS_MODE_PERF;
> > drvdata->perf_buf = etr_perf->etr_buf;
> > + drvdata->perf_handle = handle;
> > atomic_inc(csdev->refcnt);
> > }
> >
> > @@ -1666,6 +1667,7 @@ static int tmc_disable_etr_sink(struct coresight_device *csdev)
> > drvdata->mode = CS_MODE_DISABLED;
> > /* Reset perf specific data */
> > drvdata->perf_buf = NULL;
> > + drvdata->perf_handle = NULL;
> >
> > spin_unlock_irqrestore(&drvdata->spinlock, flags);
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
> > index b91ec7dde7bc9..81583ffb973dc 100644
> > --- a/drivers/hwtracing/coresight/coresight-tmc.h
> > +++ b/drivers/hwtracing/coresight/coresight-tmc.h
> > @@ -184,6 +184,7 @@ struct etr_buf {
> > * @idr_mutex: Access serialisation for idr.
> > * @sysfs_buf: SYSFS buffer for ETR.
> > * @perf_buf: PERF buffer for ETR.
> > + * @perf_handle: PERF handle for ETR.
> > */
> > struct tmc_drvdata {
> > void __iomem *base;
> > @@ -207,6 +208,7 @@ struct tmc_drvdata {
> > struct mutex idr_mutex;
> > struct etr_buf *sysfs_buf;
> > struct etr_buf *perf_buf;
> > + struct perf_output_handle *perf_handle;
> > };
> >
> > struct etr_buf_operations {
> > --
> > 2.25.1
> >