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
State New
Headers show
Series
  • mmc: core: Avoid hanging to claim host for mmc via some nested calls
Related show

Commit Message

Ulf Hansson Feb. 27, 2018, 11:22 a.m.
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(-)

-- 
2.7.4

Comments

Adrian Hunter Feb. 27, 2018, 11:40 a.m. | #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. | #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;

>   

> 



-- 
Best Regards
Shawn Lin

--
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
Ulf Hansson Feb. 27, 2018, 2:23 p.m. | #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

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;