* [PATCH] mmc: core: fix __mmc_switch timeout caused by preempt
@ 2015-11-26 2:51 Chaotian Jing
2015-11-26 11:56 ` Ulf Hansson
0 siblings, 1 reply; 2+ messages in thread
From: Chaotian Jing @ 2015-11-26 2:51 UTC (permalink / raw
To: Ulf Hansson
Cc: Matthias Brugger, Stephen Boyd, Chaotian Jing, Adrian Hunter,
Minda Chen, linux-mmc, linux-kernel, linux-arm-kernel,
linux-mediatek, srv_heupstream
there is a time window between __mmc_send_status() and time_afer(),
on some eMMC chip, the timeout_ms is only 10ms, if this thread was
scheduled out during this period, then, even card has already changes
to transfer state by the result of CMD13, this part of code also treat
it to timeout error.
So, need calculate timeout first, then call __mmc_send_status(), if
already timeout and card still in programing state, then treat it to
the real timeout error.
Signed-off-by: Chaotian Jing <chaotian.jing@mediatek.com>
---
drivers/mmc/core/mmc_ops.c | 23 ++++++++++++++---------
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 1f44426..0add634 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -489,6 +489,7 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
unsigned long timeout;
u32 status = 0;
bool use_r1b_resp = use_busy_signal;
+ bool expired = false;
mmc_retune_hold(host);
@@ -544,10 +545,22 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
/* Must check status to be sure of no errors. */
timeout = jiffies + msecs_to_jiffies(timeout_ms);
do {
+ /* Timeout if the device never leaves the program state.
+ * must check it firstly to avoid problem caused by preempt.
+ */
+ expired = time_after(jiffies, timeout);
+
if (send_status) {
err = __mmc_send_status(card, &status, ignore_crc);
- if (err)
+ if (err) {
goto out;
+ } else if (expired &&
+ R1_CURRENT_STATE(status) == R1_STATE_PRG) {
+ pr_err("%s: Card stuck in programming state! %s\n",
+ mmc_hostname(host), __func__);
+ err = -ETIMEDOUT;
+ goto out;
+ }
}
if ((host->caps & MMC_CAP_WAIT_WHILE_BUSY) && use_r1b_resp)
break;
@@ -563,14 +576,6 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
mmc_delay(timeout_ms);
goto out;
}
-
- /* Timeout if the device never leaves the program state. */
- if (time_after(jiffies, timeout)) {
- pr_err("%s: Card stuck in programming state! %s\n",
- mmc_hostname(host), __func__);
- err = -ETIMEDOUT;
- goto out;
- }
} while (R1_CURRENT_STATE(status) == R1_STATE_PRG);
err = mmc_switch_status_error(host, status);
--
1.8.1.1.dirty
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] mmc: core: fix __mmc_switch timeout caused by preempt
2015-11-26 2:51 [PATCH] mmc: core: fix __mmc_switch timeout caused by preempt Chaotian Jing
@ 2015-11-26 11:56 ` Ulf Hansson
0 siblings, 0 replies; 2+ messages in thread
From: Ulf Hansson @ 2015-11-26 11:56 UTC (permalink / raw
To: Chaotian Jing
Cc: Matthias Brugger, Stephen Boyd, Adrian Hunter, Minda Chen,
linux-mmc, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, linux-mediatek,
srv_heupstream
On 26 November 2015 at 03:51, Chaotian Jing <chaotian.jing@mediatek.com> wrote:
> there is a time window between __mmc_send_status() and time_afer(),
> on some eMMC chip, the timeout_ms is only 10ms, if this thread was
> scheduled out during this period, then, even card has already changes
> to transfer state by the result of CMD13, this part of code also treat
> it to timeout error.
> So, need calculate timeout first, then call __mmc_send_status(), if
> already timeout and card still in programing state, then treat it to
> the real timeout error.
>
> Signed-off-by: Chaotian Jing <chaotian.jing@mediatek.com>
> ---
> drivers/mmc/core/mmc_ops.c | 23 ++++++++++++++---------
> 1 file changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index 1f44426..0add634 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -489,6 +489,7 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
> unsigned long timeout;
> u32 status = 0;
> bool use_r1b_resp = use_busy_signal;
> + bool expired = false;
No need to assign an initial value.
>
> mmc_retune_hold(host);
>
> @@ -544,10 +545,22 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
> /* Must check status to be sure of no errors. */
> timeout = jiffies + msecs_to_jiffies(timeout_ms);
> do {
> + /* Timeout if the device never leaves the program state.
Please start mutli-row comment with an empty line.
> + * must check it firstly to avoid problem caused by preempt.
Please rephrase this to:
Due to the possibility of being preempted after sending the status
command, check the expiration time first.
> + */
> + expired = time_after(jiffies, timeout);
I think we can move this inside the below "if (send_status) {"
.. but of course before sending the status command.
> +
> if (send_status) {
> err = __mmc_send_status(card, &status, ignore_crc);
> - if (err)
> + if (err) {
> goto out;
> + } else if (expired &&
You may use a new "if" statement instead of an "else if", due to the
above "goto out".
> + R1_CURRENT_STATE(status) == R1_STATE_PRG) {
> + pr_err("%s: Card stuck in programming state! %s\n",
> + mmc_hostname(host), __func__);
> + err = -ETIMEDOUT;
> + goto out;
> + }
> }
> if ((host->caps & MMC_CAP_WAIT_WHILE_BUSY) && use_r1b_resp)
> break;
> @@ -563,14 +576,6 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
> mmc_delay(timeout_ms);
> goto out;
> }
> -
> - /* Timeout if the device never leaves the program state. */
> - if (time_after(jiffies, timeout)) {
> - pr_err("%s: Card stuck in programming state! %s\n",
> - mmc_hostname(host), __func__);
> - err = -ETIMEDOUT;
> - goto out;
> - }
> } while (R1_CURRENT_STATE(status) == R1_STATE_PRG);
>
> err = mmc_switch_status_error(host, status);
> --
> 1.8.1.1.dirty
>
Kind regards
Uffe
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2015-11-26 11:56 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-26 2:51 [PATCH] mmc: core: fix __mmc_switch timeout caused by preempt Chaotian Jing
2015-11-26 11:56 ` 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 read-only IMAP folder(s) and NNTP newsgroup(s).