Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Sat, 10 Jul 2021 15:05:09 +0800
From:   Leo Yan <leo.yan@linaro.org>
To:     Mathieu Poirier <mathieu.poirier@linaro.org>,
        Suzuki K Poulose <suzuki.poulose@arm.com>,
        Mike Leach <mike.leach@linaro.org>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        coresight@lists.linaro.org, linux-arm-kernel@lists.infradead.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1] coresight: tmc-etr: Speed up for bounce buffer in
 flat mode
Message-ID: <20210710070509.GB273828@leoy-ThinkPad-X240s>
References: <20210710050046.414669-1-leo.yan@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210710050046.414669-1-leo.yan@linaro.org>
Precedence: bulk

Hi all,

On Sat, Jul 10, 2021 at 01:00:46PM +0800, Leo Yan wrote:

[...]

> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> @@ -21,6 +21,7 @@
>  
>  struct etr_flat_buf {
>  	struct device	*dev;
> +	struct page	*pages;
>  	dma_addr_t	daddr;
>  	void		*vaddr;
>  	size_t		size;
> @@ -600,6 +601,7 @@ static int tmc_etr_alloc_flat_buf(struct tmc_drvdata *drvdata,
>  {
>  	struct etr_flat_buf *flat_buf;
>  	struct device *real_dev = drvdata->csdev->dev.parent;
> +	ssize_t	aligned_size;
>  
>  	/* We cannot reuse existing pages for flat buf */
>  	if (pages)
> @@ -609,12 +611,17 @@ static int tmc_etr_alloc_flat_buf(struct tmc_drvdata *drvdata,
>  	if (!flat_buf)
>  		return -ENOMEM;
>  
> -	flat_buf->vaddr = dma_alloc_coherent(real_dev, etr_buf->size,
> -					     &flat_buf->daddr, GFP_KERNEL);
> -	if (!flat_buf->vaddr) {
> -		kfree(flat_buf);
> -		return -ENOMEM;
> -	}
> +	aligned_size = PAGE_ALIGN(etr_buf->size);
> +	flat_buf->pages = alloc_pages_node(node, GFP_KERNEL | __GFP_ZERO,
> +					   get_order(aligned_size));
> +	if (!flat_buf->pages)
> +		goto fail_alloc_pages;
> +
> +	flat_buf->vaddr = page_address(flat_buf->pages);
> +	flat_buf->daddr = dma_map_page(real_dev, flat_buf->pages, 0,
> +				       aligned_size, DMA_FROM_DEVICE);
> +	if (dma_mapping_error(real_dev, flat_buf->daddr))
> +		goto fail_dma_map_page;
>  
>  	flat_buf->size = etr_buf->size;
>  	flat_buf->dev = &drvdata->csdev->dev;
> @@ -622,23 +629,34 @@ static int tmc_etr_alloc_flat_buf(struct tmc_drvdata *drvdata,
>  	etr_buf->mode = ETR_MODE_FLAT;
>  	etr_buf->private = flat_buf;
>  	return 0;
> +
> +fail_dma_map_page:
> +	__free_pages(flat_buf->pages, get_order(aligned_size));
> +fail_alloc_pages:
> +	kfree(flat_buf);
> +	return -ENOMEM;
>  }
>  
>  static void tmc_etr_free_flat_buf(struct etr_buf *etr_buf)
>  {
>  	struct etr_flat_buf *flat_buf = etr_buf->private;
>  
> -	if (flat_buf && flat_buf->daddr) {
> +	if (flat_buf && flat_buf->vaddr) {

I found here I introduced an unexpected change for checking
"flat_buf->vaddr", we should still check "flat_buf->daddr".

Sent patch v2 to address this issue; please directly review patch
v2.

Thanks,
Leo