* [PATCH] mmc: core: Prevent too long response times for suspend
@ 2011-10-13 14:03 Ulf Hansson
2011-10-15 6:47 ` Sujit Reddy Thumma
2011-10-19 22:09 ` Linus Walleij
0 siblings, 2 replies; 6+ messages in thread
From: Ulf Hansson @ 2011-10-13 14:03 UTC (permalink / raw)
To: linux-mmc, Chris Ball; +Cc: Per Forlin, Ulf Hansson, Lee Jones
While trying to suspend the mmc host there could still be
ongoing requests that we need to wait for. At the same time
a device driver must respond to a suspend request rather quickly.
Instead of potentially wait "forever" by claiming the host we now
"try" to claim the host instead. If it fails, -EBUSY is returned.
Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
---
drivers/mmc/core/core.c | 42 +++++++++++++++++++++++++++---------------
1 files changed, 27 insertions(+), 15 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 61d7730..b900932 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2121,21 +2121,33 @@ int mmc_suspend_host(struct mmc_host *host)
mmc_bus_get(host);
if (host->bus_ops && !host->bus_dead) {
- if (host->bus_ops->suspend)
- err = host->bus_ops->suspend(host);
- if (err == -ENOSYS || !host->bus_ops->resume) {
- /*
- * We simply "remove" the card in this case.
- * It will be redetected on resume.
- */
- if (host->bus_ops->remove)
- host->bus_ops->remove(host);
- mmc_claim_host(host);
- mmc_detach_bus(host);
- mmc_power_off(host);
- mmc_release_host(host);
- host->pm_flags = 0;
- err = 0;
+
+ /*
+ * A long response time is not acceptable for device drivers
+ * when doing suspend. Prevent mmc_claim_host in the suspend
+ * sequence, to potentially wait "forever" by trying to
+ * pre-claim the host.
+ */
+ if (mmc_try_claim_host(host)) {
+ if (host->bus_ops->suspend)
+ err = host->bus_ops->suspend(host);
+ if (err == -ENOSYS || !host->bus_ops->resume) {
+ /*
+ * We simply "remove" the card in this case.
+ * It will be redetected on resume.
+ */
+ if (host->bus_ops->remove)
+ host->bus_ops->remove(host);
+ mmc_claim_host(host);
+ mmc_detach_bus(host);
+ mmc_power_off(host);
+ mmc_release_host(host);
+ host->pm_flags = 0;
+ err = 0;
+ }
+ mmc_do_release_host(host);
+ } else {
+ err = -EBUSY;
}
}
mmc_bus_put(host);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] mmc: core: Prevent too long response times for suspend
2011-10-13 14:03 [PATCH] mmc: core: Prevent too long response times for suspend Ulf Hansson
@ 2011-10-15 6:47 ` Sujit Reddy Thumma
2011-10-17 9:51 ` Ulf Hansson
2011-10-19 22:09 ` Linus Walleij
1 sibling, 1 reply; 6+ messages in thread
From: Sujit Reddy Thumma @ 2011-10-15 6:47 UTC (permalink / raw)
Cc: linux-mmc, Chris Ball, Per Forlin, Ulf Hansson, Lee Jones
> While trying to suspend the mmc host there could still be
> ongoing requests that we need to wait for. At the same time
> a device driver must respond to a suspend request rather quickly.
>
> Instead of potentially wait "forever" by claiming the host we now
> "try" to claim the host instead. If it fails, -EBUSY is returned.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
> ---
> drivers/mmc/core/core.c | 42 +++++++++++++++++++++++++++---------------
> 1 files changed, 27 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 61d7730..b900932 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -2121,21 +2121,33 @@ int mmc_suspend_host(struct mmc_host *host)
>
> mmc_bus_get(host);
> if (host->bus_ops && !host->bus_dead) {
> - if (host->bus_ops->suspend)
> - err = host->bus_ops->suspend(host);
> - if (err == -ENOSYS || !host->bus_ops->resume) {
> - /*
> - * We simply "remove" the card in this case.
> - * It will be redetected on resume.
> - */
> - if (host->bus_ops->remove)
> - host->bus_ops->remove(host);
> - mmc_claim_host(host);
> - mmc_detach_bus(host);
> - mmc_power_off(host);
> - mmc_release_host(host);
> - host->pm_flags = 0;
> - err = 0;
> +
> + /*
> + * A long response time is not acceptable for device drivers
> + * when doing suspend. Prevent mmc_claim_host in the suspend
> + * sequence, to potentially wait "forever" by trying to
> + * pre-claim the host.
> + */
Why would there be pending requests while host is suspending? Is the
kernel framework not handling sync before going to suspend? However, the
mmc_blk_suspend() would be called before the host driver suspends (as all
the driver suspend routines are serialized) which means it stops block
layer to queue more I/O requests well before the host driver start
suspend. Does this sequence break in your case?
Your concern seems to be valid for SDIO case, but again the function
driver must be intelligent enough to return -EBUSY as it knows that it had
posted a request to MMC.
> + if (mmc_try_claim_host(host)) {
> + if (host->bus_ops->suspend)
> + err = host->bus_ops->suspend(host);
> + if (err == -ENOSYS || !host->bus_ops->resume) {
> + /*
> + * We simply "remove" the card in this case.
> + * It will be redetected on resume.
> + */
> + if (host->bus_ops->remove)
> + host->bus_ops->remove(host);
> + mmc_claim_host(host);
> + mmc_detach_bus(host);
> + mmc_power_off(host);
> + mmc_release_host(host);
> + host->pm_flags = 0;
> + err = 0;
> + }
> + mmc_do_release_host(host);
> + } else {
> + err = -EBUSY;
> }
> }
> mmc_bus_put(host);
> --
> 1.7.5.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Thanks,
Sujit
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mmc: core: Prevent too long response times for suspend
2011-10-15 6:47 ` Sujit Reddy Thumma
@ 2011-10-17 9:51 ` Ulf Hansson
2011-10-19 17:25 ` Sujit Reddy Thumma
0 siblings, 1 reply; 6+ messages in thread
From: Ulf Hansson @ 2011-10-17 9:51 UTC (permalink / raw)
To: Sujit Reddy Thumma
Cc: linux-mmc@vger.kernel.org, Chris Ball, Per FORLIN, Lee Jones
>
> Why would there be pending requests while host is suspending? Is the
> kernel framework not handling sync before going to suspend? However, the
> mmc_blk_suspend() would be called before the host driver suspends (as all
> the driver suspend routines are serialized) which means it stops block
> layer to queue more I/O requests well before the host driver start
> suspend. Does this sequence break in your case?
I have observed this issue for different cases (one case was logging to
eMMC). The idea is simply that we would like to be sure that we do not
wait "forever", no matter if the "upper layers" misbehaved in the
suspend sequence.
>
> Your concern seems to be valid for SDIO case, but again the function
> driver must be intelligent enough to return -EBUSY as it knows that it had
> posted a request to MMC.
>
For SDIO, should we really assume that function driver has implemented a
suspend function and moreover that it actually always behaves as we expect?
Br
Ulf Hansson
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mmc: core: Prevent too long response times for suspend
2011-10-17 9:51 ` Ulf Hansson
@ 2011-10-19 17:25 ` Sujit Reddy Thumma
0 siblings, 0 replies; 6+ messages in thread
From: Sujit Reddy Thumma @ 2011-10-19 17:25 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc@vger.kernel.org, Chris Ball, Per FORLIN, Lee Jones
On 10/17/2011 3:21 PM, Ulf Hansson wrote:
>>
>> Why would there be pending requests while host is suspending? Is the
>> kernel framework not handling sync before going to suspend? However, the
>> mmc_blk_suspend() would be called before the host driver suspends (as all
>> the driver suspend routines are serialized) which means it stops block
>> layer to queue more I/O requests well before the host driver start
>> suspend. Does this sequence break in your case?
>
> I have observed this issue for different cases (one case was logging to
> eMMC). The idea is simply that we would like to be sure that we do not
> wait "forever", no matter if the "upper layers" misbehaved in the
> suspend sequence.
>
Agree. I was just curious to know why would there be a pending requests.
If the issue is seen while logging to eMMC then there may be something
wrong, probably, we may see issue while running mmc_test and initiating
suspend. Your patch looks good to me though.
Reviewed-by: Sujit Reddy Thumma <sthumma@codeaurora.org>
>>
>> Your concern seems to be valid for SDIO case, but again the function
>> driver must be intelligent enough to return -EBUSY as it knows that it
>> had
>> posted a request to MMC.
>>
>
> For SDIO, should we really assume that function driver has implemented a
> suspend function and moreover that it actually always behaves as we expect?
I hope so, but yes, in some cases that may be broken. I agree with you.
>
> Br
> Ulf Hansson
--
Thanks & Regards,
Sujit Reddy Thumma
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mmc: core: Prevent too long response times for suspend
2011-10-13 14:03 [PATCH] mmc: core: Prevent too long response times for suspend Ulf Hansson
2011-10-15 6:47 ` Sujit Reddy Thumma
@ 2011-10-19 22:09 ` Linus Walleij
2011-10-21 4:25 ` Chris Ball
1 sibling, 1 reply; 6+ messages in thread
From: Linus Walleij @ 2011-10-19 22:09 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, Chris Ball, Per Forlin, Lee Jones
2011/10/13 Ulf Hansson <ulf.hansson@stericsson.com>:
> While trying to suspend the mmc host there could still be
> ongoing requests that we need to wait for. At the same time
> a device driver must respond to a suspend request rather quickly.
>
> Instead of potentially wait "forever" by claiming the host we now
> "try" to claim the host instead. If it fails, -EBUSY is returned.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Thanks,
Linus Walleij
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mmc: core: Prevent too long response times for suspend
2011-10-19 22:09 ` Linus Walleij
@ 2011-10-21 4:25 ` Chris Ball
0 siblings, 0 replies; 6+ messages in thread
From: Chris Ball @ 2011-10-21 4:25 UTC (permalink / raw)
To: Linus Walleij; +Cc: Ulf Hansson, linux-mmc, Per Forlin, Lee Jones
Hi,
On Wed, Oct 19 2011, Linus Walleij wrote:
>> While trying to suspend the mmc host there could still be
>> ongoing requests that we need to wait for. At the same time
>> a device driver must respond to a suspend request rather quickly.
>>
>> Instead of potentially wait "forever" by claiming the host we now
>> "try" to claim the host instead. If it fails, -EBUSY is returned.
>>
>> Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
Thanks very much, Ulf, pushed to mmc-next for 3.2.
- Chris.
--
Chris Ball <cjb@laptop.org> <http://printf.net/>
One Laptop Per Child
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-10-21 4:25 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-13 14:03 [PATCH] mmc: core: Prevent too long response times for suspend Ulf Hansson
2011-10-15 6:47 ` Sujit Reddy Thumma
2011-10-17 9:51 ` Ulf Hansson
2011-10-19 17:25 ` Sujit Reddy Thumma
2011-10-19 22:09 ` Linus Walleij
2011-10-21 4:25 ` Chris Ball
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox