Messages in this thread | | | From | Mathieu Poirier <> | Date | Thu, 15 Aug 2019 11:37:46 -0600 | Subject | Re: [PATCH v2] coresight: tmc-etr: Fix perf_data check. |
| |
On Mon, 12 Aug 2019 at 13:03, Yabin Cui <yabinc@google.com> wrote: > > When tracing etm data of multiple threads on multiple cpus through > perf interface, each cpu has a unique etr_perf_buffer while sharing > the same etr device. There is no guarantee that the last cpu starts > etm tracing also stops last. This makes perf_data check fail. > > Fix it by checking etr_buf instead of etr_perf_buffer. > > Fixes: 3147da92a8a8 ("coresight: tmc-etr: Allocate and free ETR memory buffers for CPU-wide scenarios") > Signed-off-by: Yabin Cui <yabinc@google.com> > --- > > v1 -> v2: rename perf_data to perf_buf. Add fixes tag. > > --- > drivers/hwtracing/coresight/coresight-tmc-etr.c | 6 +++--- > drivers/hwtracing/coresight/coresight-tmc.h | 6 +++--- > 2 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c > index 17006705287a..90d1548ad268 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c > +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c > @@ -1484,7 +1484,7 @@ tmc_update_etr_buffer(struct coresight_device *csdev, > goto out; > } > > - if (WARN_ON(drvdata->perf_data != etr_perf)) { > + if (WARN_ON(drvdata->perf_buf != etr_buf)) { > lost = true; > spin_unlock_irqrestore(&drvdata->spinlock, flags); > goto out; > @@ -1497,7 +1497,7 @@ tmc_update_etr_buffer(struct coresight_device *csdev, > > CS_LOCK(drvdata->base); > /* Reset perf specific data */ > - drvdata->perf_data = NULL; > + drvdata->perf_buf = NULL; > spin_unlock_irqrestore(&drvdata->spinlock, flags); > > size = etr_buf->len; > @@ -1556,7 +1556,7 @@ static int tmc_enable_etr_sink_perf(struct coresight_device *csdev, void *data) > } > > etr_perf->head = PERF_IDX2OFF(handle->head, etr_perf); > - drvdata->perf_data = etr_perf; > + drvdata->perf_buf = etr_perf->etr_buf;
Ok for the fix. Looking a things again I don't see a need to do the assignment for each event - this needs to be done only when the device is assocated with a monitored process. Please move it here [1].
Thanks, Mathieu
[1]. https://elixir.bootlin.com/linux/v5.3-rc4/source/drivers/hwtracing/coresight/coresight-tmc-etr.c#L1572 > > /* > * No HW configuration is needed if the sink is already in > diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h > index 1ed50411cc3c..f9a0c95e9ba2 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc.h > +++ b/drivers/hwtracing/coresight/coresight-tmc.h > @@ -178,8 +178,8 @@ struct etr_buf { > * device configuration register (DEVID) > * @idr: Holds etr_bufs allocated for this ETR. > * @idr_mutex: Access serialisation for idr. > - * @perf_data: PERF buffer for ETR. > - * @sysfs_data: SYSFS buffer for ETR. > + * @sysfs_buf: SYSFS buffer for ETR. > + * @perf_buf: PERF buffer for ETR. > */ > struct tmc_drvdata { > void __iomem *base; > @@ -202,7 +202,7 @@ struct tmc_drvdata { > struct idr idr; > struct mutex idr_mutex; > struct etr_buf *sysfs_buf; > - void *perf_data; > + struct etr_buf *perf_buf; > }; > > struct etr_buf_operations { > -- > 2.23.0.rc1.153.gdeed80330f-goog >
| |