Re: [PATCH 1/5] coresight: Fix buffer size in snapshot mode

[Leo Yan <leo.yan@linaro.org>]

On Wed, May 01, 2019 at 11:50:48AM -0600, Mathieu Poirier wrote:
> In snapshot mode the buffer used by the sink devices need to be
> equal to the ring buffer size in order for the user space mechanic
> to work properly.
> 
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> ---
>  drivers/hwtracing/coresight/coresight-etb10.c | 23 +++++++++++++++++++
>  .../hwtracing/coresight/coresight-tmc-etf.c   | 20 ++++++++++++++++
>  .../hwtracing/coresight/coresight-tmc-etr.c   |  8 +++++--
>  3 files changed, 49 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c
> index 4ee4c80a4354..0764647b92bc 100644
> --- a/drivers/hwtracing/coresight/coresight-etb10.c
> +++ b/drivers/hwtracing/coresight/coresight-etb10.c
> @@ -374,7 +374,30 @@ static void *etb_alloc_buffer(struct coresight_device *csdev,
>  			      int nr_pages, bool overwrite)
>  {
>  	int node, cpu = event->cpu;
> +	u32 capacity;
>  	struct cs_buffers *buf;
> +	struct etb_drvdata *drvdata;
> +
> +	/*
> +	 * In snapsot mode the size of the perf ring buffer needs to be equal
> +	 * to the size of the device's internal memory if we want to reuse the
> +	 * generic AUX buffer management mechanic.
> +	 *
> +	 * For example (assuming 4096 byte page size):

Here is delibrately to write as '4096 byte'?  Though I think should be
'4096 bytes' but I am not confident which is right ...

> +	 *
> +	 *    # cat /sys/bus/coresight/devices/20010000.etb/mgmt/rdp
> +	 *    0x2000
> +	 *    # perf record -e cs_etm/@20010000.etf/ -S -m,8 --per-thread $APP
> +	 *
> +	 */
> +	drvdata = dev_get_drvdata(csdev->dev.parent);
> +	capacity = drvdata->buffer_depth * ETB_FRAME_SIZE_WORDS;
> +
> +	if (overwrite &&
> +	    ((nr_pages << PAGE_SHIFT) != capacity)) {
> +		dev_err(&csdev->dev, "Ring buffer not equal to device buffer");
> +		return NULL;
> +	}
>  
>  	if (cpu == -1)
>  		cpu = smp_processor_id();
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> index 2527b5d3b65e..7694833b13cb 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> @@ -380,6 +380,26 @@ static void *tmc_alloc_etf_buffer(struct coresight_device *csdev,
>  {
>  	int node, cpu = event->cpu;
>  	struct cs_buffers *buf;
> +	struct tmc_drvdata *drvdata;
> +
> +	/*
> +	 * In snapsot mode the size of the perf ring buffer needs to be equal
> +	 * to the size of the device's internal memory if we want to reuse the
> +	 * generic AUX buffer management mechanic.
> +	 *
> +	 * For example (assuming 4096 byte page size):
> +	 *
> +	 *    # cat /sys/bus/coresight/devices/20010000.etf/buffer_size
> +	 *    0x10000
> +	 *    # perf record -e cs_etm/@20010000.etf/ -S -m,16 --per-thread $APP
> +	 *
> +	 */
> +	drvdata = dev_get_drvdata(csdev->dev.parent);
> +	if (overwrite &&
> +	    ((nr_pages << PAGE_SHIFT) != drvdata->size)) {
> +		dev_err(&csdev->dev, "Ring buffer not equal to device buffer");
> +		return NULL;
> +	}
>  
>  	if (cpu == -1)
>  		cpu = smp_processor_id();
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index df6e4b0b84e9..b9881d6d41ba 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> @@ -1188,9 +1188,13 @@ alloc_etr_buf(struct tmc_drvdata *drvdata, struct perf_event *event,
>  
>  	/*
>  	 * Try to match the perf ring buffer size if it is larger
> -	 * than the size requested via sysfs.
> +	 * than the size requested via sysfs.  In snapsot mode the size
> +	 * of the perf ring buffer needs to be equal to the allocated
> +	 * size if we want to reuse the generic AUX buffer management
> +	 * mechanic.
>  	 */
> -	if ((nr_pages << PAGE_SHIFT) > drvdata->size) {
> +	if (snapshot ||
> +	    (nr_pages << PAGE_SHIFT) > drvdata->size) {
>  		etr_buf = tmc_alloc_etr_buf(drvdata, (nr_pages << PAGE_SHIFT),
>  					    0, node, NULL);
>  		if (!IS_ERR(etr_buf))

If tmc_alloc_etr_buf() returns failure then it's possible to run into
the below sequence to allocate smaller buffer size for snapshot mode;
which is not expected for snapshot mode.

So here if tmc_alloc_etr_buf() fails to allocate buffer for snapshot
mode, should directly bail out with error code.

Thanks,
Leo Yan

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel