lkml.org 
[lkml]   [2008]   [Oct]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] MMC: Fix race condition in resume/card detect code
On Mon, 20 Oct 2008 22:41:48 +0300
Yauhen Kharuzhy <jekhor@gmail.com> wrote:

> When device wakes up by card change interrupt and MMC_UNSAFE_RESUME is
> enabled then race condition between mmc_rescan() and
> mmc_resume()/mmc_sd_resume() appeared.
>

Having thought a bit more about this, I'm not sure where the race is.
mmc_sd_resume() will be called before mmc_detect_change() is.

There is a race if the drivers call mmc_detect_change() before
mmc_resume_host() has returned, but that is a driver bug. I won't
object to adding a safe guard against that, but the commit message
should reflect that scenario and not something else. There should also
be some printk() to indicate that the driver is up to no good.

> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 044d84e..427f283 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -657,6 +657,9 @@ void mmc_rescan(struct work_struct *work)
> u32 ocr;
> int err;
>
> + if (host->suspended)
> + return;
> +
> mmc_bus_get(host);
>
> if (host->bus_ops == NULL) {

Was there no way to query the PM layer for this information?

> @@ -805,6 +810,8 @@ int mmc_resume_host(struct mmc_host *host)
> */
> mmc_detect_change(host, 1);
>
> + host->suspended = 0;
> +
> return 0;
> }
>

You've added a new race here. ;)

You should set suspended to 0 before calling mmc_detect_change(), not
after. The point was to protect bus_ops->resume(), nothing else.

> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 9c288c9..a584239 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -139,6 +139,9 @@ struct mmc_host {
> #ifdef CONFIG_MMC_DEBUG
> unsigned int removed:1; /* host is being removed */
> #endif
> +#ifdef CONFIG_MMC_UNSAFE_RESUME
> + unsigned int suspended:1;
> +#endif
>
> struct mmc_card *card; /* device attached to this host */
>

No #ifdef for this as there are no where this variable is referenced.

Rgds
--
-- Pierre Ossman

Linux kernel, MMC maintainer http://www.kernel.org
rdesktop, core developer http://www.rdesktop.org

WARNING: This correspondence is being monitored by the
Swedish government. Make sure your server uses encryption
for SMTP traffic and consider using PGP for end-to-end
encryption.


\
 
 \ /
  Last update: 2008-10-26 12:49    [from the cache]
©2003-2011 Jasper Spaans