Messages in this thread | | | From | Mathieu Poirier <> | Date | Wed, 14 Aug 2019 14:38:18 -0600 | Subject | Re: [PATCH v2] coresight: tmc-etr: Fix updating buffer in not-snapshot mode. |
| |
On Mon, 12 Aug 2019 at 16:11, Yabin Cui <yabinc@google.com> wrote: > > TMC etr always copies all available data to perf aux buffer, which > may exceed the available space in perf aux buffer. It isn't suitable > for not-snapshot mode, because: > 1) It may overwrite previously written data. > 2) It may make the perf_event_mmap_page->aux_head report having more > or less data than the reality. > > So change to only copy the latest data fitting the available space in > perf aux buffer. > > Signed-off-by: Yabin Cui <yabinc@google.com> > --- > > v1 -> v2: copy the latest data instead of the earliest data. > > --- > .../hwtracing/coresight/coresight-tmc-etr.c | 18 +++++++++++------- > 1 file changed, 11 insertions(+), 7 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c > index 17006705287a..676dcb4cf0e2 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c > +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c > @@ -1410,9 +1410,10 @@ static void tmc_free_etr_buffer(void *config) > * tmc_etr_sync_perf_buffer: Copy the actual trace data from the hardware > * buffer to the perf ring buffer. > */ > -static void tmc_etr_sync_perf_buffer(struct etr_perf_buffer *etr_perf) > +static void tmc_etr_sync_perf_buffer(struct etr_perf_buffer *etr_perf, > + unsigned long to_copy) > { > - long bytes, to_copy; > + long bytes; > long pg_idx, pg_offset, src_offset; > unsigned long head = etr_perf->head; > char **dst_pages, *src_buf; > @@ -1422,8 +1423,7 @@ static void tmc_etr_sync_perf_buffer(struct etr_perf_buffer *etr_perf) > pg_idx = head >> PAGE_SHIFT; > pg_offset = head & (PAGE_SIZE - 1); > dst_pages = (char **)etr_perf->pages; > - src_offset = etr_buf->offset; > - to_copy = etr_buf->len; > + src_offset = etr_buf->offset + etr_buf->len - to_copy; > > while (to_copy > 0) { > /* > @@ -1434,6 +1434,8 @@ static void tmc_etr_sync_perf_buffer(struct etr_perf_buffer *etr_perf) > * 3) what is available in the destination page. > * in one iteration. > */ > + if (src_offset >= etr_buf->size) > + src_offset -= etr_buf->size; > bytes = tmc_etr_buf_get_data(etr_buf, src_offset, to_copy, > &src_buf); > if (WARN_ON_ONCE(bytes <= 0)) > @@ -1454,8 +1456,6 @@ static void tmc_etr_sync_perf_buffer(struct etr_perf_buffer *etr_perf) > > /* Move source pointers */ > src_offset += bytes; > - if (src_offset >= etr_buf->size) > - src_offset -= etr_buf->size; > } > }
Yes, much better now. I have applied your work.
Thanks, Mathieu
> > @@ -1501,7 +1501,11 @@ tmc_update_etr_buffer(struct coresight_device *csdev, > spin_unlock_irqrestore(&drvdata->spinlock, flags); > > size = etr_buf->len; > - tmc_etr_sync_perf_buffer(etr_perf); > + if (!etr_perf->snapshot && size > handle->size) { > + size = handle->size; > + lost = true; > + } > + tmc_etr_sync_perf_buffer(etr_perf, size); > > /* > * In snapshot mode we simply increment the head by the number of byte > -- > 2.23.0.rc1.153.gdeed80330f-goog >
| |