2020-02-11 09:40:29

by Veerabhadrarao Badiganti

[permalink] [raw]
Subject: [PATCH V1] mmc: mmc_test: Pass different sg lists for non-blocking requests

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 <[email protected]>
---
drivers/mmc/core/mmc_test.c | 38 ++++++++++++++++++++++++++++++++------
1 file changed, 32 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/core/mmc_test.c b/drivers/mmc/core/mmc_test.c
index 492dd45..69bdf60 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,7 +1401,7 @@ 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;
@@ -1411,6 +1416,20 @@ 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,
+ &t->sg_len);
+ } else {
+ err = mmc_test_map_sg(t->mem, sz, t->sg_areq, 1, t->max_segs,
+ t->max_seg_sz, &t->sg_len, min_sg_len);
+ }
+
+err:
if (err)
pr_info("%s: Failed to map sg list\n",
mmc_hostname(test->card->host));
@@ -1458,15 +1477,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);
@@ -1584,6 +1604,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 +2494,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;

--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project


2020-02-11 11:45:15

by Veerabhadrarao Badiganti

[permalink] [raw]
Subject: Re: [PATCH V1] mmc: mmc_test: Pass different sg lists for non-blocking requests

Attaching the full error log

On 2/11/2020 3:06 PM, 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 <[email protected]>
> ---
> drivers/mmc/core/mmc_test.c | 38 ++++++++++++++++++++++++++++++++------
> 1 file changed, 32 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc_test.c b/drivers/mmc/core/mmc_test.c
> index 492dd45..69bdf60 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,7 +1401,7 @@ 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;
> @@ -1411,6 +1416,20 @@ 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,
> + &t->sg_len);
> + } else {
> + err = mmc_test_map_sg(t->mem, sz, t->sg_areq, 1, t->max_segs,
> + t->max_seg_sz, &t->sg_len, min_sg_len);
> + }
> +
> +err:
> if (err)
> pr_info("%s: Failed to map sg list\n",
> mmc_hostname(test->card->host));
> @@ -1458,15 +1477,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);
> @@ -1584,6 +1604,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 +2494,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;
>


Attachments:
mmc_test_areq_iommu_error.txt (4.54 kB)

2020-02-18 08:56:43

by Adrian Hunter

[permalink] [raw]
Subject: Re: [PATCH V1] mmc: mmc_test: Pass different sg lists for non-blocking requests

On 11/02/20 11:36 am, 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 <[email protected]>
> ---
> drivers/mmc/core/mmc_test.c | 38 ++++++++++++++++++++++++++++++++------
> 1 file changed, 32 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc_test.c b/drivers/mmc/core/mmc_test.c
> index 492dd45..69bdf60 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,7 +1401,7 @@ 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;
> @@ -1411,6 +1416,20 @@ 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,
> + &t->sg_len);
> + } else {
> + err = mmc_test_map_sg(t->mem, sz, t->sg_areq, 1, t->max_segs,
> + t->max_seg_sz, &t->sg_len, min_sg_len);
> + }

I suggest getting the length to a local and confirming it is the same

+ 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 +1477,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);
> @@ -1584,6 +1604,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);

Needs to be freed in mmc_test_area_cleanup()

> + 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 +2494,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;
>
>

2020-02-19 09:45:29

by Veerabhadrarao Badiganti

[permalink] [raw]
Subject: [PATCH V2] mmc: mmc_test: Pass different sg lists for non-blocking requests

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 <[email protected]>

---
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
@@ -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;

--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

2020-02-20 08:40:34

by Sai Prakash Ranjan

[permalink] [raw]
Subject: Re: [PATCH V2] mmc: mmc_test: Pass different sg lists for non-blocking requests

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 <[email protected]>
>

Tested-by: Sai Prakash Ranjan <[email protected]>

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

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation

2020-02-21 01:26:35

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH V2] mmc: mmc_test: Pass different sg lists for non-blocking requests

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 <[email protected]>
>
> ---

Reviewed-by: Stephen Boyd <[email protected]>

> 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);
>

2020-02-26 10:58:48

by Veerabhadrarao Badiganti

[permalink] [raw]
Subject: [PATCH V3] mmc: mmc_test: Pass different sg lists for non-blocking requests

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 <[email protected]>
Reviewed-by: Stephen Boyd <[email protected]>
Tested-by: Sai Prakash Ranjan <[email protected]>
---
Changes since V2:
- Simplfied mmc_test_nonblock_transter() function aruguments.

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 | 52 ++++++++++++++++++++++++++++++++++-----------
1 file changed, 40 insertions(+), 12 deletions(-)

diff --git a/drivers/mmc/core/mmc_test.c b/drivers/mmc/core/mmc_test.c
index 492dd45..c21b3cb 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,14 +838,16 @@ 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,
- unsigned dev_addr, unsigned blocks,
- unsigned blksz, int write, int count)
+ unsigned int dev_addr, int write,
+ int count)
{
struct mmc_test_req *rq1, *rq2;
struct mmc_request *mrq, *prev_mrq;
int i;
int ret = RESULT_OK;
+ struct mmc_test_area *t = &test->area;
+ struct scatterlist *sg = t->sg;
+ struct scatterlist *sg_areq = t->sg_areq;

rq1 = mmc_test_req_alloc();
rq2 = mmc_test_req_alloc();
@@ -857,8 +861,8 @@ static int mmc_test_nonblock_transfer(struct mmc_test_card *test,

for (i = 0; i < count; i++) {
mmc_test_req_reset(container_of(mrq, struct mmc_test_req, mrq));
- mmc_test_prepare_mrq(test, mrq, sg, sg_len, dev_addr, blocks,
- blksz, write);
+ mmc_test_prepare_mrq(test, mrq, sg, t->sg_len, dev_addr,
+ t->blocks, 512, write);
ret = mmc_test_start_areq(test, mrq, prev_mrq);
if (ret)
goto err;
@@ -867,7 +871,8 @@ static int mmc_test_nonblock_transfer(struct mmc_test_card *test,
prev_mrq = &rq2->mrq;

swap(mrq, prev_mrq);
- dev_addr += blocks;
+ swap(sg, sg_areq);
+ dev_addr += t->blocks;
}

ret = mmc_test_start_areq(test, NULL, prev_mrq);
@@ -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));
@@ -1440,7 +1462,6 @@ static int mmc_test_area_io_seq(struct mmc_test_card *test, unsigned long sz,
struct timespec64 ts1, ts2;
int ret = 0;
int i;
- struct mmc_test_area *t = &test->area;

/*
* In the case of a maximally scattered transfer, the maximum transfer
@@ -1458,15 +1479,14 @@ 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, dev_addr, write, count);
else
for (i = 0; i < count && ret == 0; i++) {
ret = mmc_test_area_transfer(test, dev_addr, write);
@@ -1525,6 +1545,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 +1605,13 @@ 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_areq),
+ 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 +2496,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;

--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

2020-02-27 12:58:08

by Adrian Hunter

[permalink] [raw]
Subject: Re: [PATCH V3] mmc: mmc_test: Pass different sg lists for non-blocking requests

On 26/02/20 12:57 pm, 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 <[email protected]>
> Reviewed-by: Stephen Boyd <[email protected]>
> Tested-by: Sai Prakash Ranjan <[email protected]>

Acked-by: Adrian Hunter <[email protected]>

> ---
> Changes since V2:
> - Simplfied mmc_test_nonblock_transter() function aruguments.
>
> 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 | 52 ++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 40 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc_test.c b/drivers/mmc/core/mmc_test.c
> index 492dd45..c21b3cb 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,14 +838,16 @@ 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,
> - unsigned dev_addr, unsigned blocks,
> - unsigned blksz, int write, int count)
> + unsigned int dev_addr, int write,
> + int count)
> {
> struct mmc_test_req *rq1, *rq2;
> struct mmc_request *mrq, *prev_mrq;
> int i;
> int ret = RESULT_OK;
> + struct mmc_test_area *t = &test->area;
> + struct scatterlist *sg = t->sg;
> + struct scatterlist *sg_areq = t->sg_areq;
>
> rq1 = mmc_test_req_alloc();
> rq2 = mmc_test_req_alloc();
> @@ -857,8 +861,8 @@ static int mmc_test_nonblock_transfer(struct mmc_test_card *test,
>
> for (i = 0; i < count; i++) {
> mmc_test_req_reset(container_of(mrq, struct mmc_test_req, mrq));
> - mmc_test_prepare_mrq(test, mrq, sg, sg_len, dev_addr, blocks,
> - blksz, write);
> + mmc_test_prepare_mrq(test, mrq, sg, t->sg_len, dev_addr,
> + t->blocks, 512, write);
> ret = mmc_test_start_areq(test, mrq, prev_mrq);
> if (ret)
> goto err;
> @@ -867,7 +871,8 @@ static int mmc_test_nonblock_transfer(struct mmc_test_card *test,
> prev_mrq = &rq2->mrq;
>
> swap(mrq, prev_mrq);
> - dev_addr += blocks;
> + swap(sg, sg_areq);
> + dev_addr += t->blocks;
> }
>
> ret = mmc_test_start_areq(test, NULL, prev_mrq);
> @@ -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));
> @@ -1440,7 +1462,6 @@ static int mmc_test_area_io_seq(struct mmc_test_card *test, unsigned long sz,
> struct timespec64 ts1, ts2;
> int ret = 0;
> int i;
> - struct mmc_test_area *t = &test->area;
>
> /*
> * In the case of a maximally scattered transfer, the maximum transfer
> @@ -1458,15 +1479,14 @@ 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, dev_addr, write, count);
> else
> for (i = 0; i < count && ret == 0; i++) {
> ret = mmc_test_area_transfer(test, dev_addr, write);
> @@ -1525,6 +1545,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 +1605,13 @@ 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_areq),
> + 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 +2496,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;
>
>

2020-03-04 15:37:20

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH V3] mmc: mmc_test: Pass different sg lists for non-blocking requests

On Wed, 26 Feb 2020 at 11:58, Veerabhadrarao Badiganti
<[email protected]> 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 <[email protected]>
> Reviewed-by: Stephen Boyd <[email protected]>
> Tested-by: Sai Prakash Ranjan <[email protected]>

Applied for next, thanks!

Kind regards
Uffe


> ---
> Changes since V2:
> - Simplfied mmc_test_nonblock_transter() function aruguments.
>
> 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 | 52 ++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 40 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc_test.c b/drivers/mmc/core/mmc_test.c
> index 492dd45..c21b3cb 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,14 +838,16 @@ 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,
> - unsigned dev_addr, unsigned blocks,
> - unsigned blksz, int write, int count)
> + unsigned int dev_addr, int write,
> + int count)
> {
> struct mmc_test_req *rq1, *rq2;
> struct mmc_request *mrq, *prev_mrq;
> int i;
> int ret = RESULT_OK;
> + struct mmc_test_area *t = &test->area;
> + struct scatterlist *sg = t->sg;
> + struct scatterlist *sg_areq = t->sg_areq;
>
> rq1 = mmc_test_req_alloc();
> rq2 = mmc_test_req_alloc();
> @@ -857,8 +861,8 @@ static int mmc_test_nonblock_transfer(struct mmc_test_card *test,
>
> for (i = 0; i < count; i++) {
> mmc_test_req_reset(container_of(mrq, struct mmc_test_req, mrq));
> - mmc_test_prepare_mrq(test, mrq, sg, sg_len, dev_addr, blocks,
> - blksz, write);
> + mmc_test_prepare_mrq(test, mrq, sg, t->sg_len, dev_addr,
> + t->blocks, 512, write);
> ret = mmc_test_start_areq(test, mrq, prev_mrq);
> if (ret)
> goto err;
> @@ -867,7 +871,8 @@ static int mmc_test_nonblock_transfer(struct mmc_test_card *test,
> prev_mrq = &rq2->mrq;
>
> swap(mrq, prev_mrq);
> - dev_addr += blocks;
> + swap(sg, sg_areq);
> + dev_addr += t->blocks;
> }
>
> ret = mmc_test_start_areq(test, NULL, prev_mrq);
> @@ -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));
> @@ -1440,7 +1462,6 @@ static int mmc_test_area_io_seq(struct mmc_test_card *test, unsigned long sz,
> struct timespec64 ts1, ts2;
> int ret = 0;
> int i;
> - struct mmc_test_area *t = &test->area;
>
> /*
> * In the case of a maximally scattered transfer, the maximum transfer
> @@ -1458,15 +1479,14 @@ 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, dev_addr, write, count);
> else
> for (i = 0; i < count && ret == 0; i++) {
> ret = mmc_test_area_transfer(test, dev_addr, write);
> @@ -1525,6 +1545,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 +1605,13 @@ 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_areq),
> + 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 +2496,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;
>
> --
> Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project