[PATCH 00/16] multiqueue for MMC/SD third try

[PATCH 00/16] multiqueue for MMC/SD third try

[Linus Walleij]

The following is the latest attempt at a rewriting the MMC/SD
stack to cope with multiqueueing.

If you just want to grab a branch and test the patches with
your hardware, I put a git branch with this series here:
https://git.kernel.org/cgit/linux/kernel/git/linusw/linux-stericsson.git/log/?h=mmc-mq-next-2017-02-09

It's based on Ulf's v4.10-rc3-based tree, so quick reminder:
git checkout -b test v4.10-rc3
git pull git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-stericsson.git mmc-mq-next-2017-02-09

Should get you a testable "test" branch.

These patches are clearly v4.12 material. They get increasinly
controversial and needing review the further into the series
you go. The last patch for multiqueue is marked RFC for a
reason.

Every time I do this it seems to be an extensive rewrite of the
whole world. Anyways this is based on the other ~16 patches that
were already merged for the upcoming v4.11.

The rationale for this approach was Arnd's suggestion to try to
switch the MMC/SD stack around so as to complete requests as
quickly as possible from the device driver so that new requests
can be issued. We are doing this now: the polling loop that was
pulling NULL out of the request queue and driving the pipeline
with a loop is gone.

We are not issueing new requests from interrupt context: I still
have to post a work for it. I don't know if that is possible.
There is the retune and background operations that need to be
checked after every command and yeah, it needs to happen in
blocking context as far as I know.

We have parallelism in pre/post hooks also with multiqueue.
All asynchronous optimization that was there for the old block layer
is now also there for multiqueue. There is even a new interesting
optimization that make bounce buffers be bounced asynchronously
with this change.

We still use the trick to set the queue depth to 2 to get two
parallel requests pushed down to the host.

Adrian: I know I made quite extensive violence on your queueue
handling reusing it in a way that is probably totally counter to
your command queueing patch series. I'm sorry. I guess you can
see where it is going if you follow the series. I also killed the
host context, right off, after reducing the synchronization needs
to zero. I hope you will be interested in the result though!

Does this perform? The numbers follow. I will discuss my
conclusions after the figures. All the tests are done on a cold
booted Ux500 system.

Before this patch series, based on my earlier cleanups
and refactorings on Ulf's next branch ending with
"mmc: core: start to break apart mmc_start_areq()":

time dd if=/dev/mmcblk0 of=/dev/null bs=1M count=1024
1024+0 records in
1024+0 records out
1073741824 bytes (1.0GB) copied, 45.126404 seconds, 22.7MB/s
real    0m 45.13s
user    0m 0.02s
sys     0m 7.60s

mount /dev/mmcblk0p1 /mnt/
cd /mnt/
time find . > /dev/null
real    0m 3.61s
user    0m 0.30s
sys     0m 1.56s

Command line used: iozone -az -i0 -i1 -i2 -s 20m -I -f /mnt/foo.test
Output is in kBytes/sec
                                                   random    random
   kB  reclen    write  rewrite    read    reread    read     write
20480       4     2046     2114     5981     6008     5971       40
20480       8     4825     4622     9104     9118     9070       81
20480      16     5767     5929    12250    12253    12209      166
20480      32     6242     6303    14920    14917    14879      337
20480      64     6598     5907    16758    16760    16739      695
20480     128     6807     6837    17863    17869    17788     1387
20480     256     6922     6925    18497    18490    18482     3076
20480     512     7273     7313    18636    18407    18829     7344
20480    1024     7339     7332    17695    18785    18472     7441
20480    2048     7419     7471    19166    18812    18797     7474
20480    4096     7598     7714    21006    20975    21180     7708
20480    8192     7632     7830    22328    22315    22201     7828
20480   16384     7412     7903    23070    23046    22849     7913


With "mmc: core: move the asynchronous post-processing"

time dd if=/dev/mmcblk0 of=/dev/null bs=1M count=1024
1024+0 records in
1024+0 records out
1073741824 bytes (1.0GB) copied, 52.166992 seconds, 19.6MB/s
real    0m 52.17s
user    0m 0.01s
sys     0m 6.96s

mount /dev/mmcblk0p1 /mnt/
cd /mnt/
time find . > /dev/null
real    0m 3.88s
user    0m 0.35s
sys     0m 1.60s

Command line used: iozone -az -i0 -i1 -i2 -s 20m -I -f /mnt/foo.test
Output is in kBytes/sec
                                                   random    random
   kB  reclen    write  rewrite    read    reread    read     write
20480       4     2072     2200     6030     6066     6005       40
20480       8     4847     5106     9174     9178     9123       81
20480      16     5791     5934    12301    12299    12260      166
20480      32     6252     6311    14906    14943    14919      337
20480      64     6607     6699    16776    16787    16756      690
20480     128     6836     6880    17868    17880    17873     1419
20480     256     6967     6955    18442    17112    18490     3072
20480     512     7320     7359    18818    18738    18477     7310
20480    1024     7350     7426    18297    18551    18357     7429
20480    2048     7439     7476    18035    19111    17670     7486
20480    4096     7655     7728    19688    19557    19758     7738
20480    8192     7640     7848    20675    20718    20787     7823
20480   16384     7489     7934    21225    21186    21555     7943


With "mmc: queue: issue requests in massive parallel"

time dd if=/dev/mmcblk0 of=/dev/null bs=1M count=1024
1024+0 records in
1024+0 records out
1073741824 bytes (1.0GB) copied, 49.308167 seconds, 20.8MB/s
real    0m 49.31s
user    0m 0.00s
sys     0m 7.11s

mount /dev/mmcblk0p1 /mnt/
cd /mnt/
time find . > /dev/null
real    0m 3.70s
user    0m 0.19s
sys     0m 1.73s

Command line used: iozone -az -i0 -i1 -i2 -s 20m -I -f /mnt/foo.test
Output is in kBytes/sec
                                                   random    random
   kB  reclen    write  rewrite    read    reread    read     write
20480       4     1709     1761     5963     5321     5909       40
20480       8     4736     5059     9089     9092     9055       81
20480      16     5772     5928    12217    12229    12184      165
20480      32     6237     6279    14898    14899    14875      336
20480      64     6599     6663    16759    16760    16741      683
20480     128     6804     6790    17869    17869    17864     1393
20480     256     6863     6883    18485    18488    18501     3105
20480     512     7223     7249    18807    18810    18812     7259
20480    1024     7311     7321    18684    18467    18201     7328
20480    2048     7405     7457    18560    18044    18343     7451
20480    4096     7596     7684    20742    21154    21153     7711
20480    8192     7593     7802    21743    21721    22090     7804
20480   16384     7399     7873    21539    22670    22828     7876


With "RFC: mmc: switch MMC/SD to use blk-mq multiqueueing v3"

time dd if=/dev/mmcblk0 of=/dev/null bs=1M count=1024
1024+0 records in
1024+0 records out
1073741824 bytes (1.0GB) copied, 46.240479 seconds, 22.1MB/s
real    0m 46.25s
user    0m 0.03s
sys     0m 6.42s

mount /dev/mmcblk0p1 /mnt/
cd /mnt/
time find . > /dev/null
real    0m 4.13s
user    0m 0.40s
sys     0m 1.64s

Command line used: iozone -az -i0 -i1 -i2 -s 20m -I -f /mnt/foo.test
Output is in kBytes/sec
                                                   random    random
   kB  reclen    write  rewrite    read    reread    read     write
20480       4     1786     1806     6055     6061     5360       40
20480       8     4849     5088     9167     9175     9120       81
20480      16     5807     5975    12273    12256    12240      166
20480      32     6275     6317    14929    14931    14905      338
20480      64     6629     6708    16755    16783    16758      688
20480     128     6856     6884    17890    17804    17873     1420
20480     256     6927     6946    18104    17826    18389     3038
20480     512     7296     7280    18720    18752    18819     7284
20480    1024     7286     7415    18583    18598    18516     7403
20480    2048     7435     7470    18378    18268    18682     7471
20480    4096     7670     7786    21364    21275    20761     7766
20480    8192     7637     7868    22193    21994    22100     7850
20480   16384     7416     7921    23050    23051    22726     7955


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.

You can see there are three steps:

- I do some necessary refactoring and need to move postprocessing
  to after the requests have been completed. This clearly, as you
  can see, introduce a performance regression in the dd test with
  the patch:
  "mmc: core: move the asynchronous post-processing"
  It seems the random seek with find isn't much affected.

- I continue the refactoring and get to the point of issueing
  requests immediately after every successful transfer, and the
  dd performance is restored with patch
  "mmc: queue: issue requests in massive parallel"

- Then I add multiqueue on top of the cake. So before the change
  we have the nice performance we want so we can study the effect
  of just introducing multiqueueing in the last patch
  "RFC: mmc: switch MMC/SD to use blk-mq multiqueueing v3"

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.

I do not know if my conclusions are right or anything, please
scrutinize.

Linus Walleij (16):
  mmc: core: move some code in mmc_start_areq()
  mmc: core: refactor asynchronous request finalization
  mmc: core: refactor mmc_request_done()
  mmc: core: move the asynchronous post-processing
  mmc: core: add a kthread for completing requests
  mmc: core: replace waitqueue with worker
  mmc: core: do away with is_done_rcv
  mmc: core: do away with is_new_req
  mmc: core: kill off the context info
  mmc: queue: simplify queue logic
  mmc: block: shuffle retry and error handling
  mmc: queue: stop flushing the pipeline with NULL
  mmc: queue: issue struct mmc_queue_req items
  mmc: queue: get/put struct mmc_queue_req
  mmc: queue: issue requests in massive parallel
  RFC: mmc: switch MMC/SD to use blk-mq multiqueueing v3

 drivers/mmc/core/block.c | 426 +++++++++++++++++++++++------------------------
 drivers/mmc/core/block.h |  10 +-
 drivers/mmc/core/bus.c   |   1 -
 drivers/mmc/core/core.c  | 228 ++++++++++++-------------
 drivers/mmc/core/core.h  |   2 -
 drivers/mmc/core/host.c  |   2 +-
 drivers/mmc/core/queue.c | 337 ++++++++++++++-----------------------
 drivers/mmc/core/queue.h |  21 ++-
 include/linux/mmc/core.h |   9 +-
 include/linux/mmc/host.h |  24 +--
 10 files changed, 481 insertions(+), 579 deletions(-)

-- 
2.9.3

[PATCH 01/16] mmc: core: move some code in mmc_start_areq()

[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

[PATCH 02/16] mmc: core: refactor asynchronous request finalization

[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

[PATCH 03/16] mmc: core: refactor mmc_request_done()

[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

[PATCH 04/16] mmc: core: move the asynchronous post-processing

[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

[PATCH 05/16] mmc: core: add a kthread for completing requests

[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

[PATCH 06/16] mmc: core: replace waitqueue with worker

[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

[PATCH 07/16] mmc: core: do away with is_done_rcv

[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

[PATCH 08/16] mmc: core: do away with is_new_req

[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

[PATCH 09/16] mmc: core: kill off the context info

[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

[PATCH 10/16] mmc: queue: simplify queue logic

[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

[PATCH 11/16] mmc: block: shuffle retry and error handling

[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

[PATCH 12/16] mmc: queue: stop flushing the pipeline with NULL

[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

[PATCH 13/16] mmc: queue: issue struct mmc_queue_req items

[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

[PATCH 14/16] mmc: queue: get/put struct mmc_queue_req

[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

[PATCH 15/16] mmc: queue: issue requests in massive parallel

[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

[PATCH 16/16] RFC: mmc: switch MMC/SD to use blk-mq multiqueueing v3

[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

Re: [PATCH 00/16] multiqueue for MMC/SD third try

[Christoph Hellwig]

> 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.

RE: [PATCH 00/16] multiqueue for MMC/SD third try

[Avri Altman]

> 
> 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?

RE: [PATCH 00/16] multiqueue for MMC/SD third try

[Avri Altman]

> 
> 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?

Re: [PATCH 00/16] multiqueue for MMC/SD third try

[Linus Walleij]

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

Re: [PATCH 06/16] mmc: core: replace waitqueue with worker

[Adrian Hunter]

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 )

Re: [PATCH 01/16] mmc: core: move some code in mmc_start_areq()

[Bartlomiej Zolnierkiewicz]

On Thursday, February 09, 2017 04:33:48 PM Linus Walleij wrote:
> "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>

Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH 02/16] mmc: core: refactor asynchronous request finalization

[Bartlomiej Zolnierkiewicz]

On Thursday, February 09, 2017 04:33:49 PM Linus Walleij wrote:
> 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>

Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH 03/16] mmc: core: refactor mmc_request_done()

[Bartlomiej Zolnierkiewicz]

On Thursday, February 09, 2017 04:33:50 PM Linus Walleij wrote:
> 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>

Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH 05/16] mmc: core: add a kthread for completing requests

[Bartlomiej Zolnierkiewicz]

On Thursday, February 09, 2017 04:33:52 PM Linus Walleij wrote:
> 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>

Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH 06/16] mmc: core: replace waitqueue with worker

[Bartlomiej Zolnierkiewicz]

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

Re: [PATCH 07/16] mmc: core: do away with is_done_rcv

[Bartlomiej Zolnierkiewicz]

On Thursday, February 09, 2017 04:33:54 PM Linus Walleij wrote:
> 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>

Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH 08/16] mmc: core: do away with is_new_req

[Bartlomiej Zolnierkiewicz]

On Thursday, February 09, 2017 04:33:55 PM Linus Walleij wrote:
> The host context member "is_new_req" is only assigned values,
> never checked. Delete it.
> 
> 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

Re: [PATCH 09/16] mmc: core: kill off the context info

[Bartlomiej Zolnierkiewicz]

On Thursday, February 09, 2017 04:33:56 PM Linus Walleij wrote:
> 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>

Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH 10/16] mmc: queue: simplify queue logic

[Bartlomiej Zolnierkiewicz]

On Thursday, February 09, 2017 04:33:57 PM Linus Walleij wrote:
> 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>

Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH 11/16] mmc: block: shuffle retry and error handling

[Bartlomiej Zolnierkiewicz]

On Thursday, February 09, 2017 04:33:58 PM Linus Walleij wrote:
> 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>

It seems that after this change we can end up queuing more
work for kthread from the kthread worker itself and wait
inside it for this nested work to complete.  I hope that
you've tested it with simulating errors and it all works.

Under this assumption:

Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

Also some very minor nit:

+       case MMC_BLK_DATA_ERR: {
+               int err;
+                       err = mmc_blk_reset(md, host, type);

During the code movement CodingStyle suffered.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH 12/16] mmc: queue: stop flushing the pipeline with NULL

[Bartlomiej Zolnierkiewicz]

On Thursday, February 09, 2017 04:33:59 PM Linus Walleij wrote:
> 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);

missing
				claimed_host = true;

?

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

claimed_host is never set to true

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

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH 13/16] mmc: queue: issue struct mmc_queue_req items

[Bartlomiej Zolnierkiewicz]

On Thursday, February 09, 2017 04:34:00 PM Linus Walleij wrote:
> 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) {

dropping of req checking should belong to some earlier patch

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

ditto

>  		/* 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) {

ditto

> +		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) {

ditto

>  		/* 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

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH 14/16] mmc: queue: get/put struct mmc_queue_req

[Bartlomiej Zolnierkiewicz]

On Thursday, February 09, 2017 04:34:01 PM Linus Walleij wrote:
> 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>

Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH 11/16] mmc: block: shuffle retry and error handling

[Bartlomiej Zolnierkiewicz]

Hi,

On Tuesday, February 28, 2017 06:45:20 PM Bartlomiej Zolnierkiewicz wrote:
> On Thursday, February 09, 2017 04:33:58 PM Linus Walleij wrote:
> > 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>
> 
> It seems that after this change we can end up queuing more
> work for kthread from the kthread worker itself and wait
> inside it for this nested work to complete.  I hope that

On the second look it seems that there is no waiting for
the retried areq to complete so I cannot see what protects
us from racing and trying to run two areq-s in parallel:

1st areq being retried (in the completion kthread):

	mmc_blk_rw_done()->mmc_restart_areq()->__mmc_start_data_req()

2nd areq coming from the second request in the queue
(in the queuing kthread):

	mmc_blk_issue_rw_rq()->mmc_start_areq()->__mmc_start_data_req()

(after mmc_blk_rw_done() is done in mmc_finalize_areq() 1st
areq is marked as completed by the completion kthread and
the waiting on host->areq in mmc_start_areq() of the queuing
kthread is done and 2nd areq is started while the 1st one
is still being retried)

?

Also retrying of areqs for MMC_BLK_RETRY status case got broken
(before change do {} while() loop increased retry variable,
now the loop is gone and retry variable will not be increased
correctly and we can loop forever).

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH 15/16] mmc: queue: issue requests in massive parallel

[Bartlomiej Zolnierkiewicz]

On Thursday, February 09, 2017 04:34:02 PM Linus Walleij wrote:
> 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);

What has happened to mmc_blk_rw_cmd_abort() call?

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

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH 11/16] mmc: block: shuffle retry and error handling

[Bartlomiej Zolnierkiewicz]

On Wednesday, March 01, 2017 12:45:57 PM Bartlomiej Zolnierkiewicz wrote:
> 
> Hi,
> 
> On Tuesday, February 28, 2017 06:45:20 PM Bartlomiej Zolnierkiewicz wrote:
> > On Thursday, February 09, 2017 04:33:58 PM Linus Walleij wrote:
> > > 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>
> > 
> > It seems that after this change we can end up queuing more
> > work for kthread from the kthread worker itself and wait
> > inside it for this nested work to complete.  I hope that
> 
> On the second look it seems that there is no waiting for
> the retried areq to complete so I cannot see what protects
> us from racing and trying to run two areq-s in parallel:
> 
> 1st areq being retried (in the completion kthread):
> 
> 	mmc_blk_rw_done()->mmc_restart_areq()->__mmc_start_data_req()
> 
> 2nd areq coming from the second request in the queue
> (in the queuing kthread):
> 
> 	mmc_blk_issue_rw_rq()->mmc_start_areq()->__mmc_start_data_req()
> 
> (after mmc_blk_rw_done() is done in mmc_finalize_areq() 1st
> areq is marked as completed by the completion kthread and
> the waiting on host->areq in mmc_start_areq() of the queuing
> kthread is done and 2nd areq is started while the 1st one
> is still being retried)
> 
> ?
> 
> Also retrying of areqs for MMC_BLK_RETRY status case got broken
> (before change do {} while() loop increased retry variable,
> now the loop is gone and retry variable will not be increased
> correctly and we can loop forever).

There is another problem with this patch.

During boot there is ~30 sec delay and later I get deadlock
on trying to run sync command (first thing I do after boot):

...
[    5.960623] asoc-simple-card sound: HiFi <-> 3830000.i2s mapping ok
done.
[....] Waiting for /dev to be fully populated...[   17.745887] random: crng init done
done.
[....] Activating swap...done.
[   39.767982] EXT4-fs (mmcblk0p2): re-mounted. Opts: (null)
...
root@target:~# sync
[  248.801708] INFO: task udevd:287 blocked for more than 120 seconds.
[  248.806552]       Tainted: G        W       4.10.0-rc3-00118-g4515dc6 #2736
[  248.813590] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[  248.821275] udevd           D    0   287    249 0x00000005
[  248.826815] [<c06df404>] (__schedule) from [<c06df90c>] (schedule+0x40/0xac)
[  248.833889] [<c06df90c>] (schedule) from [<c06e526c>] (schedule_timeout+0x148/0x220)
[  248.841598] [<c06e526c>] (schedule_timeout) from [<c06df24c>] (io_schedule_timeout+0x74/0xb0)
[  248.849993] [<c06df24c>] (io_schedule_timeout) from [<c0198a0c>] (__lock_page+0xe8/0x118)
[  248.858235] [<c0198a0c>] (__lock_page) from [<c01a88b0>] (truncate_inode_pages_range+0x580/0x59c)
[  248.867053] [<c01a88b0>] (truncate_inode_pages_range) from [<c01a8984>] (truncate_inode_pages+0x18/0x20)
[  248.876525] [<c01a8984>] (truncate_inode_pages) from [<c0214bf0>] (__blkdev_put+0x68/0x1d8)
[  248.884828] [<c0214bf0>] (__blkdev_put) from [<c0214ea8>] (blkdev_close+0x18/0x20)
[  248.892375] [<c0214ea8>] (blkdev_close) from [<c01e3178>] (__fput+0x84/0x1c0)
[  248.899383] [<c01e3178>] (__fput) from [<c0133d60>] (task_work_run+0xbc/0xdc)
[  248.906593] [<c0133d60>] (task_work_run) from [<c011de60>] (do_exit+0x304/0x9bc)
[  248.913938] [<c011de60>] (do_exit) from [<c011e664>] (do_group_exit+0x3c/0xbc)
[  248.921046] [<c011e664>] (do_group_exit) from [<c01278c0>] (get_signal+0x200/0x65c)
[  248.928776] [<c01278c0>] (get_signal) from [<c010ed48>] (do_signal+0x84/0x3c4)
[  248.935970] [<c010ed48>] (do_signal) from [<c010a0e4>] (do_work_pending+0xa4/0xb4)
[  248.943506] [<c010a0e4>] (do_work_pending) from [<c0107914>] (slow_work_pending+0xc/0x20)
[  248.951637] INFO: task sync:1398 blocked for more than 120 seconds.
[  248.957756]       Tainted: G        W       4.10.0-rc3-00118-g4515dc6 #2736
[  248.965052] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[  248.972681] sync            D    0  1398   1390 0x00000000
[  248.978117] [<c06df404>] (__schedule) from [<c06df90c>] (schedule+0x40/0xac)
[  248.985174] [<c06df90c>] (schedule) from [<c06dfb3c>] (schedule_preempt_disabled+0x14/0x20)
[  248.993609] [<c06dfb3c>] (schedule_preempt_disabled) from [<c06e3b18>] (__mutex_lock_slowpath+0x480/0x6ec)
[  249.003153] [<c06e3b18>] (__mutex_lock_slowpath) from [<c0215964>] (iterate_bdevs+0xb8/0x108)
[  249.011729] [<c0215964>] (iterate_bdevs) from [<c020c0ac>] (sys_sync+0x54/0x98)
[  249.018802] [<c020c0ac>] (sys_sync) from [<c01078c0>] (ret_fast_syscall+0x0/0x3c)

To be exact the same issue also sometimes happens with
previous commit 784da04 ("mmc: queue: simplify queue
logic") and I also got deadlock on boot once with commit
9a4c8a3 ("mmc: core: kill off the context info"):

...
[    5.958868] asoc-simple-card sound: HiFi <-> 3830000.i2s mapping ok
done.
[....] Waiting for /dev to be fully populated...[   16.361597] random: crng init done
done.
[  248.801776] INFO: task mmcqd/0:127 blocked for more than 120 seconds.
[  248.806795]       Tainted: G        W       4.10.0-rc3-00116-g9a4c8a3 #2735
[  248.813882] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[  248.821909] mmcqd/0         D    0   127      2 0x00000000
[  248.827031] [<c06df4b4>] (__schedule) from [<c06df9bc>] (schedule+0x40/0xac)
[  248.834098] [<c06df9bc>] (schedule) from [<c06e531c>] (schedule_timeout+0x148/0x220)
[  248.841788] [<c06e531c>] (schedule_timeout) from [<c06e02a8>] (wait_for_common+0xb8/0x144)
[  248.849969] [<c06e02a8>] (wait_for_common) from [<c05280f8>] (mmc_start_areq+0x40/0x1ac)
[  248.858092] [<c05280f8>] (mmc_start_areq) from [<c0537680>] (mmc_blk_issue_rw_rq+0x78/0x314)
[  248.866485] [<c0537680>] (mmc_blk_issue_rw_rq) from [<c0538318>] (mmc_blk_issue_rq+0x9c/0x458)
[  248.875060] [<c0538318>] (mmc_blk_issue_rq) from [<c0538820>] (mmc_queue_thread+0x90/0x16c)
[  248.883383] [<c0538820>] (mmc_queue_thread) from [<c0135604>] (kthread+0xfc/0x134)
[  248.890867] [<c0135604>] (kthread) from [<c0107978>] (ret_from_fork+0x14/0x3c)
[  248.898124] INFO: task udevd:273 blocked for more than 120 seconds.
[  248.904331]       Tainted: G        W       4.10.0-rc3-00116-g9a4c8a3 #2735
[  248.911191] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[  248.919057] udevd           D    0   273    250 0x00000005
[  248.924543] [<c06df4b4>] (__schedule) from [<c06df9bc>] (schedule+0x40/0xac)
[  248.931557] [<c06df9bc>] (schedule) from [<c06e531c>] (schedule_timeout+0x148/0x220)
[  248.939206] [<c06e531c>] (schedule_timeout) from [<c06df2fc>] (io_schedule_timeout+0x74/0xb0)
[  248.947770] [<c06df2fc>] (io_schedule_timeout) from [<c0198a0c>] (__lock_page+0xe8/0x118)
[  248.955916] [<c0198a0c>] (__lock_page) from [<c01a88b0>] (truncate_inode_pages_range+0x580/0x59c)
[  248.964751] [<c01a88b0>] (truncate_inode_pages_range) from [<c01a8984>] (truncate_inode_pages+0x18/0x20)
[  248.974401] [<c01a8984>] (truncate_inode_pages) from [<c0214bf0>] (__blkdev_put+0x68/0x1d8)
[  248.982593] [<c0214bf0>] (__blkdev_put) from [<c0214ea8>] (blkdev_close+0x18/0x20)
[  248.990088] [<c0214ea8>] (blkdev_close) from [<c01e3178>] (__fput+0x84/0x1c0)
[  248.997229] [<c01e3178>] (__fput) from [<c0133d60>] (task_work_run+0xbc/0xdc)
[  249.004380] [<c0133d60>] (task_work_run) from [<c011de60>] (do_exit+0x304/0x9bc)
[  249.011570] [<c011de60>] (do_exit) from [<c011e664>] (do_group_exit+0x3c/0xbc)
[  249.018732] [<c011e664>] (do_group_exit) from [<c01278c0>] (get_signal+0x200/0x65c)
[  249.026392] [<c01278c0>] (get_signal) from [<c010ed48>] (do_signal+0x84/0x3c4)
[  249.033577] [<c010ed48>] (do_signal) from [<c010a0e4>] (do_work_pending+0xa4/0xb4)
[  249.041086] [<c010a0e4>] (do_work_pending) from [<c0107914>] (slow_work_pending+0xc/0x20)

I assume that the problem got introduced even earlier,
commit 4515dc6 ("mmc: block: shuffle retry and error
handling") just makes it happen every time.

The hardware I use for testing is Odroid XU3-Lite.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH 11/16] mmc: block: shuffle retry and error handling

[Bartlomiej Zolnierkiewicz]

On Wednesday, March 01, 2017 04:52:38 PM Bartlomiej Zolnierkiewicz wrote:

> I assume that the problem got introduced even earlier,
> commit 4515dc6 ("mmc: block: shuffle retry and error
> handling") just makes it happen every time.

Patch #16 makes it worse as now I get deadlock on boot:

[  248.801750] INFO: task kworker/2:2:113 blocked for more than 120 seconds.
[  248.807119]       Tainted: G        W       4.10.0-rc3-00123-g1bec9a6 #2726
[  248.814162] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[  248.821943] kworker/2:2     D    0   113      2 0x00000000
[  248.827357] Workqueue: events_freezable mmc_rescan
[  248.832227] [<c06df12c>] (__schedule) from [<c06df634>] (schedule+0x40/0xac)
[  248.839123] [<c06df634>] (schedule) from [<c0527708>] (__mmc_claim_host+0x8c/0x1a0)
[  248.846851] [<c0527708>] (__mmc_claim_host) from [<c052dc54>] (mmc_attach_mmc+0xb8/0x14c)
[  248.854989] [<c052dc54>] (mmc_attach_mmc) from [<c052a124>] (mmc_rescan+0x274/0x34c)
[  248.862725] [<c052a124>] (mmc_rescan) from [<c012fdf8>] (process_one_work+0x120/0x318)
[  248.870498] [<c012fdf8>] (process_one_work) from [<c0130054>] (worker_thread+0x2c/0x4ac)
[  248.878653] [<c0130054>] (worker_thread) from [<c0135604>] (kthread+0xfc/0x134)
[  248.885934] [<c0135604>] (kthread) from [<c0107978>] (ret_from_fork+0x14/0x3c)
[  248.893098] INFO: task jbd2/mmcblk0p2-:132 blocked for more than 120 seconds.
[  248.900092]       Tainted: G        W       4.10.0-rc3-00123-g1bec9a6 #2726
[  248.907108] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[  248.914904] jbd2/mmcblk0p2- D    0   132      2 0x00000000
[  248.920319] [<c06df12c>] (__schedule) from [<c06df634>] (schedule+0x40/0xac)
[  248.927433] [<c06df634>] (schedule) from [<c06e4f94>] (schedule_timeout+0x148/0x220)
[  248.935139] [<c06e4f94>] (schedule_timeout) from [<c06def74>] (io_schedule_timeout+0x74/0xb0)
[  248.943634] [<c06def74>] (io_schedule_timeout) from [<c06df91c>] (bit_wait_io+0x10/0x58)
[  248.951684] [<c06df91c>] (bit_wait_io) from [<c06dfd3c>] (__wait_on_bit+0x84/0xbc)
[  248.959134] [<c06dfd3c>] (__wait_on_bit) from [<c06dfe60>] (out_of_line_wait_on_bit+0x68/0x70)
[  248.968142] [<c06dfe60>] (out_of_line_wait_on_bit) from [<c0295f4c>] (jbd2_journal_commit_transaction+0x1468/0x15c4)
[  248.978397] [<c0295f4c>] (jbd2_journal_commit_transaction) from [<c0298af0>] (kjournald2+0xbc/0x264)
[  248.987514] [<c0298af0>] (kjournald2) from [<c0135604>] (kthread+0xfc/0x134)
[  248.994494] [<c0135604>] (kthread) from [<c0107978>] (ret_from_fork+0x14/0x3c)
[  249.001714] INFO: task kworker/1:2H:134 blocked for more than 120 seconds.
[  249.008412]       Tainted: G        W       4.10.0-rc3-00123-g1bec9a6 #2726
[  249.015479] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[  249.023094] kworker/1:2H    D    0   134      2 0x00000000
[  249.028510] Workqueue: kblockd blk_mq_run_work_fn
[  249.033330] [<c06df12c>] (__schedule) from [<c06df634>] (schedule+0x40/0xac)
[  249.040199] [<c06df634>] (schedule) from [<c0527708>] (__mmc_claim_host+0x8c/0x1a0)
[  249.047856] [<c0527708>] (__mmc_claim_host) from [<c053881c>] (mmc_queue_rq+0x9c/0xa8)
[  249.055736] [<c053881c>] (mmc_queue_rq) from [<c0314358>] (blk_mq_dispatch_rq_list+0xd4/0x1d0)
[  249.064316] [<c0314358>] (blk_mq_dispatch_rq_list) from [<c03145d4>] (blk_mq_process_rq_list+0x180/0x198)
[  249.073845] [<c03145d4>] (blk_mq_process_rq_list) from [<c03146a4>] (__blk_mq_run_hw_queue+0xb8/0x110)
[  249.083120] [<c03146a4>] (__blk_mq_run_hw_queue) from [<c012fdf8>] (process_one_work+0x120/0x318)
[  249.092076] [<c012fdf8>] (process_one_work) from [<c0130054>] (worker_thread+0x2c/0x4ac)
[  249.099990] [<c0130054>] (worker_thread) from [<c0135604>] (kthread+0xfc/0x134)
[  249.107322] [<c0135604>] (kthread) from [<c0107978>] (ret_from_fork+0x14/0x3c)
[  249.114485] INFO: task kworker/5:2H:136 blocked for more than 120 seconds.
[  249.121326]       Tainted: G        W       4.10.0-rc3-00123-g1bec9a6 #2726
[  249.128232] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[  249.136074] kworker/5:2H    D    0   136      2 0x00000000
[  249.141544] Workqueue: kblockd blk_mq_run_work_fn
[  249.146187] [<c06df12c>] (__schedule) from [<c06df634>] (schedule+0x40/0xac)
[  249.153419] [<c06df634>] (schedule) from [<c0527708>] (__mmc_claim_host+0x8c/0x1a0)
[  249.160825] [<c0527708>] (__mmc_claim_host) from [<c053881c>] (mmc_queue_rq+0x9c/0xa8)
[  249.168755] [<c053881c>] (mmc_queue_rq) from [<c0314358>] (blk_mq_dispatch_rq_list+0xd4/0x1d0)
[  249.177318] [<c0314358>] (blk_mq_dispatch_rq_list) from [<c03145d4>] (blk_mq_process_rq_list+0x180/0x198)
[  249.186858] [<c03145d4>] (blk_mq_process_rq_list) from [<c03146a4>] (__blk_mq_run_hw_queue+0xb8/0x110)
[  249.196124] [<c03146a4>] (__blk_mq_run_hw_queue) from [<c012fdf8>] (process_one_work+0x120/0x318)
[  249.204969] [<c012fdf8>] (process_one_work) from [<c0130054>] (worker_thread+0x2c/0x4ac)
[  249.213161] [<c0130054>] (worker_thread) from [<c0135604>] (kthread+0xfc/0x134)
[  249.220270] [<c0135604>] (kthread) from [<c0107978>] (ret_from_fork+0x14/0x3c)
[  249.227505] INFO: task kworker/0:1H:145 blocked for more than 120 seconds.
[  249.234328]       Tainted: G        W       4.10.0-rc3-00123-g1bec9a6 #2726
[  249.241229] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[  249.249066] kworker/0:1H    D    0   145      2 0x00000000
[  249.254521] Workqueue: kblockd blk_mq_run_work_fn
[  249.259176] [<c06df12c>] (__schedule) from [<c06df634>] (schedule+0x40/0xac)
[  249.266233] [<c06df634>] (schedule) from [<c0527708>] (__mmc_claim_host+0x8c/0x1a0)
[  249.274001] [<c0527708>] (__mmc_claim_host) from [<c053881c>] (mmc_queue_rq+0x9c/0xa8)
[  249.281747] [<c053881c>] (mmc_queue_rq) from [<c0314358>] (blk_mq_dispatch_rq_list+0xd4/0x1d0)
[  249.290284] [<c0314358>] (blk_mq_dispatch_rq_list) from [<c03145d4>] (blk_mq_process_rq_list+0x180/0x198)
[  249.299843] [<c03145d4>] (blk_mq_process_rq_list) from [<c03146a4>] (__blk_mq_run_hw_queue+0xb8/0x110)
[  249.309122] [<c03146a4>] (__blk_mq_run_hw_queue) from [<c012fdf8>] (process_one_work+0x120/0x318)
[  249.317951] [<c012fdf8>] (process_one_work) from [<c0130054>] (worker_thread+0x2c/0x4ac)
[  249.326017] [<c0130054>] (worker_thread) from [<c0135604>] (kthread+0xfc/0x134)
[  249.333408] [<c0135604>] (kthread) from [<c0107978>] (ret_from_fork+0x14/0x3c)
[  249.340459] INFO: task udevd:280 blocked for more than 120 seconds.
[  249.346725]       Tainted: G        W       4.10.0-rc3-00123-g1bec9a6 #2726
[  249.353644] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[  249.361452] udevd           D    0   280    258 0x00000005
[  249.366885] [<c06df12c>] (__schedule) from [<c06df634>] (schedule+0x40/0xac)
[  249.373964] [<c06df634>] (schedule) from [<c06e4f94>] (schedule_timeout+0x148/0x220)
[  249.381651] [<c06e4f94>] (schedule_timeout) from [<c06def74>] (io_schedule_timeout+0x74/0xb0)
[  249.390110] [<c06def74>] (io_schedule_timeout) from [<c0198a0c>] (__lock_page+0xe8/0x118)
[  249.398399] [<c0198a0c>] (__lock_page) from [<c01a88b0>] (truncate_inode_pages_range+0x580/0x59c)
[  249.407129] [<c01a88b0>] (truncate_inode_pages_range) from [<c01a8984>] (truncate_inode_pages+0x18/0x20)
[  249.416571] [<c01a8984>] (truncate_inode_pages) from [<c0214bf0>] (__blkdev_put+0x68/0x1d8)
[  249.424892] [<c0214bf0>] (__blkdev_put) from [<c0214ea8>] (blkdev_close+0x18/0x20)
[  249.432422] [<c0214ea8>] (blkdev_close) from [<c01e3178>] (__fput+0x84/0x1c0)
[  249.439501] [<c01e3178>] (__fput) from [<c0133d60>] (task_work_run+0xbc/0xdc)
[  249.446677] [<c0133d60>] (task_work_run) from [<c011de60>] (do_exit+0x304/0x9bc)
[  249.454152] [<c011de60>] (do_exit) from [<c011e664>] (do_group_exit+0x3c/0xbc)
[  249.461165] [<c011e664>] (do_group_exit) from [<c01278c0>] (get_signal+0x200/0x65c)
[  249.468833] [<c01278c0>] (get_signal) from [<c010ed48>] (do_signal+0x84/0x3c4)
[  249.476015] [<c010ed48>] (do_signal) from [<c010a0e4>] (do_work_pending+0xa4/0xb4)
[  249.483557] [<c010a0e4>] (do_work_pending) from [<c0107914>] (slow_work_pending+0xc/0x20)
[  249.491689] INFO: task udevd:281 blocked for more than 120 seconds.
[  249.497900]       Tainted: G        W       4.10.0-rc3-00123-g1bec9a6 #2726
[  249.504892] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[  249.512771] udevd           D    0   281    258 0x00000005
[  249.518097] [<c06df12c>] (__schedule) from [<c06df634>] (schedule+0x40/0xac)
[  249.525153] [<c06df634>] (schedule) from [<c06e4f94>] (schedule_timeout+0x148/0x220)
[  249.532853] [<c06e4f94>] (schedule_timeout) from [<c06def74>] (io_schedule_timeout+0x74/0xb0)
[  249.541354] [<c06def74>] (io_schedule_timeout) from [<c0198a0c>] (__lock_page+0xe8/0x118)
[  249.549463] [<c0198a0c>] (__lock_page) from [<c01a88b0>] (truncate_inode_pages_range+0x580/0x59c)
[  249.558331] [<c01a88b0>] (truncate_inode_pages_range) from [<c01a8984>] (truncate_inode_pages+0x18/0x20)
[  249.567785] [<c01a8984>] (truncate_inode_pages) from [<c0214bf0>] (__blkdev_put+0x68/0x1d8)
[  249.576207] [<c0214bf0>] (__blkdev_put) from [<c0214ea8>] (blkdev_close+0x18/0x20)
[  249.583669] [<c0214ea8>] (blkdev_close) from [<c01e3178>] (__fput+0x84/0x1c0)
[  249.590710] [<c01e3178>] (__fput) from [<c0133d60>] (task_work_run+0xbc/0xdc)
[  249.597843] [<c0133d60>] (task_work_run) from [<c011de60>] (do_exit+0x304/0x9bc)
[  249.605217] [<c011de60>] (do_exit) from [<c011e664>] (do_group_exit+0x3c/0xbc)
[  249.612399] [<c011e664>] (do_group_exit) from [<c01278c0>] (get_signal+0x200/0x65c)
[  249.620000] [<c01278c0>] (get_signal) from [<c010ed48>] (do_signal+0x84/0x3c4)
[  249.627228] [<c010ed48>] (do_signal) from [<c010a0e4>] (do_work_pending+0xa4/0xb4)
[  249.634874] [<c010a0e4>] (do_work_pending) from [<c0107914>] (slow_work_pending+0xc/0x20)
[  249.642922] INFO: task kworker/u16:2:1268 blocked for more than 120 seconds.
[  249.649891]       Tainted: G        W       4.10.0-rc3-00123-g1bec9a6 #2726
[  249.656847] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[  249.664654] kworker/u16:2   D    0  1268      2 0x00000000
[  249.670094] Workqueue: writeback wb_workfn (flush-179:0)
[  249.675398] [<c06df12c>] (__schedule) from [<c06df634>] (schedule+0x40/0xac)
[  249.682425] [<c06df634>] (schedule) from [<c06e4f94>] (schedule_timeout+0x148/0x220)
[  249.690103] [<c06e4f94>] (schedule_timeout) from [<c06def74>] (io_schedule_timeout+0x74/0xb0)
[  249.698738] [<c06def74>] (io_schedule_timeout) from [<c03154e4>] (bt_get+0x140/0x228)
[  249.706432] [<c03154e4>] (bt_get) from [<c03156d0>] (blk_mq_get_tag+0x24/0xa8)
[  249.713613] [<c03156d0>] (blk_mq_get_tag) from [<c03119c0>] (__blk_mq_alloc_request+0x10/0x15c)
[  249.722287] [<c03119c0>] (__blk_mq_alloc_request) from [<c0311bbc>] (blk_mq_map_request+0xb0/0xfc)
[  249.731178] [<c0311bbc>] (blk_mq_map_request) from [<c03136f0>] (blk_sq_make_request+0x8c/0x298)
[  249.739962] [<c03136f0>] (blk_sq_make_request) from [<c0308e00>] (generic_make_request+0xd8/0x180)
[  249.748891] [<c0308e00>] (generic_make_request) from [<c0308f30>] (submit_bio+0x88/0x148)
[  249.757175] [<c0308f30>] (submit_bio) from [<c0256ccc>] (ext4_io_submit+0x34/0x40)
[  249.764581] [<c0256ccc>] (ext4_io_submit) from [<c0255674>] (ext4_writepages+0x484/0x670)
[  249.772722] [<c0255674>] (ext4_writepages) from [<c01a5348>] (do_writepages+0x24/0x38)
[  249.780573] [<c01a5348>] (do_writepages) from [<c0208038>] (__writeback_single_inode+0x28/0x18c)
[  249.789359] [<c0208038>] (__writeback_single_inode) from [<c02085f0>] (writeback_sb_inodes+0x1e0/0x394)
[  249.798717] [<c02085f0>] (writeback_sb_inodes) from [<c0208814>] (__writeback_inodes_wb+0x70/0xac)
[  249.807643] [<c0208814>] (__writeback_inodes_wb) from [<c02089dc>] (wb_writeback+0x18c/0x1b4)
[  249.816241] [<c02089dc>] (wb_writeback) from [<c0208d68>] (wb_workfn+0x1c8/0x388)
[  249.823590] [<c0208d68>] (wb_workfn) from [<c012fdf8>] (process_one_work+0x120/0x318)
[  249.831375] [<c012fdf8>] (process_one_work) from [<c0130054>] (worker_thread+0x2c/0x4ac)
[  249.839408] [<c0130054>] (worker_thread) from [<c0135604>] (kthread+0xfc/0x134)
[  249.846726] [<c0135604>] (kthread) from [<c0107978>] (ret_from_fork+0x14/0x3c)

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH 11/16] mmc: block: shuffle retry and error handling

[Bartlomiej Zolnierkiewicz]

On Wednesday, March 01, 2017 04:52:38 PM Bartlomiej Zolnierkiewicz wrote:

> I assume that the problem got introduced even earlier,
> commit 4515dc6 ("mmc: block: shuffle retry and error
> handling") just makes it happen every time.

It seems to be introduced by patch #6. Patch #5 survived
30 consecutive boot+sync iterations (with later patches
the issue shows up during the first 12 iterations).

root@target:~# sync
[  248.801846] INFO: task mmcqd/0:128 blocked for more than 120 seconds.
[  248.806866]       Tainted: G        W       4.10.0-rc3-00113-g5750765 #2739
[  248.814051] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[  248.821696] mmcqd/0         D    0   128      2 0x00000000
[  248.827123] [<c06df51c>] (__schedule) from [<c06dfa24>] (schedule+0x40/0xac)
[  248.834210] [<c06dfa24>] (schedule) from [<c06e5384>] (schedule_timeout+0x148/0x220)
[  248.841912] [<c06e5384>] (schedule_timeout) from [<c06e0310>] (wait_for_common+0xb8/0x144)
[  248.850058] [<c06e0310>] (wait_for_common) from [<c0528100>] (mmc_start_areq+0x40/0x1ac)
[  248.858209] [<c0528100>] (mmc_start_areq) from [<c05376c0>] (mmc_blk_issue_rw_rq+0x78/0x314)
[  248.866599] [<c05376c0>] (mmc_blk_issue_rw_rq) from [<c0538358>] (mmc_blk_issue_rq+0x9c/0x458)
[  248.875293] [<c0538358>] (mmc_blk_issue_rq) from [<c0538868>] (mmc_queue_thread+0x98/0x180)
[  248.883789] [<c0538868>] (mmc_queue_thread) from [<c0135604>] (kthread+0xfc/0x134)
[  248.891058] [<c0135604>] (kthread) from [<c0107978>] (ret_from_fork+0x14/0x3c)
[  248.898364] INFO: task jbd2/mmcblk0p2-:136 blocked for more than 120 seconds.
[  248.905400]       Tainted: G        W       4.10.0-rc3-00113-g5750765 #2739
[  248.912353] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[  248.919923] jbd2/mmcblk0p2- D    0   136      2 0x00000000
[  248.925693] [<c06df51c>] (__schedule) from [<c06dfa24>] (schedule+0x40/0xac)
[  248.932470] [<c06dfa24>] (schedule) from [<c0294ccc>] (jbd2_journal_commit_transaction+0x1e8/0x15c4)
[  248.941552] [<c0294ccc>] (jbd2_journal_commit_transaction) from [<c0298af0>] (kjournald2+0xbc/0x264)
[  248.950608] [<c0298af0>] (kjournald2) from [<c0135604>] (kthread+0xfc/0x134)
[  248.957660] [<c0135604>] (kthread) from [<c0107978>] (ret_from_fork+0x14/0x3c)
[  248.964860] INFO: task kworker/u16:2:730 blocked for more than 120 seconds.
[  248.971780]       Tainted: G        W       4.10.0-rc3-00113-g5750765 #2739
[  248.978673] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[  248.986686] kworker/u16:2   D    0   730      2 0x00000000
[  248.991993] Workqueue: writeback wb_workfn (flush-179:0)
[  248.997230] [<c06df51c>] (__schedule) from [<c06dfa24>] (schedule+0x40/0xac)
[  249.004287] [<c06dfa24>] (schedule) from [<c06e5384>] (schedule_timeout+0x148/0x220)
[  249.011997] [<c06e5384>] (schedule_timeout) from [<c06df364>] (io_schedule_timeout+0x74/0xb0)
[  249.020451] [<c06df364>] (io_schedule_timeout) from [<c06dfd0c>] (bit_wait_io+0x10/0x58)
[  249.028545] [<c06dfd0c>] (bit_wait_io) from [<c06dff1c>] (__wait_on_bit_lock+0x74/0xd0)
[  249.036513] [<c06dff1c>] (__wait_on_bit_lock) from [<c06dffe0>] (out_of_line_wait_on_bit_lock+0x68/0x70)
[  249.046231] [<c06dffe0>] (out_of_line_wait_on_bit_lock) from [<c0293dfc>] (do_get_write_access+0x3d0/0x4c4)
[  249.055729] [<c0293dfc>] (do_get_write_access) from [<c029410c>] (jbd2_journal_get_write_access+0x38/0x64)
[  249.065336] [<c029410c>] (jbd2_journal_get_write_access) from [<c0272680>] (__ext4_journal_get_write_access+0x2c/0x68)
[  249.076016] [<c0272680>] (__ext4_journal_get_write_access) from [<c0278eb8>] (ext4_mb_mark_diskspace_used+0x64/0x474)
[  249.086515] [<c0278eb8>] (ext4_mb_mark_diskspace_used) from [<c027a334>] (ext4_mb_new_blocks+0x258/0xa1c)
[  249.096040] [<c027a334>] (ext4_mb_new_blocks) from [<c026fc80>] (ext4_ext_map_blocks+0x8b4/0xf28)
[  249.104883] [<c026fc80>] (ext4_ext_map_blocks) from [<c024f318>] (ext4_map_blocks+0x144/0x5f8)
[  249.113468] [<c024f318>] (ext4_map_blocks) from [<c0254b0c>] (mpage_map_and_submit_extent+0xa4/0x788)
[  249.122641] [<c0254b0c>] (mpage_map_and_submit_extent) from [<c02556d0>] (ext4_writepages+0x4e0/0x670)
[  249.131925] [<c02556d0>] (ext4_writepages) from [<c01a5348>] (do_writepages+0x24/0x38)
[  249.139774] [<c01a5348>] (do_writepages) from [<c0208038>] (__writeback_single_inode+0x28/0x18c)
[  249.148555] [<c0208038>] (__writeback_single_inode) from [<c02085f0>] (writeback_sb_inodes+0x1e0/0x394)
[  249.157909] [<c02085f0>] (writeback_sb_inodes) from [<c0208814>] (__writeback_inodes_wb+0x70/0xac)
[  249.166833] [<c0208814>] (__writeback_inodes_wb) from [<c02089dc>] (wb_writeback+0x18c/0x1b4)
[  249.175324] [<c02089dc>] (wb_writeback) from [<c0208c74>] (wb_workfn+0xd4/0x388)
[  249.182704] [<c0208c74>] (wb_workfn) from [<c012fdf8>] (process_one_work+0x120/0x318)
[  249.190464] [<c012fdf8>] (process_one_work) from [<c0130054>] (worker_thread+0x2c/0x4ac)
[  249.198551] [<c0130054>] (worker_thread) from [<c0135604>] (kthread+0xfc/0x134)
[  249.205904] [<c0135604>] (kthread) from [<c0107978>] (ret_from_fork+0x14/0x3c)
[  249.213094] INFO: task sync:1403 blocked for more than 120 seconds.
[  249.219261]       Tainted: G        W       4.10.0-rc3-00113-g5750765 #2739
[  249.226220] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[  249.234019] sync            D    0  1403   1396 0x00000000
[  249.239424] [<c06df51c>] (__schedule) from [<c06dfa24>] (schedule+0x40/0xac)
[  249.246624] [<c06dfa24>] (schedule) from [<c02078c0>] (wb_wait_for_completion+0x50/0x7c)
[  249.254538] [<c02078c0>] (wb_wait_for_completion) from [<c0207c14>] (sync_inodes_sb+0x94/0x20c)
[  249.263200] [<c0207c14>] (sync_inodes_sb) from [<c01e4dc8>] (iterate_supers+0xac/0xd4)
[  249.271056] [<c01e4dc8>] (iterate_supers) from [<c020c088>] (sys_sync+0x30/0x98)
[  249.278446] [<c020c088>] (sys_sync) from [<c01078c0>] (ret_fast_syscall+0x0/0x3c)

I also once hit another problem with patch #6 that doesn't
happen with patch #5:

[   12.121767] Unable to handle kernel NULL pointer dereference at virtual address 00000008
[   12.129747] pgd = c0004000
[   12.132425] [00000008] *pgd=00000000
[   12.135996] Internal error: Oops: 5 [#1] PREEMPT SMP ARM
[   12.141262] Modules linked in:
[   12.144304] CPU: 0 PID: 126 Comm: mmcqd/0 Tainted: G        W       4.10.0-rc3-00113-g5750765 #2739
[   12.153296] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[   12.159367] task: edd19900 task.stack: edd66000
[   12.163900] PC is at kthread_queue_work+0x18/0x64
[   12.168574] LR is at _raw_spin_lock_irqsave+0x20/0x28
[   12.173583] pc : [<c0135b24>]    lr : [<c06e6138>]    psr: 60000193
[   12.173583] sp : edd67d10  ip : 00000000  fp : edcc9b04
[   12.185014] r10: 00000000  r9 : edd6808c  r8 : edcc9b08
[   12.190215] r7 : 00000000  r6 : edc97320  r5 : edc97324  r4 : 00000008
[   12.196714] r3 : edc97000  r2 : 00000000  r1 : 0b750b74  r0 : a0000113
[   12.203216] Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment none
[   12.210406] Control: 10c5387d  Table: 6d0e006a  DAC: 00000051
[   12.216125] Process mmcqd/0 (pid: 126, stack limit = 0xedd66210)
[   12.222102] Stack: (0xedd67d10 to 0xedd68000)
[   12.226444] 7d00:                                     edc97000 edd68004 edd6808c edc97000
[   12.234595] 7d20: 00000000 c0527ab8 edcc9a10 edc97000 edd68004 edd68004 edcc9b08 c0542834
[   12.242740] 7d40: edd680c0 edcc9a10 00000001 edd680f4 edd68004 c0542b5c edd67da4 edcc9a80
[   12.250886] 7d60: c0b108aa edcc9af0 edcc9af4 00000000 c0a62244 00000000 c0b02080 00000006
[   12.259031] 7d80: 00000101 c011f6f0 00000000 c0b02098 00000006 c0a622c8 c0b02080 c011edac
[   12.267176] 7da0: eea15160 00000001 00000000 00000009 ffff8f8d 00208840 eea15100 00000000
[   12.275322] 7dc0: 0000004b c0a65c20 00000000 00000001 ee818000 edd67e28 00000000 c011f1a8
[   12.283468] 7de0: 0000008c c016068c f0802000 c0b05724 f080200c 000003eb c0b17c30 f0803000
[   12.291614] 7e00: edd67e28 c0101470 c03448b8 20000013 ffffffff edd67e5c 00000000 edd66000
[   12.299759] 7e20: edd68004 c010b00c c08a2154 c0890cdc edd67e78 edd66000 00000000 c011f068
[   12.307904] 7e40: c0890cdc c08a2154 00000000 edd68030 edd68004 00000000 00000001 edd67e78
[   12.316050] 7e60: c011f068 c03448b8 20000013 ffffffff 00000051 00000000 edd68004 00000001
[   12.324195] 7e80: 00000000 00000201 edc97000 edd68004 00000001 c011f068 edc97000 c0527b8c
[   12.332340] 7ea0: 00000000 edd68004 edc97000 edd6813c 00000001 c0527d04 edd68044 edc97000
[   12.340487] 7ec0: 00000000 c0528208 edd68000 edd31800 edd48858 edd48858 ede6fe60 edd48840
[   12.348631] 7ee0: edd48840 00000001 00000000 c05376c0 00000000 00000001 00000000 00000000
[   12.356777] 7f00: 00000000 c013c5ec 00000100 ede6fe60 00000000 edd48858 edd48840 edd48840
[   12.364922] 7f20: edd31800 00000001 00000000 c0538358 edc18b50 edc97000 edd48860 00000001
[   12.373068] 7f40: edc18b50 edd48858 00000000 ede6fe60 edc18b50 edc97000 edd48860 00000001
[   12.381214] 7f60: 00000000 c0538868 edd19900 eeae0500 00000000 edd4e000 eeae0528 edd48858
[   12.389358] 7f80: edc87d14 c05387d0 00000000 c0135604 edd4e000 c0135508 00000000 00000000
[   12.397502] 7fa0: 00000000 00000000 00000000 c0107978 00000000 00000000 00000000 00000000
[   12.405647] 7fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[   12.413795] 7fe0: 00000000 00000000 00000000 00000000 00000013 00000000 ffffffff ffffffff
[   12.421985] [<c0135b24>] (kthread_queue_work) from [<c0527ab8>] (mmc_request_done+0xd8/0x158)
[   12.430458] [<c0527ab8>] (mmc_request_done) from [<c0542834>] (dw_mci_request_end+0xa0/0xd8)
[   12.438848] [<c0542834>] (dw_mci_request_end) from [<c0542b5c>] (dw_mci_tasklet_func+0x2f0/0x394)
[   12.447693] [<c0542b5c>] (dw_mci_tasklet_func) from [<c011f6f0>] (tasklet_action+0x84/0x12c)
[   12.456089] [<c011f6f0>] (tasklet_action) from [<c011edac>] (__do_softirq+0xec/0x244)
[   12.463885] [<c011edac>] (__do_softirq) from [<c011f1a8>] (irq_exit+0xc0/0x104)
[   12.471166] [<c011f1a8>] (irq_exit) from [<c016068c>] (__handle_domain_irq+0x70/0xe4)
[   12.478966] [<c016068c>] (__handle_domain_irq) from [<c0101470>] (gic_handle_irq+0x50/0x9c)
[   12.487280] [<c0101470>] (gic_handle_irq) from [<c010b00c>] (__irq_svc+0x6c/0xa8)
[   12.494716] Exception stack(0xedd67e28 to 0xedd67e70)
[   12.499753] 7e20:                   c08a2154 c0890cdc edd67e78 edd66000 00000000 c011f068
[   12.507902] 7e40: c0890cdc c08a2154 00000000 edd68030 edd68004 00000000 00000001 edd67e78
[   12.516039] 7e60: c011f068 c03448b8 20000013 ffffffff
[   12.521085] [<c010b00c>] (__irq_svc) from [<c03448b8>] (check_preemption_disabled+0x30/0x128)
[   12.529573] [<c03448b8>] (check_preemption_disabled) from [<c011f068>] (__local_bh_enable_ip+0xc8/0xec)
[   12.538931] [<c011f068>] (__local_bh_enable_ip) from [<c0527b8c>] (__mmc_start_request+0x54/0xdc)
[   12.547770] [<c0527b8c>] (__mmc_start_request) from [<c0527d04>] (mmc_start_request+0xf0/0x11c)
[   12.556437] [<c0527d04>] (mmc_start_request) from [<c0528208>] (mmc_start_areq+0x148/0x1ac)
[   12.564753] [<c0528208>] (mmc_start_areq) from [<c05376c0>] (mmc_blk_issue_rw_rq+0x78/0x314)
[   12.573155] [<c05376c0>] (mmc_blk_issue_rw_rq) from [<c0538358>] (mmc_blk_issue_rq+0x9c/0x458)
[   12.581733] [<c0538358>] (mmc_blk_issue_rq) from [<c0538868>] (mmc_queue_thread+0x98/0x180)
[   12.590053] [<c0538868>] (mmc_queue_thread) from [<c0135604>] (kthread+0xfc/0x134)
[   12.597603] [<c0135604>] (kthread) from [<c0107978>] (ret_from_fork+0x14/0x3c)
[   12.604782] Code: e1a06000 e1a04001 e1a00005 eb16c17c (e5943000) 
[   12.610842] ---[ end trace 86f45842e4b0b193 ]---
[   12.615426] Kernel panic - not syncing: Fatal exception in interrupt
[   12.621786] CPU1: stopping
[   12.624455] CPU: 1 PID: 0 Comm: swapper/1 Tainted: G      D W       4.10.0-rc3-00113-g5750765 #2739
[   12.633452] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[   12.639567] [<c010d830>] (unwind_backtrace) from [<c010a544>] (show_stack+0x10/0x14)
[   12.647261] [<c010a544>] (show_stack) from [<c032956c>] (dump_stack+0x74/0x94)
[   12.654445] [<c032956c>] (dump_stack) from [<c010caac>] (handle_IPI+0x170/0x1a8)
[   12.661810] [<c010caac>] (handle_IPI) from [<c01014b0>] (gic_handle_irq+0x90/0x9c)
[   12.669344] [<c01014b0>] (gic_handle_irq) from [<c010b00c>] (__irq_svc+0x6c/0xa8)
[   12.676783] Exception stack(0xee8b3f78 to 0xee8b3fc0)
[   12.681813] 3f60:                                                       00000001 00000000
[   12.689970] 3f80: ee8b3fd0 c0114060 c0b05444 c0b053e4 c0a66cc8 c0b0544c c0b108a2 00000000
[   12.698113] 3fa0: 00000000 00000000 00000001 ee8b3fc8 c01083c0 c01083c4 60000013 ffffffff
[   12.706265] [<c010b00c>] (__irq_svc) from [<c01083c4>] (arch_cpu_idle+0x30/0x3c)
[   12.713653] [<c01083c4>] (arch_cpu_idle) from [<c0152f34>] (do_idle+0x13c/0x200)
[   12.721001] [<c0152f34>] (do_idle) from [<c015328c>] (cpu_startup_entry+0x18/0x1c)
[   12.728538] [<c015328c>] (cpu_startup_entry) from [<4010154c>] (0x4010154c)
[   12.735463] CPU5: stopping
[   12.738165] CPU: 5 PID: 0 Comm: swapper/5 Tainted: G      D W       4.10.0-rc3-00113-g5750765 #2739
[   12.747156] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[   12.753291] [<c010d830>] (unwind_backtrace) from [<c010a544>] (show_stack+0x10/0x14)
[   12.760971] [<c010a544>] (show_stack) from [<c032956c>] (dump_stack+0x74/0x94)
[   12.768153] [<c032956c>] (dump_stack) from [<c010caac>] (handle_IPI+0x170/0x1a8)
[   12.775515] [<c010caac>] (handle_IPI) from [<c01014b0>] (gic_handle_irq+0x90/0x9c)
[   12.783049] [<c01014b0>] (gic_handle_irq) from [<c010b00c>] (__irq_svc+0x6c/0xa8)
[   12.790485] Exception stack(0xee8bbf78 to 0xee8bbfc0)
[   12.795517] bf60:                                                       00000001 00000000
[   12.803673] bf80: ee8bbfd0 c0114060 c0b05444 c0b053e4 c0a66cc8 c0b0544c c0b108a2 00000000
[   12.811817] bfa0: 00000000 00000000 00000001 ee8bbfc8 c01083c0 c01083c4 60000013 ffffffff
[   12.819968] [<c010b00c>] (__irq_svc) from [<c01083c4>] (arch_cpu_idle+0x30/0x3c)
[   12.827350] [<c01083c4>] (arch_cpu_idle) from [<c0152f34>] (do_idle+0x13c/0x200)
[   12.834710] [<c0152f34>] (do_idle) from [<c015328c>] (cpu_startup_entry+0x18/0x1c)
[   12.842239] [<c015328c>] (cpu_startup_entry) from [<4010154c>] (0x4010154c)
[   12.849159] CPU4: stopping
[   12.851846] CPU: 4 PID: 0 Comm: swapper/4 Tainted: G      D W       4.10.0-rc3-00113-g5750765 #2739
[   12.860840] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[   12.866957] [<c010d830>] (unwind_backtrace) from [<c010a544>] (show_stack+0x10/0x14)
[   12.874653] [<c010a544>] (show_stack) from [<c032956c>] (dump_stack+0x74/0x94)
[   12.881835] [<c032956c>] (dump_stack) from [<c010caac>] (handle_IPI+0x170/0x1a8)
[   12.889197] [<c010caac>] (handle_IPI) from [<c01014b0>] (gic_handle_irq+0x90/0x9c)
[   12.896729] [<c01014b0>] (gic_handle_irq) from [<c010b00c>] (__irq_svc+0x6c/0xa8)
[   12.904168] Exception stack(0xee8b9f78 to 0xee8b9fc0)
[   12.909204] 9f60:                                                       00000001 00000000
[   12.917356] 9f80: ee8b9fd0 c0114060 c0b05444 c0b053e4 c0a66cc8 c0b0544c c0b108a2 00000000
[   12.925499] 9fa0: 00000000 00000000 00000001 ee8b9fc8 c01083c0 c01083c4 60000013 ffffffff
[   12.933655] [<c010b00c>] (__irq_svc) from [<c01083c4>] (arch_cpu_idle+0x30/0x3c)
[   12.941028] [<c01083c4>] (arch_cpu_idle) from [<c0152f34>] (do_idle+0x13c/0x200)
[   12.948393] [<c0152f34>] (do_idle) from [<c015328c>] (cpu_startup_entry+0x18/0x1c)
[   12.955923] [<c015328c>] (cpu_startup_entry) from [<4010154c>] (0x4010154c)
[   12.962842] CPU2: stopping
[   12.965520] CPU: 2 PID: 0 Comm: swapper/2 Tainted: G      D W       4.10.0-rc3-00113-g5750765 #2739
[   12.974517] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[   12.980621] [<c010d830>] (unwind_backtrace) from [<c010a544>] (show_stack+0x10/0x14)
[   12.988321] [<c010a544>] (show_stack) from [<c032956c>] (dump_stack+0x74/0x94)
[   12.995508] [<c032956c>] (dump_stack) from [<c010caac>] (handle_IPI+0x170/0x1a8)
[   13.002875] [<c010caac>] (handle_IPI) from [<c01014b0>] (gic_handle_irq+0x90/0x9c)
[   13.010409] [<c01014b0>] (gic_handle_irq) from [<c010b00c>] (__irq_svc+0x6c/0xa8)
[   13.017849] Exception stack(0xee8b5f78 to 0xee8b5fc0)
[   13.022878] 5f60:                                                       00000001 00000000
[   13.031036] 5f80: ee8b5fd0 c0114060 c0b05444 c0b053e4 c0a66cc8 c0b0544c c0b108a2 00000000
[   13.039182] 5fa0: 00000000 00000000 00000001 ee8b5fc8 c01083c0 c01083c4 60000013 ffffffff
[   13.047329] [<c010b00c>] (__irq_svc) from [<c01083c4>] (arch_cpu_idle+0x30/0x3c)
[   13.054703] [<c01083c4>] (arch_cpu_idle) from [<c0152f34>] (do_idle+0x13c/0x200)
[   13.062066] [<c0152f34>] (do_idle) from [<c015328c>] (cpu_startup_entry+0x18/0x1c)
[   13.069600] [<c015328c>] (cpu_startup_entry) from [<4010154c>] (0x4010154c)
[   13.076519] CPU3: stopping
[   13.079209] CPU: 3 PID: 0 Comm: swapper/3 Tainted: G      D W       4.10.0-rc3-00113-g5750765 #2739
[   13.088207] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[   13.094309] [<c010d830>] (unwind_backtrace) from [<c010a544>] (show_stack+0x10/0x14)
[   13.102010] [<c010a544>] (show_stack) from [<c032956c>] (dump_stack+0x74/0x94)
[   13.109197] [<c032956c>] (dump_stack) from [<c010caac>] (handle_IPI+0x170/0x1a8)
[   13.116563] [<c010caac>] (handle_IPI) from [<c01014b0>] (gic_handle_irq+0x90/0x9c)
[   13.124099] [<c01014b0>] (gic_handle_irq) from [<c010b00c>] (__irq_svc+0x6c/0xa8)
[   13.131537] Exception stack(0xee8b7f78 to 0xee8b7fc0)
[   13.136566] 7f60:                                                       00000001 00000000
[   13.144723] 7f80: ee8b7fd0 c0114060 c0b05444 c0b053e4 c0a66cc8 c0b0544c c0b108a2 00000000
[   13.152869] 7fa0: 00000000 00000000 00000001 ee8b7fc8 c01083c0 c01083c4 60000013 ffffffff
[   13.161019] [<c010b00c>] (__irq_svc) from [<c01083c4>] (arch_cpu_idle+0x30/0x3c)
[   13.168390] [<c01083c4>] (arch_cpu_idle) from [<c0152f34>] (do_idle+0x13c/0x200)
[   13.175754] [<c0152f34>] (do_idle) from [<c015328c>] (cpu_startup_entry+0x18/0x1c)
[   13.183286] [<c015328c>] (cpu_startup_entry) from [<4010154c>] (0x4010154c)
[   13.190213] CPU6: stopping
[   13.192912] CPU: 6 PID: 0 Comm: swapper/6 Tainted: G      D W       4.10.0-rc3-00113-g5750765 #2739
[   13.201905] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[   13.208022] [<c010d830>] (unwind_backtrace) from [<c010a544>] (show_stack+0x10/0x14)
[   13.215716] [<c010a544>] (show_stack) from [<c032956c>] (dump_stack+0x74/0x94)
[   13.222899] [<c032956c>] (dump_stack) from [<c010caac>] (handle_IPI+0x170/0x1a8)
[   13.230263] [<c010caac>] (handle_IPI) from [<c01014b0>] (gic_handle_irq+0x90/0x9c)
[   13.237796] [<c01014b0>] (gic_handle_irq) from [<c010b00c>] (__irq_svc+0x6c/0xa8)
[   13.245233] Exception stack(0xee8bdf78 to 0xee8bdfc0)
[   13.250265] df60:                                                       00000001 00000000
[   13.258422] df80: ee8bdfd0 c0114060 c0b05444 c0b053e4 c0a66cc8 c0b0544c c0b108a2 00000000
[   13.266567] dfa0: 00000000 00000000 00000001 ee8bdfc8 c01083c0 c01083c4 60000013 ffffffff
[   13.274720] [<c010b00c>] (__irq_svc) from [<c01083c4>] (arch_cpu_idle+0x30/0x3c)
[   13.282096] [<c01083c4>] (arch_cpu_idle) from [<c0152f34>] (do_idle+0x13c/0x200)
[   13.289459] [<c0152f34>] (do_idle) from [<c015328c>] (cpu_startup_entry+0x18/0x1c)
[   13.296989] [<c015328c>] (cpu_startup_entry) from [<4010154c>] (0x4010154c)
[   13.303908] CPU7: stopping
[   13.306603] CPU: 7 PID: 0 Comm: swapper/7 Tainted: G      D W       4.10.0-rc3-00113-g5750765 #2739
[   13.315594] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[   13.321711] [<c010d830>] (unwind_backtrace) from [<c010a544>] (show_stack+0x10/0x14)
[   13.329407] [<c010a544>] (show_stack) from [<c032956c>] (dump_stack+0x74/0x94)
[   13.336587] [<c032956c>] (dump_stack) from [<c010caac>] (handle_IPI+0x170/0x1a8)
[   13.343950] [<c010caac>] (handle_IPI) from [<c01014b0>] (gic_handle_irq+0x90/0x9c)
[   13.351484] [<c01014b0>] (gic_handle_irq) from [<c010b00c>] (__irq_svc+0x6c/0xa8)
[   13.358923] Exception stack(0xee8bff78 to 0xee8bffc0)
[   13.363955] ff60:                                                       00000001 00000000
[   13.372113] ff80: ee8bffd0 c0114060 c0b05444 c0b053e4 c0a66cc8 c0b0544c c0b108a2 00000000
[   13.380256] ffa0: 00000000 00000000 00000001 ee8bffc8 c01083c0 c01083c4 60000013 ffffffff
[   13.388410] [<c010b00c>] (__irq_svc) from [<c01083c4>] (arch_cpu_idle+0x30/0x3c)
[   13.395786] [<c01083c4>] (arch_cpu_idle) from [<c0152f34>] (do_idle+0x13c/0x200)
[   13.403148] [<c0152f34>] (do_idle) from [<c015328c>] (cpu_startup_entry+0x18/0x1c)
[   13.410678] [<c015328c>] (cpu_startup_entry) from [<4010154c>] (0x4010154c)
[   13.417621] ---[ end Kernel panic - not syncing: Fatal exception in interrupt
[   13.424840] ------------[ cut here ]------------
[   13.429318] WARNING: CPU: 0 PID: 126 at kernel/workqueue.c:857 wq_worker_waking_up+0x70/0x80
[   13.437681] Modules linked in:
[   13.440727] CPU: 0 PID: 126 Comm: mmcqd/0 Tainted: G      D W       4.10.0-rc3-00113-g5750765 #2739
[   13.449728] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[   13.455823] [<c010d830>] (unwind_backtrace) from [<c010a544>] (show_stack+0x10/0x14)
[   13.463530] [<c010a544>] (show_stack) from [<c032956c>] (dump_stack+0x74/0x94)
[   13.470717] [<c032956c>] (dump_stack) from [<c011ad10>] (__warn+0xd4/0x100)
[   13.477650] [<c011ad10>] (__warn) from [<c011ad5c>] (warn_slowpath_null+0x20/0x28)
[   13.485194] [<c011ad5c>] (warn_slowpath_null) from [<c0130e70>] (wq_worker_waking_up+0x70/0x80)
[   13.493873] [<c0130e70>] (wq_worker_waking_up) from [<c013ba30>] (ttwu_do_activate+0x58/0x6c)
[   13.502355] [<c013ba30>] (ttwu_do_activate) from [<c013c4ec>] (try_to_wake_up+0x190/0x290)
[   13.510586] [<c013c4ec>] (try_to_wake_up) from [<c01521dc>] (__wake_up_common+0x4c/0x80)
[   13.518645] [<c01521dc>] (__wake_up_common) from [<c0152224>] (__wake_up_locked+0x14/0x1c)
[   13.526876] [<c0152224>] (__wake_up_locked) from [<c0152c24>] (complete+0x34/0x44)
[   13.534433] [<c0152c24>] (complete) from [<c04fcd34>] (exynos5_i2c_irq+0x220/0x26c)
[   13.542042] [<c04fcd34>] (exynos5_i2c_irq) from [<c0160dac>] (__handle_irq_event_percpu+0x58/0x140)
[   13.551048] [<c0160dac>] (__handle_irq_event_percpu) from [<c0160eb0>] (handle_irq_event_percpu+0x1c/0x58)
[   13.560664] [<c0160eb0>] (handle_irq_event_percpu) from [<c0160f24>] (handle_irq_event+0x38/0x5c)
[   13.569511] [<c0160f24>] (handle_irq_event) from [<c016422c>] (handle_fasteoi_irq+0xc4/0x19c)
[   13.578016] [<c016422c>] (handle_fasteoi_irq) from [<c0160574>] (generic_handle_irq+0x18/0x28)
[   13.586579] [<c0160574>] (generic_handle_irq) from [<c0160688>] (__handle_domain_irq+0x6c/0xe4)
[   13.595239] [<c0160688>] (__handle_domain_irq) from [<c0101470>] (gic_handle_irq+0x50/0x9c)
[   13.603556] [<c0101470>] (gic_handle_irq) from [<c010b00c>] (__irq_svc+0x6c/0xa8)
[   13.610994] Exception stack(0xedd67b30 to 0xedd67b78)
[   13.616028] 7b20:                                     00000041 edd19900 00000102 edd66000
[   13.624180] 7b40: c0b49ae8 00000000 c0881434 00000000 00000000 edd19900 60000193 edcc9b04
[   13.632321] 7b60: 00000001 edd67b80 c0196974 c0196978 20000113 ffffffff
[   13.638933] [<c010b00c>] (__irq_svc) from [<c0196978>] (panic+0x1e8/0x26c)
[   13.645769] [<c0196978>] (panic) from [<c010a7f8>] (die+0x2b0/0x2e0)
[   13.652099] [<c010a7f8>] (die) from [<c011514c>] (__do_kernel_fault.part.0+0x54/0x1e4)
[   13.659982] [<c011514c>] (__do_kernel_fault.part.0) from [<c0110bec>] (do_page_fault+0x26c/0x294)
[   13.668812] [<c0110bec>] (do_page_fault) from [<c0101308>] (do_DataAbort+0x34/0xb4)
[   13.676432] [<c0101308>] (do_DataAbort) from [<c010af78>] (__dabt_svc+0x58/0x80)
[   13.683783] Exception stack(0xedd67cc0 to 0xedd67d08)
[   13.688825] 7cc0: a0000113 0b750b74 00000000 edc97000 00000008 edc97324 edc97320 00000000
[   13.696970] 7ce0: edcc9b08 edd6808c 00000000 edcc9b04 00000000 edd67d10 c06e6138 c0135b24
[   13.705102] 7d00: 60000193 ffffffff
[   13.708586] [<c010af78>] (__dabt_svc) from [<c0135b24>] (kthread_queue_work+0x18/0x64)
[   13.716478] [<c0135b24>] (kthread_queue_work) from [<c0527ab8>] (mmc_request_done+0xd8/0x158)
[   13.724970] [<c0527ab8>] (mmc_request_done) from [<c0542834>] (dw_mci_request_end+0xa0/0xd8)
[   13.733373] [<c0542834>] (dw_mci_request_end) from [<c0542b5c>] (dw_mci_tasklet_func+0x2f0/0x394)
[   13.742211] [<c0542b5c>] (dw_mci_tasklet_func) from [<c011f6f0>] (tasklet_action+0x84/0x12c)
[   13.750614] [<c011f6f0>] (tasklet_action) from [<c011edac>] (__do_softirq+0xec/0x244)
[   13.758411] [<c011edac>] (__do_softirq) from [<c011f1a8>] (irq_exit+0xc0/0x104)
[   13.765689] [<c011f1a8>] (irq_exit) from [<c016068c>] (__handle_domain_irq+0x70/0xe4)
[   13.773486] [<c016068c>] (__handle_domain_irq) from [<c0101470>] (gic_handle_irq+0x50/0x9c)
[   13.781804] [<c0101470>] (gic_handle_irq) from [<c010b00c>] (__irq_svc+0x6c/0xa8)
[   13.789241] Exception stack(0xedd67e28 to 0xedd67e70)
[   13.794279] 7e20:                   c08a2154 c0890cdc edd67e78 edd66000 00000000 c011f068
[   13.802427] 7e40: c0890cdc c08a2154 00000000 edd68030 edd68004 00000000 00000001 edd67e78
[   13.810565] 7e60: c011f068 c03448b8 20000013 ffffffff
[   13.815603] [<c010b00c>] (__irq_svc) from [<c03448b8>] (check_preemption_disabled+0x30/0x128)
[   13.824098] [<c03448b8>] (check_preemption_disabled) from [<c011f068>] (__local_bh_enable_ip+0xc8/0xec)
[   13.833457] [<c011f068>] (__local_bh_enable_ip) from [<c0527b8c>] (__mmc_start_request+0x54/0xdc)
[   13.842297] [<c0527b8c>] (__mmc_start_request) from [<c0527d04>] (mmc_start_request+0xf0/0x11c)
[   13.850963] [<c0527d04>] (mmc_start_request) from [<c0528208>] (mmc_start_areq+0x148/0x1ac)
[   13.859278] [<c0528208>] (mmc_start_areq) from [<c05376c0>] (mmc_blk_issue_rw_rq+0x78/0x314)
[   13.867680] [<c05376c0>] (mmc_blk_issue_rw_rq) from [<c0538358>] (mmc_blk_issue_rq+0x9c/0x458)
[   13.876258] [<c0538358>] (mmc_blk_issue_rq) from [<c0538868>] (mmc_queue_thread+0x98/0x180)
[   13.884579] [<c0538868>] (mmc_queue_thread) from [<c0135604>] (kthread+0xfc/0x134)
[   13.892121] [<c0135604>] (kthread) from [<c0107978>] (ret_from_fork+0x14/0x3c)
[   13.899292] ---[ end trace 86f45842e4b0b194 ]---

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH 06/16] mmc: core: replace waitqueue with worker

[Linus Walleij]

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

Re: [PATCH 06/16] mmc: core: replace waitqueue with worker

[Adrian Hunter]

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?

Re: [PATCH 06/16] mmc: core: replace waitqueue with worker

[Jens Axboe]

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

Re: [PATCH 06/16] mmc: core: replace waitqueue with worker

[Adrian Hunter]

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?

Re: [PATCH 06/16] mmc: core: replace waitqueue with worker

[Jens Axboe]

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

Re: [PATCH 06/16] mmc: core: replace waitqueue with worker

[Adrian Hunter]

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.

Re: [PATCH 06/16] mmc: core: replace waitqueue with worker

[Jens Axboe]

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

Re: [PATCH 06/16] mmc: core: replace waitqueue with worker

[Christoph Hellwig]

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.

Re: [PATCH 06/16] mmc: core: replace waitqueue with worker

[Jens Axboe]

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

Re: [PATCH 06/16] mmc: core: replace waitqueue with worker

[Linus Walleij]

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

Re: [PATCH 06/16] mmc: core: replace waitqueue with worker

[Linus Walleij]

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