Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   =?iso-8859-1?Q?Christian_L=F6hle?= <CLoehle@hyperstone.com>
To:     "ulf.hansson@linaro.org" <ulf.hansson@linaro.org>,
        "linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
CC:     Avri Altman <Avri.Altman@wdc.com>
Subject: Re: [PATCH] mmc: block: fix read single on recovery logic
Thread-Topic: [PATCH] mmc: block: fix read single on recovery logic
Thread-Index: AQHYAlL02lgrcWpb+EOcEaPCcQdJl6yBx2Gn
Date:   Thu, 3 Feb 2022 10:09:18 +0000
Message-ID: <7c4757cc707740e580c61c39f963a04d@hyperstone.com>
References: <5e5f2e45d0a14a55a8b7a9357846114b@hyperstone.com>
In-Reply-To: <5e5f2e45d0a14a55a8b7a9357846114b@hyperstone.com>
Accept-Language: en-US, de-DE
Content-Language: en-US
Content-Type: text/plain;
        charset="iso-8859-1"
Content-Transfer-Encoding: 8BIT
MIME-Version: 1.0
Precedence: bulk

So could anyone take a long at this so far?


From: Christian L?hle
Sent: Wednesday, January 5, 2022 5:43 PM
To: ulf.hansson@linaro.org; Christian L?hle; linux-mmc@vger.kernel.org; linux-kernel@vger.kernel.org
Cc: Avri Altman
Subject: [PATCH] mmc: block: fix read single on recovery logic
?   
On reads with MMC_READ_MULTIPLE_BLOCK that fail,
the recovery handler will use MMC_READ_SINGLE_BLOCK for
each of the blocks, up to MMC_READ_SINGLE_RETRIES times each.
The logic for this is fixed to never report unsuccessful reads
as success to the block layer.

On command error with retries remaining, blk_update_request was
called with whatever value error was set last to.
In case it was last set to BLK_STS_OK (default), the read will be
reported as success, even though there was no data read from the device.
This could happen on a CRC mismatch for the response,
a card rejecting the command (e.g. again due to a CRC mismatch).
In case it was last set to BLK_STS_IOERR, the error is reported correctly,
but no retries will be attempted.

The patch now will count both command and data errors as retries and
send BLK_STS_IOERR if there are no retries remaining,
or BLK_STS_OK if the single read was successful in the meantime.

Signed-off-by: Christian Loehle <cloehle@hyperstone.com>
---
?drivers/mmc/core/block.c | 28 ++++++++++++++--------------
?1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 90e1bcd03b46..d7d880ce0f8a 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -1682,31 +1682,31 @@ static void mmc_blk_read_single(struct mmc_queue *mq, struct request *req)
???????? struct mmc_card *card = mq->card;
???????? struct mmc_host *host = card->host;
???????? blk_status_t error = BLK_STS_OK;
-?????? int retries = 0;
?
???????? do {
???????????????? u32 status;
???????????????? int err;
+?????????????? int retries = 0;
?
-?????????????? mmc_blk_rw_rq_prep(mqrq, card, 1, mq);
+?????????????? while (retries++ < MMC_READ_SINGLE_RETRIES) {
+?????????????????????? mmc_blk_rw_rq_prep(mqrq, card, 1, mq);
?
-?????????????? mmc_wait_for_req(host, mrq);
+?????????????????????? mmc_wait_for_req(host, mrq);
?
-?????????????? err = mmc_send_status(card, &status);
-?????????????? if (err)
-?????????????????????? goto error_exit;
-
-?????????????? if (!mmc_host_is_spi(host) &&
-?????????????????? !mmc_ready_for_data(status)) {
-?????????????????????? err = mmc_blk_fix_state(card, req);
+?????????????????????? err = mmc_send_status(card, &status);
???????????????????????? if (err)
???????????????????????????????? goto error_exit;
-?????????????? }
?
-?????????????? if (mrq->cmd->error && retries++ < MMC_READ_SINGLE_RETRIES)
-?????????????????????? continue;
+?????????????????????? if (!mmc_host_is_spi(host) &&
+?????????????????????????? !mmc_ready_for_data(status)) {
+?????????????????????????????? err = mmc_blk_fix_state(card, req);
+?????????????????????????????? if (err)
+?????????????????????????????????????? goto error_exit;
+?????????????????????? }
?
-?????????????? retries = 0;
+?????????????????????? if (!mrq->cmd->error && !mrq->data->error)
+?????????????????????????????? break;
+?????????????? }
?
???????????????? if (mrq->cmd->error ||
???????????????????? mrq->data->error ||
-- 
2.34.1
    =
Hyperstone GmbH | Reichenaustr. 39a  | 78467 Konstanz
Managing Director: Dr. Jan Peter Berns.
Commercial register of local courts: Freiburg HRB381782