[2/5] coresight: tmc-etf: Fix snapshot mode update function
diff mbox series

Message ID 20190501175052.29667-3-mathieu.poirier@linaro.org
State New, archived
Headers show
Series
  • coresight: Fix snapshot mode
Related show

Commit Message

Mathieu Poirier May 1, 2019, 5:50 p.m. UTC
When working in snapshot mode function perf_aux_output_begin()
does not set the handle->size because the size is expected to be
deduced by the placement of the "head" and "old" pointers in user
space.  As such there is no point in trying to adjust the amount
of data to copy to the ring buffer.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/hwtracing/coresight/coresight-tmc-etf.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Leo Yan May 7, 2019, 8:13 a.m. UTC | #1
On Wed, May 01, 2019 at 11:50:49AM -0600, Mathieu Poirier wrote:
> When working in snapshot mode function perf_aux_output_begin()

Do you mean perf_aux_output_end() rather than perf_aux_output_begin()?

I checked perf_aux_output_begin(), it will always set 'handle->size'
to zero.

> does not set the handle->size because the size is expected to be
> deduced by the placement of the "head" and "old" pointers in user
> space.  As such there is no point in trying to adjust the amount
> of data to copy to the ring buffer.
> 
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>

Rest looks good to me:

Reviewed-by: Leo Yan <leo.yan@linaro.org>

> ---
>  drivers/hwtracing/coresight/coresight-tmc-etf.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> index 7694833b13cb..d3025634f5e6 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> @@ -497,9 +497,11 @@ static unsigned long tmc_update_etf_buffer(struct coresight_device *csdev,
>  	/*
>  	 * The TMC RAM buffer may be bigger than the space available in the
>  	 * perf ring buffer (handle->size).  If so advance the RRP so that we
> -	 * get the latest trace data.
> +	 * get the latest trace data.  In snapshot mode none of that matters
> +	 * since we are expected to clobber stale data in favour of the latest
> +	 * traces.
>  	 */
> -	if (to_read > handle->size) {
> +	if (!buf->snapshot && to_read > handle->size) {
>  		u32 mask = 0;
>  
>  		/*
> -- 
> 2.17.1
>
Suzuki K Poulose May 7, 2019, 9:22 a.m. UTC | #2
On 01/05/2019 18:50, Mathieu Poirier wrote:
> When working in snapshot mode function perf_aux_output_begin()
> does not set the handle->size because the size is expected to be
> deduced by the placement of the "head" and "old" pointers in user
> space.  As such there is no point in trying to adjust the amount
> of data to copy to the ring buffer.
> 
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> ---
>   drivers/hwtracing/coresight/coresight-tmc-etf.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> index 7694833b13cb..d3025634f5e6 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> @@ -497,9 +497,11 @@ static unsigned long tmc_update_etf_buffer(struct coresight_device *csdev,
>   	/*
>   	 * The TMC RAM buffer may be bigger than the space available in the
>   	 * perf ring buffer (handle->size).  If so advance the RRP so that we
> -	 * get the latest trace data.
> +	 * get the latest trace data.  In snapshot mode none of that matters
> +	 * since we are expected to clobber stale data in favour of the latest
> +	 * traces.
>   	 */
> -	if (to_read > handle->size) {
> +	if (!buf->snapshot && to_read > handle->size) {
>   		u32 mask = 0;
>   
>   		/*
> 

Looks good to me.

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Mathieu Poirier May 7, 2019, 5:16 p.m. UTC | #3
On Tue, 7 May 2019 at 02:13, Leo Yan <leo.yan@linaro.org> wrote:
>
> On Wed, May 01, 2019 at 11:50:49AM -0600, Mathieu Poirier wrote:
> > When working in snapshot mode function perf_aux_output_begin()
>
> Do you mean perf_aux_output_end() rather than perf_aux_output_begin()?
>
> I checked perf_aux_output_begin(), it will always set 'handle->size'
> to zero.
>

When not operating in snapshot mode perf_aux_output_begin() sets handle->size:

https://elixir.bootlin.com/linux/latest/source/kernel/events/ring_buffer.c#L387

> > does not set the handle->size because the size is expected to be
> > deduced by the placement of the "head" and "old" pointers in user
> > space.  As such there is no point in trying to adjust the amount
> > of data to copy to the ring buffer.
> >
> > Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>
> Rest looks good to me:
>
> Reviewed-by: Leo Yan <leo.yan@linaro.org>
>
> > ---
> >  drivers/hwtracing/coresight/coresight-tmc-etf.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> > index 7694833b13cb..d3025634f5e6 100644
> > --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
> > +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> > @@ -497,9 +497,11 @@ static unsigned long tmc_update_etf_buffer(struct coresight_device *csdev,
> >       /*
> >        * The TMC RAM buffer may be bigger than the space available in the
> >        * perf ring buffer (handle->size).  If so advance the RRP so that we
> > -      * get the latest trace data.
> > +      * get the latest trace data.  In snapshot mode none of that matters
> > +      * since we are expected to clobber stale data in favour of the latest
> > +      * traces.
> >        */
> > -     if (to_read > handle->size) {
> > +     if (!buf->snapshot && to_read > handle->size) {
> >               u32 mask = 0;
> >
> >               /*
> > --
> > 2.17.1
> >

Patch
diff mbox series

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
index 7694833b13cb..d3025634f5e6 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
@@ -497,9 +497,11 @@  static unsigned long tmc_update_etf_buffer(struct coresight_device *csdev,
 	/*
 	 * The TMC RAM buffer may be bigger than the space available in the
 	 * perf ring buffer (handle->size).  If so advance the RRP so that we
-	 * get the latest trace data.
+	 * get the latest trace data.  In snapshot mode none of that matters
+	 * since we are expected to clobber stale data in favour of the latest
+	 * traces.
 	 */
-	if (to_read > handle->size) {
+	if (!buf->snapshot && to_read > handle->size) {
 		u32 mask = 0;
 
 		/*