* [PATCHv2] mmc: block: fix read single on recovery logic
[not found] ` <79d44b0c54e048b0a9cc86319a24cc19@hyperstone.com>
@ 2022-02-04 15:11 ` Christian Löhle
2022-02-08 6:37 ` Adrian Hunter
0 siblings, 1 reply; 3+ messages in thread
From: Christian Löhle @ 2022-02-04 15:11 UTC (permalink / raw)
To: Adrian Hunter, Ulf Hansson, Christian Löhle
Cc: linux-mmc, linux-kernel, Avri Altman, stable
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.
Fixes: 81196976ed946c ("mmc: block: Add blk-mq support")
Cc: stable@vger.kernel.org
Signed-off-by: Christian Loehle <cloehle@hyperstone.com>
---
v2:
- Do not allow data error retries
- Actually retry MMC_READ_SINGLE_RETRIES times instead of
MMC_READ_SINGLE_RETRIES-1
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 4e61b28a002f..8d718aa56d33 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)
+ 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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCHv2] mmc: block: fix read single on recovery logic
2022-02-04 15:11 ` [PATCHv2] mmc: block: fix read single on recovery logic Christian Löhle
@ 2022-02-08 6:37 ` Adrian Hunter
2022-02-08 15:12 ` Ulf Hansson
0 siblings, 1 reply; 3+ messages in thread
From: Adrian Hunter @ 2022-02-08 6:37 UTC (permalink / raw)
To: Christian Löhle, Ulf Hansson
Cc: linux-mmc, linux-kernel, Avri Altman, stable
On 04/02/2022 17:11, Christian Löhle wrote:
> 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.
>
> Fixes: 81196976ed946c ("mmc: block: Add blk-mq support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Christian Loehle <cloehle@hyperstone.com>
Reviewed-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> v2:
> - Do not allow data error retries
> - Actually retry MMC_READ_SINGLE_RETRIES times instead of
> MMC_READ_SINGLE_RETRIES-1
>
>
> 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 4e61b28a002f..8d718aa56d33 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)
> + break;
> + }
>
> if (mrq->cmd->error ||
> mrq->data->error ||
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCHv2] mmc: block: fix read single on recovery logic
2022-02-08 6:37 ` Adrian Hunter
@ 2022-02-08 15:12 ` Ulf Hansson
0 siblings, 0 replies; 3+ messages in thread
From: Ulf Hansson @ 2022-02-08 15:12 UTC (permalink / raw)
To: Adrian Hunter, Christian Löhle
Cc: linux-mmc, linux-kernel, Avri Altman, stable
On Tue, 8 Feb 2022 at 07:37, Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> On 04/02/2022 17:11, Christian Löhle wrote:
> > 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.
> >
> > Fixes: 81196976ed946c ("mmc: block: Add blk-mq support")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Christian Loehle <cloehle@hyperstone.com>
>
> Reviewed-by: Adrian Hunter <adrian.hunter@intel.com>
Applied for fixes, thanks to both of you!
Kind regards
Uffe
>
> > ---
> > v2:
> > - Do not allow data error retries
> > - Actually retry MMC_READ_SINGLE_RETRIES times instead of
> > MMC_READ_SINGLE_RETRIES-1
> >
> >
> > 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 4e61b28a002f..8d718aa56d33 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)
> > + break;
> > + }
> >
> > if (mrq->cmd->error ||
> > mrq->data->error ||
> >
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-02-08 15:13 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <5e5f2e45d0a14a55a8b7a9357846114b@hyperstone.com>
[not found] ` <7c4757cc707740e580c61c39f963a04d@hyperstone.com>
[not found] ` <CAPDyKFr0YXCwL-8F9M7mkpNzSQpzw6gNUq2zaiJEXj1jNxUbrg@mail.gmail.com>
[not found] ` <5c66833d-4b35-2c76-db54-0306e08843e5@intel.com>
[not found] ` <79d44b0c54e048b0a9cc86319a24cc19@hyperstone.com>
2022-02-04 15:11 ` [PATCHv2] mmc: block: fix read single on recovery logic Christian Löhle
2022-02-08 6:37 ` Adrian Hunter
2022-02-08 15:12 ` Ulf Hansson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).