* [PATCH 01/16] mmc: core: move some code in mmc_start_areq()
2017-02-09 15:33 [PATCH 00/16] multiqueue for MMC/SD third try Linus Walleij
@ 2017-02-09 15:33 ` Linus Walleij
[not found] ` <CGME20170228145506epcas1p1dd72cc5738c3f36df97ac06603ad2731@epcas1p1.samsung.com>
2017-02-09 15:33 ` [PATCH 02/16] mmc: core: refactor asynchronous request finalization Linus Walleij
` (16 subsequent siblings)
17 siblings, 1 reply; 51+ messages in thread
From: Linus Walleij @ 2017-02-09 15:33 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson, Adrian Hunter, Paolo Valente
Cc: Chunyan Zhang, Baolin Wang, linux-block, Jens Axboe,
Christoph Hellwig, Arnd Bergmann, Linus Walleij
"previous" is a better name for the variable storing the previous
asynchronous request, better than the opaque name "data" atleast.
We see that we assign the return status to the returned variable
on all code paths, so we might as well just do that immediately
after calling mmc_finalize_areq().
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/mmc/core/core.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 41b4cd01fccc..53065d1cebf7 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -683,7 +683,7 @@ struct mmc_async_req *mmc_start_areq(struct mmc_host *host,
{
enum mmc_blk_status status;
int start_err = 0;
- struct mmc_async_req *data = host->areq;
+ struct mmc_async_req *previous = host->areq;
/* Prepare a new request */
if (areq)
@@ -691,13 +691,12 @@ struct mmc_async_req *mmc_start_areq(struct mmc_host *host,
/* Finalize previous request */
status = mmc_finalize_areq(host);
+ if (ret_stat)
+ *ret_stat = status;
/* The previous request is still going on... */
- if (status == MMC_BLK_NEW_REQUEST) {
- if (ret_stat)
- *ret_stat = status;
+ if (status == MMC_BLK_NEW_REQUEST)
return NULL;
- }
/* Fine so far, start the new request! */
if (status == MMC_BLK_SUCCESS && areq)
@@ -716,9 +715,7 @@ struct mmc_async_req *mmc_start_areq(struct mmc_host *host,
else
host->areq = areq;
- if (ret_stat)
- *ret_stat = status;
- return data;
+ return previous;
}
EXPORT_SYMBOL(mmc_start_areq);
--
2.9.3
^ permalink raw reply related [flat|nested] 51+ messages in thread* [PATCH 02/16] mmc: core: refactor asynchronous request finalization
2017-02-09 15:33 [PATCH 00/16] multiqueue for MMC/SD third try Linus Walleij
2017-02-09 15:33 ` [PATCH 01/16] mmc: core: move some code in mmc_start_areq() Linus Walleij
@ 2017-02-09 15:33 ` Linus Walleij
[not found] ` <CGME20170228145552epcas5p4a43c23971d58b30ad6ab9d2c612abe9a@epcas5p4.samsung.com>
2017-02-09 15:33 ` [PATCH 03/16] mmc: core: refactor mmc_request_done() Linus Walleij
` (15 subsequent siblings)
17 siblings, 1 reply; 51+ messages in thread
From: Linus Walleij @ 2017-02-09 15:33 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson, Adrian Hunter, Paolo Valente
Cc: Chunyan Zhang, Baolin Wang, linux-block, Jens Axboe,
Christoph Hellwig, Arnd Bergmann, Linus Walleij
mmc_wait_for_data_req_done() is called in exactly one place,
and having it spread out is making things hard to oversee.
Factor this function into mmc_finalize_areq().
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/mmc/core/core.c | 86 +++++++++++++++++++------------------------------
1 file changed, 33 insertions(+), 53 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 53065d1cebf7..b2e7a6dfcbf0 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -485,56 +485,6 @@ static int __mmc_start_req(struct mmc_host *host, struct mmc_request *mrq)
return err;
}
-/*
- * mmc_wait_for_data_req_done() - wait for request completed
- * @host: MMC host to prepare the command.
- * @mrq: MMC request to wait for
- *
- * Blocks MMC context till host controller will ack end of data request
- * execution or new request notification arrives from the block layer.
- * Handles command retries.
- *
- * Returns enum mmc_blk_status after checking errors.
- */
-static enum mmc_blk_status mmc_wait_for_data_req_done(struct mmc_host *host,
- struct mmc_request *mrq)
-{
- struct mmc_command *cmd;
- struct mmc_context_info *context_info = &host->context_info;
- enum mmc_blk_status status;
-
- while (1) {
- wait_event_interruptible(context_info->wait,
- (context_info->is_done_rcv ||
- context_info->is_new_req));
-
- if (context_info->is_done_rcv) {
- context_info->is_done_rcv = false;
- cmd = mrq->cmd;
-
- if (!cmd->error || !cmd->retries ||
- mmc_card_removed(host->card)) {
- status = host->areq->err_check(host->card,
- host->areq);
- break; /* return status */
- } else {
- mmc_retune_recheck(host);
- pr_info("%s: req failed (CMD%u): %d, retrying...\n",
- mmc_hostname(host),
- cmd->opcode, cmd->error);
- cmd->retries--;
- cmd->error = 0;
- __mmc_start_request(host, mrq);
- continue; /* wait for done/new event again */
- }
- }
-
- return MMC_BLK_NEW_REQUEST;
- }
- mmc_retune_release(host);
- return status;
-}
-
void mmc_wait_for_req_done(struct mmc_host *host, struct mmc_request *mrq)
{
struct mmc_command *cmd;
@@ -639,14 +589,44 @@ static void mmc_post_req(struct mmc_host *host, struct mmc_request *mrq,
*/
static enum mmc_blk_status mmc_finalize_areq(struct mmc_host *host)
{
+ struct mmc_context_info *context_info = &host->context_info;
enum mmc_blk_status status;
if (!host->areq)
return MMC_BLK_SUCCESS;
- status = mmc_wait_for_data_req_done(host, host->areq->mrq);
- if (status == MMC_BLK_NEW_REQUEST)
- return status;
+ while (1) {
+ wait_event_interruptible(context_info->wait,
+ (context_info->is_done_rcv ||
+ context_info->is_new_req));
+
+ if (context_info->is_done_rcv) {
+ struct mmc_command *cmd;
+
+ context_info->is_done_rcv = false;
+ cmd = host->areq->mrq->cmd;
+
+ if (!cmd->error || !cmd->retries ||
+ mmc_card_removed(host->card)) {
+ status = host->areq->err_check(host->card,
+ host->areq);
+ break; /* return status */
+ } else {
+ mmc_retune_recheck(host);
+ pr_info("%s: req failed (CMD%u): %d, retrying...\n",
+ mmc_hostname(host),
+ cmd->opcode, cmd->error);
+ cmd->retries--;
+ cmd->error = 0;
+ __mmc_start_request(host, host->areq->mrq);
+ continue; /* wait for done/new event again */
+ }
+ }
+
+ return MMC_BLK_NEW_REQUEST;
+ }
+
+ mmc_retune_release(host);
/*
* Check BKOPS urgency for each R1 response
--
2.9.3
^ permalink raw reply related [flat|nested] 51+ messages in thread* [PATCH 03/16] mmc: core: refactor mmc_request_done()
2017-02-09 15:33 [PATCH 00/16] multiqueue for MMC/SD third try Linus Walleij
2017-02-09 15:33 ` [PATCH 01/16] mmc: core: move some code in mmc_start_areq() Linus Walleij
2017-02-09 15:33 ` [PATCH 02/16] mmc: core: refactor asynchronous request finalization Linus Walleij
@ 2017-02-09 15:33 ` Linus Walleij
[not found] ` <CGME20170228145627epcas1p18fb6390b7ae14a6961fac9703712e0a0@epcas1p1.samsung.com>
2017-02-09 15:33 ` [PATCH 04/16] mmc: core: move the asynchronous post-processing Linus Walleij
` (14 subsequent siblings)
17 siblings, 1 reply; 51+ messages in thread
From: Linus Walleij @ 2017-02-09 15:33 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson, Adrian Hunter, Paolo Valente
Cc: Chunyan Zhang, Baolin Wang, linux-block, Jens Axboe,
Christoph Hellwig, Arnd Bergmann, Linus Walleij
We have this construction:
if (a && b && !c)
finalize;
else
block;
finalize;
Which is equivalent by boolean logic to:
if (!a || !b || c)
block;
finalize;
Which is simpler code.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/mmc/core/core.c | 27 ++++++++++++++++-----------
1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index b2e7a6dfcbf0..8dbed198750f 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -172,14 +172,16 @@ void mmc_request_done(struct mmc_host *host, struct mmc_request *mrq)
trace_mmc_request_done(host, mrq);
- if (err && cmd->retries && !mmc_card_removed(host->card)) {
- /*
- * Request starter must handle retries - see
- * mmc_wait_for_req_done().
- */
- if (mrq->done)
- mrq->done(mrq);
- } else {
+ /*
+ * We list various conditions for the command to be considered
+ * properly done:
+ *
+ * - There was no error, OK fine then
+ * - We are not doing some kind of retry
+ * - The card was removed (...so just complete everything no matter
+ * if there are errors or retries)
+ */
+ if (!err || !cmd->retries || mmc_card_removed(host->card)) {
mmc_should_fail_request(host, mrq);
if (!host->ongoing_mrq)
@@ -211,10 +213,13 @@ void mmc_request_done(struct mmc_host *host, struct mmc_request *mrq)
mrq->stop->resp[0], mrq->stop->resp[1],
mrq->stop->resp[2], mrq->stop->resp[3]);
}
-
- if (mrq->done)
- mrq->done(mrq);
}
+ /*
+ * Request starter must handle retries - see
+ * mmc_wait_for_req_done().
+ */
+ if (mrq->done)
+ mrq->done(mrq);
}
EXPORT_SYMBOL(mmc_request_done);
--
2.9.3
^ permalink raw reply related [flat|nested] 51+ messages in thread* [PATCH 04/16] mmc: core: move the asynchronous post-processing
2017-02-09 15:33 [PATCH 00/16] multiqueue for MMC/SD third try Linus Walleij
` (2 preceding siblings ...)
2017-02-09 15:33 ` [PATCH 03/16] mmc: core: refactor mmc_request_done() Linus Walleij
@ 2017-02-09 15:33 ` Linus Walleij
2017-02-09 15:33 ` [PATCH 05/16] mmc: core: add a kthread for completing requests Linus Walleij
` (13 subsequent siblings)
17 siblings, 0 replies; 51+ messages in thread
From: Linus Walleij @ 2017-02-09 15:33 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson, Adrian Hunter, Paolo Valente
Cc: Chunyan Zhang, Baolin Wang, linux-block, Jens Axboe,
Christoph Hellwig, Arnd Bergmann, Linus Walleij
This moves the asynchronous post-processing of a request over
to the finalization function.
The patch has a slight semantic change:
Both places will be in the code path for if (host->areq) and
in the same sequence, but before this patch, the next request
was started before performing post-processing.
The effect is that whereas before, the post- and preprocessing
happened after starting the next request, now the preprocessing
will happen after the request is done and before the next has
started which would cut half of the pre/post optimizations out.
The idea is to later move the finalization to a worker started
by mmc_request_done() and introduce a completion where the code
now has a TODO comment so that we can push in a new request
as soon as the host has completed the previous one.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/mmc/core/core.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 8dbed198750f..0972c649ea7a 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -643,6 +643,9 @@ static enum mmc_blk_status mmc_finalize_areq(struct mmc_host *host)
mmc_start_bkops(host->card, true);
}
+ /* Successfully postprocess the old request at this point */
+ mmc_post_req(host, host->areq->mrq, 0);
+
return status;
}
@@ -687,10 +690,6 @@ struct mmc_async_req *mmc_start_areq(struct mmc_host *host,
if (status == MMC_BLK_SUCCESS && areq)
start_err = __mmc_start_data_req(host, areq->mrq);
- /* Postprocess the old request at this point */
- if (host->areq)
- mmc_post_req(host, host->areq->mrq, 0);
-
/* Cancel a prepared request if it was not started. */
if ((status != MMC_BLK_SUCCESS || start_err) && areq)
mmc_post_req(host, areq->mrq, -EINVAL);
--
2.9.3
^ permalink raw reply related [flat|nested] 51+ messages in thread* [PATCH 05/16] mmc: core: add a kthread for completing requests
2017-02-09 15:33 [PATCH 00/16] multiqueue for MMC/SD third try Linus Walleij
` (3 preceding siblings ...)
2017-02-09 15:33 ` [PATCH 04/16] mmc: core: move the asynchronous post-processing Linus Walleij
@ 2017-02-09 15:33 ` Linus Walleij
[not found] ` <CGME20170228145719epcas5p33d013fd48483bfba477b3f607dcdccb4@epcas5p3.samsung.com>
2017-02-09 15:33 ` [PATCH 06/16] mmc: core: replace waitqueue with worker Linus Walleij
` (12 subsequent siblings)
17 siblings, 1 reply; 51+ messages in thread
From: Linus Walleij @ 2017-02-09 15:33 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson, Adrian Hunter, Paolo Valente
Cc: Chunyan Zhang, Baolin Wang, linux-block, Jens Axboe,
Christoph Hellwig, Arnd Bergmann, Linus Walleij
As we want to complete requests autonomously from feeding the
host with new requests, we create a worker thread to deal with
this specifically in response to the callback from a host driver.
This patch just adds the worker, later patches will make use of
it.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/mmc/core/core.c | 6 ++++++
drivers/mmc/core/host.c | 2 +-
include/linux/mmc/host.h | 5 +++++
3 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 0972c649ea7a..663799240635 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2794,6 +2794,10 @@ void mmc_start_host(struct mmc_host *host)
host->f_init = max(freqs[0], host->f_min);
host->rescan_disable = 0;
host->ios.power_mode = MMC_POWER_UNDEFINED;
+ /* Worker for completing requests */
+ host->req_done_worker_task = kthread_run(kthread_worker_fn,
+ &host->req_done_worker,
+ "mmc%d-reqdone", host->index);
if (!(host->caps2 & MMC_CAP2_NO_PRESCAN_POWERUP)) {
mmc_claim_host(host);
@@ -2818,6 +2822,8 @@ void mmc_stop_host(struct mmc_host *host)
host->rescan_disable = 1;
cancel_delayed_work_sync(&host->detect);
+ kthread_flush_worker(&host->req_done_worker);
+ kthread_stop(host->req_done_worker_task);
/* clear pm flags now and let card drivers set them as needed */
host->pm_flags = 0;
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 98f25ffb4258..d33e2b260bf3 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -388,7 +388,7 @@ struct mmc_host *mmc_alloc_host(int extra, struct device *dev)
init_waitqueue_head(&host->wq);
INIT_DELAYED_WORK(&host->detect, mmc_rescan);
setup_timer(&host->retune_timer, mmc_retune_timer, (unsigned long)host);
-
+ kthread_init_worker(&host->req_done_worker);
/*
* By default, hosts do not support SGIO or large requests.
* They have to set these according to their abilities.
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 97699d55b2ae..b04f8cd51c82 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -13,6 +13,7 @@
#include <linux/sched.h>
#include <linux/device.h>
#include <linux/fault-inject.h>
+#include <linux/kthread.h>
#include <linux/mmc/core.h>
#include <linux/mmc/card.h>
@@ -375,6 +376,10 @@ struct mmc_host {
struct mmc_async_req *areq; /* active async req */
struct mmc_context_info context_info; /* async synchronization info */
+ /* finalization work thread, handles finalizing requests */
+ struct kthread_worker req_done_worker;
+ struct task_struct *req_done_worker_task;
+
/* Ongoing data transfer that allows commands during transfer */
struct mmc_request *ongoing_mrq;
--
2.9.3
^ permalink raw reply related [flat|nested] 51+ messages in thread* [PATCH 06/16] mmc: core: replace waitqueue with worker
2017-02-09 15:33 [PATCH 00/16] multiqueue for MMC/SD third try Linus Walleij
` (4 preceding siblings ...)
2017-02-09 15:33 ` [PATCH 05/16] mmc: core: add a kthread for completing requests Linus Walleij
@ 2017-02-09 15:33 ` Linus Walleij
2017-02-22 13:29 ` Adrian Hunter
[not found] ` <CGME20170228161023epcas5p3916c2e171d57b8c7814be7841fbab3aa@epcas5p3.samsung.com>
2017-02-09 15:33 ` [PATCH 07/16] mmc: core: do away with is_done_rcv Linus Walleij
` (11 subsequent siblings)
17 siblings, 2 replies; 51+ messages in thread
From: Linus Walleij @ 2017-02-09 15:33 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson, Adrian Hunter, Paolo Valente
Cc: Chunyan Zhang, Baolin Wang, linux-block, Jens Axboe,
Christoph Hellwig, Arnd Bergmann, Linus Walleij
The waitqueue in the host context is there to signal back from
mmc_request_done() through mmc_wait_data_done() that the hardware
is done with a command, and when the wait is over, the core
will typically submit the next asynchronous request that is pending
just waiting for the hardware to be available.
This is in the way for letting the mmc_request_done() trigger the
report up to the block layer that a block request is finished.
Re-jig this as a first step, remvoving the waitqueue and introducing
a work that will run after a completed asynchronous request,
finalizing that request, including retransmissions, and eventually
reporting back with a completion and a status code to the
asynchronous issue method.
This had the upside that we can remove the MMC_BLK_NEW_REQUEST
status code and the "new_request" state in the request queue
that is only there to make the state machine spin out
the first time we send a request.
Introduce a workqueue in the host for handling just this, and
then a work and completion in the asynchronous request to deal
with this mechanism.
This is a central change that let us do many other changes since
we have broken the submit and complete code paths in two, and we
can potentially remove the NULL flushing of the asynchronous
pipeline and report block requests as finished directly from
the worker.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/mmc/core/block.c | 7 ++--
drivers/mmc/core/core.c | 84 +++++++++++++++++++++++-------------------------
drivers/mmc/core/queue.c | 6 ----
drivers/mmc/core/queue.h | 1 -
include/linux/mmc/core.h | 3 +-
include/linux/mmc/host.h | 7 ++--
6 files changed, 51 insertions(+), 57 deletions(-)
diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index c49c90dba839..c459d80c66bf 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -1562,6 +1562,8 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
mqrq->areq.mrq = &brq->mrq;
mqrq->areq.err_check = mmc_blk_err_check;
+ mqrq->areq.host = card->host;
+ kthread_init_work(&mqrq->areq.finalization_work, mmc_finalize_areq);
mmc_queue_bounce_pre(mqrq);
}
@@ -1672,8 +1674,6 @@ static void mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *new_req)
* and there is nothing more to do until it is
* complete.
*/
- if (status == MMC_BLK_NEW_REQUEST)
- mq->new_request = true;
return;
}
@@ -1811,7 +1811,6 @@ void mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req)
goto out;
}
- mq->new_request = false;
if (req && req_op(req) == REQ_OP_DISCARD) {
/* complete ongoing async transfer before issuing discard */
if (card->host->areq)
@@ -1832,7 +1831,7 @@ void mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req)
}
out:
- if ((!req && !mq->new_request) || req_is_special)
+ if (!req || req_is_special)
/*
* Release host when there are no more requests
* and after special request(discard, flush) is done.
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 663799240635..8ecf61e51662 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -415,10 +415,13 @@ EXPORT_SYMBOL(mmc_start_bkops);
*/
static void mmc_wait_data_done(struct mmc_request *mrq)
{
- struct mmc_context_info *context_info = &mrq->host->context_info;
+ struct mmc_host *host = mrq->host;
+ struct mmc_context_info *context_info = &host->context_info;
+ struct mmc_async_req *areq = host->areq;
context_info->is_done_rcv = true;
- wake_up_interruptible(&context_info->wait);
+ /* Schedule a work to deal with finalizing this request */
+ kthread_queue_work(&host->req_done_worker, &areq->finalization_work);
}
static void mmc_wait_done(struct mmc_request *mrq)
@@ -592,43 +595,34 @@ static void mmc_post_req(struct mmc_host *host, struct mmc_request *mrq,
* will return MMC_BLK_SUCCESS if no request was
* going on.
*/
-static enum mmc_blk_status mmc_finalize_areq(struct mmc_host *host)
+void mmc_finalize_areq(struct kthread_work *work)
{
+ struct mmc_async_req *areq =
+ container_of(work, struct mmc_async_req, finalization_work);
+ struct mmc_host *host = areq->host;
struct mmc_context_info *context_info = &host->context_info;
- enum mmc_blk_status status;
-
- if (!host->areq)
- return MMC_BLK_SUCCESS;
-
- while (1) {
- wait_event_interruptible(context_info->wait,
- (context_info->is_done_rcv ||
- context_info->is_new_req));
+ enum mmc_blk_status status = MMC_BLK_SUCCESS;
- if (context_info->is_done_rcv) {
- struct mmc_command *cmd;
+ if (context_info->is_done_rcv) {
+ struct mmc_command *cmd;
- context_info->is_done_rcv = false;
- cmd = host->areq->mrq->cmd;
+ context_info->is_done_rcv = false;
+ cmd = areq->mrq->cmd;
- if (!cmd->error || !cmd->retries ||
- mmc_card_removed(host->card)) {
- status = host->areq->err_check(host->card,
- host->areq);
- break; /* return status */
- } else {
- mmc_retune_recheck(host);
- pr_info("%s: req failed (CMD%u): %d, retrying...\n",
- mmc_hostname(host),
- cmd->opcode, cmd->error);
- cmd->retries--;
- cmd->error = 0;
- __mmc_start_request(host, host->areq->mrq);
- continue; /* wait for done/new event again */
- }
+ if (!cmd->error || !cmd->retries ||
+ mmc_card_removed(host->card)) {
+ status = areq->err_check(host->card,
+ areq);
+ } else {
+ mmc_retune_recheck(host);
+ pr_info("%s: req failed (CMD%u): %d, retrying...\n",
+ mmc_hostname(host),
+ cmd->opcode, cmd->error);
+ cmd->retries--;
+ cmd->error = 0;
+ __mmc_start_request(host, areq->mrq);
+ return; /* wait for done/new event again */
}
-
- return MMC_BLK_NEW_REQUEST;
}
mmc_retune_release(host);
@@ -644,10 +638,12 @@ static enum mmc_blk_status mmc_finalize_areq(struct mmc_host *host)
}
/* Successfully postprocess the old request at this point */
- mmc_post_req(host, host->areq->mrq, 0);
+ mmc_post_req(host, areq->mrq, 0);
- return status;
+ areq->finalization_status = status;
+ complete(&areq->complete);
}
+EXPORT_SYMBOL(mmc_finalize_areq);
/**
* mmc_start_areq - start an asynchronous request
@@ -677,18 +673,21 @@ struct mmc_async_req *mmc_start_areq(struct mmc_host *host,
if (areq)
mmc_pre_req(host, areq->mrq);
- /* Finalize previous request */
- status = mmc_finalize_areq(host);
+ /* Finalize previous request, if there is one */
+ if (previous) {
+ wait_for_completion(&previous->complete);
+ status = previous->finalization_status;
+ } else {
+ status = MMC_BLK_SUCCESS;
+ }
if (ret_stat)
*ret_stat = status;
- /* The previous request is still going on... */
- if (status == MMC_BLK_NEW_REQUEST)
- return NULL;
-
/* Fine so far, start the new request! */
- if (status == MMC_BLK_SUCCESS && areq)
+ if (status == MMC_BLK_SUCCESS && areq) {
+ init_completion(&areq->complete);
start_err = __mmc_start_data_req(host, areq->mrq);
+ }
/* Cancel a prepared request if it was not started. */
if ((status != MMC_BLK_SUCCESS || start_err) && areq)
@@ -2996,7 +2995,6 @@ void mmc_init_context_info(struct mmc_host *host)
host->context_info.is_new_req = false;
host->context_info.is_done_rcv = false;
host->context_info.is_waiting_last_req = false;
- init_waitqueue_head(&host->context_info.wait);
}
static int __init mmc_init(void)
diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c
index 5cb369c2664b..73250ed8f093 100644
--- a/drivers/mmc/core/queue.c
+++ b/drivers/mmc/core/queue.c
@@ -86,11 +86,6 @@ static int mmc_queue_thread(void *d)
set_current_state(TASK_RUNNING);
mmc_blk_issue_rq(mq, req);
cond_resched();
- if (mq->new_request) {
- mq->new_request = false;
- continue; /* fetch again */
- }
-
/*
* Current request becomes previous request
* and vice versa.
@@ -143,7 +138,6 @@ static void mmc_request_fn(struct request_queue *q)
if (cntx->is_waiting_last_req) {
cntx->is_new_req = true;
- wake_up_interruptible(&cntx->wait);
}
if (mq->asleep)
diff --git a/drivers/mmc/core/queue.h b/drivers/mmc/core/queue.h
index e298f100101b..39d8e710287e 100644
--- a/drivers/mmc/core/queue.h
+++ b/drivers/mmc/core/queue.h
@@ -40,7 +40,6 @@ struct mmc_queue {
struct mmc_card *card;
struct task_struct *thread;
struct semaphore thread_sem;
- bool new_request;
bool suspended;
bool asleep;
struct mmc_blk_data *blkdata;
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index a0c63ea28796..5db0fb722c37 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -9,6 +9,7 @@
#define LINUX_MMC_CORE_H
#include <linux/completion.h>
+#include <linux/kthread.h>
#include <linux/types.h>
struct mmc_data;
@@ -23,7 +24,6 @@ enum mmc_blk_status {
MMC_BLK_DATA_ERR,
MMC_BLK_ECC_ERR,
MMC_BLK_NOMEDIUM,
- MMC_BLK_NEW_REQUEST,
};
struct mmc_command {
@@ -158,6 +158,7 @@ struct mmc_request {
struct mmc_card;
struct mmc_async_req;
+void mmc_finalize_areq(struct kthread_work *work);
struct mmc_async_req *mmc_start_areq(struct mmc_host *host,
struct mmc_async_req *areq,
enum mmc_blk_status *ret_stat);
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index b04f8cd51c82..c5f61f2f2310 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -14,6 +14,7 @@
#include <linux/device.h>
#include <linux/fault-inject.h>
#include <linux/kthread.h>
+#include <linux/completion.h>
#include <linux/mmc/core.h>
#include <linux/mmc/card.h>
@@ -169,6 +170,10 @@ struct mmc_async_req {
* Returns 0 if success otherwise non zero.
*/
enum mmc_blk_status (*err_check)(struct mmc_card *, struct mmc_async_req *);
+ struct kthread_work finalization_work;
+ enum mmc_blk_status finalization_status;
+ struct completion complete;
+ struct mmc_host *host;
};
/**
@@ -192,13 +197,11 @@ struct mmc_slot {
* @is_done_rcv wake up reason was done request
* @is_new_req wake up reason was new request
* @is_waiting_last_req mmc context waiting for single running request
- * @wait wait queue
*/
struct mmc_context_info {
bool is_done_rcv;
bool is_new_req;
bool is_waiting_last_req;
- wait_queue_head_t wait;
};
struct regulator;
--
2.9.3
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH 06/16] mmc: core: replace waitqueue with worker
2017-02-09 15:33 ` [PATCH 06/16] mmc: core: replace waitqueue with worker Linus Walleij
@ 2017-02-22 13:29 ` Adrian Hunter
2017-03-09 22:49 ` Linus Walleij
[not found] ` <CGME20170228161023epcas5p3916c2e171d57b8c7814be7841fbab3aa@epcas5p3.samsung.com>
1 sibling, 1 reply; 51+ messages in thread
From: Adrian Hunter @ 2017-02-22 13:29 UTC (permalink / raw)
To: Linus Walleij, linux-mmc, Ulf Hansson, Paolo Valente
Cc: Chunyan Zhang, Baolin Wang, linux-block, Jens Axboe,
Christoph Hellwig, Arnd Bergmann
On 09/02/17 17:33, Linus Walleij wrote:
> The waitqueue in the host context is there to signal back from
> mmc_request_done() through mmc_wait_data_done() that the hardware
> is done with a command, and when the wait is over, the core
> will typically submit the next asynchronous request that is pending
> just waiting for the hardware to be available.
>
> This is in the way for letting the mmc_request_done() trigger the
> report up to the block layer that a block request is finished.
>
> Re-jig this as a first step, remvoving the waitqueue and introducing
> a work that will run after a completed asynchronous request,
> finalizing that request, including retransmissions, and eventually
> reporting back with a completion and a status code to the
> asynchronous issue method.
>
> This had the upside that we can remove the MMC_BLK_NEW_REQUEST
> status code and the "new_request" state in the request queue
> that is only there to make the state machine spin out
> the first time we send a request.
>
> Introduce a workqueue in the host for handling just this, and
> then a work and completion in the asynchronous request to deal
> with this mechanism.
>
> This is a central change that let us do many other changes since
> we have broken the submit and complete code paths in two, and we
> can potentially remove the NULL flushing of the asynchronous
> pipeline and report block requests as finished directly from
> the worker.
This needs more thought. The completion should go straight to the mmc block
driver from the ->done() callback. And from there straight back to the
block layer if recovery is not needed. We want to stop using
mmc_start_areq() altogether because we never want to wait - we always want
to issue (if possible) and return.
The core API to use is __mmc_start_req() but the block driver should
populate mrq->done with its own handler. i.e. change __mmc_start_req()
- mrq->done = mmc_wait_done;
+ if (!mrq->done)
+ mrq->done = mmc_wait_done;
mrq->done() would complete the request (e.g. via blk_complete_request()) if
it has no errors (and doesn't need polling), and wake up the queue thread to
finish up everything else and start the next request.
For the blk-mq port, the queue thread should also be retained, partly
because it solves some synchronization problems, but mostly because, at this
stage, we anyway don't have solutions for all the different ways the driver
can block.
(as listed here https://marc.info/?l=linux-mmc&m=148336571720463&w=2 )
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH 06/16] mmc: core: replace waitqueue with worker
2017-02-22 13:29 ` Adrian Hunter
@ 2017-03-09 22:49 ` Linus Walleij
2017-03-10 14:21 ` Adrian Hunter
0 siblings, 1 reply; 51+ messages in thread
From: Linus Walleij @ 2017-03-09 22:49 UTC (permalink / raw)
To: Adrian Hunter
Cc: linux-mmc, Ulf Hansson, Paolo Valente, Chunyan Zhang,
Baolin Wang, linux-block, Jens Axboe, Christoph Hellwig,
Arnd Bergmann
On Wed, Feb 22, 2017 at 2:29 PM, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 09/02/17 17:33, Linus Walleij wrote:
>> The waitqueue in the host context is there to signal back from
>> mmc_request_done() through mmc_wait_data_done() that the hardware
>> is done with a command, and when the wait is over, the core
>> will typically submit the next asynchronous request that is pending
>> just waiting for the hardware to be available.
>>
>> This is in the way for letting the mmc_request_done() trigger the
>> report up to the block layer that a block request is finished.
>>
>> Re-jig this as a first step, remvoving the waitqueue and introducing
>> a work that will run after a completed asynchronous request,
>> finalizing that request, including retransmissions, and eventually
>> reporting back with a completion and a status code to the
>> asynchronous issue method.
>>
>> This had the upside that we can remove the MMC_BLK_NEW_REQUEST
>> status code and the "new_request" state in the request queue
>> that is only there to make the state machine spin out
>> the first time we send a request.
>>
>> Introduce a workqueue in the host for handling just this, and
>> then a work and completion in the asynchronous request to deal
>> with this mechanism.
>>
>> This is a central change that let us do many other changes since
>> we have broken the submit and complete code paths in two, and we
>> can potentially remove the NULL flushing of the asynchronous
>> pipeline and report block requests as finished directly from
>> the worker.
>
> This needs more thought. The completion should go straight to the mmc block
> driver from the ->done() callback. And from there straight back to the
> block layer if recovery is not needed. We want to stop using
> mmc_start_areq() altogether because we never want to wait - we always want
> to issue (if possible) and return.
I don't quite follow this. Isn't what you request exactly what
patch 15/16 "mmc: queue: issue requests in massive parallel"
is doing?
The whole patch series leads up to that.
> The core API to use is __mmc_start_req() but the block driver should
> populate mrq->done with its own handler. i.e. change __mmc_start_req()
>
> - mrq->done = mmc_wait_done;
> + if (!mrq->done)
> + mrq->done = mmc_wait_done;
>
> mrq->done() would complete the request (e.g. via blk_complete_request()) if
> it has no errors (and doesn't need polling), and wake up the queue thread to
> finish up everything else and start the next request.
I think this is what it does at the end of the patch series, patch 15/16.
I have to split it somehow...
> For the blk-mq port, the queue thread should also be retained, partly
> because it solves some synchronization problems, but mostly because, at this
> stage, we anyway don't have solutions for all the different ways the driver
> can block.
> (as listed here https://marc.info/?l=linux-mmc&m=148336571720463&w=2 )
Essentially I take out that thread and replace it with this one worker
introduced in this very patch. I agree the driver can block in many ways
and that is why I need to have it running in process context, and this
is what the worker introduced here provides.
Maybe I'm getting something backwards, sorry then :/
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 06/16] mmc: core: replace waitqueue with worker
2017-03-09 22:49 ` Linus Walleij
@ 2017-03-10 14:21 ` Adrian Hunter
2017-03-10 22:05 ` Jens Axboe
2017-03-28 7:46 ` Linus Walleij
0 siblings, 2 replies; 51+ messages in thread
From: Adrian Hunter @ 2017-03-10 14:21 UTC (permalink / raw)
To: Linus Walleij
Cc: linux-mmc, Ulf Hansson, Paolo Valente, Chunyan Zhang,
Baolin Wang, linux-block, Jens Axboe, Christoph Hellwig,
Arnd Bergmann
On 10/03/17 00:49, Linus Walleij wrote:
> On Wed, Feb 22, 2017 at 2:29 PM, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> On 09/02/17 17:33, Linus Walleij wrote:
>>> The waitqueue in the host context is there to signal back from
>>> mmc_request_done() through mmc_wait_data_done() that the hardware
>>> is done with a command, and when the wait is over, the core
>>> will typically submit the next asynchronous request that is pending
>>> just waiting for the hardware to be available.
>>>
>>> This is in the way for letting the mmc_request_done() trigger the
>>> report up to the block layer that a block request is finished.
>>>
>>> Re-jig this as a first step, remvoving the waitqueue and introducing
>>> a work that will run after a completed asynchronous request,
>>> finalizing that request, including retransmissions, and eventually
>>> reporting back with a completion and a status code to the
>>> asynchronous issue method.
>>>
>>> This had the upside that we can remove the MMC_BLK_NEW_REQUEST
>>> status code and the "new_request" state in the request queue
>>> that is only there to make the state machine spin out
>>> the first time we send a request.
>>>
>>> Introduce a workqueue in the host for handling just this, and
>>> then a work and completion in the asynchronous request to deal
>>> with this mechanism.
>>>
>>> This is a central change that let us do many other changes since
>>> we have broken the submit and complete code paths in two, and we
>>> can potentially remove the NULL flushing of the asynchronous
>>> pipeline and report block requests as finished directly from
>>> the worker.
>>
>> This needs more thought. The completion should go straight to the mmc block
>> driver from the ->done() callback. And from there straight back to the
>> block layer if recovery is not needed. We want to stop using
>> mmc_start_areq() altogether because we never want to wait - we always want
>> to issue (if possible) and return.
>
> I don't quite follow this. Isn't what you request exactly what
> patch 15/16 "mmc: queue: issue requests in massive parallel"
> is doing?
There is the latency for the worker that runs mmc_finalize_areq() and then
another latency to wake up the worker that is running mmc_start_areq().
That is 2 wake-ups instead of 1.
As a side note, ideally we would be able to issue the next request from the
interrupt or soft interrupt context of the completion (i.e. 0 wake-ups
between requests), but we would probably have to look at the host API to
support that.
>
> The whole patch series leads up to that.
>
>> The core API to use is __mmc_start_req() but the block driver should
>> populate mrq->done with its own handler. i.e. change __mmc_start_req()
>>
>> - mrq->done = mmc_wait_done;
>> + if (!mrq->done)
>> + mrq->done = mmc_wait_done;
>>
>> mrq->done() would complete the request (e.g. via blk_complete_request()) if
>> it has no errors (and doesn't need polling), and wake up the queue thread to
>> finish up everything else and start the next request.
>
> I think this is what it does at the end of the patch series, patch 15/16.
> I have to split it somehow...
>
>> For the blk-mq port, the queue thread should also be retained, partly
>> because it solves some synchronization problems, but mostly because, at this
>> stage, we anyway don't have solutions for all the different ways the driver
>> can block.
>> (as listed here https://marc.info/?l=linux-mmc&m=148336571720463&w=2 )
>
> Essentially I take out that thread and replace it with this one worker
> introduced in this very patch. I agree the driver can block in many ways
> and that is why I need to have it running in process context, and this
> is what the worker introduced here provides.
The last time I looked at the blk-mq I/O scheduler code, it pulled up to
qdepth requests from the I/O scheduler and left them on a local list while
running ->queue_rq(). That means blocking in ->queue_rq() leaves some
number of requests in limbo (not issued but also not in the I/O scheduler)
for that time.
Maybe blk-mq should offer a pull interface to I/O scheduler users?
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 06/16] mmc: core: replace waitqueue with worker
2017-03-10 14:21 ` Adrian Hunter
@ 2017-03-10 22:05 ` Jens Axboe
2017-03-13 9:25 ` Adrian Hunter
2017-03-28 7:46 ` Linus Walleij
1 sibling, 1 reply; 51+ messages in thread
From: Jens Axboe @ 2017-03-10 22:05 UTC (permalink / raw)
To: Adrian Hunter, Linus Walleij
Cc: linux-mmc, Ulf Hansson, Paolo Valente, Chunyan Zhang,
Baolin Wang, linux-block, Christoph Hellwig, Arnd Bergmann
On 03/10/2017 07:21 AM, Adrian Hunter wrote:
>> Essentially I take out that thread and replace it with this one worker
>> introduced in this very patch. I agree the driver can block in many ways
>> and that is why I need to have it running in process context, and this
>> is what the worker introduced here provides.
>
> The last time I looked at the blk-mq I/O scheduler code, it pulled up to
> qdepth requests from the I/O scheduler and left them on a local list while
> running ->queue_rq(). That means blocking in ->queue_rq() leaves some
> number of requests in limbo (not issued but also not in the I/O scheduler)
> for that time.
Look again, if we're not handling the requeued dispatches, we pull one
at the time from the scheduler.
--
Jens Axboe
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 06/16] mmc: core: replace waitqueue with worker
2017-03-10 22:05 ` Jens Axboe
@ 2017-03-13 9:25 ` Adrian Hunter
2017-03-13 14:19 ` Jens Axboe
0 siblings, 1 reply; 51+ messages in thread
From: Adrian Hunter @ 2017-03-13 9:25 UTC (permalink / raw)
To: Jens Axboe, Linus Walleij
Cc: linux-mmc, Ulf Hansson, Paolo Valente, Chunyan Zhang,
Baolin Wang, linux-block, Christoph Hellwig, Arnd Bergmann
On 11/03/17 00:05, Jens Axboe wrote:
> On 03/10/2017 07:21 AM, Adrian Hunter wrote:
>>> Essentially I take out that thread and replace it with this one worker
>>> introduced in this very patch. I agree the driver can block in many ways
>>> and that is why I need to have it running in process context, and this
>>> is what the worker introduced here provides.
>>
>> The last time I looked at the blk-mq I/O scheduler code, it pulled up to
>> qdepth requests from the I/O scheduler and left them on a local list while
>> running ->queue_rq(). That means blocking in ->queue_rq() leaves some
>> number of requests in limbo (not issued but also not in the I/O scheduler)
>> for that time.
>
> Look again, if we're not handling the requeued dispatches, we pull one
> at the time from the scheduler.
>
That's good :-)
Now the next thing ;-)
It looks like we either set BLK_MQ_F_BLOCKING and miss the possibility of
issuing synchronous requests immediately, or we don't set BLK_MQ_F_BLOCKING
in which case we are never allowed to sleep in ->queue_rq(). Is that true?
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 06/16] mmc: core: replace waitqueue with worker
2017-03-13 9:25 ` Adrian Hunter
@ 2017-03-13 14:19 ` Jens Axboe
2017-03-14 12:59 ` Adrian Hunter
0 siblings, 1 reply; 51+ messages in thread
From: Jens Axboe @ 2017-03-13 14:19 UTC (permalink / raw)
To: Adrian Hunter, Linus Walleij
Cc: linux-mmc, Ulf Hansson, Paolo Valente, Chunyan Zhang,
Baolin Wang, linux-block, Christoph Hellwig, Arnd Bergmann
On 03/13/2017 03:25 AM, Adrian Hunter wrote:
> On 11/03/17 00:05, Jens Axboe wrote:
>> On 03/10/2017 07:21 AM, Adrian Hunter wrote:
>>>> Essentially I take out that thread and replace it with this one worker
>>>> introduced in this very patch. I agree the driver can block in many ways
>>>> and that is why I need to have it running in process context, and this
>>>> is what the worker introduced here provides.
>>>
>>> The last time I looked at the blk-mq I/O scheduler code, it pulled up to
>>> qdepth requests from the I/O scheduler and left them on a local list while
>>> running ->queue_rq(). That means blocking in ->queue_rq() leaves some
>>> number of requests in limbo (not issued but also not in the I/O scheduler)
>>> for that time.
>>
>> Look again, if we're not handling the requeued dispatches, we pull one
>> at the time from the scheduler.
>>
>
> That's good :-)
>
> Now the next thing ;-)
>
> It looks like we either set BLK_MQ_F_BLOCKING and miss the possibility of
> issuing synchronous requests immediately, or we don't set BLK_MQ_F_BLOCKING
> in which case we are never allowed to sleep in ->queue_rq(). Is that true?
Only one of those statements is true - if you don't set BLK_MQ_F_BLOCKING,
then you may never block in your ->queue_rq() function. But if you do set it,
it does not preclude immediate issue of sync requests.
--
Jens Axboe
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 06/16] mmc: core: replace waitqueue with worker
2017-03-13 14:19 ` Jens Axboe
@ 2017-03-14 12:59 ` Adrian Hunter
2017-03-14 14:36 ` Jens Axboe
0 siblings, 1 reply; 51+ messages in thread
From: Adrian Hunter @ 2017-03-14 12:59 UTC (permalink / raw)
To: Jens Axboe, Linus Walleij
Cc: linux-mmc, Ulf Hansson, Paolo Valente, Chunyan Zhang,
Baolin Wang, linux-block, Christoph Hellwig, Arnd Bergmann
On 13/03/17 16:19, Jens Axboe wrote:
> On 03/13/2017 03:25 AM, Adrian Hunter wrote:
>> On 11/03/17 00:05, Jens Axboe wrote:
>>> On 03/10/2017 07:21 AM, Adrian Hunter wrote:
>>>>> Essentially I take out that thread and replace it with this one worker
>>>>> introduced in this very patch. I agree the driver can block in many ways
>>>>> and that is why I need to have it running in process context, and this
>>>>> is what the worker introduced here provides.
>>>>
>>>> The last time I looked at the blk-mq I/O scheduler code, it pulled up to
>>>> qdepth requests from the I/O scheduler and left them on a local list while
>>>> running ->queue_rq(). That means blocking in ->queue_rq() leaves some
>>>> number of requests in limbo (not issued but also not in the I/O scheduler)
>>>> for that time.
>>>
>>> Look again, if we're not handling the requeued dispatches, we pull one
>>> at the time from the scheduler.
>>>
>>
>> That's good :-)
>>
>> Now the next thing ;-)
>>
>> It looks like we either set BLK_MQ_F_BLOCKING and miss the possibility of
>> issuing synchronous requests immediately, or we don't set BLK_MQ_F_BLOCKING
>> in which case we are never allowed to sleep in ->queue_rq(). Is that true?
>
> Only one of those statements is true - if you don't set BLK_MQ_F_BLOCKING,
> then you may never block in your ->queue_rq() function. But if you do set it,
> it does not preclude immediate issue of sync requests.
I meant it gets put to the workqueue rather than issued in the context of
the submitter.
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 06/16] mmc: core: replace waitqueue with worker
2017-03-14 12:59 ` Adrian Hunter
@ 2017-03-14 14:36 ` Jens Axboe
2017-03-14 14:43 ` Christoph Hellwig
2017-03-28 7:47 ` Linus Walleij
0 siblings, 2 replies; 51+ messages in thread
From: Jens Axboe @ 2017-03-14 14:36 UTC (permalink / raw)
To: Adrian Hunter, Linus Walleij
Cc: linux-mmc, Ulf Hansson, Paolo Valente, Chunyan Zhang,
Baolin Wang, linux-block, Christoph Hellwig, Arnd Bergmann
On 03/14/2017 06:59 AM, Adrian Hunter wrote:
> On 13/03/17 16:19, Jens Axboe wrote:
>> On 03/13/2017 03:25 AM, Adrian Hunter wrote:
>>> On 11/03/17 00:05, Jens Axboe wrote:
>>>> On 03/10/2017 07:21 AM, Adrian Hunter wrote:
>>>>>> Essentially I take out that thread and replace it with this one worker
>>>>>> introduced in this very patch. I agree the driver can block in many ways
>>>>>> and that is why I need to have it running in process context, and this
>>>>>> is what the worker introduced here provides.
>>>>>
>>>>> The last time I looked at the blk-mq I/O scheduler code, it pulled up to
>>>>> qdepth requests from the I/O scheduler and left them on a local list while
>>>>> running ->queue_rq(). That means blocking in ->queue_rq() leaves some
>>>>> number of requests in limbo (not issued but also not in the I/O scheduler)
>>>>> for that time.
>>>>
>>>> Look again, if we're not handling the requeued dispatches, we pull one
>>>> at the time from the scheduler.
>>>>
>>>
>>> That's good :-)
>>>
>>> Now the next thing ;-)
>>>
>>> It looks like we either set BLK_MQ_F_BLOCKING and miss the possibility of
>>> issuing synchronous requests immediately, or we don't set BLK_MQ_F_BLOCKING
>>> in which case we are never allowed to sleep in ->queue_rq(). Is that true?
>>
>> Only one of those statements is true - if you don't set BLK_MQ_F_BLOCKING,
>> then you may never block in your ->queue_rq() function. But if you do set it,
>> it does not preclude immediate issue of sync requests.
>
> I meant it gets put to the workqueue rather than issued in the context of
> the submitter.
There's one case that doesn't look like it was converted properly, but
that's a mistake. The general insert-and-run cases run inline if we can,
but the direct-issue needs a fixup, see below.
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 159187a28d66..4196d6bee92d 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1434,7 +1434,8 @@ static blk_qc_t request_to_qc_t(struct blk_mq_hw_ctx *hctx, struct request *rq)
return blk_tag_to_qc_t(rq->internal_tag, hctx->queue_num, true);
}
-static void blk_mq_try_issue_directly(struct request *rq, blk_qc_t *cookie)
+static void blk_mq_try_issue_directly(struct request *rq, blk_qc_t *cookie,
+ bool can_block)
{
struct request_queue *q = rq->q;
struct blk_mq_queue_data bd = {
@@ -1475,7 +1476,7 @@ static void blk_mq_try_issue_directly(struct request *rq, blk_qc_t *cookie)
}
insert:
- blk_mq_sched_insert_request(rq, false, true, true, false);
+ blk_mq_sched_insert_request(rq, false, true, false, can_block);
}
/*
@@ -1569,11 +1570,11 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio)
if (!(data.hctx->flags & BLK_MQ_F_BLOCKING)) {
rcu_read_lock();
- blk_mq_try_issue_directly(old_rq, &cookie);
+ blk_mq_try_issue_directly(old_rq, &cookie, false);
rcu_read_unlock();
} else {
srcu_idx = srcu_read_lock(&data.hctx->queue_rq_srcu);
- blk_mq_try_issue_directly(old_rq, &cookie);
+ blk_mq_try_issue_directly(old_rq, &cookie, true);
srcu_read_unlock(&data.hctx->queue_rq_srcu, srcu_idx);
}
goto done;
--
Jens Axboe
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH 06/16] mmc: core: replace waitqueue with worker
2017-03-14 14:36 ` Jens Axboe
@ 2017-03-14 14:43 ` Christoph Hellwig
2017-03-14 14:52 ` Jens Axboe
2017-03-28 7:47 ` Linus Walleij
1 sibling, 1 reply; 51+ messages in thread
From: Christoph Hellwig @ 2017-03-14 14:43 UTC (permalink / raw)
To: Jens Axboe
Cc: Adrian Hunter, Linus Walleij, linux-mmc, Ulf Hansson,
Paolo Valente, Chunyan Zhang, Baolin Wang, linux-block,
Christoph Hellwig, Arnd Bergmann
On Tue, Mar 14, 2017 at 08:36:26AM -0600, Jens Axboe wrote:
> There's one case that doesn't look like it was converted properly, but
> that's a mistake. The general insert-and-run cases run inline if we can,
> but the direct-issue needs a fixup, see below.
Note that blk_mq_try_issue_directly is only called for the multiple
hardware queue case, so MMC would not hit it.
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 06/16] mmc: core: replace waitqueue with worker
2017-03-14 14:43 ` Christoph Hellwig
@ 2017-03-14 14:52 ` Jens Axboe
0 siblings, 0 replies; 51+ messages in thread
From: Jens Axboe @ 2017-03-14 14:52 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Adrian Hunter, Linus Walleij, linux-mmc, Ulf Hansson,
Paolo Valente, Chunyan Zhang, Baolin Wang, linux-block,
Arnd Bergmann
On 03/14/2017 08:43 AM, Christoph Hellwig wrote:
> On Tue, Mar 14, 2017 at 08:36:26AM -0600, Jens Axboe wrote:
>> There's one case that doesn't look like it was converted properly, but
>> that's a mistake. The general insert-and-run cases run inline if we can,
>> but the direct-issue needs a fixup, see below.
>
> Note that blk_mq_try_issue_directly is only called for the multiple
> hardware queue case, so MMC would not hit it.
Right, which is why I said that the general case works fine, it's
only the explicit issue-direct that currently does not. Not by
design, just an error.
--
Jens Axboe
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 06/16] mmc: core: replace waitqueue with worker
2017-03-14 14:36 ` Jens Axboe
2017-03-14 14:43 ` Christoph Hellwig
@ 2017-03-28 7:47 ` Linus Walleij
1 sibling, 0 replies; 51+ messages in thread
From: Linus Walleij @ 2017-03-28 7:47 UTC (permalink / raw)
To: Jens Axboe
Cc: Adrian Hunter, linux-mmc, Ulf Hansson, Paolo Valente,
Chunyan Zhang, Baolin Wang, linux-block, Christoph Hellwig,
Arnd Bergmann
On Tue, Mar 14, 2017 at 3:36 PM, Jens Axboe <axboe@kernel.dk> wrote:
> There's one case that doesn't look like it was converted properly, but
> that's a mistake. The general insert-and-run cases run inline if we can,
> but the direct-issue needs a fixup, see below.
>
>
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 159187a28d66..4196d6bee92d 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -1434,7 +1434,8 @@ static blk_qc_t request_to_qc_t(struct blk_mq_hw_ctx *hctx, struct request *rq)
> return blk_tag_to_qc_t(rq->internal_tag, hctx->queue_num, true);
> }
>
> -static void blk_mq_try_issue_directly(struct request *rq, blk_qc_t *cookie)
> +static void blk_mq_try_issue_directly(struct request *rq, blk_qc_t *cookie,
> + bool can_block)
> {
> struct request_queue *q = rq->q;
> struct blk_mq_queue_data bd = {
> @@ -1475,7 +1476,7 @@ static void blk_mq_try_issue_directly(struct request *rq, blk_qc_t *cookie)
> }
>
> insert:
> - blk_mq_sched_insert_request(rq, false, true, true, false);
> + blk_mq_sched_insert_request(rq, false, true, false, can_block);
> }
>
> /*
> @@ -1569,11 +1570,11 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio)
>
> if (!(data.hctx->flags & BLK_MQ_F_BLOCKING)) {
> rcu_read_lock();
> - blk_mq_try_issue_directly(old_rq, &cookie);
> + blk_mq_try_issue_directly(old_rq, &cookie, false);
> rcu_read_unlock();
> } else {
> srcu_idx = srcu_read_lock(&data.hctx->queue_rq_srcu);
> - blk_mq_try_issue_directly(old_rq, &cookie);
> + blk_mq_try_issue_directly(old_rq, &cookie, true);
> srcu_read_unlock(&data.hctx->queue_rq_srcu, srcu_idx);
> }
> goto done;
Jens do you have this in your patch queue or is it something you want
us to test and
submit back to you?
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 06/16] mmc: core: replace waitqueue with worker
2017-03-10 14:21 ` Adrian Hunter
2017-03-10 22:05 ` Jens Axboe
@ 2017-03-28 7:46 ` Linus Walleij
1 sibling, 0 replies; 51+ messages in thread
From: Linus Walleij @ 2017-03-28 7:46 UTC (permalink / raw)
To: Adrian Hunter
Cc: linux-mmc, Ulf Hansson, Paolo Valente, Chunyan Zhang,
Baolin Wang, linux-block, Jens Axboe, Christoph Hellwig,
Arnd Bergmann
On Fri, Mar 10, 2017 at 3:21 PM, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 10/03/17 00:49, Linus Walleij wrote:
>> On Wed, Feb 22, 2017 at 2:29 PM, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>> On 09/02/17 17:33, Linus Walleij wrote:
>>>> This is a central change that let us do many other changes since
>>>> we have broken the submit and complete code paths in two, and we
>>>> can potentially remove the NULL flushing of the asynchronous
>>>> pipeline and report block requests as finished directly from
>>>> the worker.
>>>
>>> This needs more thought. The completion should go straight to the mmc block
>>> driver from the ->done() callback. And from there straight back to the
>>> block layer if recovery is not needed. We want to stop using
>>> mmc_start_areq() altogether because we never want to wait - we always want
>>> to issue (if possible) and return.
>>
>> I don't quite follow this. Isn't what you request exactly what
>> patch 15/16 "mmc: queue: issue requests in massive parallel"
>> is doing?
>
> There is the latency for the worker that runs mmc_finalize_areq() and then
> another latency to wake up the worker that is running mmc_start_areq().
> That is 2 wake-ups instead of 1.
That is correct (though the measured effect is small).
However when we switch to MQ it must happen like this due to its asynchronous
nature of issuing requests to us.
Then we have MQ's submission thread coming in from one en and our worker
to manage retries and errors on the other side. We obviously cannot do
the retries and resends in MQs context as it blocks subsequent requests.
> As a side note, ideally we would be able to issue the next request from the
> interrupt or soft interrupt context of the completion (i.e. 0 wake-ups
> between requests), but we would probably have to look at the host API to
> support that.
I looked at that and couldn't find a good way to get to that point.
Mainly because of mmc_start_bkops() that may
arbitrarily fire after every command and start new requests to the
card, and that of course require a process context to happen. Then there
is the retune thing that I do not fully understand how it schedules, but
it might be fine since I'm under the impression that it is done at the
start of the next request if need be. Maybe both can be overcome by
quick checks in IRQ context and then this can be done. (I'm not smart enough
to see that yet, sorry.)
However since we activate the blocking context in MQ I don't know
if it can even deal with having requests completed in interrupt context
so that the next thing that happens after completing the request and
returning from the interrupt is that the block layer thread gets scheduled
(unless something more important is going on), I guess it is possible?
It looks like it could be really efficient.
>>> For the blk-mq port, the queue thread should also be retained, partly
>>> because it solves some synchronization problems, but mostly because, at this
>>> stage, we anyway don't have solutions for all the different ways the driver
>>> can block.
>>> (as listed here https://marc.info/?l=linux-mmc&m=148336571720463&w=2 )
>>
>> Essentially I take out that thread and replace it with this one worker
>> introduced in this very patch. I agree the driver can block in many ways
>> and that is why I need to have it running in process context, and this
>> is what the worker introduced here provides.
>
> The last time I looked at the blk-mq I/O scheduler code, it pulled up to
> qdepth requests from the I/O scheduler and left them on a local list while
> running ->queue_rq(). That means blocking in ->queue_rq() leaves some
> number of requests in limbo (not issued but also not in the I/O scheduler)
> for that time.
I think Jens provided a patch for this bug (don't see the patch
upstream though).
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 51+ messages in thread
[parent not found: <CGME20170228161023epcas5p3916c2e171d57b8c7814be7841fbab3aa@epcas5p3.samsung.com>]
* Re: [PATCH 06/16] mmc: core: replace waitqueue with worker
[not found] ` <CGME20170228161023epcas5p3916c2e171d57b8c7814be7841fbab3aa@epcas5p3.samsung.com>
@ 2017-02-28 16:10 ` Bartlomiej Zolnierkiewicz
0 siblings, 0 replies; 51+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2017-02-28 16:10 UTC (permalink / raw)
To: Linus Walleij
Cc: linux-mmc, Ulf Hansson, Adrian Hunter, Paolo Valente,
Chunyan Zhang, Baolin Wang, linux-block, Jens Axboe,
Christoph Hellwig, Arnd Bergmann
On Thursday, February 09, 2017 04:33:53 PM Linus Walleij wrote:
> The waitqueue in the host context is there to signal back from
> mmc_request_done() through mmc_wait_data_done() that the hardware
> is done with a command, and when the wait is over, the core
> will typically submit the next asynchronous request that is pending
> just waiting for the hardware to be available.
>
> This is in the way for letting the mmc_request_done() trigger the
> report up to the block layer that a block request is finished.
>
> Re-jig this as a first step, remvoving the waitqueue and introducing
> a work that will run after a completed asynchronous request,
> finalizing that request, including retransmissions, and eventually
> reporting back with a completion and a status code to the
> asynchronous issue method.
>
> This had the upside that we can remove the MMC_BLK_NEW_REQUEST
> status code and the "new_request" state in the request queue
> that is only there to make the state machine spin out
> the first time we send a request.
>
> Introduce a workqueue in the host for handling just this, and
> then a work and completion in the asynchronous request to deal
> with this mechanism.
>
> This is a central change that let us do many other changes since
> we have broken the submit and complete code paths in two, and we
> can potentially remove the NULL flushing of the asynchronous
> pipeline and report block requests as finished directly from
> the worker.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 07/16] mmc: core: do away with is_done_rcv
2017-02-09 15:33 [PATCH 00/16] multiqueue for MMC/SD third try Linus Walleij
` (5 preceding siblings ...)
2017-02-09 15:33 ` [PATCH 06/16] mmc: core: replace waitqueue with worker Linus Walleij
@ 2017-02-09 15:33 ` Linus Walleij
[not found] ` <CGME20170228161047epcas1p2f307733cb1c441d0c290e794a04a06a8@epcas1p2.samsung.com>
2017-02-09 15:33 ` [PATCH 08/16] mmc: core: do away with is_new_req Linus Walleij
` (10 subsequent siblings)
17 siblings, 1 reply; 51+ messages in thread
From: Linus Walleij @ 2017-02-09 15:33 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson, Adrian Hunter, Paolo Valente
Cc: Chunyan Zhang, Baolin Wang, linux-block, Jens Axboe,
Christoph Hellwig, Arnd Bergmann, Linus Walleij
The "is_done_rcv" in the context info for the host is no longer
needed: it is clear from context (ha!) that as long as we are
waiting for the asynchronous request to come to completion,
we are not done receiving data, and when the finalization work
has run and completed the completion, we are indeed done.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/mmc/core/core.c | 40 ++++++++++++++++------------------------
include/linux/mmc/host.h | 2 --
2 files changed, 16 insertions(+), 26 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 8ecf61e51662..fcb40ade9b82 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -416,10 +416,8 @@ EXPORT_SYMBOL(mmc_start_bkops);
static void mmc_wait_data_done(struct mmc_request *mrq)
{
struct mmc_host *host = mrq->host;
- struct mmc_context_info *context_info = &host->context_info;
struct mmc_async_req *areq = host->areq;
- context_info->is_done_rcv = true;
/* Schedule a work to deal with finalizing this request */
kthread_queue_work(&host->req_done_worker, &areq->finalization_work);
}
@@ -551,7 +549,7 @@ EXPORT_SYMBOL(mmc_wait_for_req_done);
bool mmc_is_req_done(struct mmc_host *host, struct mmc_request *mrq)
{
if (host->areq)
- return host->context_info.is_done_rcv;
+ return completion_done(&host->areq->complete);
else
return completion_done(&mrq->completion);
}
@@ -600,29 +598,24 @@ void mmc_finalize_areq(struct kthread_work *work)
struct mmc_async_req *areq =
container_of(work, struct mmc_async_req, finalization_work);
struct mmc_host *host = areq->host;
- struct mmc_context_info *context_info = &host->context_info;
enum mmc_blk_status status = MMC_BLK_SUCCESS;
+ struct mmc_command *cmd;
- if (context_info->is_done_rcv) {
- struct mmc_command *cmd;
-
- context_info->is_done_rcv = false;
- cmd = areq->mrq->cmd;
+ cmd = areq->mrq->cmd;
- if (!cmd->error || !cmd->retries ||
- mmc_card_removed(host->card)) {
- status = areq->err_check(host->card,
- areq);
- } else {
- mmc_retune_recheck(host);
- pr_info("%s: req failed (CMD%u): %d, retrying...\n",
- mmc_hostname(host),
- cmd->opcode, cmd->error);
- cmd->retries--;
- cmd->error = 0;
- __mmc_start_request(host, areq->mrq);
- return; /* wait for done/new event again */
- }
+ if (!cmd->error || !cmd->retries ||
+ mmc_card_removed(host->card)) {
+ status = areq->err_check(host->card,
+ areq);
+ } else {
+ mmc_retune_recheck(host);
+ pr_info("%s: req failed (CMD%u): %d, retrying...\n",
+ mmc_hostname(host),
+ cmd->opcode, cmd->error);
+ cmd->retries--;
+ cmd->error = 0;
+ __mmc_start_request(host, areq->mrq);
+ return; /* wait for done/new event again */
}
mmc_retune_release(host);
@@ -2993,7 +2986,6 @@ void mmc_unregister_pm_notifier(struct mmc_host *host)
void mmc_init_context_info(struct mmc_host *host)
{
host->context_info.is_new_req = false;
- host->context_info.is_done_rcv = false;
host->context_info.is_waiting_last_req = false;
}
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index c5f61f2f2310..cbb40682024a 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -194,12 +194,10 @@ struct mmc_slot {
/**
* mmc_context_info - synchronization details for mmc context
- * @is_done_rcv wake up reason was done request
* @is_new_req wake up reason was new request
* @is_waiting_last_req mmc context waiting for single running request
*/
struct mmc_context_info {
- bool is_done_rcv;
bool is_new_req;
bool is_waiting_last_req;
};
--
2.9.3
^ permalink raw reply related [flat|nested] 51+ messages in thread* [PATCH 08/16] mmc: core: do away with is_new_req
2017-02-09 15:33 [PATCH 00/16] multiqueue for MMC/SD third try Linus Walleij
` (6 preceding siblings ...)
2017-02-09 15:33 ` [PATCH 07/16] mmc: core: do away with is_done_rcv Linus Walleij
@ 2017-02-09 15:33 ` Linus Walleij
[not found] ` <CGME20170228161102epcas5p25dc3b560013599fda6cc750f6d528595@epcas5p2.samsung.com>
2017-02-09 15:33 ` [PATCH 09/16] mmc: core: kill off the context info Linus Walleij
` (9 subsequent siblings)
17 siblings, 1 reply; 51+ messages in thread
From: Linus Walleij @ 2017-02-09 15:33 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson, Adrian Hunter, Paolo Valente
Cc: Chunyan Zhang, Baolin Wang, linux-block, Jens Axboe,
Christoph Hellwig, Arnd Bergmann, Linus Walleij
The host context member "is_new_req" is only assigned values,
never checked. Delete it.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/mmc/core/core.c | 1 -
drivers/mmc/core/queue.c | 5 -----
include/linux/mmc/host.h | 2 --
3 files changed, 8 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index fcb40ade9b82..933a4d1f20d5 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2985,7 +2985,6 @@ void mmc_unregister_pm_notifier(struct mmc_host *host)
*/
void mmc_init_context_info(struct mmc_host *host)
{
- host->context_info.is_new_req = false;
host->context_info.is_waiting_last_req = false;
}
diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c
index 73250ed8f093..63927ffd6825 100644
--- a/drivers/mmc/core/queue.c
+++ b/drivers/mmc/core/queue.c
@@ -66,7 +66,6 @@ static int mmc_queue_thread(void *d)
req = blk_fetch_request(q);
mq->asleep = false;
cntx->is_waiting_last_req = false;
- cntx->is_new_req = false;
if (!req) {
/*
* Dispatch queue is empty so set flags for
@@ -136,10 +135,6 @@ static void mmc_request_fn(struct request_queue *q)
cntx = &mq->card->host->context_info;
- if (cntx->is_waiting_last_req) {
- cntx->is_new_req = true;
- }
-
if (mq->asleep)
wake_up_process(mq->thread);
}
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index cbb40682024a..970d7f9b1eba 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -194,11 +194,9 @@ struct mmc_slot {
/**
* mmc_context_info - synchronization details for mmc context
- * @is_new_req wake up reason was new request
* @is_waiting_last_req mmc context waiting for single running request
*/
struct mmc_context_info {
- bool is_new_req;
bool is_waiting_last_req;
};
--
2.9.3
^ permalink raw reply related [flat|nested] 51+ messages in thread* [PATCH 09/16] mmc: core: kill off the context info
2017-02-09 15:33 [PATCH 00/16] multiqueue for MMC/SD third try Linus Walleij
` (7 preceding siblings ...)
2017-02-09 15:33 ` [PATCH 08/16] mmc: core: do away with is_new_req Linus Walleij
@ 2017-02-09 15:33 ` Linus Walleij
[not found] ` <CGME20170228161117epcas5p20a6e62146733466b98c0ef4ea6efbb5f@epcas5p2.samsung.com>
2017-02-09 15:33 ` [PATCH 10/16] mmc: queue: simplify queue logic Linus Walleij
` (8 subsequent siblings)
17 siblings, 1 reply; 51+ messages in thread
From: Linus Walleij @ 2017-02-09 15:33 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson, Adrian Hunter, Paolo Valente
Cc: Chunyan Zhang, Baolin Wang, linux-block, Jens Axboe,
Christoph Hellwig, Arnd Bergmann, Linus Walleij
The last member of the context info: is_waiting_last_req is
just assigned values, never checked. Delete that and the whole
context info as a result.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/mmc/core/bus.c | 1 -
drivers/mmc/core/core.c | 13 -------------
drivers/mmc/core/core.h | 2 --
drivers/mmc/core/queue.c | 9 +--------
include/linux/mmc/host.h | 9 ---------
5 files changed, 1 insertion(+), 33 deletions(-)
diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
index 301246513a37..22ed11ac961b 100644
--- a/drivers/mmc/core/bus.c
+++ b/drivers/mmc/core/bus.c
@@ -348,7 +348,6 @@ int mmc_add_card(struct mmc_card *card)
#ifdef CONFIG_DEBUG_FS
mmc_add_card_debugfs(card);
#endif
- mmc_init_context_info(card->host);
card->dev.of_node = mmc_of_find_child_device(card->host, 0);
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 933a4d1f20d5..4b84f18518ac 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2975,19 +2975,6 @@ void mmc_unregister_pm_notifier(struct mmc_host *host)
}
#endif
-/**
- * mmc_init_context_info() - init synchronization context
- * @host: mmc host
- *
- * Init struct context_info needed to implement asynchronous
- * request mechanism, used by mmc core, host driver and mmc requests
- * supplier.
- */
-void mmc_init_context_info(struct mmc_host *host)
-{
- host->context_info.is_waiting_last_req = false;
-}
-
static int __init mmc_init(void)
{
int ret;
diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
index 8a95c82554be..620bea373c3a 100644
--- a/drivers/mmc/core/core.h
+++ b/drivers/mmc/core/core.h
@@ -90,8 +90,6 @@ void mmc_remove_host_debugfs(struct mmc_host *host);
void mmc_add_card_debugfs(struct mmc_card *card);
void mmc_remove_card_debugfs(struct mmc_card *card);
-void mmc_init_context_info(struct mmc_host *host);
-
int mmc_execute_tuning(struct mmc_card *card);
int mmc_hs200_to_hs400(struct mmc_card *card);
int mmc_hs400_to_hs200(struct mmc_card *card);
diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c
index 63927ffd6825..a845fe8d4fd1 100644
--- a/drivers/mmc/core/queue.c
+++ b/drivers/mmc/core/queue.c
@@ -53,7 +53,6 @@ static int mmc_queue_thread(void *d)
{
struct mmc_queue *mq = d;
struct request_queue *q = mq->queue;
- struct mmc_context_info *cntx = &mq->card->host->context_info;
current->flags |= PF_MEMALLOC;
@@ -65,15 +64,12 @@ static int mmc_queue_thread(void *d)
set_current_state(TASK_INTERRUPTIBLE);
req = blk_fetch_request(q);
mq->asleep = false;
- cntx->is_waiting_last_req = false;
if (!req) {
/*
* Dispatch queue is empty so set flags for
* mmc_request_fn() to wake us up.
*/
- if (mq->mqrq_prev->req)
- cntx->is_waiting_last_req = true;
- else
+ if (!mq->mqrq_prev->req)
mq->asleep = true;
}
mq->mqrq_cur->req = req;
@@ -123,7 +119,6 @@ static void mmc_request_fn(struct request_queue *q)
{
struct mmc_queue *mq = q->queuedata;
struct request *req;
- struct mmc_context_info *cntx;
if (!mq) {
while ((req = blk_fetch_request(q)) != NULL) {
@@ -133,8 +128,6 @@ static void mmc_request_fn(struct request_queue *q)
return;
}
- cntx = &mq->card->host->context_info;
-
if (mq->asleep)
wake_up_process(mq->thread);
}
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 970d7f9b1eba..a7c0ed887391 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -192,14 +192,6 @@ struct mmc_slot {
void *handler_priv;
};
-/**
- * mmc_context_info - synchronization details for mmc context
- * @is_waiting_last_req mmc context waiting for single running request
- */
-struct mmc_context_info {
- bool is_waiting_last_req;
-};
-
struct regulator;
struct mmc_pwrseq;
@@ -373,7 +365,6 @@ struct mmc_host {
struct dentry *debugfs_root;
struct mmc_async_req *areq; /* active async req */
- struct mmc_context_info context_info; /* async synchronization info */
/* finalization work thread, handles finalizing requests */
struct kthread_worker req_done_worker;
--
2.9.3
^ permalink raw reply related [flat|nested] 51+ messages in thread* [PATCH 10/16] mmc: queue: simplify queue logic
2017-02-09 15:33 [PATCH 00/16] multiqueue for MMC/SD third try Linus Walleij
` (8 preceding siblings ...)
2017-02-09 15:33 ` [PATCH 09/16] mmc: core: kill off the context info Linus Walleij
@ 2017-02-09 15:33 ` Linus Walleij
[not found] ` <CGME20170228161132epcas5p265793e8675aa2f1e5dd199a9ee0ab6f1@epcas5p2.samsung.com>
2017-02-09 15:33 ` [PATCH 11/16] mmc: block: shuffle retry and error handling Linus Walleij
` (7 subsequent siblings)
17 siblings, 1 reply; 51+ messages in thread
From: Linus Walleij @ 2017-02-09 15:33 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson, Adrian Hunter, Paolo Valente
Cc: Chunyan Zhang, Baolin Wang, linux-block, Jens Axboe,
Christoph Hellwig, Arnd Bergmann, Linus Walleij
The if() statment checking if there is no current or previous
request is now just looking ahead at something that will be
concluded a few lines below. Simplify the logic by moving the
assignment of .asleep.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/mmc/core/queue.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c
index a845fe8d4fd1..bc116709c806 100644
--- a/drivers/mmc/core/queue.c
+++ b/drivers/mmc/core/queue.c
@@ -64,14 +64,6 @@ static int mmc_queue_thread(void *d)
set_current_state(TASK_INTERRUPTIBLE);
req = blk_fetch_request(q);
mq->asleep = false;
- if (!req) {
- /*
- * Dispatch queue is empty so set flags for
- * mmc_request_fn() to wake us up.
- */
- if (!mq->mqrq_prev->req)
- mq->asleep = true;
- }
mq->mqrq_cur->req = req;
spin_unlock_irq(q->queue_lock);
@@ -95,6 +87,7 @@ static int mmc_queue_thread(void *d)
mq->mqrq_prev->req = NULL;
swap(mq->mqrq_prev, mq->mqrq_cur);
} else {
+ mq->asleep = true;
if (kthread_should_stop()) {
set_current_state(TASK_RUNNING);
break;
--
2.9.3
^ permalink raw reply related [flat|nested] 51+ messages in thread* [PATCH 11/16] mmc: block: shuffle retry and error handling
2017-02-09 15:33 [PATCH 00/16] multiqueue for MMC/SD third try Linus Walleij
` (9 preceding siblings ...)
2017-02-09 15:33 ` [PATCH 10/16] mmc: queue: simplify queue logic Linus Walleij
@ 2017-02-09 15:33 ` Linus Walleij
[not found] ` <CGME20170228174522epcas5p34dce6477eb96f7e0fb38431c4de35f60@epcas5p3.samsung.com>
2017-02-09 15:33 ` [PATCH 12/16] mmc: queue: stop flushing the pipeline with NULL Linus Walleij
` (6 subsequent siblings)
17 siblings, 1 reply; 51+ messages in thread
From: Linus Walleij @ 2017-02-09 15:33 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson, Adrian Hunter, Paolo Valente
Cc: Chunyan Zhang, Baolin Wang, linux-block, Jens Axboe,
Christoph Hellwig, Arnd Bergmann, Linus Walleij
Instead of doing retries at the same time as trying to submit new
requests, do the retries when the request is reported as completed
by the driver, in the finalization worker.
This is achieved by letting the core worker call back into the block
layer using mmc_blk_rw_done(), that will read the status and repeatedly
try to hammer the request using single request etc by calling back to
the core layer using mmc_restart_areq()
The beauty of it is that the completion will not complete until the
block layer has had the opportunity to hammer a bit at the card using
a bunch of different approaches in the while() loop in
mmc_blk_rw_done()
The algorithm for recapture, retry and handle errors is essentially
identical to the one we used to have in mmc_blk_issue_rw_rq(),
only augmented to get called in another path.
We have to add and initialize a pointer back to the struct mmc_queue
from the struct mmc_queue_req to find the queue from the asynchronous
request.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/mmc/core/block.c | 307 +++++++++++++++++++++++------------------------
drivers/mmc/core/block.h | 3 +
drivers/mmc/core/core.c | 23 +++-
drivers/mmc/core/queue.c | 2 +
drivers/mmc/core/queue.h | 1 +
include/linux/mmc/core.h | 1 +
include/linux/mmc/host.h | 1 -
7 files changed, 177 insertions(+), 161 deletions(-)
diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index c459d80c66bf..0bd9070f5f2e 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -1614,182 +1614,181 @@ static void mmc_blk_rw_cmd_abort(struct mmc_card *card, struct request *req)
* @mq: the queue with the card and host to restart
* @req: a new request that want to be started after the current one
*/
-static void mmc_blk_rw_try_restart(struct mmc_queue *mq, struct request *req)
+static void mmc_blk_rw_try_restart(struct mmc_queue *mq)
{
- if (!req)
- return;
-
- /*
- * If the card was removed, just cancel everything and return.
- */
- if (mmc_card_removed(mq->card)) {
- req->rq_flags |= RQF_QUIET;
- blk_end_request_all(req, -EIO);
- return;
- }
- /* Else proceed and try to restart the current async request */
+ /* Proceed and try to restart the current async request */
mmc_blk_rw_rq_prep(mq->mqrq_cur, mq->card, 0, mq);
- mmc_start_areq(mq->card->host, &mq->mqrq_cur->areq, NULL);
+ mmc_restart_areq(mq->card->host, &mq->mqrq_cur->areq);
}
-static void mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *new_req)
+void mmc_blk_rw_done(struct mmc_async_req *areq,
+ enum mmc_blk_status status)
{
- struct mmc_blk_data *md = mq->blkdata;
- struct mmc_card *card = md->queue.card;
- struct mmc_blk_request *brq;
- int disable_multi = 0, retry = 0, type, retune_retry_done = 0;
- enum mmc_blk_status status;
+ struct mmc_queue *mq;
struct mmc_queue_req *mq_rq;
+ struct mmc_blk_request *brq;
+ struct mmc_blk_data *md;
struct request *old_req;
- struct mmc_async_req *new_areq;
- struct mmc_async_req *old_areq;
+ struct mmc_card *card;
+ struct mmc_host *host;
+ int disable_multi = 0, retry = 0, type, retune_retry_done = 0;
bool req_pending = true;
- if (!new_req && !mq->mqrq_prev->req)
- return;
-
- do {
- if (new_req) {
- /*
- * When 4KB native sector is enabled, only 8 blocks
- * multiple read or write is allowed
- */
- if (mmc_large_sector(card) &&
- !IS_ALIGNED(blk_rq_sectors(new_req), 8)) {
- pr_err("%s: Transfer size is not 4KB sector size aligned\n",
- new_req->rq_disk->disk_name);
- mmc_blk_rw_cmd_abort(card, new_req);
- return;
- }
-
- mmc_blk_rw_rq_prep(mq->mqrq_cur, card, 0, mq);
- new_areq = &mq->mqrq_cur->areq;
- } else
- new_areq = NULL;
-
- old_areq = mmc_start_areq(card->host, new_areq, &status);
- if (!old_areq) {
- /*
- * We have just put the first request into the pipeline
- * and there is nothing more to do until it is
- * complete.
- */
- return;
- }
-
+ /*
+ * An asynchronous request has been completed and we proceed
+ * to handle the result of it.
+ */
+ mq_rq = container_of(areq, struct mmc_queue_req, areq);
+ mq = mq_rq->mq;
+ md = mq->blkdata;
+ card = mq->card;
+ host = card->host;
+ brq = &mq_rq->brq;
+ old_req = mq_rq->req;
+ type = rq_data_dir(old_req) == READ ? MMC_BLK_READ : MMC_BLK_WRITE;
+
+ mmc_queue_bounce_post(mq_rq);
+
+ switch (status) {
+ case MMC_BLK_SUCCESS:
+ case MMC_BLK_PARTIAL:
/*
- * An asynchronous request has been completed and we proceed
- * to handle the result of it.
+ * A block was successfully transferred.
*/
- mq_rq = container_of(old_areq, struct mmc_queue_req, areq);
- brq = &mq_rq->brq;
- old_req = mq_rq->req;
- type = rq_data_dir(old_req) == READ ? MMC_BLK_READ : MMC_BLK_WRITE;
- mmc_queue_bounce_post(mq_rq);
-
- switch (status) {
- case MMC_BLK_SUCCESS:
- case MMC_BLK_PARTIAL:
- /*
- * A block was successfully transferred.
- */
- mmc_blk_reset_success(md, type);
+ mmc_blk_reset_success(md, type);
- req_pending = blk_end_request(old_req, 0,
- brq->data.bytes_xfered);
- /*
- * If the blk_end_request function returns non-zero even
- * though all data has been transferred and no errors
- * were returned by the host controller, it's a bug.
- */
- if (status == MMC_BLK_SUCCESS && req_pending) {
- pr_err("%s BUG rq_tot %d d_xfer %d\n",
- __func__, blk_rq_bytes(old_req),
- brq->data.bytes_xfered);
- mmc_blk_rw_cmd_abort(card, old_req);
- return;
- }
- break;
- case MMC_BLK_CMD_ERR:
- req_pending = mmc_blk_rw_cmd_err(md, card, brq, old_req, req_pending);
- if (mmc_blk_reset(md, card->host, type)) {
- mmc_blk_rw_cmd_abort(card, old_req);
- mmc_blk_rw_try_restart(mq, new_req);
- return;
- }
- if (!req_pending) {
- mmc_blk_rw_try_restart(mq, new_req);
- return;
- }
- break;
- case MMC_BLK_RETRY:
- retune_retry_done = brq->retune_retry_done;
- if (retry++ < 5)
- break;
- /* Fall through */
- case MMC_BLK_ABORT:
- if (!mmc_blk_reset(md, card->host, type))
- break;
+ req_pending = blk_end_request(old_req, 0,
+ brq->data.bytes_xfered);
+ /*
+ * If the blk_end_request function returns non-zero even
+ * though all data has been transferred and no errors
+ * were returned by the host controller, it's a bug.
+ */
+ if (status == MMC_BLK_SUCCESS && req_pending) {
+ pr_err("%s BUG rq_tot %d d_xfer %d\n",
+ __func__, blk_rq_bytes(old_req),
+ brq->data.bytes_xfered);
mmc_blk_rw_cmd_abort(card, old_req);
- mmc_blk_rw_try_restart(mq, new_req);
return;
- case MMC_BLK_DATA_ERR: {
- int err;
-
- err = mmc_blk_reset(md, card->host, type);
- if (!err)
- break;
- if (err == -ENODEV) {
- mmc_blk_rw_cmd_abort(card, old_req);
- mmc_blk_rw_try_restart(mq, new_req);
- return;
- }
- /* Fall through */
}
- case MMC_BLK_ECC_ERR:
- if (brq->data.blocks > 1) {
- /* Redo read one sector at a time */
- pr_warn("%s: retrying using single block read\n",
- old_req->rq_disk->disk_name);
- disable_multi = 1;
- break;
- }
- /*
- * After an error, we redo I/O one sector at a
- * time, so we only reach here after trying to
- * read a single sector.
- */
- req_pending = blk_end_request(old_req, -EIO,
- brq->data.blksz);
- if (!req_pending) {
- mmc_blk_rw_try_restart(mq, new_req);
- return;
- }
- break;
- case MMC_BLK_NOMEDIUM:
+ break;
+ case MMC_BLK_CMD_ERR:
+ req_pending = mmc_blk_rw_cmd_err(md, card, brq, old_req, req_pending);
+ if (mmc_blk_reset(md, host, type)) {
mmc_blk_rw_cmd_abort(card, old_req);
- mmc_blk_rw_try_restart(mq, new_req);
+ mmc_blk_rw_try_restart(mq);
return;
- default:
- pr_err("%s: Unhandled return value (%d)",
- old_req->rq_disk->disk_name, status);
+ }
+ if (!req_pending) {
+ mmc_blk_rw_try_restart(mq);
+ return;
+ }
+ break;
+ case MMC_BLK_RETRY:
+ retune_retry_done = brq->retune_retry_done;
+ if (retry++ < 5)
+ break;
+ /* Fall through */
+ case MMC_BLK_ABORT:
+ if (!mmc_blk_reset(md, host, type))
+ break;
+ mmc_blk_rw_cmd_abort(card, old_req);
+ mmc_blk_rw_try_restart(mq);
+ return;
+ case MMC_BLK_DATA_ERR: {
+ int err;
+ err = mmc_blk_reset(md, host, type);
+ if (!err)
+ break;
+ if (err == -ENODEV) {
mmc_blk_rw_cmd_abort(card, old_req);
- mmc_blk_rw_try_restart(mq, new_req);
+ mmc_blk_rw_try_restart(mq);
return;
}
+ /* Fall through */
+ }
+ case MMC_BLK_ECC_ERR:
+ if (brq->data.blocks > 1) {
+ /* Redo read one sector at a time */
+ pr_warn("%s: retrying using single block read\n",
+ old_req->rq_disk->disk_name);
+ disable_multi = 1;
+ break;
+ }
+ /*
+ * After an error, we redo I/O one sector at a
+ * time, so we only reach here after trying to
+ * read a single sector.
+ */
+ req_pending = blk_end_request(old_req, -EIO,
+ brq->data.blksz);
+ if (!req_pending) {
+ mmc_blk_rw_try_restart(mq);
+ return;
+ }
+ break;
+ case MMC_BLK_NOMEDIUM:
+ mmc_blk_rw_cmd_abort(card, old_req);
+ mmc_blk_rw_try_restart(mq);
+ return;
+ default:
+ pr_err("%s: Unhandled return value (%d)",
+ old_req->rq_disk->disk_name, status);
+ mmc_blk_rw_cmd_abort(card, old_req);
+ mmc_blk_rw_try_restart(mq);
+ return;
+ }
- if (req_pending) {
- /*
- * In case of a incomplete request
- * prepare it again and resend.
- */
- mmc_blk_rw_rq_prep(mq_rq, card,
- disable_multi, mq);
- mmc_start_areq(card->host,
- &mq_rq->areq, NULL);
- mq_rq->brq.retune_retry_done = retune_retry_done;
+ if (req_pending) {
+ /*
+ * In case of a incomplete request
+ * prepare it again and resend.
+ */
+ mmc_blk_rw_rq_prep(mq_rq, card,
+ disable_multi, mq);
+ mq_rq->brq.retune_retry_done = retune_retry_done;
+ mmc_restart_areq(host, &mq->mqrq_cur->areq);
+ }
+}
+
+static void mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *new_req)
+{
+ enum mmc_blk_status status;
+ struct mmc_async_req *new_areq;
+ struct mmc_async_req *old_areq;
+ struct mmc_card *card = mq->card;
+
+ if (!new_req && !mq->mqrq_prev->req)
+ return;
+
+ if (new_req) {
+ /*
+ * When 4KB native sector is enabled, only 8 blocks
+ * multiple read or write is allowed
+ */
+ if (mmc_large_sector(card) &&
+ !IS_ALIGNED(blk_rq_sectors(new_req), 8)) {
+ pr_err("%s: Transfer size is not 4KB sector size aligned\n",
+ new_req->rq_disk->disk_name);
+ mmc_blk_rw_cmd_abort(card, new_req);
+ return;
}
- } while (req_pending);
+
+ mmc_blk_rw_rq_prep(mq->mqrq_cur, card, 0, mq);
+ new_areq = &mq->mqrq_cur->areq;
+ } else
+ new_areq = NULL;
+
+ old_areq = mmc_start_areq(card->host, new_areq, &status);
+ if (!old_areq) {
+ /*
+ * We have just put the first request into the pipeline
+ * and there is nothing more to do until it is
+ * complete.
+ */
+ return;
+ }
+ /* FIXME: yes, we just disregard the old_areq */
}
void mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req)
diff --git a/drivers/mmc/core/block.h b/drivers/mmc/core/block.h
index 860ca7c8df86..b4b489911599 100644
--- a/drivers/mmc/core/block.h
+++ b/drivers/mmc/core/block.h
@@ -1,9 +1,12 @@
#ifndef _MMC_CORE_BLOCK_H
#define _MMC_CORE_BLOCK_H
+struct mmc_async_req;
+enum mmc_blk_status;
struct mmc_queue;
struct request;
+void mmc_blk_rw_done(struct mmc_async_req *areq, enum mmc_blk_status status);
void mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req);
#endif
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 4b84f18518ac..34337ef6705e 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -39,6 +39,7 @@
#define CREATE_TRACE_POINTS
#include <trace/events/mmc.h>
+#include "block.h"
#include "core.h"
#include "card.h"
#include "bus.h"
@@ -632,13 +633,25 @@ void mmc_finalize_areq(struct kthread_work *work)
/* Successfully postprocess the old request at this point */
mmc_post_req(host, areq->mrq, 0);
+ mmc_blk_rw_done(areq, status);
- areq->finalization_status = status;
complete(&areq->complete);
}
EXPORT_SYMBOL(mmc_finalize_areq);
/**
+ * mmc_restart_areq() - restart an asynchronous request
+ * @host: MMC host to restart the command on
+ * @areq: the asynchronous request to restart
+ */
+int mmc_restart_areq(struct mmc_host *host,
+ struct mmc_async_req *areq)
+{
+ return __mmc_start_data_req(host, areq->mrq);
+}
+EXPORT_SYMBOL(mmc_restart_areq);
+
+/**
* mmc_start_areq - start an asynchronous request
* @host: MMC host to start command
* @areq: asynchronous request to start
@@ -667,12 +680,10 @@ struct mmc_async_req *mmc_start_areq(struct mmc_host *host,
mmc_pre_req(host, areq->mrq);
/* Finalize previous request, if there is one */
- if (previous) {
+ if (previous)
wait_for_completion(&previous->complete);
- status = previous->finalization_status;
- } else {
- status = MMC_BLK_SUCCESS;
- }
+
+ status = MMC_BLK_SUCCESS;
if (ret_stat)
*ret_stat = status;
diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c
index bc116709c806..ae6837317fe0 100644
--- a/drivers/mmc/core/queue.c
+++ b/drivers/mmc/core/queue.c
@@ -268,7 +268,9 @@ int mmc_init_queue(struct mmc_queue *mq, struct mmc_card *card,
if (!mq->mqrq)
goto blk_cleanup;
mq->mqrq_cur = &mq->mqrq[0];
+ mq->mqrq_cur->mq = mq;
mq->mqrq_prev = &mq->mqrq[1];
+ mq->mqrq_prev->mq = mq;
mq->queue->queuedata = mq;
blk_queue_prep_rq(mq->queue, mmc_prep_request);
diff --git a/drivers/mmc/core/queue.h b/drivers/mmc/core/queue.h
index 39d8e710287e..c18d3f908433 100644
--- a/drivers/mmc/core/queue.h
+++ b/drivers/mmc/core/queue.h
@@ -34,6 +34,7 @@ struct mmc_queue_req {
struct scatterlist *bounce_sg;
unsigned int bounce_sg_len;
struct mmc_async_req areq;
+ struct mmc_queue *mq;
};
struct mmc_queue {
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index 5db0fb722c37..55b45dcddee6 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -159,6 +159,7 @@ struct mmc_card;
struct mmc_async_req;
void mmc_finalize_areq(struct kthread_work *work);
+int mmc_restart_areq(struct mmc_host *host, struct mmc_async_req *areq);
struct mmc_async_req *mmc_start_areq(struct mmc_host *host,
struct mmc_async_req *areq,
enum mmc_blk_status *ret_stat);
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index a7c0ed887391..47d80b8470cd 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -171,7 +171,6 @@ struct mmc_async_req {
*/
enum mmc_blk_status (*err_check)(struct mmc_card *, struct mmc_async_req *);
struct kthread_work finalization_work;
- enum mmc_blk_status finalization_status;
struct completion complete;
struct mmc_host *host;
};
--
2.9.3
^ permalink raw reply related [flat|nested] 51+ messages in thread* [PATCH 12/16] mmc: queue: stop flushing the pipeline with NULL
2017-02-09 15:33 [PATCH 00/16] multiqueue for MMC/SD third try Linus Walleij
` (10 preceding siblings ...)
2017-02-09 15:33 ` [PATCH 11/16] mmc: block: shuffle retry and error handling Linus Walleij
@ 2017-02-09 15:33 ` Linus Walleij
[not found] ` <CGME20170228180309epcas5p317af83f41d3b0426868dcfd660bd0aec@epcas5p3.samsung.com>
2017-02-09 15:34 ` [PATCH 13/16] mmc: queue: issue struct mmc_queue_req items Linus Walleij
` (5 subsequent siblings)
17 siblings, 1 reply; 51+ messages in thread
From: Linus Walleij @ 2017-02-09 15:33 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson, Adrian Hunter, Paolo Valente
Cc: Chunyan Zhang, Baolin Wang, linux-block, Jens Axboe,
Christoph Hellwig, Arnd Bergmann, Linus Walleij
Remove all the pipeline flush: i.e. repeatedly sending NULL
down to the core layer to flush out asynchronous requests,
and also sending NULL after "special" commands to achieve the
same flush.
Instead: let the "special" commands wait for any ongoing
asynchronous transfers using the completion, and apart from
that expect the core.c and block.c layers to deal with the
ongoing requests autonomously without any "push" from the
queue.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/mmc/core/block.c | 80 +++++++++++++++++-------------------------------
drivers/mmc/core/core.c | 37 ++++++++++------------
drivers/mmc/core/queue.c | 18 ++++++++---
include/linux/mmc/core.h | 5 ++-
4 files changed, 60 insertions(+), 80 deletions(-)
diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 0bd9070f5f2e..4952a105780e 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -1753,42 +1753,27 @@ void mmc_blk_rw_done(struct mmc_async_req *areq,
static void mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *new_req)
{
- enum mmc_blk_status status;
- struct mmc_async_req *new_areq;
- struct mmc_async_req *old_areq;
struct mmc_card *card = mq->card;
- if (!new_req && !mq->mqrq_prev->req)
+ if (!new_req) {
+ pr_err("%s: NULL request!\n", __func__);
return;
+ }
- if (new_req) {
- /*
- * When 4KB native sector is enabled, only 8 blocks
- * multiple read or write is allowed
- */
- if (mmc_large_sector(card) &&
- !IS_ALIGNED(blk_rq_sectors(new_req), 8)) {
- pr_err("%s: Transfer size is not 4KB sector size aligned\n",
- new_req->rq_disk->disk_name);
- mmc_blk_rw_cmd_abort(card, new_req);
- return;
- }
-
- mmc_blk_rw_rq_prep(mq->mqrq_cur, card, 0, mq);
- new_areq = &mq->mqrq_cur->areq;
- } else
- new_areq = NULL;
-
- old_areq = mmc_start_areq(card->host, new_areq, &status);
- if (!old_areq) {
- /*
- * We have just put the first request into the pipeline
- * and there is nothing more to do until it is
- * complete.
- */
+ /*
+ * When 4KB native sector is enabled, only 8 blocks
+ * multiple read or write is allowed
+ */
+ if (mmc_large_sector(card) &&
+ !IS_ALIGNED(blk_rq_sectors(new_req), 8)) {
+ pr_err("%s: Transfer size is not 4KB sector size aligned\n",
+ new_req->rq_disk->disk_name);
+ mmc_blk_rw_cmd_abort(card, new_req);
return;
}
- /* FIXME: yes, we just disregard the old_areq */
+
+ mmc_blk_rw_rq_prep(mq->mqrq_cur, card, 0, mq);
+ mmc_start_areq(card->host, &mq->mqrq_cur->areq);
}
void mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req)
@@ -1796,48 +1781,39 @@ void mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req)
int ret;
struct mmc_blk_data *md = mq->blkdata;
struct mmc_card *card = md->queue.card;
- bool req_is_special = mmc_req_is_special(req);
-
- if (req && !mq->mqrq_prev->req)
- /* claim host only for the first request */
- mmc_get_card(card);
ret = mmc_blk_part_switch(card, md);
if (ret) {
if (req) {
blk_end_request_all(req, -EIO);
}
- goto out;
+ return;
}
if (req && req_op(req) == REQ_OP_DISCARD) {
/* complete ongoing async transfer before issuing discard */
- if (card->host->areq)
- mmc_blk_issue_rw_rq(mq, NULL);
+ if (card->host->areq) {
+ wait_for_completion(&card->host->areq->complete);
+ card->host->areq = NULL;
+ }
mmc_blk_issue_discard_rq(mq, req);
} else if (req && req_op(req) == REQ_OP_SECURE_ERASE) {
/* complete ongoing async transfer before issuing secure erase*/
- if (card->host->areq)
- mmc_blk_issue_rw_rq(mq, NULL);
+ if (card->host->areq) {
+ wait_for_completion(&card->host->areq->complete);
+ card->host->areq = NULL;
+ }
mmc_blk_issue_secdiscard_rq(mq, req);
} else if (req && req_op(req) == REQ_OP_FLUSH) {
/* complete ongoing async transfer before issuing flush */
- if (card->host->areq)
- mmc_blk_issue_rw_rq(mq, NULL);
+ if (card->host->areq) {
+ wait_for_completion(&card->host->areq->complete);
+ card->host->areq = NULL;
+ }
mmc_blk_issue_flush(mq, req);
} else {
mmc_blk_issue_rw_rq(mq, req);
}
-
-out:
- if (!req || req_is_special)
- /*
- * Release host when there are no more requests
- * and after special request(discard, flush) is done.
- * In case sepecial request, there is no reentry to
- * the 'mmc_blk_issue_rq' with 'mqrq_prev->req'.
- */
- mmc_put_card(card);
}
static inline int mmc_blk_readonly(struct mmc_card *card)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 34337ef6705e..03c290e5e2c9 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -667,42 +667,37 @@ EXPORT_SYMBOL(mmc_restart_areq);
* return the completed request. If there is no ongoing request, NULL
* is returned without waiting. NULL is not an error condition.
*/
-struct mmc_async_req *mmc_start_areq(struct mmc_host *host,
- struct mmc_async_req *areq,
- enum mmc_blk_status *ret_stat)
+int mmc_start_areq(struct mmc_host *host,
+ struct mmc_async_req *areq)
{
- enum mmc_blk_status status;
- int start_err = 0;
+ int ret;
struct mmc_async_req *previous = host->areq;
/* Prepare a new request */
- if (areq)
- mmc_pre_req(host, areq->mrq);
+ if (!areq) {
+ pr_err("%s: NULL asynchronous request!\n", __func__);
+ return -EIO;
+ }
+
+ mmc_pre_req(host, areq->mrq);
/* Finalize previous request, if there is one */
if (previous)
wait_for_completion(&previous->complete);
- status = MMC_BLK_SUCCESS;
- if (ret_stat)
- *ret_stat = status;
-
/* Fine so far, start the new request! */
- if (status == MMC_BLK_SUCCESS && areq) {
- init_completion(&areq->complete);
- start_err = __mmc_start_data_req(host, areq->mrq);
- }
+ init_completion(&areq->complete);
+ ret = __mmc_start_data_req(host, areq->mrq);
/* Cancel a prepared request if it was not started. */
- if ((status != MMC_BLK_SUCCESS || start_err) && areq)
+ if (ret) {
mmc_post_req(host, areq->mrq, -EINVAL);
-
- if (status != MMC_BLK_SUCCESS)
host->areq = NULL;
- else
- host->areq = areq;
+ pr_err("%s: failed to start request\n", __func__);
+ }
+ host->areq = areq;
- return previous;
+ return ret;
}
EXPORT_SYMBOL(mmc_start_areq);
diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c
index ae6837317fe0..c9f28de7b0f4 100644
--- a/drivers/mmc/core/queue.c
+++ b/drivers/mmc/core/queue.c
@@ -53,6 +53,7 @@ static int mmc_queue_thread(void *d)
{
struct mmc_queue *mq = d;
struct request_queue *q = mq->queue;
+ bool claimed_host = false;
current->flags |= PF_MEMALLOC;
@@ -67,9 +68,11 @@ static int mmc_queue_thread(void *d)
mq->mqrq_cur->req = req;
spin_unlock_irq(q->queue_lock);
- if (req || mq->mqrq_prev->req) {
+ if (req) {
bool req_is_special = mmc_req_is_special(req);
+ if (!claimed_host)
+ mmc_get_card(mq->card);
set_current_state(TASK_RUNNING);
mmc_blk_issue_rq(mq, req);
cond_resched();
@@ -78,11 +81,14 @@ static int mmc_queue_thread(void *d)
* and vice versa.
* In case of special requests, current request
* has been finished. Do not assign it to previous
- * request.
+ * request. Always unclaim the host after special
+ * commands.
*/
- if (req_is_special)
+ if (req_is_special) {
mq->mqrq_cur->req = NULL;
-
+ mmc_put_card(mq->card);
+ claimed_host = false;
+ }
mq->mqrq_prev->brq.mrq.data = NULL;
mq->mqrq_prev->req = NULL;
swap(mq->mqrq_prev, mq->mqrq_cur);
@@ -97,6 +103,10 @@ static int mmc_queue_thread(void *d)
down(&mq->thread_sem);
}
} while (1);
+
+ if (claimed_host)
+ mmc_put_card(mq->card);
+
up(&mq->thread_sem);
return 0;
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index 55b45dcddee6..af651e723ba2 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -160,9 +160,8 @@ struct mmc_async_req;
void mmc_finalize_areq(struct kthread_work *work);
int mmc_restart_areq(struct mmc_host *host, struct mmc_async_req *areq);
-struct mmc_async_req *mmc_start_areq(struct mmc_host *host,
- struct mmc_async_req *areq,
- enum mmc_blk_status *ret_stat);
+int mmc_start_areq(struct mmc_host *host,
+ struct mmc_async_req *areq);
void mmc_wait_for_req(struct mmc_host *host, struct mmc_request *mrq);
int mmc_wait_for_cmd(struct mmc_host *host, struct mmc_command *cmd,
int retries);
--
2.9.3
^ permalink raw reply related [flat|nested] 51+ messages in thread* [PATCH 13/16] mmc: queue: issue struct mmc_queue_req items
2017-02-09 15:33 [PATCH 00/16] multiqueue for MMC/SD third try Linus Walleij
` (11 preceding siblings ...)
2017-02-09 15:33 ` [PATCH 12/16] mmc: queue: stop flushing the pipeline with NULL Linus Walleij
@ 2017-02-09 15:34 ` Linus Walleij
[not found] ` <CGME20170228181009epcas1p4ca0e714214097d07d7172182ba8e032b@epcas1p4.samsung.com>
2017-02-09 15:34 ` [PATCH 14/16] mmc: queue: get/put struct mmc_queue_req Linus Walleij
` (4 subsequent siblings)
17 siblings, 1 reply; 51+ messages in thread
From: Linus Walleij @ 2017-02-09 15:34 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson, Adrian Hunter, Paolo Valente
Cc: Chunyan Zhang, Baolin Wang, linux-block, Jens Axboe,
Christoph Hellwig, Arnd Bergmann, Linus Walleij
Instead of passing two pointers around and messing and reassigning
to the left and right, issue mmc_queue_req and dereference
the queue from the request where needed. The struct mmc_queue_req
is the thing that has a lifecycle after all: this is what we are
keepin in out queue. Augment all users to be passed the struct
mmc_queue_req as well.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/mmc/core/block.c | 88 ++++++++++++++++++++++++------------------------
drivers/mmc/core/block.h | 5 ++-
drivers/mmc/core/queue.c | 6 ++--
3 files changed, 50 insertions(+), 49 deletions(-)
diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 4952a105780e..628a22b9bf41 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -1151,9 +1151,9 @@ int mmc_access_rpmb(struct mmc_queue *mq)
return false;
}
-static void mmc_blk_issue_discard_rq(struct mmc_queue *mq, struct request *req)
+static void mmc_blk_issue_discard_rq(struct mmc_queue_req *mq_rq)
{
- struct mmc_blk_data *md = mq->blkdata;
+ struct mmc_blk_data *md = mq_rq->mq->blkdata;
struct mmc_card *card = md->queue.card;
unsigned int from, nr, arg;
int err = 0, type = MMC_BLK_DISCARD;
@@ -1163,8 +1163,8 @@ static void mmc_blk_issue_discard_rq(struct mmc_queue *mq, struct request *req)
goto fail;
}
- from = blk_rq_pos(req);
- nr = blk_rq_sectors(req);
+ from = blk_rq_pos(mq_rq->req);
+ nr = blk_rq_sectors(mq_rq->req);
if (mmc_can_discard(card))
arg = MMC_DISCARD_ARG;
@@ -1188,13 +1188,12 @@ static void mmc_blk_issue_discard_rq(struct mmc_queue *mq, struct request *req)
if (!err)
mmc_blk_reset_success(md, type);
fail:
- blk_end_request(req, err, blk_rq_bytes(req));
+ blk_end_request(mq_rq->req, err, blk_rq_bytes(mq_rq->req));
}
-static void mmc_blk_issue_secdiscard_rq(struct mmc_queue *mq,
- struct request *req)
+static void mmc_blk_issue_secdiscard_rq(struct mmc_queue_req *mq_rq)
{
- struct mmc_blk_data *md = mq->blkdata;
+ struct mmc_blk_data *md = mq_rq->mq->blkdata;
struct mmc_card *card = md->queue.card;
unsigned int from, nr, arg;
int err = 0, type = MMC_BLK_SECDISCARD;
@@ -1204,8 +1203,8 @@ static void mmc_blk_issue_secdiscard_rq(struct mmc_queue *mq,
goto out;
}
- from = blk_rq_pos(req);
- nr = blk_rq_sectors(req);
+ from = blk_rq_pos(mq_rq->req);
+ nr = blk_rq_sectors(mq_rq->req);
if (mmc_can_trim(card) && !mmc_erase_group_aligned(card, from, nr))
arg = MMC_SECURE_TRIM1_ARG;
@@ -1253,12 +1252,12 @@ static void mmc_blk_issue_secdiscard_rq(struct mmc_queue *mq,
if (!err)
mmc_blk_reset_success(md, type);
out:
- blk_end_request(req, err, blk_rq_bytes(req));
+ blk_end_request(mq_rq->req, err, blk_rq_bytes(mq_rq->req));
}
-static void mmc_blk_issue_flush(struct mmc_queue *mq, struct request *req)
+static void mmc_blk_issue_flush(struct mmc_queue_req *mq_rq)
{
- struct mmc_blk_data *md = mq->blkdata;
+ struct mmc_blk_data *md = mq_rq->mq->blkdata;
struct mmc_card *card = md->queue.card;
int ret = 0;
@@ -1266,7 +1265,7 @@ static void mmc_blk_issue_flush(struct mmc_queue *mq, struct request *req)
if (ret)
ret = -EIO;
- blk_end_request_all(req, ret);
+ blk_end_request_all(mq_rq->req, ret);
}
/*
@@ -1614,11 +1613,13 @@ static void mmc_blk_rw_cmd_abort(struct mmc_card *card, struct request *req)
* @mq: the queue with the card and host to restart
* @req: a new request that want to be started after the current one
*/
-static void mmc_blk_rw_try_restart(struct mmc_queue *mq)
+static void mmc_blk_rw_try_restart(struct mmc_queue_req *mq_rq)
{
+ struct mmc_queue *mq = mq_rq->mq;
+
/* Proceed and try to restart the current async request */
- mmc_blk_rw_rq_prep(mq->mqrq_cur, mq->card, 0, mq);
- mmc_restart_areq(mq->card->host, &mq->mqrq_cur->areq);
+ mmc_blk_rw_rq_prep(mq_rq, mq->card, 0, mq);
+ mmc_restart_areq(mq->card->host, &mq_rq->areq);
}
void mmc_blk_rw_done(struct mmc_async_req *areq,
@@ -1676,11 +1677,11 @@ void mmc_blk_rw_done(struct mmc_async_req *areq,
req_pending = mmc_blk_rw_cmd_err(md, card, brq, old_req, req_pending);
if (mmc_blk_reset(md, host, type)) {
mmc_blk_rw_cmd_abort(card, old_req);
- mmc_blk_rw_try_restart(mq);
+ mmc_blk_rw_try_restart(mq_rq);
return;
}
if (!req_pending) {
- mmc_blk_rw_try_restart(mq);
+ mmc_blk_rw_try_restart(mq_rq);
return;
}
break;
@@ -1693,7 +1694,7 @@ void mmc_blk_rw_done(struct mmc_async_req *areq,
if (!mmc_blk_reset(md, host, type))
break;
mmc_blk_rw_cmd_abort(card, old_req);
- mmc_blk_rw_try_restart(mq);
+ mmc_blk_rw_try_restart(mq_rq);
return;
case MMC_BLK_DATA_ERR: {
int err;
@@ -1702,7 +1703,7 @@ void mmc_blk_rw_done(struct mmc_async_req *areq,
break;
if (err == -ENODEV) {
mmc_blk_rw_cmd_abort(card, old_req);
- mmc_blk_rw_try_restart(mq);
+ mmc_blk_rw_try_restart(mq_rq);
return;
}
/* Fall through */
@@ -1723,19 +1724,19 @@ void mmc_blk_rw_done(struct mmc_async_req *areq,
req_pending = blk_end_request(old_req, -EIO,
brq->data.blksz);
if (!req_pending) {
- mmc_blk_rw_try_restart(mq);
+ mmc_blk_rw_try_restart(mq_rq);
return;
}
break;
case MMC_BLK_NOMEDIUM:
mmc_blk_rw_cmd_abort(card, old_req);
- mmc_blk_rw_try_restart(mq);
+ mmc_blk_rw_try_restart(mq_rq);
return;
default:
pr_err("%s: Unhandled return value (%d)",
old_req->rq_disk->disk_name, status);
mmc_blk_rw_cmd_abort(card, old_req);
- mmc_blk_rw_try_restart(mq);
+ mmc_blk_rw_try_restart(mq_rq);
return;
}
@@ -1747,15 +1748,16 @@ void mmc_blk_rw_done(struct mmc_async_req *areq,
mmc_blk_rw_rq_prep(mq_rq, card,
disable_multi, mq);
mq_rq->brq.retune_retry_done = retune_retry_done;
- mmc_restart_areq(host, &mq->mqrq_cur->areq);
+ mmc_restart_areq(host, &mq_rq->areq);
}
}
-static void mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *new_req)
+static void mmc_blk_issue_rw_rq(struct mmc_queue_req *mq_rq)
{
+ struct mmc_queue *mq = mq_rq->mq;
struct mmc_card *card = mq->card;
- if (!new_req) {
+ if (!mq_rq->req) {
pr_err("%s: NULL request!\n", __func__);
return;
}
@@ -1765,54 +1767,52 @@ static void mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *new_req)
* multiple read or write is allowed
*/
if (mmc_large_sector(card) &&
- !IS_ALIGNED(blk_rq_sectors(new_req), 8)) {
+ !IS_ALIGNED(blk_rq_sectors(mq_rq->req), 8)) {
pr_err("%s: Transfer size is not 4KB sector size aligned\n",
- new_req->rq_disk->disk_name);
- mmc_blk_rw_cmd_abort(card, new_req);
+ mq_rq->req->rq_disk->disk_name);
+ mmc_blk_rw_cmd_abort(card, mq_rq->req);
return;
}
- mmc_blk_rw_rq_prep(mq->mqrq_cur, card, 0, mq);
- mmc_start_areq(card->host, &mq->mqrq_cur->areq);
+ mmc_blk_rw_rq_prep(mq_rq, card, 0, mq);
+ mmc_start_areq(card->host, &mq_rq->areq);
}
-void mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req)
+void mmc_blk_issue_rq(struct mmc_queue_req *mq_rq)
{
int ret;
- struct mmc_blk_data *md = mq->blkdata;
+ struct mmc_blk_data *md = mq_rq->mq->blkdata;
struct mmc_card *card = md->queue.card;
ret = mmc_blk_part_switch(card, md);
if (ret) {
- if (req) {
- blk_end_request_all(req, -EIO);
- }
+ blk_end_request_all(mq_rq->req, -EIO);
return;
}
- if (req && req_op(req) == REQ_OP_DISCARD) {
+ if (req_op(mq_rq->req) == REQ_OP_DISCARD) {
/* complete ongoing async transfer before issuing discard */
if (card->host->areq) {
wait_for_completion(&card->host->areq->complete);
card->host->areq = NULL;
}
- mmc_blk_issue_discard_rq(mq, req);
- } else if (req && req_op(req) == REQ_OP_SECURE_ERASE) {
+ mmc_blk_issue_discard_rq(mq_rq);
+ } else if (req_op(mq_rq->req) == REQ_OP_SECURE_ERASE) {
/* complete ongoing async transfer before issuing secure erase*/
if (card->host->areq) {
wait_for_completion(&card->host->areq->complete);
card->host->areq = NULL;
}
- mmc_blk_issue_secdiscard_rq(mq, req);
- } else if (req && req_op(req) == REQ_OP_FLUSH) {
+ mmc_blk_issue_secdiscard_rq(mq_rq);
+ } else if (req_op(mq_rq->req) == REQ_OP_FLUSH) {
/* complete ongoing async transfer before issuing flush */
if (card->host->areq) {
wait_for_completion(&card->host->areq->complete);
card->host->areq = NULL;
}
- mmc_blk_issue_flush(mq, req);
+ mmc_blk_issue_flush(mq_rq);
} else {
- mmc_blk_issue_rw_rq(mq, req);
+ mmc_blk_issue_rw_rq(mq_rq);
}
}
diff --git a/drivers/mmc/core/block.h b/drivers/mmc/core/block.h
index b4b489911599..0326fa5d8217 100644
--- a/drivers/mmc/core/block.h
+++ b/drivers/mmc/core/block.h
@@ -3,10 +3,9 @@
struct mmc_async_req;
enum mmc_blk_status;
-struct mmc_queue;
-struct request;
+struct mmc_queue_req;
void mmc_blk_rw_done(struct mmc_async_req *areq, enum mmc_blk_status status);
-void mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req);
+void mmc_blk_issue_rq(struct mmc_queue_req *mq_rq);
#endif
diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c
index c9f28de7b0f4..c4e1ced55796 100644
--- a/drivers/mmc/core/queue.c
+++ b/drivers/mmc/core/queue.c
@@ -54,6 +54,7 @@ static int mmc_queue_thread(void *d)
struct mmc_queue *mq = d;
struct request_queue *q = mq->queue;
bool claimed_host = false;
+ struct mmc_queue_req *mq_rq;
current->flags |= PF_MEMALLOC;
@@ -65,7 +66,8 @@ static int mmc_queue_thread(void *d)
set_current_state(TASK_INTERRUPTIBLE);
req = blk_fetch_request(q);
mq->asleep = false;
- mq->mqrq_cur->req = req;
+ mq_rq = mq->mqrq_cur;
+ mq_rq->req = req;
spin_unlock_irq(q->queue_lock);
if (req) {
@@ -74,7 +76,7 @@ static int mmc_queue_thread(void *d)
if (!claimed_host)
mmc_get_card(mq->card);
set_current_state(TASK_RUNNING);
- mmc_blk_issue_rq(mq, req);
+ mmc_blk_issue_rq(mq_rq);
cond_resched();
/*
* Current request becomes previous request
--
2.9.3
^ permalink raw reply related [flat|nested] 51+ messages in thread* [PATCH 14/16] mmc: queue: get/put struct mmc_queue_req
2017-02-09 15:33 [PATCH 00/16] multiqueue for MMC/SD third try Linus Walleij
` (12 preceding siblings ...)
2017-02-09 15:34 ` [PATCH 13/16] mmc: queue: issue struct mmc_queue_req items Linus Walleij
@ 2017-02-09 15:34 ` Linus Walleij
[not found] ` <CGME20170228182149epcas1p28789bce5433cee1579e8b8d083ba5811@epcas1p2.samsung.com>
2017-02-09 15:34 ` [PATCH 15/16] mmc: queue: issue requests in massive parallel Linus Walleij
` (3 subsequent siblings)
17 siblings, 1 reply; 51+ messages in thread
From: Linus Walleij @ 2017-02-09 15:34 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson, Adrian Hunter, Paolo Valente
Cc: Chunyan Zhang, Baolin Wang, linux-block, Jens Axboe,
Christoph Hellwig, Arnd Bergmann, Linus Walleij
The per-hardware-transaction struct mmc_queue_req is assigned
from a pool of 2 requests using a current/previous scheme and
then swapped around.
This is confusing, especially if we need more than two to make
our work efficient and parallel.
Rewrite the mechanism to have a pool of struct mmc_queue_req
and take one when we need one and put it back when we don't
need it anymore.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/mmc/core/block.c | 3 +++
drivers/mmc/core/core.c | 4 ++++
drivers/mmc/core/queue.c | 57 ++++++++++++++++++++++++++++++++++++++----------
drivers/mmc/core/queue.h | 8 ++++---
4 files changed, 57 insertions(+), 15 deletions(-)
diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 628a22b9bf41..acca15cc1807 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -1797,6 +1797,7 @@ void mmc_blk_issue_rq(struct mmc_queue_req *mq_rq)
card->host->areq = NULL;
}
mmc_blk_issue_discard_rq(mq_rq);
+ mmc_queue_req_put(mq_rq);
} else if (req_op(mq_rq->req) == REQ_OP_SECURE_ERASE) {
/* complete ongoing async transfer before issuing secure erase*/
if (card->host->areq) {
@@ -1804,6 +1805,7 @@ void mmc_blk_issue_rq(struct mmc_queue_req *mq_rq)
card->host->areq = NULL;
}
mmc_blk_issue_secdiscard_rq(mq_rq);
+ mmc_queue_req_put(mq_rq);
} else if (req_op(mq_rq->req) == REQ_OP_FLUSH) {
/* complete ongoing async transfer before issuing flush */
if (card->host->areq) {
@@ -1811,6 +1813,7 @@ void mmc_blk_issue_rq(struct mmc_queue_req *mq_rq)
card->host->areq = NULL;
}
mmc_blk_issue_flush(mq_rq);
+ mmc_queue_req_put(mq_rq);
} else {
mmc_blk_issue_rw_rq(mq_rq);
}
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 03c290e5e2c9..50a8942b98c2 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -39,6 +39,7 @@
#define CREATE_TRACE_POINTS
#include <trace/events/mmc.h>
+#include "queue.h"
#include "block.h"
#include "core.h"
#include "card.h"
@@ -598,6 +599,8 @@ void mmc_finalize_areq(struct kthread_work *work)
{
struct mmc_async_req *areq =
container_of(work, struct mmc_async_req, finalization_work);
+ struct mmc_queue_req *mq_rq = container_of(areq, struct mmc_queue_req,
+ areq);
struct mmc_host *host = areq->host;
enum mmc_blk_status status = MMC_BLK_SUCCESS;
struct mmc_command *cmd;
@@ -636,6 +639,7 @@ void mmc_finalize_areq(struct kthread_work *work)
mmc_blk_rw_done(areq, status);
complete(&areq->complete);
+ mmc_queue_req_put(mq_rq);
}
EXPORT_SYMBOL(mmc_finalize_areq);
diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c
index c4e1ced55796..cab0f51dbb4d 100644
--- a/drivers/mmc/core/queue.c
+++ b/drivers/mmc/core/queue.c
@@ -49,6 +49,42 @@ static int mmc_prep_request(struct request_queue *q, struct request *req)
return BLKPREP_OK;
}
+/**
+ * Get an available queue item from the pool.
+ */
+static struct mmc_queue_req *mmc_queue_req_get(struct mmc_queue *mq)
+{
+ int i;
+
+ /*
+ * This simply cannot fail so we just spin here
+ * until we get a queue request to work on.
+ */
+ while (1) {
+ /* Just take the first unused queue request */
+ spin_lock_irq(&mq->mqrq_lock);
+ for (i = 0; i < mq->qdepth; i++) {
+ if (!mq->mqrq[i].in_use) {
+ mq->mqrq[i].in_use = true;
+ spin_unlock_irq(&mq->mqrq_lock);
+ return &mq->mqrq[i];
+ }
+ }
+ spin_unlock_irq(&mq->mqrq_lock);
+
+ pr_warn_once("%s: out of queue items, spinning\n", __func__);
+ }
+}
+
+void mmc_queue_req_put(struct mmc_queue_req *mq_rq)
+{
+ mq_rq->brq.mrq.data = NULL;
+ mq_rq->req = NULL;
+ spin_lock_irq(&mq_rq->mq->mqrq_lock);
+ mq_rq->in_use = false;
+ spin_unlock_irq(&mq_rq->mq->mqrq_lock);
+}
+
static int mmc_queue_thread(void *d)
{
struct mmc_queue *mq = d;
@@ -62,17 +98,17 @@ static int mmc_queue_thread(void *d)
do {
struct request *req = NULL;
- spin_lock_irq(q->queue_lock);
set_current_state(TASK_INTERRUPTIBLE);
+ spin_lock_irq(q->queue_lock);
req = blk_fetch_request(q);
- mq->asleep = false;
- mq_rq = mq->mqrq_cur;
- mq_rq->req = req;
spin_unlock_irq(q->queue_lock);
+ mq->asleep = false;
if (req) {
bool req_is_special = mmc_req_is_special(req);
+ mq_rq = mmc_queue_req_get(mq);
+ mq_rq->req = req;
if (!claimed_host)
mmc_get_card(mq->card);
set_current_state(TASK_RUNNING);
@@ -87,13 +123,9 @@ static int mmc_queue_thread(void *d)
* commands.
*/
if (req_is_special) {
- mq->mqrq_cur->req = NULL;
mmc_put_card(mq->card);
claimed_host = false;
}
- mq->mqrq_prev->brq.mrq.data = NULL;
- mq->mqrq_prev->req = NULL;
- swap(mq->mqrq_prev, mq->mqrq_cur);
} else {
mq->asleep = true;
if (kthread_should_stop()) {
@@ -265,6 +297,7 @@ int mmc_init_queue(struct mmc_queue *mq, struct mmc_card *card,
u64 limit = BLK_BOUNCE_HIGH;
bool bounce = false;
int ret = -ENOMEM;
+ int i;
if (mmc_dev(host)->dma_mask && *mmc_dev(host)->dma_mask)
limit = (u64)dma_max_pfn(mmc_dev(host)) << PAGE_SHIFT;
@@ -275,14 +308,14 @@ int mmc_init_queue(struct mmc_queue *mq, struct mmc_card *card,
return -ENOMEM;
mq->qdepth = 2;
+ spin_lock_init(&mq->mqrq_lock);
mq->mqrq = kcalloc(mq->qdepth, sizeof(struct mmc_queue_req),
GFP_KERNEL);
if (!mq->mqrq)
goto blk_cleanup;
- mq->mqrq_cur = &mq->mqrq[0];
- mq->mqrq_cur->mq = mq;
- mq->mqrq_prev = &mq->mqrq[1];
- mq->mqrq_prev->mq = mq;
+ for (i = 0; i < mq->qdepth; i++)
+ mq->mqrq[i].mq = mq;
+
mq->queue->queuedata = mq;
blk_queue_prep_rq(mq->queue, mmc_prep_request);
diff --git a/drivers/mmc/core/queue.h b/drivers/mmc/core/queue.h
index c18d3f908433..886a05482b74 100644
--- a/drivers/mmc/core/queue.h
+++ b/drivers/mmc/core/queue.h
@@ -2,6 +2,7 @@
#define MMC_QUEUE_H
#include <linux/types.h>
+#include <linux/spinlock.h>
#include <linux/blkdev.h>
#include <linux/mmc/core.h>
#include <linux/mmc/host.h>
@@ -27,6 +28,7 @@ struct mmc_blk_request {
};
struct mmc_queue_req {
+ bool in_use;
struct request *req;
struct mmc_blk_request brq;
struct scatterlist *sg;
@@ -45,12 +47,12 @@ struct mmc_queue {
bool asleep;
struct mmc_blk_data *blkdata;
struct request_queue *queue;
+ spinlock_t mqrq_lock;
struct mmc_queue_req *mqrq;
- struct mmc_queue_req *mqrq_cur;
- struct mmc_queue_req *mqrq_prev;
- int qdepth;
+ unsigned int qdepth;
};
+extern void mmc_queue_req_put(struct mmc_queue_req *mq_rq);
extern int mmc_init_queue(struct mmc_queue *, struct mmc_card *, spinlock_t *,
const char *);
extern void mmc_cleanup_queue(struct mmc_queue *);
--
2.9.3
^ permalink raw reply related [flat|nested] 51+ messages in thread* [PATCH 15/16] mmc: queue: issue requests in massive parallel
2017-02-09 15:33 [PATCH 00/16] multiqueue for MMC/SD third try Linus Walleij
` (13 preceding siblings ...)
2017-02-09 15:34 ` [PATCH 14/16] mmc: queue: get/put struct mmc_queue_req Linus Walleij
@ 2017-02-09 15:34 ` Linus Walleij
[not found] ` <CGME20170301120247epcas1p1ad2be24dc9bbd1218dab8f565fb82b27@epcas1p1.samsung.com>
2017-02-09 15:34 ` [PATCH 16/16] RFC: mmc: switch MMC/SD to use blk-mq multiqueueing v3 Linus Walleij
` (2 subsequent siblings)
17 siblings, 1 reply; 51+ messages in thread
From: Linus Walleij @ 2017-02-09 15:34 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson, Adrian Hunter, Paolo Valente
Cc: Chunyan Zhang, Baolin Wang, linux-block, Jens Axboe,
Christoph Hellwig, Arnd Bergmann, Linus Walleij
This makes a crucial change to the issueing mechanism for the
MMC requests:
Before commit "mmc: core: move the asynchronous post-processing"
some parallelism on the read/write requests was achieved by
speculatively postprocessing a request and re-preprocess and
re-issue the request if something went wrong, which we discover
later when checking for an error.
This is kind of ugly. Instead we need a mechanism like here:
We issue requests, and when they come back from the hardware,
we know if they finished successfully or not. If the request
was successful, we complete the asynchronous request and let a
new request immediately start on the hardware. If, and only if,
it returned an error from the hardware we go down the error
path.
This is achieved by splitting the work path from the hardware
in two: a successful path ending up calling down to
mmc_blk_rw_done_success() and an errorpath calling down to
mmc_blk_rw_done_error().
This has a profound effect: we reintroduce the parallelism on
the successful path as mmc_post_req() can now be called in
while the next request is in transit (just like prior to
commit "mmc: core: move the asynchronous post-processing")
but ALSO we can call mmc_queue_bounce_post() and
blk_end_request() in parallel.
The latter has the profound effect of issuing a new request
again so that we actually need to have at least three requests
in transit at the same time: we haven't yet dropped the
reference to our struct mmc_queue_req so we need at least
three. I put the pool to 4 requests for now.
I expect the imrovement to be noticeable on systems that use
bounce buffers since they can now process requests in parallel
with post-processing their bounce buffers, but I don't have a
test target for that.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/mmc/core/block.c | 61 +++++++++++++++++++++++++++++++++++++-----------
drivers/mmc/core/block.h | 4 +++-
drivers/mmc/core/core.c | 27 ++++++++++++++++++---
drivers/mmc/core/queue.c | 2 +-
4 files changed, 75 insertions(+), 19 deletions(-)
diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index acca15cc1807..f1008ce5376b 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -1622,8 +1622,51 @@ static void mmc_blk_rw_try_restart(struct mmc_queue_req *mq_rq)
mmc_restart_areq(mq->card->host, &mq_rq->areq);
}
-void mmc_blk_rw_done(struct mmc_async_req *areq,
- enum mmc_blk_status status)
+/**
+ * Final handling of an asynchronous request if there was no error.
+ * This is the common path that we take when everything is nice
+ * and smooth. The status from the command is always MMC_BLK_SUCCESS.
+ */
+void mmc_blk_rw_done_success(struct mmc_async_req *areq)
+{
+ struct mmc_queue_req *mq_rq;
+ struct mmc_blk_request *brq;
+ struct mmc_blk_data *md;
+ struct request *old_req;
+ bool req_pending;
+ int type;
+
+ mq_rq = container_of(areq, struct mmc_queue_req, areq);
+ md = mq_rq->mq->blkdata;
+ brq = &mq_rq->brq;
+ old_req = mq_rq->req;
+ type = rq_data_dir(old_req) == READ ? MMC_BLK_READ : MMC_BLK_WRITE;
+
+ mmc_queue_bounce_post(mq_rq);
+ mmc_blk_reset_success(md, type);
+ req_pending = blk_end_request(old_req, 0,
+ brq->data.bytes_xfered);
+ /*
+ * If the blk_end_request function returns non-zero even
+ * though all data has been transferred and no errors
+ * were returned by the host controller, it's a bug.
+ */
+ if (req_pending) {
+ pr_err("%s BUG rq_tot %d d_xfer %d\n",
+ __func__, blk_rq_bytes(old_req),
+ brq->data.bytes_xfered);
+ return;
+ }
+}
+
+/**
+ * Error, recapture, retry etc for asynchronous requests.
+ * This is the error path that we take when there is bad status
+ * coming back from the hardware and we need to do a bit of
+ * cleverness.
+ */
+void mmc_blk_rw_done_error(struct mmc_async_req *areq,
+ enum mmc_blk_status status)
{
struct mmc_queue *mq;
struct mmc_queue_req *mq_rq;
@@ -1652,6 +1695,8 @@ void mmc_blk_rw_done(struct mmc_async_req *areq,
switch (status) {
case MMC_BLK_SUCCESS:
+ pr_err("%s: MMC_BLK_SUCCESS on error path\n", __func__);
+ /* This should not happen: anyway fall through */
case MMC_BLK_PARTIAL:
/*
* A block was successfully transferred.
@@ -1660,18 +1705,6 @@ void mmc_blk_rw_done(struct mmc_async_req *areq,
req_pending = blk_end_request(old_req, 0,
brq->data.bytes_xfered);
- /*
- * If the blk_end_request function returns non-zero even
- * though all data has been transferred and no errors
- * were returned by the host controller, it's a bug.
- */
- if (status == MMC_BLK_SUCCESS && req_pending) {
- pr_err("%s BUG rq_tot %d d_xfer %d\n",
- __func__, blk_rq_bytes(old_req),
- brq->data.bytes_xfered);
- mmc_blk_rw_cmd_abort(card, old_req);
- return;
- }
break;
case MMC_BLK_CMD_ERR:
req_pending = mmc_blk_rw_cmd_err(md, card, brq, old_req, req_pending);
diff --git a/drivers/mmc/core/block.h b/drivers/mmc/core/block.h
index 0326fa5d8217..eae47ae7c903 100644
--- a/drivers/mmc/core/block.h
+++ b/drivers/mmc/core/block.h
@@ -5,7 +5,9 @@ struct mmc_async_req;
enum mmc_blk_status;
struct mmc_queue_req;
-void mmc_blk_rw_done(struct mmc_async_req *areq, enum mmc_blk_status status);
+void mmc_blk_rw_done_success(struct mmc_async_req *areq);
+void mmc_blk_rw_done_error(struct mmc_async_req *areq,
+ enum mmc_blk_status status);
void mmc_blk_issue_rq(struct mmc_queue_req *mq_rq);
#endif
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 50a8942b98c2..04666ad91df0 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -634,11 +634,32 @@ void mmc_finalize_areq(struct kthread_work *work)
mmc_start_bkops(host->card, true);
}
- /* Successfully postprocess the old request at this point */
- mmc_post_req(host, areq->mrq, 0);
- mmc_blk_rw_done(areq, status);
+ /*
+ * Postprocess the old request at this point:
+ * on success: take a fast path!
+ * on error: take the slow path, retrying etc
+ */
+ if (status != MMC_BLK_SUCCESS) {
+ mmc_post_req(host, areq->mrq, 0);
+ /*
+ * This call can lead to retransmissions using
+ * mmc_restart_areq() so do not complete until
+ * after this call!
+ */
+ mmc_blk_rw_done_error(areq, status);
+ complete(&areq->complete);
+ mmc_queue_req_put(mq_rq);
+ return;
+ }
+ /*
+ * There will not be any retransmissions etc
+ * at this point, so let the next request get
+ * access to the hardware.
+ */
complete(&areq->complete);
+ mmc_post_req(host, areq->mrq, 0);
+ mmc_blk_rw_done_success(areq);
mmc_queue_req_put(mq_rq);
}
EXPORT_SYMBOL(mmc_finalize_areq);
diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c
index cab0f51dbb4d..e7ba5bef2df3 100644
--- a/drivers/mmc/core/queue.c
+++ b/drivers/mmc/core/queue.c
@@ -307,7 +307,7 @@ int mmc_init_queue(struct mmc_queue *mq, struct mmc_card *card,
if (!mq->queue)
return -ENOMEM;
- mq->qdepth = 2;
+ mq->qdepth = 4;
spin_lock_init(&mq->mqrq_lock);
mq->mqrq = kcalloc(mq->qdepth, sizeof(struct mmc_queue_req),
GFP_KERNEL);
--
2.9.3
^ permalink raw reply related [flat|nested] 51+ messages in thread* [PATCH 16/16] RFC: mmc: switch MMC/SD to use blk-mq multiqueueing v3
2017-02-09 15:33 [PATCH 00/16] multiqueue for MMC/SD third try Linus Walleij
` (14 preceding siblings ...)
2017-02-09 15:34 ` [PATCH 15/16] mmc: queue: issue requests in massive parallel Linus Walleij
@ 2017-02-09 15:34 ` Linus Walleij
2017-02-09 15:39 ` [PATCH 00/16] multiqueue for MMC/SD third try Christoph Hellwig
2017-02-11 13:03 ` Avri Altman
17 siblings, 0 replies; 51+ messages in thread
From: Linus Walleij @ 2017-02-09 15:34 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson, Adrian Hunter, Paolo Valente
Cc: Chunyan Zhang, Baolin Wang, linux-block, Jens Axboe,
Christoph Hellwig, Arnd Bergmann, Linus Walleij
HACK ALERT: DO NOT MERGE THIS! IT IS A FYI PATCH FOR DISCUSSION
ONLY.
This is a totally new implementation of how to do multiqueue
in the MMC/SD stack. It is based on top of my refactorings in the
series which ends with this patch, and now makes proper use of
.init_request() and .exit_request() to initialize the per-request
hardware context, reusing good old struct mmc_queue_req which is
what is actually intended by these functions.
We kill off the kthread that was just calling blk_fetch_request()
and let blk-mq drive all traffic, nice, that is how it should work.
Due to having switched the submission mechanics around so that
the completion of requests is now triggered from the host
callbacks, we manage to keep the same performance for linear
reads/writes as we have for the old block layer.
Some open questions:
- We used to issue mmc_get_card() when the first request comes
in and mmc_put_card() when we get NULL from blk_fetch_request().
Well as we are not pushed any NULL requests anymore we need
another way for the queue to tell us it is idle, or we should
just set up a delayed work and release the card if no new
requests appear for some time.
- The flush was handled by issueing blk_end_request_all() in
the old scheduler. Is blk_mq_complete_request() really doing
the same job, or is there some extra magic needed here?
- We can sometime get a partial read from a MMC command, meaning
some of the request has been handled. We know how many bytes
were read/written. We used to report this to the block layer
using blk_end_request(old_req, 0, bytes_xfered) but the MQ
scheduler seems to be missing a command that reports
partial completion. How do we handle this?
Apart from that my only remaining worries are about the
block scheduler, but I hear Jens and Paolo are working to fix
this.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/mmc/core/block.c | 66 ++++-----
drivers/mmc/core/core.c | 4 -
drivers/mmc/core/queue.c | 355 ++++++++++++++++-------------------------------
drivers/mmc/core/queue.h | 15 +-
4 files changed, 159 insertions(+), 281 deletions(-)
diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index f1008ce5376b..f977117f7435 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -28,6 +28,7 @@
#include <linux/hdreg.h>
#include <linux/kdev_t.h>
#include <linux/blkdev.h>
+#include <linux/blk-mq.h>
#include <linux/mutex.h>
#include <linux/scatterlist.h>
#include <linux/string_helpers.h>
@@ -96,7 +97,6 @@ static DEFINE_SPINLOCK(mmc_blk_lock);
* There is one mmc_blk_data per slot.
*/
struct mmc_blk_data {
- spinlock_t lock;
struct device *parent;
struct gendisk *disk;
struct mmc_queue queue;
@@ -1188,7 +1188,7 @@ static void mmc_blk_issue_discard_rq(struct mmc_queue_req *mq_rq)
if (!err)
mmc_blk_reset_success(md, type);
fail:
- blk_end_request(mq_rq->req, err, blk_rq_bytes(mq_rq->req));
+ blk_mq_complete_request(mq_rq->req, err);
}
static void mmc_blk_issue_secdiscard_rq(struct mmc_queue_req *mq_rq)
@@ -1265,7 +1265,8 @@ static void mmc_blk_issue_flush(struct mmc_queue_req *mq_rq)
if (ret)
ret = -EIO;
- blk_end_request_all(mq_rq->req, ret);
+ /* FIXME: was using blk_end_request_all() to flush */
+ blk_mq_complete_request(mq_rq->req, ret);
}
/*
@@ -1589,12 +1590,15 @@ static bool mmc_blk_rw_cmd_err(struct mmc_blk_data *md, struct mmc_card *card,
int err;
err = mmc_sd_num_wr_blocks(card, &blocks);
- if (err)
+ if (err) {
req_pending = old_req_pending;
- else
- req_pending = blk_end_request(req, 0, blocks << 9);
+ } else {
+ blk_mq_complete_request(req, 0);
+ req_pending = false;
+ }
} else {
- req_pending = blk_end_request(req, 0, brq->data.bytes_xfered);
+ blk_mq_complete_request(req, 0);
+ req_pending = false;
}
return req_pending;
}
@@ -1630,33 +1634,18 @@ static void mmc_blk_rw_try_restart(struct mmc_queue_req *mq_rq)
void mmc_blk_rw_done_success(struct mmc_async_req *areq)
{
struct mmc_queue_req *mq_rq;
- struct mmc_blk_request *brq;
struct mmc_blk_data *md;
struct request *old_req;
- bool req_pending;
int type;
mq_rq = container_of(areq, struct mmc_queue_req, areq);
md = mq_rq->mq->blkdata;
- brq = &mq_rq->brq;
old_req = mq_rq->req;
type = rq_data_dir(old_req) == READ ? MMC_BLK_READ : MMC_BLK_WRITE;
mmc_queue_bounce_post(mq_rq);
mmc_blk_reset_success(md, type);
- req_pending = blk_end_request(old_req, 0,
- brq->data.bytes_xfered);
- /*
- * If the blk_end_request function returns non-zero even
- * though all data has been transferred and no errors
- * were returned by the host controller, it's a bug.
- */
- if (req_pending) {
- pr_err("%s BUG rq_tot %d d_xfer %d\n",
- __func__, blk_rq_bytes(old_req),
- brq->data.bytes_xfered);
- return;
- }
+ blk_mq_complete_request(old_req, 0);
}
/**
@@ -1702,9 +1691,16 @@ void mmc_blk_rw_done_error(struct mmc_async_req *areq,
* A block was successfully transferred.
*/
mmc_blk_reset_success(md, type);
-
- req_pending = blk_end_request(old_req, 0,
- brq->data.bytes_xfered);
+ /*
+ * FIXME:
+ * How do we handle a partial request?
+ * brq->data.bytes_xfered contains the number of
+ * successfully transfered bytes, how to report
+ * this to the MQ block layer and get a new, smaller
+ * request back? Currently just requeueing.
+ */
+ blk_mq_requeue_request(old_req, false);
+ req_pending = false;
break;
case MMC_BLK_CMD_ERR:
req_pending = mmc_blk_rw_cmd_err(md, card, brq, old_req, req_pending);
@@ -1754,12 +1750,9 @@ void mmc_blk_rw_done_error(struct mmc_async_req *areq,
* time, so we only reach here after trying to
* read a single sector.
*/
- req_pending = blk_end_request(old_req, -EIO,
- brq->data.blksz);
- if (!req_pending) {
- mmc_blk_rw_try_restart(mq_rq);
- return;
- }
+ blk_mq_complete_request(mq_rq->req, -EIO);
+ req_pending = false;
+ mmc_blk_rw_try_restart(mq_rq);
break;
case MMC_BLK_NOMEDIUM:
mmc_blk_rw_cmd_abort(card, old_req);
@@ -1819,7 +1812,8 @@ void mmc_blk_issue_rq(struct mmc_queue_req *mq_rq)
ret = mmc_blk_part_switch(card, md);
if (ret) {
- blk_end_request_all(mq_rq->req, -EIO);
+ /* FIXME: was blk_end_request_all() to flush */
+ blk_mq_complete_request(mq_rq->req, -EIO);
return;
}
@@ -1830,7 +1824,6 @@ void mmc_blk_issue_rq(struct mmc_queue_req *mq_rq)
card->host->areq = NULL;
}
mmc_blk_issue_discard_rq(mq_rq);
- mmc_queue_req_put(mq_rq);
} else if (req_op(mq_rq->req) == REQ_OP_SECURE_ERASE) {
/* complete ongoing async transfer before issuing secure erase*/
if (card->host->areq) {
@@ -1838,7 +1831,6 @@ void mmc_blk_issue_rq(struct mmc_queue_req *mq_rq)
card->host->areq = NULL;
}
mmc_blk_issue_secdiscard_rq(mq_rq);
- mmc_queue_req_put(mq_rq);
} else if (req_op(mq_rq->req) == REQ_OP_FLUSH) {
/* complete ongoing async transfer before issuing flush */
if (card->host->areq) {
@@ -1846,7 +1838,6 @@ void mmc_blk_issue_rq(struct mmc_queue_req *mq_rq)
card->host->areq = NULL;
}
mmc_blk_issue_flush(mq_rq);
- mmc_queue_req_put(mq_rq);
} else {
mmc_blk_issue_rw_rq(mq_rq);
}
@@ -1906,11 +1897,10 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
goto err_kfree;
}
- spin_lock_init(&md->lock);
INIT_LIST_HEAD(&md->part);
md->usage = 1;
- ret = mmc_init_queue(&md->queue, card, &md->lock, subname);
+ ret = mmc_init_queue(&md->queue, card, subname);
if (ret)
goto err_putdisk;
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 04666ad91df0..a81b6baa3bee 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -599,8 +599,6 @@ void mmc_finalize_areq(struct kthread_work *work)
{
struct mmc_async_req *areq =
container_of(work, struct mmc_async_req, finalization_work);
- struct mmc_queue_req *mq_rq = container_of(areq, struct mmc_queue_req,
- areq);
struct mmc_host *host = areq->host;
enum mmc_blk_status status = MMC_BLK_SUCCESS;
struct mmc_command *cmd;
@@ -648,7 +646,6 @@ void mmc_finalize_areq(struct kthread_work *work)
*/
mmc_blk_rw_done_error(areq, status);
complete(&areq->complete);
- mmc_queue_req_put(mq_rq);
return;
}
@@ -660,7 +657,6 @@ void mmc_finalize_areq(struct kthread_work *work)
complete(&areq->complete);
mmc_post_req(host, areq->mrq, 0);
mmc_blk_rw_done_success(areq);
- mmc_queue_req_put(mq_rq);
}
EXPORT_SYMBOL(mmc_finalize_areq);
diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c
index e7ba5bef2df3..9850d7342763 100644
--- a/drivers/mmc/core/queue.c
+++ b/drivers/mmc/core/queue.c
@@ -14,6 +14,7 @@
#include <linux/kthread.h>
#include <linux/scatterlist.h>
#include <linux/dma-mapping.h>
+#include <linux/blk-mq.h>
#include <linux/mmc/card.h>
#include <linux/mmc/host.h>
@@ -49,126 +50,6 @@ static int mmc_prep_request(struct request_queue *q, struct request *req)
return BLKPREP_OK;
}
-/**
- * Get an available queue item from the pool.
- */
-static struct mmc_queue_req *mmc_queue_req_get(struct mmc_queue *mq)
-{
- int i;
-
- /*
- * This simply cannot fail so we just spin here
- * until we get a queue request to work on.
- */
- while (1) {
- /* Just take the first unused queue request */
- spin_lock_irq(&mq->mqrq_lock);
- for (i = 0; i < mq->qdepth; i++) {
- if (!mq->mqrq[i].in_use) {
- mq->mqrq[i].in_use = true;
- spin_unlock_irq(&mq->mqrq_lock);
- return &mq->mqrq[i];
- }
- }
- spin_unlock_irq(&mq->mqrq_lock);
-
- pr_warn_once("%s: out of queue items, spinning\n", __func__);
- }
-}
-
-void mmc_queue_req_put(struct mmc_queue_req *mq_rq)
-{
- mq_rq->brq.mrq.data = NULL;
- mq_rq->req = NULL;
- spin_lock_irq(&mq_rq->mq->mqrq_lock);
- mq_rq->in_use = false;
- spin_unlock_irq(&mq_rq->mq->mqrq_lock);
-}
-
-static int mmc_queue_thread(void *d)
-{
- struct mmc_queue *mq = d;
- struct request_queue *q = mq->queue;
- bool claimed_host = false;
- struct mmc_queue_req *mq_rq;
-
- current->flags |= PF_MEMALLOC;
-
- down(&mq->thread_sem);
- do {
- struct request *req = NULL;
-
- set_current_state(TASK_INTERRUPTIBLE);
- spin_lock_irq(q->queue_lock);
- req = blk_fetch_request(q);
- spin_unlock_irq(q->queue_lock);
- mq->asleep = false;
-
- if (req) {
- bool req_is_special = mmc_req_is_special(req);
-
- mq_rq = mmc_queue_req_get(mq);
- mq_rq->req = req;
- if (!claimed_host)
- mmc_get_card(mq->card);
- set_current_state(TASK_RUNNING);
- mmc_blk_issue_rq(mq_rq);
- cond_resched();
- /*
- * Current request becomes previous request
- * and vice versa.
- * In case of special requests, current request
- * has been finished. Do not assign it to previous
- * request. Always unclaim the host after special
- * commands.
- */
- if (req_is_special) {
- mmc_put_card(mq->card);
- claimed_host = false;
- }
- } else {
- mq->asleep = true;
- if (kthread_should_stop()) {
- set_current_state(TASK_RUNNING);
- break;
- }
- up(&mq->thread_sem);
- schedule();
- down(&mq->thread_sem);
- }
- } while (1);
-
- if (claimed_host)
- mmc_put_card(mq->card);
-
- up(&mq->thread_sem);
-
- return 0;
-}
-
-/*
- * Generic MMC request handler. This is called for any queue on a
- * particular host. When the host is not busy, we look for a request
- * on any queue on this host, and attempt to issue it. This may
- * not be the queue we were asked to process.
- */
-static void mmc_request_fn(struct request_queue *q)
-{
- struct mmc_queue *mq = q->queuedata;
- struct request *req;
-
- if (!mq) {
- while ((req = blk_fetch_request(q)) != NULL) {
- req->rq_flags |= RQF_QUIET;
- __blk_end_request_all(req, -EIO);
- }
- return;
- }
-
- if (mq->asleep)
- wake_up_process(mq->thread);
-}
-
static struct scatterlist *mmc_alloc_sg(int sg_len, int *err)
{
struct scatterlist *sg;
@@ -205,55 +86,80 @@ static void mmc_queue_setup_discard(struct request_queue *q,
queue_flag_set_unlocked(QUEUE_FLAG_SECERASE, q);
}
-#ifdef CONFIG_MMC_BLOCK_BOUNCE
-static bool mmc_queue_alloc_bounce_bufs(struct mmc_queue *mq,
- unsigned int bouncesz)
+static int mmc_queue_rq(struct blk_mq_hw_ctx *hctx,
+ const struct blk_mq_queue_data *bd)
{
- int i;
+ struct mmc_queue_req *mq_rq = blk_mq_rq_to_pdu(bd->rq);
+ struct mmc_queue *mq = mq_rq->mq;
+ bool req_is_special = mmc_req_is_special(bd->rq);
- for (i = 0; i < mq->qdepth; i++) {
- mq->mqrq[i].bounce_buf = kmalloc(bouncesz, GFP_KERNEL);
- if (!mq->mqrq[i].bounce_buf)
- goto out_err;
+ /* start this request */
+ mq_rq->req = bd->rq;
+ mq_rq->brq.mrq.data = NULL;
+ blk_mq_start_request(mq_rq->req);
+
+ if (!mq->claimed_host) {
+ mmc_get_card(mq->card);
+ mq->claimed_host = true;
+ }
+ set_current_state(TASK_RUNNING);
+ mmc_blk_issue_rq(mq_rq);
+ cond_resched();
+ /*
+ * In case of special requests, current request
+ * has been finished. Always unclaim the host after special
+ * commands.
+ */
+ if (req_is_special) {
+ mmc_put_card(mq->card);
+ mq->claimed_host = false;
}
- return true;
+ /*
+ * FIXME: unclaim host after timeout?
+ * Can blk-mq notify us that there are no requests coming
+ * for a while so we can relax the host?
+ */
+ // if (claimed_host)
+ // mmc_put_card(mq->card);
-out_err:
- while (--i >= 0) {
- kfree(mq->mqrq[i].bounce_buf);
- mq->mqrq[i].bounce_buf = NULL;
- }
- pr_warn("%s: unable to allocate bounce buffers\n",
- mmc_card_name(mq->card));
- return false;
+ return BLK_MQ_RQ_QUEUE_OK;
}
-static int mmc_queue_alloc_bounce_sgs(struct mmc_queue *mq,
- unsigned int bouncesz)
+static int mmc_init_request(void *data, struct request *rq,
+ unsigned int hctx_idx, unsigned int request_idx,
+ unsigned int numa_node)
{
- int i, ret;
+ struct mmc_queue_req *mq_rq = blk_mq_rq_to_pdu(rq);
+ struct mmc_queue *mq = data;
+ struct mmc_card *card = mq->card;
+ struct mmc_host *host = card->host;
+ int ret;
- for (i = 0; i < mq->qdepth; i++) {
- mq->mqrq[i].sg = mmc_alloc_sg(1, &ret);
- if (ret)
- return ret;
+ mq_rq->dev = host->parent;
+ mq_rq->mq = mq;
+ /* Set up bounces etc */
+ dev_info(mq_rq->dev, "%s\n", __func__);
- mq->mqrq[i].bounce_sg = mmc_alloc_sg(bouncesz / 512, &ret);
- if (ret)
- return ret;
- }
+#ifdef CONFIG_MMC_BLOCK_BOUNCE
+ if (mq->bouncesz) {
+ mq_rq->bounce_buf = kmalloc(mq->bouncesz, GFP_KERNEL);
+ if (!mq_rq->bounce_buf)
+ return -ENOMEM;
+ if (mq->bouncesz > 512) {
+ mq_rq->sg = mmc_alloc_sg(1, &ret);
+ if (ret)
+ return ret;
- return 0;
-}
+ mq_rq->bounce_sg = mmc_alloc_sg(mq->bouncesz / 512,
+ &ret);
+ if (ret)
+ return ret;
+ }
+ } else
#endif
-
-static int mmc_queue_alloc_sgs(struct mmc_queue *mq, int max_segs)
-{
- int i, ret;
-
- for (i = 0; i < mq->qdepth; i++) {
- mq->mqrq[i].sg = mmc_alloc_sg(max_segs, &ret);
+ {
+ mq_rq->sg = mmc_alloc_sg(host->max_segs, &ret);
if (ret)
return ret;
}
@@ -261,26 +167,35 @@ static int mmc_queue_alloc_sgs(struct mmc_queue *mq, int max_segs)
return 0;
}
-static void mmc_queue_req_free_bufs(struct mmc_queue_req *mqrq)
+static void mmc_exit_request(void *data, struct request *rq,
+ unsigned int hctx_idx, unsigned int request_idx)
{
- kfree(mqrq->bounce_sg);
- mqrq->bounce_sg = NULL;
+ struct mmc_queue_req *mq_rq = blk_mq_rq_to_pdu(rq);
- kfree(mqrq->sg);
- mqrq->sg = NULL;
+ dev_info(mq_rq->dev, "%s: hctx_idx = %u, request_idx = %u\n",
+ __func__, hctx_idx, request_idx);
- kfree(mqrq->bounce_buf);
- mqrq->bounce_buf = NULL;
-}
+ kfree(mq_rq->bounce_sg);
+ mq_rq->bounce_sg = NULL;
-static void mmc_queue_reqs_free_bufs(struct mmc_queue *mq)
-{
- int i;
+ kfree(mq_rq->sg);
+ mq_rq->sg = NULL;
- for (i = 0; i < mq->qdepth; i++)
- mmc_queue_req_free_bufs(&mq->mqrq[i]);
+ kfree(mq_rq->bounce_buf);
+ mq_rq->bounce_buf = NULL;
}
+static struct blk_mq_ops mmc_mq_ops = {
+ .queue_rq = mmc_queue_rq,
+ .init_request = mmc_init_request,
+ .exit_request = mmc_exit_request,
+ /*
+ * .exit_request() will only be invoked if we explcitly call
+ * blk_mq_end_request() on all requests. Why would we do that,
+ * we will just call blk_mq_complete_request().
+ */
+};
+
/**
* mmc_init_queue - initialise a queue structure.
* @mq: mmc queue
@@ -291,31 +206,47 @@ static void mmc_queue_reqs_free_bufs(struct mmc_queue *mq)
* Initialise a MMC card request queue.
*/
int mmc_init_queue(struct mmc_queue *mq, struct mmc_card *card,
- spinlock_t *lock, const char *subname)
+ const char *subname)
{
struct mmc_host *host = card->host;
u64 limit = BLK_BOUNCE_HIGH;
bool bounce = false;
int ret = -ENOMEM;
- int i;
if (mmc_dev(host)->dma_mask && *mmc_dev(host)->dma_mask)
limit = (u64)dma_max_pfn(mmc_dev(host)) << PAGE_SHIFT;
mq->card = card;
- mq->queue = blk_init_queue(mmc_request_fn, lock);
- if (!mq->queue)
+ mq->tag_set.ops = &mmc_mq_ops;
+ /* The MMC/SD protocols have only one command pipe */
+ mq->tag_set.nr_hw_queues = 1;
+ /* Set this to 2 to simulate async requests */
+ mq->tag_set.queue_depth = 2;
+ /*
+ * The extra data allocated per block request.
+ */
+ mq->tag_set.cmd_size = sizeof(struct mmc_queue_req);
+ mq->tag_set.numa_node = NUMA_NO_NODE;
+ /* We use blocking requests */
+ mq->tag_set.flags = BLK_MQ_F_SHOULD_MERGE | BLK_MQ_F_BLOCKING;
+ // BLK_MQ_F_SG_MERGE?
+ mq->tag_set.driver_data = mq;
+
+ ret = blk_mq_alloc_tag_set(&mq->tag_set);
+ if (ret) {
+ dev_err(card->host->parent, "failed to allocate MQ tag set\n");
return -ENOMEM;
+ }
- mq->qdepth = 4;
- spin_lock_init(&mq->mqrq_lock);
- mq->mqrq = kcalloc(mq->qdepth, sizeof(struct mmc_queue_req),
- GFP_KERNEL);
- if (!mq->mqrq)
- goto blk_cleanup;
- for (i = 0; i < mq->qdepth; i++)
- mq->mqrq[i].mq = mq;
+ mq->queue = blk_mq_init_queue(&mq->tag_set);
+ if (!mq->queue) {
+ dev_err(card->host->parent, "failed to initialize block MQ\n");
+ goto cleanup_free_tag_set;
+ }
+
+ blk_queue_max_segments(mq->queue, host->max_segs);
+ mq->qdepth = 4;
mq->queue->queuedata = mq;
blk_queue_prep_rq(mq->queue, mmc_prep_request);
@@ -337,18 +268,15 @@ int mmc_init_queue(struct mmc_queue *mq, struct mmc_card *card,
if (bouncesz > (host->max_blk_count * 512))
bouncesz = host->max_blk_count * 512;
- if (bouncesz > 512 &&
- mmc_queue_alloc_bounce_bufs(mq, bouncesz)) {
+ if (bouncesz > 512) {
blk_queue_bounce_limit(mq->queue, BLK_BOUNCE_ANY);
blk_queue_max_hw_sectors(mq->queue, bouncesz / 512);
blk_queue_max_segments(mq->queue, bouncesz / 512);
blk_queue_max_segment_size(mq->queue, bouncesz);
-
- ret = mmc_queue_alloc_bounce_sgs(mq, bouncesz);
- if (ret)
- goto cleanup_queue;
bounce = true;
}
+
+ mq->bouncesz = bouncesz;
}
#endif
@@ -358,53 +286,29 @@ int mmc_init_queue(struct mmc_queue *mq, struct mmc_card *card,
min(host->max_blk_count, host->max_req_size / 512));
blk_queue_max_segments(mq->queue, host->max_segs);
blk_queue_max_segment_size(mq->queue, host->max_seg_size);
-
- ret = mmc_queue_alloc_sgs(mq, host->max_segs);
- if (ret)
- goto cleanup_queue;
- }
-
- sema_init(&mq->thread_sem, 1);
-
- mq->thread = kthread_run(mmc_queue_thread, mq, "mmcqd/%d%s",
- host->index, subname ? subname : "");
-
- if (IS_ERR(mq->thread)) {
- ret = PTR_ERR(mq->thread);
- goto cleanup_queue;
}
return 0;
- cleanup_queue:
- mmc_queue_reqs_free_bufs(mq);
- kfree(mq->mqrq);
- mq->mqrq = NULL;
-blk_cleanup:
- blk_cleanup_queue(mq->queue);
+cleanup_free_tag_set:
+ blk_mq_free_tag_set(&mq->tag_set);
+
return ret;
}
void mmc_cleanup_queue(struct mmc_queue *mq)
{
struct request_queue *q = mq->queue;
- unsigned long flags;
/* Make sure the queue isn't suspended, as that will deadlock */
mmc_queue_resume(mq);
- /* Then terminate our worker thread */
- kthread_stop(mq->thread);
-
/* Empty the queue */
- spin_lock_irqsave(q->queue_lock, flags);
q->queuedata = NULL;
blk_start_queue(q);
- spin_unlock_irqrestore(q->queue_lock, flags);
- mmc_queue_reqs_free_bufs(mq);
- kfree(mq->mqrq);
- mq->mqrq = NULL;
+ blk_cleanup_queue(mq->queue);
+ blk_mq_free_tag_set(&mq->tag_set);
mq->card = NULL;
}
@@ -414,23 +318,16 @@ EXPORT_SYMBOL(mmc_cleanup_queue);
* mmc_queue_suspend - suspend a MMC request queue
* @mq: MMC queue to suspend
*
- * Stop the block request queue, and wait for our thread to
- * complete any outstanding requests. This ensures that we
+ * Stop the block request queue. This ensures that we
* won't suspend while a request is being processed.
*/
void mmc_queue_suspend(struct mmc_queue *mq)
{
struct request_queue *q = mq->queue;
- unsigned long flags;
if (!mq->suspended) {
- mq->suspended |= true;
-
- spin_lock_irqsave(q->queue_lock, flags);
+ mq->suspended = true;
blk_stop_queue(q);
- spin_unlock_irqrestore(q->queue_lock, flags);
-
- down(&mq->thread_sem);
}
}
@@ -441,16 +338,10 @@ void mmc_queue_suspend(struct mmc_queue *mq)
void mmc_queue_resume(struct mmc_queue *mq)
{
struct request_queue *q = mq->queue;
- unsigned long flags;
if (mq->suspended) {
mq->suspended = false;
-
- up(&mq->thread_sem);
-
- spin_lock_irqsave(q->queue_lock, flags);
blk_start_queue(q);
- spin_unlock_irqrestore(q->queue_lock, flags);
}
}
diff --git a/drivers/mmc/core/queue.h b/drivers/mmc/core/queue.h
index 886a05482b74..0a30fcc115ee 100644
--- a/drivers/mmc/core/queue.h
+++ b/drivers/mmc/core/queue.h
@@ -4,6 +4,7 @@
#include <linux/types.h>
#include <linux/spinlock.h>
#include <linux/blkdev.h>
+#include <linux/blk-mq.h>
#include <linux/mmc/core.h>
#include <linux/mmc/host.h>
@@ -28,7 +29,7 @@ struct mmc_blk_request {
};
struct mmc_queue_req {
- bool in_use;
+ struct device *dev;
struct request *req;
struct mmc_blk_request brq;
struct scatterlist *sg;
@@ -41,20 +42,20 @@ struct mmc_queue_req {
struct mmc_queue {
struct mmc_card *card;
- struct task_struct *thread;
- struct semaphore thread_sem;
bool suspended;
- bool asleep;
+ bool claimed_host;
struct mmc_blk_data *blkdata;
struct request_queue *queue;
+ struct blk_mq_tag_set tag_set;
spinlock_t mqrq_lock;
struct mmc_queue_req *mqrq;
unsigned int qdepth;
+#ifdef CONFIG_MMC_BLOCK_BOUNCE
+ unsigned int bouncesz;
+#endif
};
-extern void mmc_queue_req_put(struct mmc_queue_req *mq_rq);
-extern int mmc_init_queue(struct mmc_queue *, struct mmc_card *, spinlock_t *,
- const char *);
+extern int mmc_init_queue(struct mmc_queue *, struct mmc_card *, const char *);
extern void mmc_cleanup_queue(struct mmc_queue *);
extern void mmc_queue_suspend(struct mmc_queue *);
extern void mmc_queue_resume(struct mmc_queue *);
--
2.9.3
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH 00/16] multiqueue for MMC/SD third try
2017-02-09 15:33 [PATCH 00/16] multiqueue for MMC/SD third try Linus Walleij
` (15 preceding siblings ...)
2017-02-09 15:34 ` [PATCH 16/16] RFC: mmc: switch MMC/SD to use blk-mq multiqueueing v3 Linus Walleij
@ 2017-02-09 15:39 ` Christoph Hellwig
2017-02-11 13:03 ` Avri Altman
17 siblings, 0 replies; 51+ messages in thread
From: Christoph Hellwig @ 2017-02-09 15:39 UTC (permalink / raw)
To: Linus Walleij
Cc: linux-mmc, Ulf Hansson, Adrian Hunter, Paolo Valente,
Chunyan Zhang, Baolin Wang, linux-block, Jens Axboe,
Christoph Hellwig, Arnd Bergmann
> What immediately jumps out at you is that linear read/writes
> perform just as nicely or actually better with MQ than with the
> old block layer.
>
> What is amazing is that just a little randomness, such as the
> find . > /dev/null immediately seems to visibly regress with MQ.
> My best guess is that it is caused by the absence of the block
> scheduler.
The block tree now has a basic deadline scheduler. So for now I'd
suggest to kick off the discussion on all these prep patches,
and then retest with 4.11-rc1 or 2 that will have the scheduler.
^ permalink raw reply [flat|nested] 51+ messages in thread* RE: [PATCH 00/16] multiqueue for MMC/SD third try
2017-02-09 15:33 [PATCH 00/16] multiqueue for MMC/SD third try Linus Walleij
@ 2017-02-11 13:03 ` Avri Altman
2017-02-09 15:33 ` [PATCH 02/16] mmc: core: refactor asynchronous request finalization Linus Walleij
` (16 subsequent siblings)
17 siblings, 0 replies; 51+ messages in thread
From: Avri Altman @ 2017-02-11 13:03 UTC (permalink / raw)
To: Linus Walleij, linux-mmc, Ulf Hansson, Adrian Hunter, Paolo Valente
Cc: Chunyan Zhang, Baolin Wang, linux-block, Jens Axboe,
Christoph Hellwig, Arnd Bergmann
>
> The iozone results seem a bit consistent and all values seem to be noisy and
> not say much. I don't know why really, maybe the test is simply not relevant,
> the tests don't seem to be significantly affected by any of the patches, so
> let's focus on the dd and find tests.
Maybe use a more selective testing mode instead of -az.
Also maybe you want to clear the cache between the sequential and random tests:
#sync�
#echo 3 > /proc/sys/vm/drop_caches�
#sync�
It helps to obtain a more robust results.
> What immediately jumps out at you is that linear read/writes perform just as
> nicely or actually better with MQ than with the old block layer.
How come 22.7MB/s before vs. 22.1MB/s after is better? or did I misunderstand the output?
Also as dd is probably using the buffer cache, unlike the iozone test in which you properly used -I
for direct mode to isolate the blk-mq effect - does it really say much?
^ permalink raw reply [flat|nested] 51+ messages in thread* RE: [PATCH 00/16] multiqueue for MMC/SD third try
@ 2017-02-11 13:03 ` Avri Altman
0 siblings, 0 replies; 51+ messages in thread
From: Avri Altman @ 2017-02-11 13:03 UTC (permalink / raw)
To: Linus Walleij, linux-mmc, Ulf Hansson, Adrian Hunter, Paolo Valente
Cc: Chunyan Zhang, Baolin Wang, linux-block, Jens Axboe,
Christoph Hellwig, Arnd Bergmann
>
> The iozone results seem a bit consistent and all values seem to be noisy and
> not say much. I don't know why really, maybe the test is simply not relevant,
> the tests don't seem to be significantly affected by any of the patches, so
> let's focus on the dd and find tests.
Maybe use a more selective testing mode instead of -az.
Also maybe you want to clear the cache between the sequential and random tests:
#sync
#echo 3 > /proc/sys/vm/drop_caches
#sync
It helps to obtain a more robust results.
> What immediately jumps out at you is that linear read/writes perform just as
> nicely or actually better with MQ than with the old block layer.
How come 22.7MB/s before vs. 22.1MB/s after is better? or did I misunderstand the output?
Also as dd is probably using the buffer cache, unlike the iozone test in which you properly used -I
for direct mode to isolate the blk-mq effect - does it really say much?
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 00/16] multiqueue for MMC/SD third try
2017-02-11 13:03 ` Avri Altman
(?)
@ 2017-02-12 16:16 ` Linus Walleij
-1 siblings, 0 replies; 51+ messages in thread
From: Linus Walleij @ 2017-02-12 16:16 UTC (permalink / raw)
To: Avri Altman
Cc: linux-mmc, Ulf Hansson, Adrian Hunter, Paolo Valente,
Chunyan Zhang, Baolin Wang, linux-block, Jens Axboe,
Christoph Hellwig, Arnd Bergmann
On Sat, Feb 11, 2017 at 2:03 PM, Avri Altman <Avri.Altman@sandisk.com> wrote:
>>
>> The iozone results seem a bit consistent and all values seem to be noisy and
>> not say much. I don't know why really, maybe the test is simply not relevant,
>> the tests don't seem to be significantly affected by any of the patches, so
>> let's focus on the dd and find tests.
>
> Maybe use a more selective testing mode instead of -az.
> Also maybe you want to clear the cache between the sequential and random tests:
> #sync
> #echo 3 > /proc/sys/vm/drop_caches
> #sync
> It helps to obtain a more robust results.
OK I'll try that. I actually cold booted the system between each test to
avoid cache effects.
>> What immediately jumps out at you is that linear read/writes perform just as
>> nicely or actually better with MQ than with the old block layer.
>
> How come 22.7MB/s before vs. 22.1MB/s after is better? or did I misunderstand the output?
> Also as dd is probably using the buffer cache, unlike the iozone test in which you properly used -I
> for direct mode to isolate the blk-mq effect - does it really say much?
Sorry I guess I was a bit too enthusiastic there. The difference is in
the error margin, it is just based on a single test. I guess I should re-run
them with a few iterations, then drop caches iterate drop caches iterate
and get some more stable figures.
We need to understand what is meant by "better" too:
quicker compared to wall clock time (real), user
or sys.
So for the dd command:
real user sys
Before patches: 45.13 0.02 7.60
Move asynch pp 52.17 0.01 6.96
Issue in parallel 49.31 0.00 7.11
Multiqueue 46.25 0.03 6.42
For these pure kernel patches only the last figure (sys) is really relevant
IIUC. The other figures are just system noise, but still the eventual
throughput figure from dd is including the time spent on other processes
in the system etc, so that value is not relevant.
But I guess Paolo may need to beat me up a bit here: what the user
percieves in the end if of course the most relevant for any human ...
Nevertheless if we just look at sys then MQ is already winning this test.
I just think there is too little tested here.
I think 1GiB is maybe too little. Maybe I need to read the entire card
a few times or something?
Since dd is just using sequenctially blocks from mmcblk0 on a cold
booted system I think the buffer cache is empty except for maybe
the partition table blocks. But I dunno. I will use your trick the next
time to drop caches.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 51+ messages in thread