| Message ID | 20190812221154.46875-1-yabinc@google.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 13af88f312fc57becacfcbb1cc77f844281a30ec |
| Headers | show |
| Series | [v2] coresight: tmc-etr: Fix updating buffer in not-snapshot mode. | expand |
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 >
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; } } @@ -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
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(-)