| Message ID | 20190815184628.183186-1-yabinc@google.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | bbedcb91cc3bf252e6031e199ab3d1f07107f7c5 |
| Headers | show |
| Series | [v3] coresight: tmc-etr: Fix perf_data check. | expand |
On Thu, 15 Aug 2019 at 12:46, 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. > Also move the code setting and clearing perf_buf to more suitable > places. > > Fixes: 3147da92a8a8 ("coresight: tmc-etr: Allocate and free ETR memory buffers for CPU-wide scenarios") > Signed-off-by: Yabin Cui <yabinc@google.com> > --- > > v2 -> v3: > moved place of setting drvdata->perf_buf based on review comment. > Also moved place of clearing drvdata->perf_buf from > tmc_update_etr_buffer() to tmc_disable_etr_sink() for below > situation: > > cpu 0 enable etr device (set perf_buf) > cpu 0 update etr buffer (clear perf_buf) > cpu 1 enable etr device (perf_buf isn't set because pid set by cpu 0) > cpu 0 disable etr device > cpu 1 update etr buffer (perf_buf == NULL, check fails) > > To fix it, we need to move clearing perf_buf to the disable function. > > --- > drivers/hwtracing/coresight/coresight-tmc-etr.c | 8 ++++---- > drivers/hwtracing/coresight/coresight-tmc.h | 6 +++--- > 2 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c > index 17006705287a..80af313f55d7 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; > @@ -1496,8 +1496,6 @@ tmc_update_etr_buffer(struct coresight_device *csdev, > tmc_sync_etr_buf(drvdata); > > CS_LOCK(drvdata->base); > - /* Reset perf specific data */ > - drvdata->perf_data = NULL; > spin_unlock_irqrestore(&drvdata->spinlock, flags); > > size = etr_buf->len; > @@ -1556,7 +1554,6 @@ 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; > > /* > * No HW configuration is needed if the sink is already in > @@ -1572,6 +1569,7 @@ static int tmc_enable_etr_sink_perf(struct coresight_device *csdev, void *data) > /* Associate with monitored process. */ > drvdata->pid = pid; > drvdata->mode = CS_MODE_PERF; > + drvdata->perf_buf = etr_perf->etr_buf; > atomic_inc(csdev->refcnt); > } > > @@ -1617,6 +1615,8 @@ static int tmc_disable_etr_sink(struct coresight_device *csdev) > /* Dissociate from monitored process. */ > drvdata->pid = -1; > drvdata->mode = CS_MODE_DISABLED; > + /* Reset perf specific data */ > + drvdata->perf_buf = NULL; > > spin_unlock_irqrestore(&drvdata->spinlock, flags); > > 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; > }; I have applied your patch. Thanks, Mathieu > > struct etr_buf_operations { > -- > 2.23.0.rc1.153.gdeed80330f-goog >
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index 17006705287a..80af313f55d7 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; @@ -1496,8 +1496,6 @@ tmc_update_etr_buffer(struct coresight_device *csdev, tmc_sync_etr_buf(drvdata); CS_LOCK(drvdata->base); - /* Reset perf specific data */ - drvdata->perf_data = NULL; spin_unlock_irqrestore(&drvdata->spinlock, flags); size = etr_buf->len; @@ -1556,7 +1554,6 @@ 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; /* * No HW configuration is needed if the sink is already in @@ -1572,6 +1569,7 @@ static int tmc_enable_etr_sink_perf(struct coresight_device *csdev, void *data) /* Associate with monitored process. */ drvdata->pid = pid; drvdata->mode = CS_MODE_PERF; + drvdata->perf_buf = etr_perf->etr_buf; atomic_inc(csdev->refcnt); } @@ -1617,6 +1615,8 @@ static int tmc_disable_etr_sink(struct coresight_device *csdev) /* Dissociate from monitored process. */ drvdata->pid = -1; drvdata->mode = CS_MODE_DISABLED; + /* Reset perf specific data */ + drvdata->perf_buf = NULL; spin_unlock_irqrestore(&drvdata->spinlock, flags); 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 {
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. Also move the code setting and clearing perf_buf to more suitable places. Fixes: 3147da92a8a8 ("coresight: tmc-etr: Allocate and free ETR memory buffers for CPU-wide scenarios") Signed-off-by: Yabin Cui <yabinc@google.com> --- v2 -> v3: moved place of setting drvdata->perf_buf based on review comment. Also moved place of clearing drvdata->perf_buf from tmc_update_etr_buffer() to tmc_disable_etr_sink() for below situation: cpu 0 enable etr device (set perf_buf) cpu 0 update etr buffer (clear perf_buf) cpu 1 enable etr device (perf_buf isn't set because pid set by cpu 0) cpu 0 disable etr device cpu 1 update etr buffer (perf_buf == NULL, check fails) To fix it, we need to move clearing perf_buf to the disable function. --- drivers/hwtracing/coresight/coresight-tmc-etr.c | 8 ++++---- drivers/hwtracing/coresight/coresight-tmc.h | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-)