| Message ID | 1582105474-27866-1-git-send-email-vbadigan@codeaurora.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [V2] mmc: mmc_test: Pass different sg lists for non-blocking requests | expand |
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
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 --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;
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(-)