[PATCH V2] mmc: core: Fix deadlock when the CONFIG_MMC_UNSAFE_RESUME is not defined

[PATCH V2] mmc: core: Fix deadlock when the CONFIG_MMC_UNSAFE_RESUME is not defined

[Sujit Reddy Thumma]

mmc_suspend_host() tries to claim host during suspend
and release it only when the bus suspend operation is
compeleted. If CONFIG_MMC_UNSAFE_RESUME is defined and
the host is flagged as removable, mmc_suspend_host()
tries to remove the card. In this process, the file system
sync can get blocked trying to acquire host which is already
claimed by mmc_suspend_host() causing deadlock.

Fix this deadlock by releasing host before ->remove() is called.

Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org>

---
Changes in v2:
	- Addressed review comment from Ulf Hansson.
---
 drivers/mmc/core/core.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 271efea..950b97d 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2333,6 +2333,8 @@ int mmc_suspend_host(struct mmc_host *host)
 				mmc_poweroff_notify(host);
 				err = host->bus_ops->suspend(host);
 			}
+			mmc_do_release_host(host);
+
 			if (err == -ENOSYS || !host->bus_ops->resume) {
 				/*
 				 * We simply "remove" the card in this case.
@@ -2347,7 +2349,6 @@ int mmc_suspend_host(struct mmc_host *host)
 				host->pm_flags = 0;
 				err = 0;
 			}
-			mmc_do_release_host(host);
 		} else {
 			err = -EBUSY;
 		}
-- 
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH V2] mmc: core: Fix deadlock when the CONFIG_MMC_UNSAFE_RESUME is not defined

[Ulf Hansson]

Sujit Reddy Thumma wrote:
> mmc_suspend_host() tries to claim host during suspend
> and release it only when the bus suspend operation is
> compeleted. If CONFIG_MMC_UNSAFE_RESUME is defined and
> the host is flagged as removable, mmc_suspend_host()
> tries to remove the card. In this process, the file system
> sync can get blocked trying to acquire host which is already
> claimed by mmc_suspend_host() causing deadlock.
> 
> Fix this deadlock by releasing host before ->remove() is called.
> 
> Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org>
> 

Looks good to me!

Acked-by: Ulf Hansson <ulf.hansson@stericsson.com>

Re: [PATCH V2] mmc: core: Fix deadlock when the CONFIG_MMC_UNSAFE_RESUME is not defined

[Linus Walleij]

On Wed, Nov 23, 2011 at 4:13 AM, Sujit Reddy Thumma
<sthumma@codeaurora.org> wrote:

> mmc_suspend_host() tries to claim host during suspend
> and release it only when the bus suspend operation is
> compeleted. If CONFIG_MMC_UNSAFE_RESUME is defined and
> the host is flagged as removable, mmc_suspend_host()
> tries to remove the card. In this process, the file system
> sync can get blocked trying to acquire host which is already
> claimed by mmc_suspend_host() causing deadlock.
>
> Fix this deadlock by releasing host before ->remove() is called.
>
> Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org>

Looks correct!
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH V2] mmc: core: Fix deadlock when the CONFIG_MMC_UNSAFE_RESUME is not defined

[Chris Ball]

Hi,

On Tue, Nov 22 2011, Sujit Reddy Thumma wrote:
> mmc_suspend_host() tries to claim host during suspend
> and release it only when the bus suspend operation is
> compeleted. If CONFIG_MMC_UNSAFE_RESUME is defined and
> the host is flagged as removable, mmc_suspend_host()
> tries to remove the card. In this process, the file system
> sync can get blocked trying to acquire host which is already
> claimed by mmc_suspend_host() causing deadlock.
>
> Fix this deadlock by releasing host before ->remove() is called.
>
> Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org>
>
> ---
> Changes in v2:
> 	- Addressed review comment from Ulf Hansson.
> ---
>  drivers/mmc/core/core.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 271efea..950b97d 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -2333,6 +2333,8 @@ int mmc_suspend_host(struct mmc_host *host)
>  				mmc_poweroff_notify(host);
>  				err = host->bus_ops->suspend(host);
>  			}
> +			mmc_do_release_host(host);
> +
>  			if (err == -ENOSYS || !host->bus_ops->resume) {
>  				/*
>  				 * We simply "remove" the card in this case.
> @@ -2347,7 +2349,6 @@ int mmc_suspend_host(struct mmc_host *host)
>  				host->pm_flags = 0;
>  				err = 0;
>  			}
> -			mmc_do_release_host(host);
>  		} else {
>  			err = -EBUSY;
>  		}

Thanks very much, pushed to mmc-next for 3.2 with ACKs from Ulf and Linus.

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child