Re: [PATCH] mmc: fix async request mechanism for sequential read scenarios
On Sun, Oct 14, 2012 at 6:17 PM, Konstantin Dorfman kdorf...@codeaurora.org wrote: On Thu, 11 Oct 2012 17:19:01 +0200, Per Forlin per.l...@gmail.com wrote: Hello Per, I would like to start with some basic comments. 1. Is this read sequential issue specific to MMC? 2. Or is it common with all other block-drivers that gets data from the block layer (SCSI/SATA etc) ? If (#2) can the issue be addressed inside the block layer instead? BR Per This issue specific to MMC, others block drivers probably not using MMC mechanism for async request (or have more kernel threads for processing incoming blk requests). I think, since MMC actively fetches requests from block layer queue, the solution has nothing to do with block layer context. On Tue, Oct 2, 2012 at 5:39 PM, Konstantin Dorfman kdorf...@codeaurora.org wrote: The main assumption of the async request design is that the file system adds block requests to the block device queue asynchronously without waiting for completion (see the Rationale section of https://wiki.linaro.org/WorkingGroups/Kernel/Specs /StoragePerfMMC-async-req). We found out that in case of sequential read operations this is not the case, due to the read ahead mechanism. Would it be possible to improve this mechanism to achieve the same result? Allow an outstanding read ahead request on top of the current ongoing one. I need to look on this mechanism, but from first glance such behaviour may be result of libc/vfs/fs decisions and too complex comparing to the patch we are talking about. One observation I have made is that if setting the mmc_req_size to half READ_AHEAD changes the way block layer adds request to the MMC queue. Extract from https://wiki.linaro.org/WorkingGroups/Kernel/Specs/StoragePerfMMC-async-req#Unresolved_issues Forcing mmc host driver to set mmc_req_size 64k results in this behaviour. dd if=/dev/mmcblk0 of=/dev/null bs=4k count=256 [mmc_queue_thread] req d955f9b0 blocks 32 [mmc_queue_thread] req (null) blocks 0 [mmc_queue_thread] req (null) blocks 0 [mmc_queue_thread] req d955f9b0 blocks 64 [mmc_queue_thread] req (null) blocks 0 [mmc_queue_thread] req d955f8d8 blocks 128 [mmc_queue_thread] req (null) blocks 0 [mmc_queue_thread] req d955f9b0 blocks 128 [mmc_queue_thread] req d955f800 blocks 128 [mmc_queue_thread] req d955f8d8 blocks 128 [mmc_queue_thread] req d955fec0 blocks 128 [mmc_queue_thread] req d955f800 blocks 128 [mmc_queue_thread] req d955f9b0 blocks 128 [mmc_queue_thread] req d967cd30 blocks 128 This shows that the block layer can add request in a more asynchronous manner. I have not investigate that mechanism enough to say what can be done. Do you have an explanation to why the block layer behaves like this? BR Per -- Konstantin Dorfman, QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mmc: fix async request mechanism for sequential read scenarios
On Thu, 11 Oct 2012 17:19:01 +0200, Per Forlin per.l...@gmail.com wrote: Hello Per, I would like to start with some basic comments. 1. Is this read sequential issue specific to MMC? 2. Or is it common with all other block-drivers that gets data from the block layer (SCSI/SATA etc) ? If (#2) can the issue be addressed inside the block layer instead? BR Per This issue specific to MMC, others block drivers probably not using MMC mechanism for async request (or have more kernel threads for processing incoming blk requests). I think, since MMC actively fetches requests from block layer queue, the solution has nothing to do with block layer context. On Tue, Oct 2, 2012 at 5:39 PM, Konstantin Dorfman kdorf...@codeaurora.org wrote: The main assumption of the async request design is that the file system adds block requests to the block device queue asynchronously without waiting for completion (see the Rationale section of https://wiki.linaro.org/WorkingGroups/Kernel/Specs /StoragePerfMMC-async-req). We found out that in case of sequential read operations this is not the case, due to the read ahead mechanism. Would it be possible to improve this mechanism to achieve the same result? Allow an outstanding read ahead request on top of the current ongoing one. I need to look on this mechanism, but from first glance such behaviour may be result of libc/vfs/fs decisions and too complex comparing to the patch we are talking about. -- Konstantin Dorfman, QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mmc: fix async request mechanism for sequential read scenarios
Hi Konstantin,
Thanks for addressing this issue. I have just started to look at this
patch and I haven't got into the details yet.
I would like to start with some basic comments.
1. Is this read sequential issue specific to MMC?
2. Or is it common with all other block-drivers that gets data from
the block layer (SCSI/SATA etc) ?
If (#2) can the issue be addressed inside the block layer instead?
BR
Per
On Tue, Oct 2, 2012 at 5:39 PM, Konstantin Dorfman
kdorf...@codeaurora.org wrote:
The main assumption of the async request design is that the file
system adds block requests to the block device queue asynchronously
without waiting for completion (see the Rationale section of
https://wiki.linaro.org/WorkingGroups/Kernel/Specs
/StoragePerfMMC-async-req).
We found out that in case of sequential read operations this is not
the case, due to the read ahead mechanism.
Would it be possible to improve this mechanism to achieve the same result?
Allow an outstanding read ahead request on top of the current ongoing one.
When mmcqt reports on completion of a request there should be
a context switch to allow the insertion of the next read ahead BIOs
to the block layer. Since the mmcqd tries to fetch another request
immediately after the completion of the previous request it gets NULL
and starts waiting for the completion of the previous request.
This wait on completion gives the FS the opportunity to insert the next
request but the MMC layer is already blocked on the previous request
completion and is not aware of the new request waiting to be fetched.
This patch fixes the above behavior and allows the async request mechanism
to work in case of sequential read scenarios.
The main idea is to replace the blocking wait for a completion with an
event driven mechanism and add a new event of new_request.
When the block layer notifies the MMC layer on a new request, we check
for the above case where MMC layer is waiting on a previous request
completion and the current request is NULL.
In such a case the new_request event will be triggered to wakeup
the waiting thread. MMC layer will then fetch the new request
and after its preparation will go back to waiting on the completion.
Our tests showed that this fix improves the read sequential throughput
by 16%.
Signed-off-by: Konstantin Dorfman kdorf...@codeaurora.org
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 172a768..cb32d4c 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -112,17 +112,6 @@ struct mmc_blk_data {
static DEFINE_MUTEX(open_lock);
-enum mmc_blk_status {
- MMC_BLK_SUCCESS = 0,
- MMC_BLK_PARTIAL,
- MMC_BLK_CMD_ERR,
- MMC_BLK_RETRY,
- MMC_BLK_ABORT,
- MMC_BLK_DATA_ERR,
- MMC_BLK_ECC_ERR,
- MMC_BLK_NOMEDIUM,
-};
-
module_param(perdev_minors, int, 0444);
MODULE_PARM_DESC(perdev_minors, Minors numbers to allocate per device);
@@ -1224,6 +1213,7 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req
*mqrq,
}
mqrq-mmc_active.mrq = brq-mrq;
+ mqrq-mmc_active.mrq-sync_data = mq-sync_data;
mqrq-mmc_active.err_check = mmc_blk_err_check;
mmc_queue_bounce_pre(mqrq);
@@ -1284,9 +1274,12 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq,
struct request *rqc)
areq = mq-mqrq_cur-mmc_active;
} else
areq = NULL;
- areq = mmc_start_req(card-host, areq, (int *) status);
- if (!areq)
+ areq = mmc_start_data_req(card-host, areq, (int *) status);
+ if (!areq) {
+ if (status == MMC_BLK_NEW_PACKET)
+ return status;
return 0;
+ }
mq_rq = container_of(areq, struct mmc_queue_req, mmc_active);
brq = mq_rq-brq;
@@ -1295,6 +1288,9 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq,
struct request *rqc)
mmc_queue_bounce_post(mq_rq);
switch (status) {
+ case MMC_BLK_NEW_PACKET:
+ BUG_ON(1); /* should never get here */
+ return MMC_BLK_NEW_PACKET;
case MMC_BLK_SUCCESS:
case MMC_BLK_PARTIAL:
/*
@@ -1367,7 +1363,8 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq,
struct request *rqc)
* prepare it again and resend.
*/
mmc_blk_rw_rq_prep(mq_rq, card, disable_multi, mq);
- mmc_start_req(card-host, mq_rq-mmc_active, NULL);
+ mmc_start_req(card-host, mq_rq-mmc_active,
+ (int *) status);
}
} while (ret);
@@ -1382,7 +1379,7 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq,
struct request
[PATCH] mmc: fix async request mechanism for sequential read scenarios
The main assumption of the async request design is that the file system adds block requests to the block device queue asynchronously without waiting for completion (see the Rationale section of https://wiki.linaro.org/WorkingGroups/Kernel/Specs/StoragePerfMMC-async-req). We found out that in case of sequential read operations this is not the case, due to the read ahead mechanism. When mmcqt reports on completion of a request there should be a context switch to allow the insertion of the next read ahead BIOs to the block layer. Since the mmcqd tries to fetch another request immediately after the completion of the previous request it gets NULL and starts waiting for the completion of the previous request. This wait on completion gives the FS the opportunity to insert the next request but the MMC layer is already blocked on the previous request completion and is not aware of the new request waiting to be fetched. This patch fixes the above behavior and allows the async request mechanism to work in case of sequential read scenarios. The main idea is to replace the blocking wait for a completion with an event driven mechanism and add a new event of new_request. When the block layer notifies the MMC layer on a new request, we check for the above case where MMC layer is waiting on a previous request completion and the current request is NULL. In such a case the new_request event will be triggered to wakeup the waiting thread. MMC layer will then fetch the new request and after its preparation will go back to waiting on the completion. Our tests showed that this fix improves the read sequential throughput by 16%. Konstantin Dorfman (1): mmc: fix async request mechanism for sequential read scenarios drivers/mmc/card/block.c | 30 - drivers/mmc/card/queue.c | 34 - drivers/mmc/card/queue.h |1 + drivers/mmc/core/core.c | 165 ++ include/linux/mmc/card.h | 12 include/linux/mmc/core.h | 21 ++ 6 files changed, 243 insertions(+), 20 deletions(-) -- 1.7.6 -- Konstantin Dorfman, QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] mmc: fix async request mechanism for sequential read scenarios
The main assumption of the async request design is that the file
system adds block requests to the block device queue asynchronously
without waiting for completion (see the Rationale section of
https://wiki.linaro.org/WorkingGroups/Kernel/Specs
/StoragePerfMMC-async-req).
We found out that in case of sequential read operations this is not
the case, due to the read ahead mechanism.
When mmcqt reports on completion of a request there should be
a context switch to allow the insertion of the next read ahead BIOs
to the block layer. Since the mmcqd tries to fetch another request
immediately after the completion of the previous request it gets NULL
and starts waiting for the completion of the previous request.
This wait on completion gives the FS the opportunity to insert the next
request but the MMC layer is already blocked on the previous request
completion and is not aware of the new request waiting to be fetched.
This patch fixes the above behavior and allows the async request mechanism
to work in case of sequential read scenarios.
The main idea is to replace the blocking wait for a completion with an
event driven mechanism and add a new event of new_request.
When the block layer notifies the MMC layer on a new request, we check
for the above case where MMC layer is waiting on a previous request
completion and the current request is NULL.
In such a case the new_request event will be triggered to wakeup
the waiting thread. MMC layer will then fetch the new request
and after its preparation will go back to waiting on the completion.
Our tests showed that this fix improves the read sequential throughput
by 16%.
Signed-off-by: Konstantin Dorfman kdorf...@codeaurora.org
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 172a768..cb32d4c 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -112,17 +112,6 @@ struct mmc_blk_data {
static DEFINE_MUTEX(open_lock);
-enum mmc_blk_status {
- MMC_BLK_SUCCESS = 0,
- MMC_BLK_PARTIAL,
- MMC_BLK_CMD_ERR,
- MMC_BLK_RETRY,
- MMC_BLK_ABORT,
- MMC_BLK_DATA_ERR,
- MMC_BLK_ECC_ERR,
- MMC_BLK_NOMEDIUM,
-};
-
module_param(perdev_minors, int, 0444);
MODULE_PARM_DESC(perdev_minors, Minors numbers to allocate per device);
@@ -1224,6 +1213,7 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
}
mqrq-mmc_active.mrq = brq-mrq;
+ mqrq-mmc_active.mrq-sync_data = mq-sync_data;
mqrq-mmc_active.err_check = mmc_blk_err_check;
mmc_queue_bounce_pre(mqrq);
@@ -1284,9 +1274,12 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq,
struct request *rqc)
areq = mq-mqrq_cur-mmc_active;
} else
areq = NULL;
- areq = mmc_start_req(card-host, areq, (int *) status);
- if (!areq)
+ areq = mmc_start_data_req(card-host, areq, (int *) status);
+ if (!areq) {
+ if (status == MMC_BLK_NEW_PACKET)
+ return status;
return 0;
+ }
mq_rq = container_of(areq, struct mmc_queue_req, mmc_active);
brq = mq_rq-brq;
@@ -1295,6 +1288,9 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq,
struct request *rqc)
mmc_queue_bounce_post(mq_rq);
switch (status) {
+ case MMC_BLK_NEW_PACKET:
+ BUG_ON(1); /* should never get here */
+ return MMC_BLK_NEW_PACKET;
case MMC_BLK_SUCCESS:
case MMC_BLK_PARTIAL:
/*
@@ -1367,7 +1363,8 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq,
struct request *rqc)
* prepare it again and resend.
*/
mmc_blk_rw_rq_prep(mq_rq, card, disable_multi, mq);
- mmc_start_req(card-host, mq_rq-mmc_active, NULL);
+ mmc_start_req(card-host, mq_rq-mmc_active,
+ (int *) status);
}
} while (ret);
@@ -1382,7 +1379,7 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq,
struct request *rqc)
start_new_req:
if (rqc) {
mmc_blk_rw_rq_prep(mq-mqrq_cur, card, 0, mq);
- mmc_start_req(card-host, mq-mqrq_cur-mmc_active, NULL);
+ mmc_start_data_req(card-host, mq-mqrq_cur-mmc_active, NULL);
}
return 0;
@@ -1426,9 +1423,10 @@ static int mmc_blk_issue_rq(struct mmc_queue *mq, struct
request *req)
}
out:
- if (!req)
+ if (!req (ret != MMC_BLK_NEW_PACKET))
/* release host only when there are no more requests */
mmc_release_host(card-host);
+
return ret;
}
diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
index e360a97..71e2610 100644
---
