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

[PATCH] 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>
---
 drivers/mmc/core/core.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 271efea..8adbadf 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2338,6 +2338,7 @@ int mmc_suspend_host(struct mmc_host *host)
 				 * We simply "remove" the card in this case.
 				 * It will be redetected on resume.
 				 */
+				mmc_do_release_host(host);
 				if (host->bus_ops->remove)
 					host->bus_ops->remove(host);
 				mmc_claim_host(host);
@@ -2346,8 +2347,9 @@ int mmc_suspend_host(struct mmc_host *host)
 				mmc_release_host(host);
 				host->pm_flags = 0;
 				err = 0;
+			} else {
+				mmc_do_release_host(host);
 			}
-			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] 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>
> ---
>  drivers/mmc/core/core.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 271efea..8adbadf 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -2338,6 +2338,7 @@ int mmc_suspend_host(struct mmc_host *host)
>  				 * We simply "remove" the card in this case.
>  				 * It will be redetected on resume.
>  				 */
> +				mmc_do_release_host(host);

Instead of having an "else" below move mmc_do_release_host to be done 
before "if (err == -ENOSYS || !host->bus_ops->resume)"

>  				if (host->bus_ops->remove)
>  					host->bus_ops->remove(host);
>  				mmc_claim_host(host);
> @@ -2346,8 +2347,9 @@ int mmc_suspend_host(struct mmc_host *host)
>  				mmc_release_host(host);
>  				host->pm_flags = 0;
>  				err = 0;
> +			} else {
> +				mmc_do_release_host(host);
>  			}

Remove else statement entirely.

> -			mmc_do_release_host(host);
>  		} else {
>  			err = -EBUSY;
>  		}

Otherwise it looks good to me! Thanks, for fixing this issue!

Br
Ulf Hansson

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

[Sujit Reddy Thumma]

On 11/22/2011 9:13 PM, Ulf Hansson wrote:
> 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>
>> ---
>> drivers/mmc/core/core.c | 4 +++-
>> 1 files changed, 3 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>> index 271efea..8adbadf 100644
>> --- a/drivers/mmc/core/core.c
>> +++ b/drivers/mmc/core/core.c
>> @@ -2338,6 +2338,7 @@ int mmc_suspend_host(struct mmc_host *host)
>> * We simply "remove" the card in this case.
>> * It will be redetected on resume.
>> */
>> + mmc_do_release_host(host);
>
> Instead of having an "else" below move mmc_do_release_host to be done
> before "if (err == -ENOSYS || !host->bus_ops->resume)"

Sure.

>
>> if (host->bus_ops->remove)
>> host->bus_ops->remove(host);
>> mmc_claim_host(host);
>> @@ -2346,8 +2347,9 @@ int mmc_suspend_host(struct mmc_host *host)
>> mmc_release_host(host);
>> host->pm_flags = 0;
>> err = 0;
>> + } else {
>> + mmc_do_release_host(host);
>> }
>
> Remove else statement entirely.

Okay. Will do it.


Thanks,
Sujit