* FAILED: patch "[PATCH] mmc: block: fix read single on recovery logic" failed to apply to 4.19-stable tree
@ 2022-02-17 19:14 gregkh
2022-02-18 9:50 ` [PATCH] mmc: block: fix read single on recovery logic Christian Löhle
0 siblings, 1 reply; 12+ messages in thread
From: gregkh @ 2022-02-17 19:14 UTC (permalink / raw)
To: CLoehle, adrian.hunter, cloehle, ulf.hansson; +Cc: stable
The patch below does not apply to the 4.19-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 54309fde1a352ad2674ebba004a79f7d20b9f037 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20L=C3=B6hle?= <CLoehle@hyperstone.com>
Date: Fri, 4 Feb 2022 15:11:37 +0000
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.
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>
Link: https://lore.kernel.org/r/bc706a6ab08c4fe2834ba0c05a804672@hyperstone.com
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
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 related [flat|nested] 12+ messages in thread
* [PATCH] mmc: block: fix read single on recovery logic
2022-02-17 19:14 FAILED: patch "[PATCH] mmc: block: fix read single on recovery logic" failed to apply to 4.19-stable tree gregkh
@ 2022-02-18 9:50 ` Christian Löhle
2022-02-18 10:33 ` gregkh
0 siblings, 1 reply; 12+ messages in thread
From: Christian Löhle @ 2022-02-18 9:50 UTC (permalink / raw)
To: gregkh, stable; +Cc: Ulf Hansson, Adrian Hunter
commit 54309fde1a352ad2674ebba004a79f7d20b9f037 upstream.
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")
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 d1cc0fdbc51c..630f3bcba56d 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -1678,31 +1678,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_blk_in_tran_state(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_blk_in_tran_state(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] 12+ messages in thread
* Re: [PATCH] mmc: block: fix read single on recovery logic
2022-02-18 9:50 ` [PATCH] mmc: block: fix read single on recovery logic Christian Löhle
@ 2022-02-18 10:33 ` gregkh
2022-02-18 10:36 ` Christian Löhle
0 siblings, 1 reply; 12+ messages in thread
From: gregkh @ 2022-02-18 10:33 UTC (permalink / raw)
To: Christian Löhle; +Cc: stable, Ulf Hansson, Adrian Hunter
On Fri, Feb 18, 2022 at 09:50:54AM +0000, Christian Löhle wrote:
> commit 54309fde1a352ad2674ebba004a79f7d20b9f037 upstream.
>
> 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")
>
> Signed-off-by: Christian Loehle <cloehle@hyperstone.com>
> ---
> drivers/mmc/core/block.c | 28 ++++++++++++++--------------
> 1 file changed, 14 insertions(+), 14 deletions(-)
What stable branch(s) is this backport for?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] mmc: block: fix read single on recovery logic
2022-02-18 10:33 ` gregkh
@ 2022-02-18 10:36 ` Christian Löhle
2022-02-18 10:49 ` gregkh
[not found] ` <916c2090-5b39-e9e8-190e-d0f8a77adfa3@intel.com>
0 siblings, 2 replies; 12+ messages in thread
From: Christian Löhle @ 2022-02-18 10:36 UTC (permalink / raw)
To: gregkh, stable; +Cc: Ulf Hansson, Adrian Hunter
This is the backport for 4.19, it applied for more recent branches and is not applicable to 4.14.
From: gregkh@linuxfoundation.org <gregkh@linuxfoundation.org>
Sent: Friday, February 18, 2022 11:33 AM
To: Christian Löhle
Cc: stable@vger.kernel.org; Ulf Hansson; Adrian Hunter
Subject: Re: [PATCH] mmc: block: fix read single on recovery logic
On Fri, Feb 18, 2022 at 09:50:54AM +0000, Christian Löhle wrote:
> commit 54309fde1a352ad2674ebba004a79f7d20b9f037 upstream.
>
> 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")
>
> Signed-off-by: Christian Loehle <cloehle@hyperstone.com>
> ---
> drivers/mmc/core/block.c | 28 ++++++++++++++--------------
> 1 file changed, 14 insertions(+), 14 deletions(-)
What stable branch(s) is this backport for?
thanks,
greg k-h
=
Hyperstone GmbH | Reichenaustr. 39a | 78467 Konstanz
Managing Director: Dr. Jan Peter Berns.
Commercial register of local courts: Freiburg HRB381782
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] mmc: block: fix read single on recovery logic
2022-02-18 10:36 ` Christian Löhle
@ 2022-02-18 10:49 ` gregkh
[not found] ` <916c2090-5b39-e9e8-190e-d0f8a77adfa3@intel.com>
1 sibling, 0 replies; 12+ messages in thread
From: gregkh @ 2022-02-18 10:49 UTC (permalink / raw)
To: Christian Löhle; +Cc: stable, Ulf Hansson, Adrian Hunter
On Fri, Feb 18, 2022 at 10:36:47AM +0000, Christian Löhle wrote:
> This is the backport for 4.19, it applied for more recent branches and
> is not applicable to 4.14.
Thanks, now queued up.
greg k-h
^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <916c2090-5b39-e9e8-190e-d0f8a77adfa3@intel.com>]
* Re: [PATCH] mmc: block: fix read single on recovery logic
[not found] ` <916c2090-5b39-e9e8-190e-d0f8a77adfa3@intel.com>
@ 2022-02-18 11:05 ` Christian Löhle
2022-02-18 11:47 ` Greg Kroah-Hartman
0 siblings, 1 reply; 12+ messages in thread
From: Christian Löhle @ 2022-02-18 11:05 UTC (permalink / raw)
To: Adrian Hunter, Greg Kroah-Hartman, stable
You're right, I did not get an email about 5.4, maybe it was caught up somewhere.
Greg, can you apply this to 5.4, too?
From: Adrian Hunter <adrian.hunter@intel.com>
Sent: Friday, February 18, 2022 12:02 PM
To: Christian Löhle
Subject: Re: [PATCH] mmc: block: fix read single on recovery logic
On 18/02/2022 12:36, Christian Löhle wrote:
> This is the backport for 4.19, it applied for more recent branches and is not applicable to 4.14.
Was 5.4 done? There was a "failed to apply email for that also"
>
>
>
> From: gregkh@linuxfoundation.org <gregkh@linuxfoundation.org>
> Sent: Friday, February 18, 2022 11:33 AM
> To: Christian Löhle
> Cc: stable@vger.kernel.org; Ulf Hansson; Adrian Hunter
> Subject: Re: [PATCH] mmc: block: fix read single on recovery logic
>
> On Fri, Feb 18, 2022 at 09:50:54AM +0000, Christian Löhle wrote:
>> commit 54309fde1a352ad2674ebba004a79f7d20b9f037 upstream.
>>
>> 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")
>>
>> Signed-off-by: Christian Loehle <cloehle@hyperstone.com>
>> ---
>> drivers/mmc/core/block.c | 28 ++++++++++++++--------------
>> 1 file changed, 14 insertions(+), 14 deletions(-)
>
> What stable branch(s) is this backport for?
>
> thanks,
>
> greg k-h
> =
> Hyperstone GmbH | Reichenaustr. 39a | 78467 Konstanz
> Managing Director: Dr. Jan Peter Berns.
> Commercial register of local courts: Freiburg HRB381782
>
=
Hyperstone GmbH | Reichenaustr. 39a | 78467 Konstanz
Managing Director: Dr. Jan Peter Berns.
Commercial register of local courts: Freiburg HRB381782
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] mmc: block: fix read single on recovery logic
2022-02-18 11:05 ` Christian Löhle
@ 2022-02-18 11:47 ` Greg Kroah-Hartman
0 siblings, 0 replies; 12+ messages in thread
From: Greg Kroah-Hartman @ 2022-02-18 11:47 UTC (permalink / raw)
To: Christian Löhle; +Cc: Adrian Hunter, stable
On Fri, Feb 18, 2022 at 11:05:21AM +0000, Christian Löhle wrote:
> You're right, I did not get an email about 5.4, maybe it was caught up somewhere.
> Greg, can you apply this to 5.4, too?
Oops, forgot that one, now queued up.
greg k-h
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] mmc: block: fix read single on recovery logic
@ 2022-01-05 16:43 Christian Löhle
2022-02-03 10:09 ` Christian Löhle
0 siblings, 1 reply; 12+ messages in thread
From: Christian Löhle @ 2022-01-05 16:43 UTC (permalink / raw)
To: ulf.hansson, Christian Löhle, linux-mmc, linux-kernel; +Cc: Avri Altman
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] mmc: block: fix read single on recovery logic
2022-01-05 16:43 Christian Löhle
@ 2022-02-03 10:09 ` Christian Löhle
2022-02-04 9:47 ` Ulf Hansson
0 siblings, 1 reply; 12+ messages in thread
From: Christian Löhle @ 2022-02-03 10:09 UTC (permalink / raw)
To: ulf.hansson, linux-mmc, linux-kernel; +Cc: Avri Altman
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] mmc: block: fix read single on recovery logic
2022-02-03 10:09 ` Christian Löhle
@ 2022-02-04 9:47 ` Ulf Hansson
2022-02-04 11:26 ` Adrian Hunter
0 siblings, 1 reply; 12+ messages in thread
From: Ulf Hansson @ 2022-02-04 9:47 UTC (permalink / raw)
To: Christian Löhle; +Cc: linux-mmc, linux-kernel, Avri Altman, Adrian Hunter
+ Adrian
On Thu, 3 Feb 2022 at 11:09, Christian Löhle <CLoehle@hyperstone.com> wrote:
>
> So could anyone take a long at this so far?
>
Thanks for pinging. Apologize for the delay, it's on top of my "to-review" list.
I have added Adrian too, who knows this code very well too.
Kind regards
Uffe
>
>
> 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
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] mmc: block: fix read single on recovery logic
2022-02-04 9:47 ` Ulf Hansson
@ 2022-02-04 11:26 ` Adrian Hunter
2022-02-04 14:34 ` Christian Löhle
0 siblings, 1 reply; 12+ messages in thread
From: Adrian Hunter @ 2022-02-04 11:26 UTC (permalink / raw)
To: Ulf Hansson, Christian Löhle; +Cc: linux-mmc, linux-kernel, Avri Altman
On 04/02/2022 11:47, Ulf Hansson wrote:
> + Adrian
>
> On Thu, 3 Feb 2022 at 11:09, Christian Löhle <CLoehle@hyperstone.com> wrote:
>>
>> So could anyone take a long at this so far?
>>
>
> Thanks for pinging. Apologize for the delay, it's on top of my "to-review" list.
>
> I have added Adrian too, who knows this code very well too.
>
> Kind regards
> Uffe
>
>>
>>
>> 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>
Thanks for the patch.
Looks OK, although a couple of comments below, plus it needs
a Fixes tag, and Cc for stable.
Fixes: 81196976ed946c ("mmc: block: Add blk-mq support")
Cc: stable@vger.kernel.org
>> ---
>> 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) {
Because this is now checked at the top of the loop, wouldn't that
result in one fewer retries than before? So, maybe:
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)
We weren't retrying for data errors before, and I don't think we want to
because single block read can be very slow. i.e. just
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 [flat|nested] 12+ messages in thread
* Re: [PATCH] mmc: block: fix read single on recovery logic
2022-02-04 11:26 ` Adrian Hunter
@ 2022-02-04 14:34 ` Christian Löhle
0 siblings, 0 replies; 12+ messages in thread
From: Christian Löhle @ 2022-02-04 14:34 UTC (permalink / raw)
To: Adrian Hunter, Ulf Hansson; +Cc: linux-mmc, linux-kernel, Avri Altman
Thanks for the comments Adrian!
>> + while (retries++ < MMC_READ_SINGLE_RETRIES) {
>
>Because this is now checked at the top of the loop, wouldn't that
>result in one fewer retries than before? So, maybe:
>
> while (retries++ <= MMC_READ_SINGLE_RETRIES) {
Yes, you are correct. Will be fixed in v2.
>> + if (!mrq->cmd->error && !mrq->data->error)
>
>We weren't retrying for data errors before, and I don't think we want to
>because single block read can be very slow. i.e. just
>
> if (!mrq->cmd->error)
That was intentional by me, it was very unintuitive to my you would not retry for data errors.
(Considering a data error is likely how you got into the whole recovery in the first place.)
But yes I see your point, a very large request might block this for quite a while.
Will change in v2, too.
From: Adrian Hunter <adrian.hunter@intel.com>
Sent: Friday, February 4, 2022 12:26 PM
To: Ulf Hansson; Christian Löhle
Cc: linux-mmc@vger.kernel.org; linux-kernel@vger.kernel.org; Avri Altman
Subject: Re: [PATCH] mmc: block: fix read single on recovery logic
On 04/02/2022 11:47, Ulf Hansson wrote:
> + Adrian
>
> On Thu, 3 Feb 2022 at 11:09, Christian Löhle <CLoehle@hyperstone.com> wrote:
>>
>> So could anyone take a long at this so far?
>>
>
> Thanks for pinging. Apologize for the delay, it's on top of my "to-review" list.
>
> I have added Adrian too, who knows this code very well too.
>
> Kind regards
> Uffe
>
>>
>>
>> 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>
Thanks for the patch.
Looks OK, although a couple of comments below, plus it needs
a Fixes tag, and Cc for stable.
Fixes: 81196976ed946c ("mmc: block: Add blk-mq support")
Cc: stable@vger.kernel.org
>> ---
>> 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) {
Because this is now checked at the top of the loop, wouldn't that
result in one fewer retries than before? So, maybe:
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)
We weren't retrying for data errors before, and I don't think we want to
because single block read can be very slow. i.e. just
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
>>
=
Hyperstone GmbH | Reichenaustr. 39a | 78467 Konstanz
Managing Director: Dr. Jan Peter Berns.
Commercial register of local courts: Freiburg HRB381782
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-02-18 11:48 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-17 19:14 FAILED: patch "[PATCH] mmc: block: fix read single on recovery logic" failed to apply to 4.19-stable tree gregkh
2022-02-18 9:50 ` [PATCH] mmc: block: fix read single on recovery logic Christian Löhle
2022-02-18 10:33 ` gregkh
2022-02-18 10:36 ` Christian Löhle
2022-02-18 10:49 ` gregkh
[not found] ` <916c2090-5b39-e9e8-190e-d0f8a77adfa3@intel.com>
2022-02-18 11:05 ` Christian Löhle
2022-02-18 11:47 ` Greg Kroah-Hartman
-- strict thread matches above, loose matches on Subject: below --
2022-01-05 16:43 Christian Löhle
2022-02-03 10:09 ` Christian Löhle
2022-02-04 9:47 ` Ulf Hansson
2022-02-04 11:26 ` Adrian Hunter
2022-02-04 14:34 ` Christian Löhle
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.