| Message ID | 20190501175052.29667-4-mathieu.poirier@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | coresight: Fix snapshot mode | expand |
On Wed, May 01, 2019 at 11:50:50AM -0600, Mathieu Poirier wrote: > This patch avoids setting the truncated flag when operaring in snapshot s/operaring/operating > mode since the trace buffer is expected to be truncated and discontinuous > from one snapshot to another. Moreover when the truncated flag is set > the perf core stops enabling the event, waiting for user space to consume > the data. In snapshot mode this is clearly not what we want since it > results in stale data. Not sure if I understand correctly or not. If set TRUNCATED flag and the user space has finished to read out the trace data, will perf not re-enable the event anymore for snapshot mode? Seems to me, the perf core code cannot handle properly for TRUNCATED flag with snapshot mode. Sorry if introduce noise, will look into the perf core code. Thanks, Leo Yan > Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org> > --- > drivers/hwtracing/coresight/coresight-etb10.c | 8 +++++++- > drivers/hwtracing/coresight/coresight-tmc-etf.c | 8 +++++++- > drivers/hwtracing/coresight/coresight-tmc-etr.c | 8 +++++++- > 3 files changed, 21 insertions(+), 3 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c > index 0764647b92bc..6ff48be91f61 100644 > --- a/drivers/hwtracing/coresight/coresight-etb10.c > +++ b/drivers/hwtracing/coresight/coresight-etb10.c > @@ -535,7 +535,13 @@ static unsigned long etb_update_buffer(struct coresight_device *csdev, > lost = true; > } > > - if (lost) > + /* > + * Don't set the TRUNCATED flag in snapshot mode because 1) the > + * captured buffer is expected to be truncated and 2) a full buffer > + * prevents the event from being re-enabled by the perf core, > + * resulting in stale data being send to user space. > + */ > + if (!buf->snapshot && lost) > perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED); > > /* finally tell HW where we want to start reading from */ > diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c > index d3025634f5e6..8039bd389034 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c > +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c > @@ -538,7 +538,13 @@ static unsigned long tmc_update_etf_buffer(struct coresight_device *csdev, > lost = true; > } > > - if (lost) > + /* > + * Don't set the TRUNCATED flag in snapshot mode because 1) the > + * captured buffer is expected to be truncated and 2) a full buffer > + * prevents the event from being re-enabled by the perf core, > + * resulting in stale data being send to user space. > + */ > + if (!buf->snapshot && lost) > perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED); > > cur = buf->cur; > diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c > index b9881d6d41ba..718586a083af 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c > +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c > @@ -1516,7 +1516,13 @@ tmc_update_etr_buffer(struct coresight_device *csdev, > > lost |= etr_buf->full; > out: > - if (lost) > + /* > + * Don't set the TRUNCATED flag in snapshot mode because 1) the > + * captured buffer is expected to be truncated and 2) a full buffer > + * prevents the event from being re-enabled by the perf core, > + * resulting in stale data being send to user space. > + */ > + if (!etr_perf->snapshot && lost) > perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED); > return size; > } > -- > 2.17.1 >
On Tue, 7 May 2019 at 02:29, Leo Yan <leo.yan@linaro.org> wrote: > > On Wed, May 01, 2019 at 11:50:50AM -0600, Mathieu Poirier wrote: > > This patch avoids setting the truncated flag when operaring in snapshot > > s/operaring/operating Ok > > > mode since the trace buffer is expected to be truncated and discontinuous > > from one snapshot to another. Moreover when the truncated flag is set > > the perf core stops enabling the event, waiting for user space to consume > > the data. In snapshot mode this is clearly not what we want since it > > results in stale data. > > Not sure if I understand correctly or not. > > If set TRUNCATED flag and the user space has finished to read out the > trace data, will perf not re-enable the event anymore for snapshot mode? You are correct, once user space has read truncated trace data the event is scheduled again. But here we want to continue accumulating data in the ring buffer without sending it to user space, that is the idea behind snaphsot mode. As such we will get truncated data but we want to continue accumulating it until user space decides that it wants to read a snapshot. If the TRUNCATED flag is set when the ring buffer has been filled the event is not scheduled and trace data not accumulated anymore, leading to a stale snapshot when user space requests it. > > Seems to me, the perf core code cannot handle properly for TRUNCATED > flag with snapshot mode. Sorry if introduce noise, will look into the > perf core code. > > Thanks, > Leo Yan > > > Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org> > > --- > > drivers/hwtracing/coresight/coresight-etb10.c | 8 +++++++- > > drivers/hwtracing/coresight/coresight-tmc-etf.c | 8 +++++++- > > drivers/hwtracing/coresight/coresight-tmc-etr.c | 8 +++++++- > > 3 files changed, 21 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c > > index 0764647b92bc..6ff48be91f61 100644 > > --- a/drivers/hwtracing/coresight/coresight-etb10.c > > +++ b/drivers/hwtracing/coresight/coresight-etb10.c > > @@ -535,7 +535,13 @@ static unsigned long etb_update_buffer(struct coresight_device *csdev, > > lost = true; > > } > > > > - if (lost) > > + /* > > + * Don't set the TRUNCATED flag in snapshot mode because 1) the > > + * captured buffer is expected to be truncated and 2) a full buffer > > + * prevents the event from being re-enabled by the perf core, > > + * resulting in stale data being send to user space. > > + */ > > + if (!buf->snapshot && lost) > > perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED); > > > > /* finally tell HW where we want to start reading from */ > > diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c > > index d3025634f5e6..8039bd389034 100644 > > --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c > > +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c > > @@ -538,7 +538,13 @@ static unsigned long tmc_update_etf_buffer(struct coresight_device *csdev, > > lost = true; > > } > > > > - if (lost) > > + /* > > + * Don't set the TRUNCATED flag in snapshot mode because 1) the > > + * captured buffer is expected to be truncated and 2) a full buffer > > + * prevents the event from being re-enabled by the perf core, > > + * resulting in stale data being send to user space. > > + */ > > + if (!buf->snapshot && lost) > > perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED); > > > > cur = buf->cur; > > diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c > > index b9881d6d41ba..718586a083af 100644 > > --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c > > +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c > > @@ -1516,7 +1516,13 @@ tmc_update_etr_buffer(struct coresight_device *csdev, > > > > lost |= etr_buf->full; > > out: > > - if (lost) > > + /* > > + * Don't set the TRUNCATED flag in snapshot mode because 1) the > > + * captured buffer is expected to be truncated and 2) a full buffer > > + * prevents the event from being re-enabled by the perf core, > > + * resulting in stale data being send to user space. > > + */ > > + if (!etr_perf->snapshot && lost) > > perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED); > > return size; > > } > > -- > > 2.17.1 > >
diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c index 0764647b92bc..6ff48be91f61 100644 --- a/drivers/hwtracing/coresight/coresight-etb10.c +++ b/drivers/hwtracing/coresight/coresight-etb10.c @@ -535,7 +535,13 @@ static unsigned long etb_update_buffer(struct coresight_device *csdev, lost = true; } - if (lost) + /* + * Don't set the TRUNCATED flag in snapshot mode because 1) the + * captured buffer is expected to be truncated and 2) a full buffer + * prevents the event from being re-enabled by the perf core, + * resulting in stale data being send to user space. + */ + if (!buf->snapshot && lost) perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED); /* finally tell HW where we want to start reading from */ diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c index d3025634f5e6..8039bd389034 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c @@ -538,7 +538,13 @@ static unsigned long tmc_update_etf_buffer(struct coresight_device *csdev, lost = true; } - if (lost) + /* + * Don't set the TRUNCATED flag in snapshot mode because 1) the + * captured buffer is expected to be truncated and 2) a full buffer + * prevents the event from being re-enabled by the perf core, + * resulting in stale data being send to user space. + */ + if (!buf->snapshot && lost) perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED); cur = buf->cur; diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index b9881d6d41ba..718586a083af 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -1516,7 +1516,13 @@ tmc_update_etr_buffer(struct coresight_device *csdev, lost |= etr_buf->full; out: - if (lost) + /* + * Don't set the TRUNCATED flag in snapshot mode because 1) the + * captured buffer is expected to be truncated and 2) a full buffer + * prevents the event from being re-enabled by the perf core, + * resulting in stale data being send to user space. + */ + if (!etr_perf->snapshot && lost) perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED); return size; }
This patch avoids setting the truncated flag when operaring in snapshot mode since the trace buffer is expected to be truncated and discontinuous from one snapshot to another. Moreover when the truncated flag is set the perf core stops enabling the event, waiting for user space to consume the data. In snapshot mode this is clearly not what we want since it results in stale data. Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org> --- drivers/hwtracing/coresight/coresight-etb10.c | 8 +++++++- drivers/hwtracing/coresight/coresight-tmc-etf.c | 8 +++++++- drivers/hwtracing/coresight/coresight-tmc-etr.c | 8 +++++++- 3 files changed, 21 insertions(+), 3 deletions(-)