From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jaehoon Chung Subject: Re: [PATCH v2] mmc: fix async request mechanism for sequential read scenarios Date: Mon, 05 Nov 2012 16:15:14 +0900 Message-ID: <50976782.5080808@samsung.com> References: <1351780852-1293-1-git-send-email-kdorfman@codeaurora.org> <50975AA8.8030304@stericsson.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: Received: from mailout1.samsung.com ([203.254.224.24]:35394 "EHLO mailout1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751453Ab2KEHPY (ORCPT ); Mon, 5 Nov 2012 02:15:24 -0500 Received: from epcpsbgm2.samsung.com (epcpsbgm2 [203.254.230.27]) by mailout1.samsung.com (Oracle Communications Messaging Server 7u4-24.01(7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTP id <0MD000EC76TJ1JQ0@mailout1.samsung.com> for linux-mmc@vger.kernel.org; Mon, 05 Nov 2012 16:15:22 +0900 (KST) Received: from [10.90.51.55] by mmp2.samsung.com (Oracle Communications Messaging Server 7u4-24.01(7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTPA id <0MD00018P6TLUN40@mmp2.samsung.com> for linux-mmc@vger.kernel.org; Mon, 05 Nov 2012 16:15:22 +0900 (KST) In-reply-to: <50975AA8.8030304@stericsson.com> Sender: linux-mmc-owner@vger.kernel.org List-Id: linux-mmc@vger.kernel.org To: =?ISO-8859-1?Q?Per_F=F6rlin?= Cc: Konstantin Dorfman , "cjb@laptop.org" , "linux-mmc@vger.kernel.org" , "per.lkml@gmail.com" Hi Konstantin, On 11/05/2012 03:20 PM, Per F=F6rlin wrote: > Hi Konstantin, >=20 > On 11/01/2012 03:40 PM, Konstantin Dorfman wrote: >> When current request is running on the bus and if next request fetch= ed >> by mmcqd is NULL, mmc context (mmcqd thread) gets blocked until the >> current request completes. This means if new request comes in while >> the mmcqd thread is blocked, this new request can not be prepared in >> parallel to current ongoing request. This may result in latency to >> start new request. >> >> This change allows to wake up the MMC thread (which >> is waiting for the current running request to complete). Now once th= e >> MMC thread is woken up, new request can be fetched and prepared in >> parallel to current running request which means this new request can >> be started immediately after the current running request completes. >> >> With this change read throughput is improved by 16%. > What HW and what test cases have you been running? I also want to know which benchmark use? If you can share it, i will test with yours. Best Regards, Jaehoon Chung >=20 >> >> Signed-off-by: Konstantin Dorfman >> --- > Please add a change log here with a brief description of the changes = since last version >=20 >> drivers/mmc/card/block.c | 26 +++++------- >> drivers/mmc/card/queue.c | 26 ++++++++++- >> drivers/mmc/card/queue.h | 3 + >> drivers/mmc/core/core.c | 102 +++++++++++++++++++++++++++++++++++= +++++++++- >> include/linux/mmc/card.h | 12 +++++ >> include/linux/mmc/core.h | 15 +++++++ >> 6 files changed, 163 insertions(+), 21 deletions(-) >> >> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c >> index 172a768..0e9bedb 100644 >> --- a/drivers/mmc/card/block.c >> +++ b/drivers/mmc/card/block.c >> @@ -112,17 +112,6 @@ struct mmc_blk_data { >> >> static DEFINE_MUTEX(open_lock); >> >> -enum mmc_blk_status { >> - MMC_BLK_SUCCESS =3D 0, >> - MMC_BLK_PARTIAL, >> - MMC_BLK_CMD_ERR, >> - MMC_BLK_RETRY, >> - MMC_BLK_ABORT, >> - MMC_BLK_DATA_ERR, >> - MMC_BLK_ECC_ERR, >> - MMC_BLK_NOMEDIUM, >> -}; >> - >> module_param(perdev_minors, int, 0444); >> MODULE_PARM_DESC(perdev_minors, "Minors numbers to allocate per dev= ice"); >> >> @@ -1225,6 +1214,7 @@ static void mmc_blk_rw_rq_prep(struct mmc_queu= e_req *mqrq, >> >> mqrq->mmc_active.mrq =3D &brq->mrq; >> mqrq->mmc_active.err_check =3D mmc_blk_err_check; >> + mqrq->mmc_active.mrq->context_info =3D &mq->context_info; >> >> mmc_queue_bounce_pre(mqrq); >> } >> @@ -1284,9 +1274,12 @@ static int mmc_blk_issue_rw_rq(struct mmc_que= ue *mq, struct request *rqc) >> areq =3D &mq->mqrq_cur->mmc_active; >> } else >> areq =3D NULL; >> - areq =3D mmc_start_req(card->host, areq, (int *) &st= atus); >> - if (!areq) >> + areq =3D mmc_start_req(card->host, areq, (int *)&sta= tus); >> + if (!areq) { >> + if (status =3D=3D MMC_BLK_NEW_REQUEST) >> + mq->flags |=3D MMC_QUEUE_NEW_REQUEST= ; >> return 0; >> + } >> >> mq_rq =3D container_of(areq, struct mmc_queue_req, m= mc_active); >> brq =3D &mq_rq->brq; >> @@ -1295,6 +1288,8 @@ static int mmc_blk_issue_rw_rq(struct mmc_queu= e *mq, struct request *rqc) >> mmc_queue_bounce_post(mq_rq); >> >> switch (status) { >> + case MMC_BLK_NEW_REQUEST: >> + BUG_ON(1); /* should never get here */ >> case MMC_BLK_SUCCESS: >> case MMC_BLK_PARTIAL: >> /* >> @@ -1367,7 +1362,7 @@ static int mmc_blk_issue_rw_rq(struct mmc_queu= e *mq, struct request *rqc) >> * prepare it again and resend. >> */ >> mmc_blk_rw_rq_prep(mq_rq, card, disable_mult= i, mq); >> - mmc_start_req(card->host, &mq_rq->mmc_active= , NULL); >> + mmc_start_req(card->host, &mq_rq->mmc_active= , (int *)&status); >> } >> } while (ret); >> >> @@ -1406,6 +1401,7 @@ static int mmc_blk_issue_rq(struct mmc_queue *= mq, struct request *req) >> ret =3D 0; >> goto out; >> } >> + mq->flags &=3D ~MMC_QUEUE_NEW_REQUEST; >> >> if (req && req->cmd_flags & REQ_DISCARD) { >> /* complete ongoing async transfer before issuing di= scard */ >> @@ -1426,7 +1422,7 @@ static int mmc_blk_issue_rq(struct mmc_queue *= mq, struct request *req) >> } >> >> out: >> - if (!req) >> + if (!req && !(mq->flags & MMC_QUEUE_NEW_REQUEST)) >> /* release host only when there are no more requests= */ >> mmc_release_host(card->host); >> return ret; >> diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c >> index fadf52e..7375476 100644 >> --- a/drivers/mmc/card/queue.c >> +++ b/drivers/mmc/card/queue.c >> @@ -22,7 +22,6 @@ >> >> #define MMC_QUEUE_BOUNCESZ 65536 >> >> -#define MMC_QUEUE_SUSPENDED (1 << 0) >> >> /* >> * Prepare a MMC request. This just filters out odd stuff. >> @@ -63,11 +62,17 @@ static int mmc_queue_thread(void *d) >> set_current_state(TASK_INTERRUPTIBLE); >> req =3D blk_fetch_request(q); >> mq->mqrq_cur->req =3D req; >> + if (!req && mq->mqrq_prev->req) >> + mq->context_info.is_waiting_last_req =3D tru= e; >> spin_unlock_irq(q->queue_lock); >> >> if (req || mq->mqrq_prev->req) { >> set_current_state(TASK_RUNNING); >> mq->issue_fn(mq, req); >> + if (mq->flags & MMC_QUEUE_NEW_REQUEST) { >> + mq->flags &=3D ~MMC_QUEUE_NEW_REQUES= T; >> + continue; /* fetch again */ >> + } >> >> /* >> * Current request becomes previous request >> @@ -111,8 +116,19 @@ static void mmc_request_fn(struct request_queue= *q) >> } >> return; >> } >> - >> - if (!mq->mqrq_cur->req && !mq->mqrq_prev->req) >> + if (!mq->mqrq_cur->req && mq->mqrq_prev->req && >> + !(mq->mqrq_prev->req->cmd_flags & REQ_FLUSH) && >> + !(mq->mqrq_prev->req->cmd_flags & REQ_DISCARD)) { >> + /* >> + * New MMC request arrived when MMC thread may be >> + * blocked on the previous request to be complete >> + * with no current request fetched >> + */ >> + if (mq->context_info.is_waiting_last_req) { >> + mq->context_info.is_new_req =3D true; >> + wake_up_interruptible(&mq->context_info.wait= ); >> + } >> + } else if (!mq->mqrq_cur->req && !mq->mqrq_prev->req) >> wake_up_process(mq->thread); >> } >> >> @@ -262,6 +278,10 @@ int mmc_init_queue(struct mmc_queue *mq, struct= mmc_card *card, >> } >> >> sema_init(&mq->thread_sem, 1); >> + mq->context_info.is_new_req =3D false; >> + mq->context_info.is_done_rcv =3D false; >> + mq->context_info.is_waiting_last_req =3D false; >> + init_waitqueue_head(&mq->context_info.wait); >> >> mq->thread =3D kthread_run(mmc_queue_thread, mq, "mmcqd/%d%s= ", >> host->index, subname ? subname : ""); >> diff --git a/drivers/mmc/card/queue.h b/drivers/mmc/card/queue.h >> index d2a1eb4..f5885e9 100644 >> --- a/drivers/mmc/card/queue.h >> +++ b/drivers/mmc/card/queue.h >> @@ -26,6 +26,8 @@ struct mmc_queue { >> struct mmc_card *card; >> struct task_struct *thread; >> struct semaphore thread_sem; >> +#define MMC_QUEUE_SUSPENDED (1 << 0) >> +#define MMC_QUEUE_NEW_REQUEST (1 << 1) >> unsigned int flags; >> int (*issue_fn)(struct mmc_queue *, stru= ct request *); >> void *data; >> @@ -33,6 +35,7 @@ struct mmc_queue { >> struct mmc_queue_req mqrq[2]; >> struct mmc_queue_req *mqrq_cur; >> struct mmc_queue_req *mqrq_prev; >> + struct mmc_context_info context_info; >> }; >> >> extern int mmc_init_queue(struct mmc_queue *, struct mmc_card *, sp= inlock_t *, >> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c >> index 06c42cf..a24d298 100644 >> --- a/drivers/mmc/core/core.c >> +++ b/drivers/mmc/core/core.c >> @@ -316,11 +316,43 @@ out: >> } >> EXPORT_SYMBOL(mmc_start_bkops); >> >> +/* >> + * mmc_wait_data_done() - done callback for data request >> + * @mrq: done data request >> + * >> + * Wakes up mmc context, passed as callback to host controller driv= er >> + */ >> +static void mmc_wait_data_done(struct mmc_request *mrq) >> +{ >> + mrq->context_info->is_done_rcv =3D true; >> + wake_up_interruptible(&mrq->context_info->wait); >> +} >> + >> static void mmc_wait_done(struct mmc_request *mrq) >> { >> complete(&mrq->completion); >> } >> >> +/* >> + *__mmc_start_data_req() - starts data request >> + * @host: MMC host to start the request >> + * @mrq: data request to start >> + * >> + * Fills done callback that will be used when request are done by c= ard. >> + * Starts data mmc request execution >> + */ >> +static int __mmc_start_data_req(struct mmc_host *host, struct mmc_r= equest *mrq) >> +{ >> + mrq->done =3D mmc_wait_data_done; >> + if (mmc_card_removed(host->card)) { >> + mrq->cmd->error =3D -ENOMEDIUM; >> + return -ENOMEDIUM; >> + } >> + mmc_start_request(host, mrq); >> + >> + return 0; >> +} >> + >> static int __mmc_start_req(struct mmc_host *host, struct mmc_reques= t *mrq) >> { >> init_completion(&mrq->completion); >> @@ -334,6 +366,57 @@ static int __mmc_start_req(struct mmc_host *hos= t, struct mmc_request *mrq) >> return 0; >> } >> >> +/* >> + * mmc_wait_for_data_req_done() - wait for request completed or new >> + * request notification arrives >> + * @host: MMC host to prepare the command. >> + * @mrq: MMC request to wait for >> + * >> + * Blocks MMC context till host controller will ack end of data req= uest >> + * execution or new request arrives from block layer. Handles >> + * command retries. >> + * >> + * Returns enum mmc_blk_status after checking errors. >> + */ >> +static int mmc_wait_for_data_req_done(struct mmc_host *host, >> + struct mmc_request *mrq) >> +{ >> + struct mmc_command *cmd; >> + struct mmc_context_info *context_info =3D mrq->context_info; >> + int err; >> + >> + while (1) { >> + wait_event_interruptible(context_info->wait, >> + (context_info->is_done_rcv || >> + context_info->is_new_req)); >> + context_info->is_waiting_last_req =3D false; >> + if (context_info->is_done_rcv) { >> + context_info->is_done_rcv =3D false; >> + context_info->is_new_req =3D false; >> + cmd =3D mrq->cmd; >> + if (!cmd->error || !cmd->retries || >> + mmc_card_removed(host->card)= ) { >> + err =3D host->areq->err_check(host->= card, >> + host->areq); >> + break; /* return err */ >> + } else { >> + pr_info("%s: req failed (CMD%u): %d,= retrying...\n", >> + mmc_hostname(host), >> + cmd->opcode, cmd->er= ror); >> + cmd->retries--; >> + cmd->error =3D 0; >> + host->ops->request(host, mrq); >> + continue; /* wait for done/new event= again */ >> + } >> + } else if (context_info->is_new_req) { >> + context_info->is_new_req =3D false; >> + err =3D MMC_BLK_NEW_REQUEST; >> + break; /* return err */ >> + } >> + } /* while */ >> + return err; >> +} >> + >> static void mmc_wait_for_req_done(struct mmc_host *host, >> struct mmc_request *mrq) >> { >> @@ -423,8 +506,20 @@ struct mmc_async_req *mmc_start_req(struct mmc_= host *host, >> mmc_pre_req(host, areq->mrq, !host->areq); >> >> if (host->areq) { >> - mmc_wait_for_req_done(host, host->areq->mrq); >> - err =3D host->areq->err_check(host->card, host->areq= ); >> + err =3D mmc_wait_for_data_req_done(host, host->areq-= >mrq); >> + if (err =3D=3D MMC_BLK_NEW_REQUEST) { >> + if (areq) { >> + pr_err("%s: new request while areq =3D= %p", >> + mmc_hostname(host), = areq); >> + BUG_ON(1); >> + } >> + *error =3D err; >> + /* >> + * The previous request was not completed, >> + * nothing to return >> + */ >> + return NULL; >> + } >> /* >> * Check BKOPS urgency for each R1 response >> */ >> @@ -436,7 +531,7 @@ struct mmc_async_req *mmc_start_req(struct mmc_h= ost *host, >> } >> >> if (!err && areq) >> - start_err =3D __mmc_start_req(host, areq->mrq); >> + start_err =3D __mmc_start_data_req(host, areq->mrq); >> >> if (host->areq) >> mmc_post_req(host, host->areq->mrq, 0); >> @@ -452,6 +547,7 @@ struct mmc_async_req *mmc_start_req(struct mmc_h= ost *host, >> >> if (error) >> *error =3D err; >> + >> return data; >> } >> EXPORT_SYMBOL(mmc_start_req); >> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h >> index 943550d..9681d8f 100644 >> --- a/include/linux/mmc/card.h >> +++ b/include/linux/mmc/card.h >> @@ -186,6 +186,18 @@ struct sdio_func_tuple; >> >> #define SDIO_MAX_FUNCS 7 >> >> +enum mmc_blk_status { >> + MMC_BLK_SUCCESS =3D 0, >> + MMC_BLK_PARTIAL, >> + MMC_BLK_CMD_ERR, >> + MMC_BLK_RETRY, >> + MMC_BLK_ABORT, >> + MMC_BLK_DATA_ERR, >> + MMC_BLK_ECC_ERR, >> + MMC_BLK_NOMEDIUM, >> + MMC_BLK_NEW_REQUEST, >> +}; >> + >> /* The number of MMC physical partitions. These consist of: >> * boot partitions (2), general purpose partitions (4) in MMC v4.4. >> */ >> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h >> index 9b9cdaf..fc2d095 100644 >> --- a/include/linux/mmc/core.h >> +++ b/include/linux/mmc/core.h >> @@ -120,6 +120,20 @@ struct mmc_data { >> s32 host_cookie; /* host private data= */ >> }; >> >> +/** >> + * 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 runni= ng 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 mmc_request { >> struct mmc_command *sbc; /* SET_BLOCK_COUNT f= or multiblock */ >> struct mmc_command *cmd; >> @@ -128,6 +142,7 @@ struct mmc_request { >> >> struct completion completion; >> void (*done)(struct mmc_request *);/* com= pletion function */ >> + struct mmc_context_info *context_info; >> }; >> >> struct mmc_host; >> -- >> 1.7.6 >> >=20 > -- > To unsubscribe from this list: send the line "unsubscribe linux-mmc" = in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >=20