From patchwork Thu Oct 26 12:57:54 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Linus Walleij X-Patchwork-Id: 10028107 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 3F5166022E for ; Thu, 26 Oct 2017 12:59:02 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 2FE9328DCD for ; Thu, 26 Oct 2017 12:59:02 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 2486728DF7; Thu, 26 Oct 2017 12:59:02 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.5 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 08CC128DCD for ; Thu, 26 Oct 2017 12:59:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932338AbdJZM7A (ORCPT ); Thu, 26 Oct 2017 08:59:00 -0400 Received: from mail-lf0-f68.google.com ([209.85.215.68]:55870 "EHLO mail-lf0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932344AbdJZM66 (ORCPT ); Thu, 26 Oct 2017 08:58:58 -0400 Received: by mail-lf0-f68.google.com with SMTP id p184so3616061lfe.12 for ; Thu, 26 Oct 2017 05:58:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=fUKcryUX/11Qz8brlBoUPHXhgqGtlgZ5RF/MwAgECsQ=; b=MYvYD5RoWsa43ppBBziz4ZsvsLVdgHGXK1a41z1f+gmAzedeKBBKZP6/Ob0effnaHS preQYgagq7OijYbmYZRmabUrkU48K6yRbydAfBdWTccHoQjYd+FwTvz/QpMT5sApjBh9 VpZVm3xWKalh4g5oR4RK0Ez5qTtxXWvnWEYG8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=fUKcryUX/11Qz8brlBoUPHXhgqGtlgZ5RF/MwAgECsQ=; b=F+yZLdqL2fWHElwuCq5g9DYCZ8wU3BOic4MFb6XotjHVsZpiZTrQ+Z36AOLLC4B6uI H2FLnivV0AebqgRD8tx6u2brU+A+toR4pJzD7e75KVzQE+Hp2rLlAWZ3Nq6LWJOBb+Nz 7LxdVzgwReS0LEL7baXtSIJOCNdNo9GGBlA6dbsKg9+Qt07AcH9m93UdwTvn6xLUhqaE TW8gcFjukwUdC/VInD3gdrD+J3VchB175BG4jPf1P4Jfb7y4dPSJONFJQ2ul6ehWc5ub Dsxav43Xy7gINSp8L7oK1F9gr6jOjke6Ff974q6/wLRhbKT/SBMGNo1bbGcQQ3x7MeUD oSVg== X-Gm-Message-State: AMCzsaU9n7a77msHg26P8MIk31ReYhkySOl8sUuPFEAeNFnUDrBTJbOn 87LVUb6DvVnJMffiQXwhJSRa/A== X-Google-Smtp-Source: ABhQp+RjL3XXcRbLfLF6zkM3333DAd90AqIbhi63FJOPeNW9SymVlwQRfOT/TBkVWwQIciexm1aU6w== X-Received: by 10.46.64.141 with SMTP id r13mr9399294lje.112.1509022737192; Thu, 26 Oct 2017 05:58:57 -0700 (PDT) Received: from genomnajs.ideon.se ([85.235.10.227]) by smtp.gmail.com with ESMTPSA id 34sm1165600lfr.25.2017.10.26.05.58.55 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 26 Oct 2017 05:58:56 -0700 (PDT) From: Linus Walleij To: linux-mmc@vger.kernel.org, Ulf Hansson Cc: linux-block@vger.kernel.org, Jens Axboe , Christoph Hellwig , Arnd Bergmann , Bartlomiej Zolnierkiewicz , Paolo Valente , Avri Altman , Adrian Hunter , Linus Walleij Subject: [PATCH 09/12 v4] mmc: queue: stop flushing the pipeline with NULL Date: Thu, 26 Oct 2017 14:57:54 +0200 Message-Id: <20171026125757.10200-10-linus.walleij@linaro.org> X-Mailer: git-send-email 2.13.6 In-Reply-To: <20171026125757.10200-1-linus.walleij@linaro.org> References: <20171026125757.10200-1-linus.walleij@linaro.org> Sender: linux-block-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-block@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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. Add a function in the core to wait for an asynchronous request to complete. Update the tests to use the new function prototypes. This kills off some FIXME's such as gettin rid of the mq->qcnt queue depth variable that was introduced a while back. It is a vital step toward multiqueue enablement that we stop pulling NULL off the end of the request queue to flush the asynchronous issueing mechanism. Signed-off-by: Linus Walleij --- drivers/mmc/core/block.c | 168 ++++++++++++++++---------------------------- drivers/mmc/core/core.c | 50 +++++++------ drivers/mmc/core/core.h | 6 +- drivers/mmc/core/mmc_test.c | 31 ++------ drivers/mmc/core/queue.c | 11 ++- drivers/mmc/core/queue.h | 7 -- 6 files changed, 106 insertions(+), 167 deletions(-) diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index c1178fa83f75..ab01cab4a026 100644 --- a/drivers/mmc/core/block.c +++ b/drivers/mmc/core/block.c @@ -1805,7 +1805,6 @@ static void mmc_blk_rw_cmd_abort(struct mmc_queue *mq, struct mmc_card *card, if (mmc_card_removed(card)) req->rq_flags |= RQF_QUIET; while (blk_end_request(req, BLK_STS_IOERR, blk_rq_cur_bytes(req))); - mq->qcnt--; } /** @@ -1873,13 +1872,10 @@ static void mmc_blk_rw_done(struct mmc_async_req *areq, enum mmc_blk_status stat if (mmc_blk_reset(md, card->host, type)) { if (req_pending) mmc_blk_rw_cmd_abort(mq, card, old_req, mq_rq); - else - mq->qcnt--; mmc_blk_rw_try_restart(mq, mq_rq); return; } if (!req_pending) { - mq->qcnt--; mmc_blk_rw_try_restart(mq, mq_rq); return; } @@ -1923,7 +1919,6 @@ static void mmc_blk_rw_done(struct mmc_async_req *areq, enum mmc_blk_status stat req_pending = blk_end_request(old_req, BLK_STS_IOERR, brq->data.blksz); if (!req_pending) { - mq->qcnt--; mmc_blk_rw_try_restart(mq, mq_rq); return; } @@ -1947,26 +1942,16 @@ static void mmc_blk_rw_done(struct mmc_async_req *areq, enum mmc_blk_status stat */ mmc_blk_rw_rq_prep(mq_rq, card, disable_multi, mq); - mmc_start_areq(card->host, areq, NULL); + mmc_start_areq(card->host, areq); mq_rq->brq.retune_retry_done = retune_retry_done; - } else { - /* Else, this request is done */ - mq->qcnt--; } } 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->qcnt++; - - if (!mq->qcnt) - return; + struct mmc_queue_req *mqrq_cur = req_to_mmc_queue_req(new_req); + struct mmc_async_req *areq = &mqrq_cur->areq; /* * If the card was removed, just cancel everything and return. @@ -1974,44 +1959,25 @@ static void mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *new_req) if (mmc_card_removed(card)) { new_req->rq_flags |= RQF_QUIET; blk_end_request_all(new_req, BLK_STS_IOERR); - mq->qcnt--; /* FIXME: just set to 0? */ return; } - if (new_req) { - struct mmc_queue_req *mqrq_cur = req_to_mmc_queue_req(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(mq, card, new_req, mqrq_cur); - return; - } - - mmc_blk_rw_rq_prep(mqrq_cur, card, 0, mq); - new_areq = &mqrq_cur->areq; - new_areq->report_done_status = mmc_blk_rw_done; - } 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 discard the old_areq, it will be - * post-processed when done, in mmc_blk_rw_done(). We clean - * this up in later patches. + * 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(mq, card, new_req, mqrq_cur); + return; + } + + mmc_blk_rw_rq_prep(mqrq_cur, card, 0, mq); + areq->report_done_status = mmc_blk_rw_done; + mmc_start_areq(card->host, areq); } void mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req) @@ -2020,70 +1986,56 @@ void mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req) struct mmc_blk_data *md = mq->blkdata; struct mmc_card *card = md->queue.card; - if (req && !mq->qcnt) - /* claim host only for the first request */ - mmc_get_card(card, NULL); + if (!req) { + pr_err("%s: tried to issue NULL request\n", __func__); + return; + } ret = mmc_blk_part_switch(card, md->part_type); if (ret) { - if (req) { - blk_end_request_all(req, BLK_STS_IOERR); - } - goto out; + blk_end_request_all(req, BLK_STS_IOERR); + return; } - if (req) { - switch (req_op(req)) { - case REQ_OP_DRV_IN: - case REQ_OP_DRV_OUT: - /* - * Complete ongoing async transfer before issuing - * ioctl()s - */ - if (mq->qcnt) - mmc_blk_issue_rw_rq(mq, NULL); - mmc_blk_issue_drv_op(mq, req); - break; - case REQ_OP_DISCARD: - /* - * Complete ongoing async transfer before issuing - * discard. - */ - if (mq->qcnt) - mmc_blk_issue_rw_rq(mq, NULL); - mmc_blk_issue_discard_rq(mq, req); - break; - case REQ_OP_SECURE_ERASE: - /* - * Complete ongoing async transfer before issuing - * secure erase. - */ - if (mq->qcnt) - mmc_blk_issue_rw_rq(mq, NULL); - mmc_blk_issue_secdiscard_rq(mq, req); - break; - case REQ_OP_FLUSH: - /* - * Complete ongoing async transfer before issuing - * flush. - */ - if (mq->qcnt) - mmc_blk_issue_rw_rq(mq, NULL); - mmc_blk_issue_flush(mq, req); - break; - default: - /* Normal request, just issue it */ - mmc_blk_issue_rw_rq(mq, req); - break; - } - } else { - /* No request, flushing the pipeline with NULL */ - mmc_blk_issue_rw_rq(mq, NULL); + switch (req_op(req)) { + case REQ_OP_DRV_IN: + case REQ_OP_DRV_OUT: + /* + * Complete ongoing async transfer before issuing + * ioctl()s + */ + mmc_wait_for_areq(card->host); + mmc_blk_issue_drv_op(mq, req); + break; + case REQ_OP_DISCARD: + /* + * Complete ongoing async transfer before issuing + * discard. + */ + mmc_wait_for_areq(card->host); + mmc_blk_issue_discard_rq(mq, req); + break; + case REQ_OP_SECURE_ERASE: + /* + * Complete ongoing async transfer before issuing + * secure erase. + */ + mmc_wait_for_areq(card->host); + mmc_blk_issue_secdiscard_rq(mq, req); + break; + case REQ_OP_FLUSH: + /* + * Complete ongoing async transfer before issuing + * flush. + */ + mmc_wait_for_areq(card->host); + mmc_blk_issue_flush(mq, req); + break; + default: + /* Normal request, just issue it */ + mmc_blk_issue_rw_rq(mq, req); + break; } - -out: - if (!mq->qcnt) - mmc_put_card(card, NULL); } 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 620dcbed15b7..209ebb8a7f3f 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -747,6 +747,15 @@ void mmc_finalize_areq(struct work_struct *work) } EXPORT_SYMBOL(mmc_finalize_areq); +void mmc_wait_for_areq(struct mmc_host *host) +{ + if (host->areq) { + wait_for_completion(&host->areq->complete); + host->areq = NULL; + } +} +EXPORT_SYMBOL(mmc_wait_for_areq); + /** * mmc_restart_areq() - restart an asynchronous request * @host: MMC host to restart the command on @@ -775,40 +784,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) { - int start_err = 0; struct mmc_async_req *previous = host->areq; + int ret; + + /* Delete this check when we trust the code */ + if (!areq) + pr_err("%s: NULL asynchronous request!\n", __func__); /* Prepare a new request */ - if (areq) - mmc_pre_req(host, areq->mrq); + mmc_pre_req(host, areq->mrq); /* Finalize previous request, if there is one */ if (previous) wait_for_completion(&previous->complete); - /* Just always succeed */ - if (ret_stat) - *ret_stat = MMC_BLK_SUCCESS; - /* Fine so far, start the new request! */ - if (areq) { - init_completion(&areq->complete); - areq->mrq->areq = areq; - start_err = __mmc_start_data_req(host, areq->mrq); - /* Cancel a prepared request if it was not started. */ - if (start_err) { - mmc_post_req(host, areq->mrq, -EINVAL); - host->areq = NULL; - } else { - host->areq = areq; - } + init_completion(&areq->complete); + areq->mrq->areq = areq; + ret = __mmc_start_data_req(host, areq->mrq); + /* Cancel a prepared request if it was not started. */ + if (ret) { + mmc_post_req(host, areq->mrq, -EINVAL); + host->areq = NULL; + pr_err("%s: failed to start request\n", __func__); + } else { + host->areq = areq; } - return previous; + return ret; } EXPORT_SYMBOL(mmc_start_areq); diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h index 1859804ecd80..5b8d0f1147ef 100644 --- a/drivers/mmc/core/core.h +++ b/drivers/mmc/core/core.h @@ -113,9 +113,9 @@ struct mmc_async_req; void mmc_finalize_areq(struct work_struct *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_areq(struct mmc_host *host); int mmc_erase(struct mmc_card *card, unsigned int from, unsigned int nr, unsigned int arg); diff --git a/drivers/mmc/core/mmc_test.c b/drivers/mmc/core/mmc_test.c index 478869805b96..256fdce38449 100644 --- a/drivers/mmc/core/mmc_test.c +++ b/drivers/mmc/core/mmc_test.c @@ -839,10 +839,8 @@ static int mmc_test_nonblock_transfer(struct mmc_test_card *test, { struct mmc_test_req *rq1, *rq2; struct mmc_test_async_req test_areq[2]; - struct mmc_async_req *done_areq; struct mmc_async_req *cur_areq = &test_areq[0].areq; struct mmc_async_req *other_areq = &test_areq[1].areq; - enum mmc_blk_status status; int i; int ret = RESULT_OK; @@ -864,25 +862,16 @@ static int mmc_test_nonblock_transfer(struct mmc_test_card *test, for (i = 0; i < count; i++) { mmc_test_prepare_mrq(test, cur_areq->mrq, sg, sg_len, dev_addr, blocks, blksz, write); - done_areq = mmc_start_areq(test->card->host, cur_areq, &status); + ret = mmc_start_areq(test->card->host, cur_areq); + mmc_wait_for_areq(test->card->host); - if (status != MMC_BLK_SUCCESS || (!done_areq && i > 0)) { - ret = RESULT_FAIL; - goto err; - } - - if (done_areq) - mmc_test_req_reset(container_of(done_areq->mrq, + mmc_test_req_reset(container_of(cur_areq->mrq, struct mmc_test_req, mrq)); swap(cur_areq, other_areq); dev_addr += blocks; } - done_areq = mmc_start_areq(test->card->host, NULL, &status); - if (status != MMC_BLK_SUCCESS) - ret = RESULT_FAIL; - err: kfree(rq1); kfree(rq2); @@ -2360,7 +2349,6 @@ static int mmc_test_ongoing_transfer(struct mmc_test_card *test, struct mmc_request *mrq; unsigned long timeout; bool expired = false; - enum mmc_blk_status blkstat = MMC_BLK_SUCCESS; int ret = 0, cmd_ret; u32 status = 0; int count = 0; @@ -2388,11 +2376,8 @@ static int mmc_test_ongoing_transfer(struct mmc_test_card *test, /* Start ongoing data request */ if (use_areq) { - mmc_start_areq(host, &test_areq.areq, &blkstat); - if (blkstat != MMC_BLK_SUCCESS) { - ret = RESULT_FAIL; - goto out_free; - } + mmc_start_areq(host, &test_areq.areq); + mmc_wait_for_areq(host); } else { mmc_wait_for_req(host, mrq); } @@ -2425,11 +2410,7 @@ static int mmc_test_ongoing_transfer(struct mmc_test_card *test, } while (repeat_cmd && R1_CURRENT_STATE(status) != R1_STATE_TRAN); /* Wait for data request to complete */ - if (use_areq) { - mmc_start_areq(host, NULL, &blkstat); - if (blkstat != MMC_BLK_SUCCESS) - ret = RESULT_FAIL; - } else { + if (!use_areq) { mmc_wait_for_req_done(test->card->host, mrq); } diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c index db1fa11d9870..cf43a2d5410d 100644 --- a/drivers/mmc/core/queue.c +++ b/drivers/mmc/core/queue.c @@ -42,6 +42,7 @@ static int mmc_queue_thread(void *d) { struct mmc_queue *mq = d; struct request_queue *q = mq->queue; + bool claimed_card = false; current->flags |= PF_MEMALLOC; @@ -55,7 +56,11 @@ static int mmc_queue_thread(void *d) mq->asleep = false; spin_unlock_irq(q->queue_lock); - if (req || mq->qcnt) { + if (req) { + if (!claimed_card) { + mmc_get_card(mq->card, NULL); + claimed_card = true; + } set_current_state(TASK_RUNNING); mmc_blk_issue_rq(mq, req); cond_resched(); @@ -72,6 +77,9 @@ static int mmc_queue_thread(void *d) } while (1); up(&mq->thread_sem); + if (claimed_card) + mmc_put_card(mq->card, NULL); + return 0; } @@ -207,7 +215,6 @@ int mmc_init_queue(struct mmc_queue *mq, struct mmc_card *card, mq->queue->exit_rq_fn = mmc_exit_request; mq->queue->cmd_size = sizeof(struct mmc_queue_req); mq->queue->queuedata = mq; - mq->qcnt = 0; ret = blk_init_allocated_queue(mq->queue); if (ret) { blk_cleanup_queue(mq->queue); diff --git a/drivers/mmc/core/queue.h b/drivers/mmc/core/queue.h index dce7cedb9d0b..67ae311b107f 100644 --- a/drivers/mmc/core/queue.h +++ b/drivers/mmc/core/queue.h @@ -67,13 +67,6 @@ struct mmc_queue { bool asleep; struct mmc_blk_data *blkdata; struct request_queue *queue; - /* - * FIXME: this counter is not a very reliable way of keeping - * track of how many requests that are ongoing. Switch to just - * letting the block core keep track of requests and per-request - * associated mmc_queue_req data. - */ - int qcnt; }; extern int mmc_init_queue(struct mmc_queue *, struct mmc_card *, spinlock_t *,