diff mbox

mmc: core: Avoid hanging to claim host for mmc via some nested calls

Message ID 1519730535-9991-1-git-send-email-ulf.hansson@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Ulf Hansson Feb. 27, 2018, 11:22 a.m. UTC
As the block layer, since the conversion to blkmq, claims the host using a
context, a following nested call to mmc_claim_host(), which isn't using a
context, may hang.

Calling mmc_interrupt_hpi() and mmc_read_bkops_status() via the mmc block
layer, may suffer from this problem, as these functions are calling
mmc_claim|release_host().

Let's fix the problem by removing the calls to mmc_claim|release_host()
from the above mentioned functions and instead make the callers responsible
of claiming/releasing the host. As a matter of fact, the existing callers
already deals with it.

Fixes: 81196976ed94 ("mmc: block: Add blk-mq support")
Reported-by: Dmitry Osipenko <digetx@gmail.com>
Suggested-by: Adrian Hunter <adrian.hunter@intel.com>
Tested-by: Dmitry Osipenko <digetx@gmail.com>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/core/mmc_ops.c | 4 ----
 1 file changed, 4 deletions(-)

Comments

Adrian Hunter Feb. 27, 2018, 11:40 a.m. UTC | #1
On 27/02/18 13:22, Ulf Hansson wrote:
> As the block layer, since the conversion to blkmq, claims the host using a
> context, a following nested call to mmc_claim_host(), which isn't using a
> context, may hang.
> 
> Calling mmc_interrupt_hpi() and mmc_read_bkops_status() via the mmc block
> layer, may suffer from this problem, as these functions are calling
> mmc_claim|release_host().
> 
> Let's fix the problem by removing the calls to mmc_claim|release_host()
> from the above mentioned functions and instead make the callers responsible
> of claiming/releasing the host. As a matter of fact, the existing callers
> already deals with it.
> 
> Fixes: 81196976ed94 ("mmc: block: Add blk-mq support")
> Reported-by: Dmitry Osipenko <digetx@gmail.com>
> Suggested-by: Adrian Hunter <adrian.hunter@intel.com>
> Tested-by: Dmitry Osipenko <digetx@gmail.com>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/core/mmc_ops.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index 908e4db..42d6aa8 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -848,7 +848,6 @@ int mmc_interrupt_hpi(struct mmc_card *card)
>  		return 1;
>  	}
>  
> -	mmc_claim_host(card->host);
>  	err = mmc_send_status(card, &status);
>  	if (err) {
>  		pr_err("%s: Get card status fail\n", mmc_hostname(card->host));
> @@ -890,7 +889,6 @@ int mmc_interrupt_hpi(struct mmc_card *card)
>  	} while (!err);
>  
>  out:
> -	mmc_release_host(card->host);
>  	return err;
>  }
>  
> @@ -932,9 +930,7 @@ static int mmc_read_bkops_status(struct mmc_card *card)
>  	int err;
>  	u8 *ext_csd;
>  
> -	mmc_claim_host(card->host);
>  	err = mmc_get_ext_csd(card, &ext_csd);
> -	mmc_release_host(card->host);
>  	if (err)
>  		return err;
>  
> 

--
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
Shawn Lin Feb. 27, 2018, 1:26 p.m. UTC | #2
On 2018/2/27 19:22, Ulf Hansson wrote:
> As the block layer, since the conversion to blkmq, claims the host using a
> context, a following nested call to mmc_claim_host(), which isn't using a
> context, may hang.
> 
> Calling mmc_interrupt_hpi() and mmc_read_bkops_status() via the mmc block
> layer, may suffer from this problem, as these functions are calling
> mmc_claim|release_host().
> 
> Let's fix the problem by removing the calls to mmc_claim|release_host()
> from the above mentioned functions and instead make the callers responsible
> of claiming/releasing the host. As a matter of fact, the existing callers
> already deals with it.
> 

Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>

> Fixes: 81196976ed94 ("mmc: block: Add blk-mq support")
> Reported-by: Dmitry Osipenko <digetx@gmail.com>
> Suggested-by: Adrian Hunter <adrian.hunter@intel.com>
> Tested-by: Dmitry Osipenko <digetx@gmail.com>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>   drivers/mmc/core/mmc_ops.c | 4 ----
>   1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index 908e4db..42d6aa8 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -848,7 +848,6 @@ int mmc_interrupt_hpi(struct mmc_card *card)
>   		return 1;
>   	}
>   
> -	mmc_claim_host(card->host);
>   	err = mmc_send_status(card, &status);
>   	if (err) {
>   		pr_err("%s: Get card status fail\n", mmc_hostname(card->host));
> @@ -890,7 +889,6 @@ int mmc_interrupt_hpi(struct mmc_card *card)
>   	} while (!err);
>   
>   out:
> -	mmc_release_host(card->host);
>   	return err;
>   }
>   
> @@ -932,9 +930,7 @@ static int mmc_read_bkops_status(struct mmc_card *card)
>   	int err;
>   	u8 *ext_csd;
>   
> -	mmc_claim_host(card->host);
>   	err = mmc_get_ext_csd(card, &ext_csd);
> -	mmc_release_host(card->host);
>   	if (err)
>   		return err;
>   
>
Ulf Hansson Feb. 27, 2018, 2:23 p.m. UTC | #3
On 27 February 2018 at 12:22, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> As the block layer, since the conversion to blkmq, claims the host using a
> context, a following nested call to mmc_claim_host(), which isn't using a
> context, may hang.
>
> Calling mmc_interrupt_hpi() and mmc_read_bkops_status() via the mmc block
> layer, may suffer from this problem, as these functions are calling
> mmc_claim|release_host().
>
> Let's fix the problem by removing the calls to mmc_claim|release_host()
> from the above mentioned functions and instead make the callers responsible
> of claiming/releasing the host. As a matter of fact, the existing callers
> already deals with it.
>
> Fixes: 81196976ed94 ("mmc: block: Add blk-mq support")
> Reported-by: Dmitry Osipenko <digetx@gmail.com>
> Suggested-by: Adrian Hunter <adrian.hunter@intel.com>
> Tested-by: Dmitry Osipenko <digetx@gmail.com>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

I have queued up this for fixes, thanks all involved for helping out!

Kind regards
Uffe

> ---
>  drivers/mmc/core/mmc_ops.c | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index 908e4db..42d6aa8 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -848,7 +848,6 @@ int mmc_interrupt_hpi(struct mmc_card *card)
>                 return 1;
>         }
>
> -       mmc_claim_host(card->host);
>         err = mmc_send_status(card, &status);
>         if (err) {
>                 pr_err("%s: Get card status fail\n", mmc_hostname(card->host));
> @@ -890,7 +889,6 @@ int mmc_interrupt_hpi(struct mmc_card *card)
>         } while (!err);
>
>  out:
> -       mmc_release_host(card->host);
>         return err;
>  }
>
> @@ -932,9 +930,7 @@ static int mmc_read_bkops_status(struct mmc_card *card)
>         int err;
>         u8 *ext_csd;
>
> -       mmc_claim_host(card->host);
>         err = mmc_get_ext_csd(card, &ext_csd);
> -       mmc_release_host(card->host);
>         if (err)
>                 return err;
>
> --
> 2.7.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
diff mbox

Patch

diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 908e4db..42d6aa8 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -848,7 +848,6 @@  int mmc_interrupt_hpi(struct mmc_card *card)
 		return 1;
 	}
 
-	mmc_claim_host(card->host);
 	err = mmc_send_status(card, &status);
 	if (err) {
 		pr_err("%s: Get card status fail\n", mmc_hostname(card->host));
@@ -890,7 +889,6 @@  int mmc_interrupt_hpi(struct mmc_card *card)
 	} while (!err);
 
 out:
-	mmc_release_host(card->host);
 	return err;
 }
 
@@ -932,9 +930,7 @@  static int mmc_read_bkops_status(struct mmc_card *card)
 	int err;
 	u8 *ext_csd;
 
-	mmc_claim_host(card->host);
 	err = mmc_get_ext_csd(card, &ext_csd);
-	mmc_release_host(card->host);
 	if (err)
 		return err;