diff mbox series

[V2] mmc: mmc_test: Pass different sg lists for non-blocking requests

Message ID 1582105474-27866-1-git-send-email-vbadigan@codeaurora.org
State Superseded
Headers show
Series [V2] mmc: mmc_test: Pass different sg lists for non-blocking requests | expand

Commit Message

Veerabhadrarao Badiganti Feb. 19, 2020, 9:44 a.m. UTC
Supply a separate sg list for each of the request in non-blocking
IO test cases where two requests will be issued at same time.

Otherwise, sg memory may get unmapped when a request is done while
same memory is being accessed by controller from the other request,
and it leads to iommu errors with below call stack:

	__arm_lpae_unmap+0x2e0/0x478
	arm_lpae_unmap+0x54/0x70
	arm_smmu_unmap+0x64/0xa4
	__iommu_unmap+0xb8/0x1f0
	iommu_unmap_fast+0x38/0x48
	__iommu_dma_unmap+0x88/0x108
	iommu_dma_unmap_sg+0x90/0xa4
	sdhci_post_req+0x5c/0x78
	mmc_test_start_areq+0x10c/0x120 [mmc_test]
	mmc_test_area_io_seq+0x150/0x264 [mmc_test]
	mmc_test_rw_multiple+0x174/0x1c0 [mmc_test]
	mmc_test_rw_multiple_sg_len+0x44/0x6c [mmc_test]
	mmc_test_profile_sglen_wr_nonblock_perf+0x6c/0x94 [mmc_test]
	mtf_test_write+0x238/0x3cc [mmc_test]

Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>

---
Changes since V1:
	- Freeing-up sg_areq memory.
	- Added check to ensure sg length is equal for both the sg-lists
	  supplied in case of non-blocking requests.
---
 drivers/mmc/core/mmc_test.c | 42 ++++++++++++++++++++++++++++++++++++------
 1 file changed, 36 insertions(+), 6 deletions(-)

Comments

Sai Prakash Ranjan Feb. 20, 2020, 8:39 a.m. UTC | #1
On 2020-02-19 15:14, Veerabhadrarao Badiganti wrote:
> Supply a separate sg list for each of the request in non-blocking
> IO test cases where two requests will be issued at same time.
> 
> Otherwise, sg memory may get unmapped when a request is done while
> same memory is being accessed by controller from the other request,
> and it leads to iommu errors with below call stack:
> 
> 	__arm_lpae_unmap+0x2e0/0x478
> 	arm_lpae_unmap+0x54/0x70
> 	arm_smmu_unmap+0x64/0xa4
> 	__iommu_unmap+0xb8/0x1f0
> 	iommu_unmap_fast+0x38/0x48
> 	__iommu_dma_unmap+0x88/0x108
> 	iommu_dma_unmap_sg+0x90/0xa4
> 	sdhci_post_req+0x5c/0x78
> 	mmc_test_start_areq+0x10c/0x120 [mmc_test]
> 	mmc_test_area_io_seq+0x150/0x264 [mmc_test]
> 	mmc_test_rw_multiple+0x174/0x1c0 [mmc_test]
> 	mmc_test_rw_multiple_sg_len+0x44/0x6c [mmc_test]
> 	mmc_test_profile_sglen_wr_nonblock_perf+0x6c/0x94 [mmc_test]
> 	mtf_test_write+0x238/0x3cc [mmc_test]
> 
> Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
> 

Tested-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>

This is reproducible on Dragonboard845c as well when performing 
nonblocking tests
and the patch fixes the issue.

/ # emmc_dev=mmc0:0002
  / # echo $emmc_dev > /sys/bus/mmc/drivers/mmcblk/unbind
  / # echo $emmc_dev > /sys/bus/mmc/drivers/mmc_test/bind
  [   23.715673] mmc_test mmc0:0002: Card claimed for testing.
  / # cd /sys/kernel/debug/mmc*/${emmc_dev}
  /sys/kernel/debug/mmc0/mmc0:0002 # echo 42 > test
  [   32.074182] mmc0: Starting tests of card mmc0:0002...
  [   32.079484] mmc0: Test case 42. Write performance non-blocking req 1 
to 512 sg elems...
  [   32.783439] mmc0: ADMA error: 0x02000000
  [   32.787411] mmc0: sdhci: ============ SDHCI REGISTER DUMP 
===========
  [   32.796336] mmc0: sdhci: Sys addr:  0x00000000 | Version:  
0x00006c02
  [   32.804533] mmc0: sdhci: Blk size:  0x00000200 | Blk cnt:  
0x00000280
  [   32.811046] mmc0: sdhci: Argument:  0x001d0400 | Trn mode: 
0x00000023
  [   32.817555] mmc0: sdhci: Present:   0x01f801f6 | Host ctl: 
0x0000001f
  [   32.824062] mmc0: sdhci: Power:     0x00000001 | Blk gap:  
0x00000000
  [   32.830566] mmc0: sdhci: Wake-up:   0x00000000 | Clock:    
0x00000007
  [   32.837074] mmc0: sdhci: Timeout:   0x0000000e | Int stat: 
0x02008000
  [   32.843582] mmc0: sdhci: Int enab:  0x03ff100b | Sig enab: 
0x03ff100b
  [   32.850089] mmc0: sdhci: ACmd stat: 0x00000000 | Slot int: 
0x00000000
  [   32.856596] mmc0: sdhci: Caps:      0x362dc8b2 | Caps_1:   
0x0000808f
  [   32.863104] mmc0: sdhci: Cmd:       0x0000193a | Max curr: 
0x00000000
  [   32.869612] mmc0: sdhci: Resp[0]:   0x00000900 | Resp[1]:  
0x5b590000
  [   32.876121] mmc0: sdhci: Resp[2]:   0x1d9b7f80 | Resp[3]:  
0x0a40003a
  [   32.882627] mmc0: sdhci: Host ctl2: 0x00000000
  [   32.887121] mmc0: sdhci: ADMA Err:  0x00000001 | ADMA Ptr: 
0x0000000ffffff248
  [   32.894320] mmc0: sdhci: 
============================================
  [   32.900831] mmc0: sdhci: ffffff200: DMA 0x0000000fffe80000, LEN 
0x0000, Attr=0x21
  [   32.908391] mmc0: sdhci: ffffff20c: DMA 0x0000000fffe90000, LEN 
0x0000, Attr=0x21
  [   32.915951] mmc0: sdhci: ffffff218: DMA 0x0000000fffea0000, LEN 
0x0000, Attr=0x21
  [   32.923511] mmc0: sdhci: ffffff224: DMA 0x0000000fffeb0000, LEN 
0x0000, Attr=0x21
  [   32.931070] mmc0: sdhci: ffffff230: DMA 0x0000000fffec0000, LEN 
0x0000, Attr=0x21
  [   32.938630] mmc0: sdhci: ffffff23c: DMA 0x0000000fffed0000, LEN 
0x0000, Attr=0x21
  [   32.946190] mmc0: sdhci: ffffff248: DMA 0x0000000fffee0000, LEN 
0x0000, Attr=0x21
  [   32.953750] mmc0: sdhci: ffffff254: DMA 0x0000000fffef0000, LEN 
0x0000, Attr=0x21
  [   32.961309] mmc0: sdhci: ffffff260: DMA 0x0000000000000000, LEN 
0x0000, Attr=0x03
  [   32.968982] arm-smmu 15000000.iommu: Unhandled context fault: 
fsr=0x402, iova=0xfffe80480, fsynr=0x390003, cbfrsynra=0xa0, cb=1
<snip>...
  [   33.424884] [mmc_test_rw_multiple] error
  [   33.428849] mmc0: Result: ERROR (-5)
  [   33.432465] mmc0: Tests completed.
  /sys/kernel/debug/mmc0/mmc0:0002 #

-Sai
Stephen Boyd Feb. 21, 2020, 1:25 a.m. UTC | #2
Quoting Veerabhadrarao Badiganti (2020-02-19 01:44:31)
> Supply a separate sg list for each of the request in non-blocking
> IO test cases where two requests will be issued at same time.
> 
> Otherwise, sg memory may get unmapped when a request is done while
> same memory is being accessed by controller from the other request,
> and it leads to iommu errors with below call stack:
> 
>         __arm_lpae_unmap+0x2e0/0x478
>         arm_lpae_unmap+0x54/0x70
>         arm_smmu_unmap+0x64/0xa4
>         __iommu_unmap+0xb8/0x1f0
>         iommu_unmap_fast+0x38/0x48
>         __iommu_dma_unmap+0x88/0x108
>         iommu_dma_unmap_sg+0x90/0xa4
>         sdhci_post_req+0x5c/0x78
>         mmc_test_start_areq+0x10c/0x120 [mmc_test]
>         mmc_test_area_io_seq+0x150/0x264 [mmc_test]
>         mmc_test_rw_multiple+0x174/0x1c0 [mmc_test]
>         mmc_test_rw_multiple_sg_len+0x44/0x6c [mmc_test]
>         mmc_test_profile_sglen_wr_nonblock_perf+0x6c/0x94 [mmc_test]
>         mtf_test_write+0x238/0x3cc [mmc_test]
> 
> Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
> 
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

> Changes since V1:
>         - Freeing-up sg_areq memory.
>         - Added check to ensure sg length is equal for both the sg-lists
>           supplied in case of non-blocking requests.
> ---
>  drivers/mmc/core/mmc_test.c | 42 ++++++++++++++++++++++++++++++++++++------
>  1 file changed, 36 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mmc/core/mmc_test.c b/drivers/mmc/core/mmc_test.c
> index 492dd45..f8f884a 100644
> --- a/drivers/mmc/core/mmc_test.c
> +++ b/drivers/mmc/core/mmc_test.c
> @@ -1411,6 +1417,22 @@ static int mmc_test_area_map(struct mmc_test_card *test, unsigned long sz,
>                 err = mmc_test_map_sg(t->mem, sz, t->sg, 1, t->max_segs,
>                                       t->max_seg_sz, &t->sg_len, min_sg_len);
>         }
> +
> +       if (err || !nonblock)
> +               goto err;
> +
> +       if (max_scatter) {
> +               err = mmc_test_map_sg_max_scatter(t->mem, sz, t->sg_areq,
> +                                                 t->max_segs, t->max_seg_sz,
> +                                                 &sg_len);
> +       } else {
> +               err = mmc_test_map_sg(t->mem, sz, t->sg_areq, 1, t->max_segs,

'repeat' is always set to 1. Why not remove that argument and update the
code? As a follow up patch.

> +                                     t->max_seg_sz, &sg_len, min_sg_len);
> +       }
> +       if (!err && sg_len != t->sg_len)
> +               err = -EINVAL;
> +
> +err:
>         if (err)
>                 pr_info("%s: Failed to map sg list\n",
>                        mmc_hostname(test->card->host));
> @@ -1458,15 +1480,16 @@ static int mmc_test_area_io_seq(struct mmc_test_card *test, unsigned long sz,
>                         sz = max_tfr;
>         }
>  
> -       ret = mmc_test_area_map(test, sz, max_scatter, min_sg_len);
> +       ret = mmc_test_area_map(test, sz, max_scatter, min_sg_len, nonblock);
>         if (ret)
>                 return ret;
>  
>         if (timed)
>                 ktime_get_ts64(&ts1);
>         if (nonblock)
> -               ret = mmc_test_nonblock_transfer(test, t->sg, t->sg_len,
> -                                dev_addr, t->blocks, 512, write, count);
> +               ret = mmc_test_nonblock_transfer(test, t->sg, t->sg_areq,
> +                                t->sg_len, dev_addr, t->blocks, 512, write,
> +                                count);

This is only called one time so it may be simpler to pass 't' instead of
pick it apart and pass it as many arguments. Not a problem in this
patch, besides that we're now passing even more arguments here making
this harder to read. Also, the blksz could be hardcoded in the function
instead of passing it as 512.

>         else
>                 for (i = 0; i < count && ret == 0; i++) {
>                         ret = mmc_test_area_transfer(test, dev_addr, write);
> @@ -1584,6 +1608,12 @@ static int mmc_test_area_init(struct mmc_test_card *test, int erase, int fill)
>                 goto out_free;
>         }
>  
> +       t->sg_areq = kmalloc_array(t->max_segs, sizeof(*t->sg), GFP_KERNEL);

It's more idiomatic to use sizeof(*t->sq_areq) here.

> +       if (!t->sg_areq) {
> +               ret = -ENOMEM;
> +               goto out_free;
> +       }
> +
>         t->dev_addr = mmc_test_capacity(test->card) / 2;
>         t->dev_addr -= t->dev_addr % (t->max_sz >> 9);
>
diff mbox series

Patch

diff --git a/drivers/mmc/core/mmc_test.c b/drivers/mmc/core/mmc_test.c
index 492dd45..f8f884a 100644
--- a/drivers/mmc/core/mmc_test.c
+++ b/drivers/mmc/core/mmc_test.c
@@ -71,6 +71,7 @@  struct mmc_test_mem {
  * @sg_len: length of currently mapped scatterlist @sg
  * @mem: allocated memory
  * @sg: scatterlist
+ * @sg_areq: scatterlist for non-blocking request
  */
 struct mmc_test_area {
 	unsigned long max_sz;
@@ -82,6 +83,7 @@  struct mmc_test_area {
 	unsigned int sg_len;
 	struct mmc_test_mem *mem;
 	struct scatterlist *sg;
+	struct scatterlist *sg_areq;
 };
 
 /**
@@ -836,7 +838,9 @@  static int mmc_test_start_areq(struct mmc_test_card *test,
 }
 
 static int mmc_test_nonblock_transfer(struct mmc_test_card *test,
-				      struct scatterlist *sg, unsigned sg_len,
+				      struct scatterlist *sg,
+				      struct scatterlist *sg_areq,
+				      unsigned int sg_len,
 				      unsigned dev_addr, unsigned blocks,
 				      unsigned blksz, int write, int count)
 {
@@ -867,6 +871,7 @@  static int mmc_test_nonblock_transfer(struct mmc_test_card *test,
 			prev_mrq = &rq2->mrq;
 
 		swap(mrq, prev_mrq);
+		swap(sg, sg_areq);
 		dev_addr += blocks;
 	}
 
@@ -1396,10 +1401,11 @@  static int mmc_test_no_highmem(struct mmc_test_card *test)
  * Map sz bytes so that it can be transferred.
  */
 static int mmc_test_area_map(struct mmc_test_card *test, unsigned long sz,
-			     int max_scatter, int min_sg_len)
+			     int max_scatter, int min_sg_len, bool nonblock)
 {
 	struct mmc_test_area *t = &test->area;
 	int err;
+	unsigned int sg_len = 0;
 
 	t->blocks = sz >> 9;
 
@@ -1411,6 +1417,22 @@  static int mmc_test_area_map(struct mmc_test_card *test, unsigned long sz,
 		err = mmc_test_map_sg(t->mem, sz, t->sg, 1, t->max_segs,
 				      t->max_seg_sz, &t->sg_len, min_sg_len);
 	}
+
+	if (err || !nonblock)
+		goto err;
+
+	if (max_scatter) {
+		err = mmc_test_map_sg_max_scatter(t->mem, sz, t->sg_areq,
+						  t->max_segs, t->max_seg_sz,
+						  &sg_len);
+	} else {
+		err = mmc_test_map_sg(t->mem, sz, t->sg_areq, 1, t->max_segs,
+				      t->max_seg_sz, &sg_len, min_sg_len);
+	}
+	if (!err && sg_len != t->sg_len)
+		err = -EINVAL;
+
+err:
 	if (err)
 		pr_info("%s: Failed to map sg list\n",
 		       mmc_hostname(test->card->host));
@@ -1458,15 +1480,16 @@  static int mmc_test_area_io_seq(struct mmc_test_card *test, unsigned long sz,
 			sz = max_tfr;
 	}
 
-	ret = mmc_test_area_map(test, sz, max_scatter, min_sg_len);
+	ret = mmc_test_area_map(test, sz, max_scatter, min_sg_len, nonblock);
 	if (ret)
 		return ret;
 
 	if (timed)
 		ktime_get_ts64(&ts1);
 	if (nonblock)
-		ret = mmc_test_nonblock_transfer(test, t->sg, t->sg_len,
-				 dev_addr, t->blocks, 512, write, count);
+		ret = mmc_test_nonblock_transfer(test, t->sg, t->sg_areq,
+				 t->sg_len, dev_addr, t->blocks, 512, write,
+				 count);
 	else
 		for (i = 0; i < count && ret == 0; i++) {
 			ret = mmc_test_area_transfer(test, dev_addr, write);
@@ -1525,6 +1548,7 @@  static int mmc_test_area_cleanup(struct mmc_test_card *test)
 	struct mmc_test_area *t = &test->area;
 
 	kfree(t->sg);
+	kfree(t->sg_areq);
 	mmc_test_free_mem(t->mem);
 
 	return 0;
@@ -1584,6 +1608,12 @@  static int mmc_test_area_init(struct mmc_test_card *test, int erase, int fill)
 		goto out_free;
 	}
 
+	t->sg_areq = kmalloc_array(t->max_segs, sizeof(*t->sg), GFP_KERNEL);
+	if (!t->sg_areq) {
+		ret = -ENOMEM;
+		goto out_free;
+	}
+
 	t->dev_addr = mmc_test_capacity(test->card) / 2;
 	t->dev_addr -= t->dev_addr % (t->max_sz >> 9);
 
@@ -2468,7 +2498,7 @@  static int __mmc_test_cmds_during_tfr(struct mmc_test_card *test,
 	if (!(test->card->host->caps & MMC_CAP_CMD_DURING_TFR))
 		return RESULT_UNSUP_HOST;
 
-	ret = mmc_test_area_map(test, sz, 0, 0);
+	ret = mmc_test_area_map(test, sz, 0, 0, use_areq);
 	if (ret)
 		return ret;