Re: [PATCH V14 14/24] mmc: block: Add CQE support

[Ulf Hansson]

On 21 November 2017 at 14:42, Adrian Hunter  wrote:
> Add CQE support to the block driver, including:
> - optionally using DCMD for flush requests
> - "manually" issuing discard requests
> - issuing read / write requests to the CQE
> - supporting block-layer timeouts
> - handling recovery
> - supporting re-tuning
>
> CQE offers 25% - 50% better random multi-threaded I/O.  There is a slight
> (e.g. 2%) drop in sequential read speed but no observable change to sequential
> write.
>
> CQE automatically sends the commands to complete requests.  However it only
> supports reads / writes and so-called "direct commands" (DCMD).  Furthermore
> DCMD is limited to one command at a time, but discards require 3 commands.
> That makes issuing discards through CQE very awkward, but some CQE's don't
> support DCMD anyway.  So for discards, the existing non-CQE approach is
> taken, where the mmc core code issues the 3 commands one at a time i.e.
> mmc_erase(). Where DCMD is used, is for issuing flushes.
>
> Signed-off-by: Adrian Hunter 

This looks good to me!

I only have one, very minor comment.

[...]

> @@ -370,10 +514,14 @@ static int mmc_mq_init_queue(struct mmc_queue *mq, int 
> q_depth,
>  static int mmc_mq_init(struct mmc_queue *mq, struct mmc_card *card,
>  spinlock_t *lock)
>  {
> +   struct mmc_host *host = card->host;
> int q_depth;
> int ret;
>
> -   q_depth = MMC_QUEUE_DEPTH;
> +   if (mq->use_cqe)
> +   q_depth = min_t(int, card->ext_csd.cmdq_depth, 
> host->cqe_qdepth);

To make it clear why this is needed, could you please add some comment
in the code?

As I was trying to point out in the other reply about queue depth, for
patch 13, this is weird to me.
This may mean that we end up using queue_depth being less than
MMC_QUEUE_DEPTH (64) for the CQE case. While in fact, in the CQE case
the HW actually supports a bigger queue depth, comparing when not
using CQE.

Anyway, it seems like that will have to be a separate topic to discuss
with the blkmq experts.

> +   else
> +   q_depth = MMC_QUEUE_DEPTH;
>

[...]

Kind regards
Uffe

[PATCH V14 14/24] mmc: block: Add CQE support

[Adrian Hunter]

Add CQE support to the block driver, including:
- optionally using DCMD for flush requests
- "manually" issuing discard requests
- issuing read / write requests to the CQE
- supporting block-layer timeouts
- handling recovery
- supporting re-tuning

CQE offers 25% - 50% better random multi-threaded I/O.  There is a slight
(e.g. 2%) drop in sequential read speed but no observable change to sequential
write.

CQE automatically sends the commands to complete requests.  However it only
supports reads / writes and so-called "direct commands" (DCMD).  Furthermore
DCMD is limited to one command at a time, but discards require 3 commands.
That makes issuing discards through CQE very awkward, but some CQE's don't
support DCMD anyway.  So for discards, the existing non-CQE approach is
taken, where the mmc core code issues the 3 commands one at a time i.e.
mmc_erase(). Where DCMD is used, is for issuing flushes.

Signed-off-by: Adrian Hunter 
---
 drivers/mmc/core/block.c | 150 +++-
 drivers/mmc/core/block.h |   2 +
 drivers/mmc/core/queue.c | 158 +--
 drivers/mmc/core/queue.h |  18 ++
 4 files changed, 322 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index a08d727d100b..2aacd3fa0d1a 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -111,6 +111,7 @@ struct mmc_blk_data {
 #define MMC_BLK_WRITE  BIT(1)
 #define MMC_BLK_DISCARDBIT(2)
 #define MMC_BLK_SECDISCARD BIT(3)
+#define MMC_BLK_CQE_RECOVERY   BIT(4)
 
/*
 * Only set in main mmc_blk_data associated
@@ -1785,6 +1786,138 @@ static void mmc_blk_data_prep(struct mmc_queue *mq, 
struct mmc_queue_req *mqrq,
*do_data_tag_p = do_data_tag;
 }
 
+#define MMC_CQE_RETRIES 2
+
+static void mmc_blk_cqe_complete_rq(struct mmc_queue *mq, struct request *req)
+{
+   struct mmc_queue_req *mqrq = req_to_mmc_queue_req(req);
+   struct mmc_request *mrq = >brq.mrq;
+   struct request_queue *q = req->q;
+   struct mmc_host *host = mq->card->host;
+   unsigned long flags;
+   bool put_card;
+   int err;
+
+   mmc_cqe_post_req(host, mrq);
+
+   if (mrq->cmd && mrq->cmd->error)
+   err = mrq->cmd->error;
+   else if (mrq->data && mrq->data->error)
+   err = mrq->data->error;
+   else
+   err = 0;
+
+   if (err) {
+   if (mqrq->retries++ < MMC_CQE_RETRIES)
+   blk_mq_requeue_request(req, true);
+   else
+   blk_mq_end_request(req, BLK_STS_IOERR);
+   } else if (mrq->data) {
+   if (blk_update_request(req, BLK_STS_OK, 
mrq->data->bytes_xfered))
+   blk_mq_requeue_request(req, true);
+   else
+   __blk_mq_end_request(req, BLK_STS_OK);
+   } else {
+   blk_mq_end_request(req, BLK_STS_OK);
+   }
+
+   spin_lock_irqsave(q->queue_lock, flags);
+
+   mq->in_flight[mmc_issue_type(mq, req)] -= 1;
+
+   put_card = (mmc_tot_in_flight(mq) == 0);
+
+   mmc_cqe_check_busy(mq);
+
+   spin_unlock_irqrestore(q->queue_lock, flags);
+
+   if (!mq->cqe_busy)
+   blk_mq_run_hw_queues(q, true);
+
+   if (put_card)
+   mmc_put_card(mq->card, >ctx);
+}
+
+void mmc_blk_cqe_recovery(struct mmc_queue *mq)
+{
+   struct mmc_card *card = mq->card;
+   struct mmc_host *host = card->host;
+   int err;
+
+   pr_debug("%s: CQE recovery start\n", mmc_hostname(host));
+
+   err = mmc_cqe_recovery(host);
+   if (err)
+   mmc_blk_reset(mq->blkdata, host, MMC_BLK_CQE_RECOVERY);
+   else
+   mmc_blk_reset_success(mq->blkdata, MMC_BLK_CQE_RECOVERY);
+
+   pr_debug("%s: CQE recovery done\n", mmc_hostname(host));
+}
+
+static void mmc_blk_cqe_req_done(struct mmc_request *mrq)
+{
+   struct mmc_queue_req *mqrq = container_of(mrq, struct mmc_queue_req,
+ brq.mrq);
+   struct request *req = mmc_queue_req_to_req(mqrq);
+   struct request_queue *q = req->q;
+   struct mmc_queue *mq = q->queuedata;
+
+   /*
+* Block layer timeouts race with completions which means the normal
+* completion path cannot be used during recovery.
+*/
+   if (mq->in_recovery)
+   mmc_blk_cqe_complete_rq(mq, req);
+   else
+   blk_mq_complete_request(req);
+}
+
+static int mmc_blk_cqe_start_req(struct mmc_host *host, struct mmc_request 
*mrq)
+{
+   mrq->done   = mmc_blk_cqe_req_done;
+   mrq->recovery_notifier  = mmc_cqe_recovery_notifier;
+
+   return mmc_cqe_start_req(host, mrq);
+}
+
+static struct mmc_request *mmc_blk_cqe_prep_dcmd(struct mmc_queue_req *mqrq,
+