[PATCH] mmc: block: release host in case of error

[PATCH] mmc: block: release host in case of error

[Per Forlin]

Host is claimed as long as there are requests in the block queue
and all request are completed successfully. If an error occur release
the host in case someone else needs to claim it, for instance if the card
is removed during a transfer.

Signed-off-by: Per Forlin <per.forlin@stericsson.com>
---
 drivers/mmc/card/block.c |   37 +++++++++++++++++++++++++++++--------
 1 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index c80bb6d..c21fd2c 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -1158,6 +1158,28 @@ static int mmc_blk_cmd_err(struct mmc_blk_data *md, struct mmc_card *card,
 	return ret;
 }
 
+/*
+ * This function should be called to resend a request after failure.
+ * Prepares and starts the request.
+ */
+static inline struct mmc_async_req *mmc_blk_resend(struct mmc_card *card,
+						   struct mmc_queue *mq,
+						   struct mmc_queue_req *mqrq,
+						   int disable_multi,
+						   struct mmc_async_req *areq)
+{
+	/*
+	 * Release host after failure in case the host is needed
+	 * by someone else. For instance, if the card is removed the
+	 * worker thread needs to claim the host in order to do mmc_rescan.
+	 */
+	mmc_release_host(card->host);
+	mmc_claim_host(card->host);
+
+	mmc_blk_rw_rq_prep(mqrq, card, disable_multi, mq);
+	return mmc_start_req(card->host, areq, NULL);
+}
+
 static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
 {
 	struct mmc_blk_data *md = mq->data;
@@ -1257,14 +1279,14 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
 			break;
 		}
 
-		if (ret) {
+		if (ret)
 			/*
 			 * In case of a incomplete request
 			 * prepare it again and resend.
 			 */
-			mmc_blk_rw_rq_prep(mq_rq, card, disable_multi, mq);
-			mmc_start_req(card->host, &mq_rq->mmc_active, NULL);
-		}
+			mmc_blk_resend(card, mq, mq_rq, disable_multi,
+				       &mq_rq->mmc_active);
+
 	} while (ret);
 
 	return 1;
@@ -1276,10 +1298,9 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
 	spin_unlock_irq(&md->lock);
 
  start_new_req:
-	if (rqc) {
-		mmc_blk_rw_rq_prep(mq->mqrq_cur, card, 0, mq);
-		mmc_start_req(card->host, &mq->mqrq_cur->mmc_active, NULL);
-	}
+	if (rqc)
+		mmc_blk_resend(card, mq, mq->mqrq_cur, 0,
+			       &mq->mqrq_cur->mmc_active);
 
 	return 0;
 }
-- 
1.6.3.3

Re: [PATCH] mmc: block: release host in case of error

[Sujit Reddy Thumma]

On 11/14/2011 4:42 PM, Per Forlin wrote:
> Host is claimed as long as there are requests in the block queue
> and all request are completed successfully. If an error occur release
> the host in case someone else needs to claim it, for instance if the card
> is removed during a transfer.
>
> Signed-off-by: Per Forlin<per.forlin@stericsson.com>
> ---
>   drivers/mmc/card/block.c |   37 +++++++++++++++++++++++++++++--------
>   1 files changed, 29 insertions(+), 8 deletions(-)

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


Thanks
Sujit

Re: [PATCH] mmc: block: release host in case of error

[Linus Walleij]

On Mon, Nov 14, 2011 at 12:12 PM, Per Forlin <per.forlin@stericsson.com> wrote:

> Host is claimed as long as there are requests in the block queue
> and all request are completed successfully. If an error occur release
> the host in case someone else needs to claim it, for instance if the card
> is removed during a transfer.
>
> Signed-off-by: Per Forlin <per.forlin@stericsson.com>

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

Thanks,
Linus Walleij

Re: [PATCH] mmc: block: release host in case of error

[Adrian Hunter]

On 14/11/11 13:12, Per Forlin wrote:
> Host is claimed as long as there are requests in the block queue
> and all request are completed successfully. If an error occur release
> the host in case someone else needs to claim it, for instance if the card
> is removed during a transfer.
> 
> Signed-off-by: Per Forlin <per.forlin@stericsson.com>
> ---
>  drivers/mmc/card/block.c |   37 +++++++++++++++++++++++++++++--------
>  1 files changed, 29 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index c80bb6d..c21fd2c 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -1158,6 +1158,28 @@ static int mmc_blk_cmd_err(struct mmc_blk_data *md, struct mmc_card *card,
>  	return ret;
>  }
>  
> +/*
> + * This function should be called to resend a request after failure.
> + * Prepares and starts the request.
> + */
> +static inline struct mmc_async_req *mmc_blk_resend(struct mmc_card *card,
> +						   struct mmc_queue *mq,
> +						   struct mmc_queue_req *mqrq,
> +						   int disable_multi,
> +						   struct mmc_async_req *areq)
> +{
> +	/*
> +	 * Release host after failure in case the host is needed
> +	 * by someone else. For instance, if the card is removed the
> +	 * worker thread needs to claim the host in order to do mmc_rescan.
> +	 */
> +	mmc_release_host(card->host);
> +	mmc_claim_host(card->host);

Does this work?  Won't the current thread win the race
to claim the host again?


> +
> +	mmc_blk_rw_rq_prep(mqrq, card, disable_multi, mq);
> +	return mmc_start_req(card->host, areq, NULL);
> +}
> +
>  static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
>  {
>  	struct mmc_blk_data *md = mq->data;
> @@ -1257,14 +1279,14 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
>  			break;
>  		}
>  
> -		if (ret) {
> +		if (ret)
>  			/*
>  			 * In case of a incomplete request
>  			 * prepare it again and resend.
>  			 */
> -			mmc_blk_rw_rq_prep(mq_rq, card, disable_multi, mq);
> -			mmc_start_req(card->host, &mq_rq->mmc_active, NULL);
> -		}
> +			mmc_blk_resend(card, mq, mq_rq, disable_multi,
> +				       &mq_rq->mmc_active);
> +
>  	} while (ret);
>  
>  	return 1;
> @@ -1276,10 +1298,9 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
>  	spin_unlock_irq(&md->lock);
>  
>   start_new_req:
> -	if (rqc) {
> -		mmc_blk_rw_rq_prep(mq->mqrq_cur, card, 0, mq);
> -		mmc_start_req(card->host, &mq->mqrq_cur->mmc_active, NULL);
> -	}
> +	if (rqc)
> +		mmc_blk_resend(card, mq, mq->mqrq_cur, 0,
> +			       &mq->mqrq_cur->mmc_active);
>  
>  	return 0;
>  }

Re: [PATCH] mmc: block: release host in case of error

[Per Förlin]

On 11/17/2011 10:18 AM, Adrian Hunter wrote:
> On 14/11/11 13:12, Per Forlin wrote:
>> Host is claimed as long as there are requests in the block queue
>> and all request are completed successfully. If an error occur release
>> the host in case someone else needs to claim it, for instance if the card
>> is removed during a transfer.
>>
>> Signed-off-by: Per Forlin <per.forlin@stericsson.com>
>> ---
>>  drivers/mmc/card/block.c |   37 +++++++++++++++++++++++++++++--------
>>  1 files changed, 29 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>> index c80bb6d..c21fd2c 100644
>> --- a/drivers/mmc/card/block.c
>> +++ b/drivers/mmc/card/block.c
>> @@ -1158,6 +1158,28 @@ static int mmc_blk_cmd_err(struct mmc_blk_data *md, struct mmc_card *card,
>>  	return ret;
>>  }
>>  
>> +/*
>> + * This function should be called to resend a request after failure.
>> + * Prepares and starts the request.
>> + */
>> +static inline struct mmc_async_req *mmc_blk_resend(struct mmc_card *card,
>> +						   struct mmc_queue *mq,
>> +						   struct mmc_queue_req *mqrq,
>> +						   int disable_multi,
>> +						   struct mmc_async_req *areq)
>> +{
>> +	/*
>> +	 * Release host after failure in case the host is needed
>> +	 * by someone else. For instance, if the card is removed the
>> +	 * worker thread needs to claim the host in order to do mmc_rescan.
>> +	 */
>> +	mmc_release_host(card->host);
>> +	mmc_claim_host(card->host);
> 
> Does this work?  Won't the current thread win the race
> to claim the host again?
> 
Good question. I've tested it and I haven't seen any cases where current has claimed the host again. Sujit has tested the patch as well.
But I can't say that your scenario can't happen. I will study the wake_up and wait_queue code to see if I can find the answer.

Thanks,
Per

Re: [PATCH] mmc: block: release host in case of error

[Per Forlin]

Hi Adrian,

On Fri, Nov 18, 2011 at 10:56 AM, Per Förlin <per.forlin@stericsson.com> wrote:
> On 11/17/2011 10:18 AM, Adrian Hunter wrote:
>> On 14/11/11 13:12, Per Forlin wrote:
>>> Host is claimed as long as there are requests in the block queue
>>> and all request are completed successfully. If an error occur release
>>> the host in case someone else needs to claim it, for instance if the card
>>> is removed during a transfer.
>>>
>>> Signed-off-by: Per Forlin <per.forlin@stericsson.com>
>>> ---
>>>  drivers/mmc/card/block.c |   37 +++++++++++++++++++++++++++++--------
>>>  1 files changed, 29 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>>> index c80bb6d..c21fd2c 100644
>>> --- a/drivers/mmc/card/block.c
>>> +++ b/drivers/mmc/card/block.c
>>> @@ -1158,6 +1158,28 @@ static int mmc_blk_cmd_err(struct mmc_blk_data *md, struct mmc_card *card,
>>>      return ret;
>>>  }
>>>
>>> +/*
>>> + * This function should be called to resend a request after failure.
>>> + * Prepares and starts the request.
>>> + */
>>> +static inline struct mmc_async_req *mmc_blk_resend(struct mmc_card *card,
>>> +                                               struct mmc_queue *mq,
>>> +                                               struct mmc_queue_req *mqrq,
>>> +                                               int disable_multi,
>>> +                                               struct mmc_async_req *areq)
>>> +{
>>> +    /*
>>> +     * Release host after failure in case the host is needed
>>> +     * by someone else. For instance, if the card is removed the
>>> +     * worker thread needs to claim the host in order to do mmc_rescan.
>>> +     */
>>> +    mmc_release_host(card->host);
>>> +    mmc_claim_host(card->host);
>>
>> Does this work?  Won't the current thread win the race
>> to claim the host again?
>>
> Good question. I've tested it and I haven't seen any cases where current has claimed the host again. Sujit has tested the patch as well.
> But I can't say that your scenario can't happen. I will study the wake_up and wait_queue code to see if I can find the answer.
>

mmc_release_host() -> wake_up() -> schedule(). If the waking process
has higher prio than current it will preempt current on NOSMP. If SMP,
current and waking process may be on separate CPUs and in that case
it's difficult to guarantee that the waking process will win the race.
I'm proposing to add yield() in order to give the waking process
better chances to win the race.
Here's a patch:
--------------------------------
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index c21fd2c..add1c38 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -1173,8 +1173,11 @@ static inline struct mmc_async_req
*mmc_blk_resend(struct mmc_card *card,
         * by someone else. For instance, if the card is removed the
         * worker thread needs to claim the host in order to do mmc_rescan.
         */
-       mmc_release_host(card->host);
-       mmc_claim_host(card->host);
+       if (mmc_card_rescan(card)) {
+               mmc_release_host(card->host);
+               yield();
+               mmc_claim_host(card->host);
+       }

        mmc_blk_rw_rq_prep(mqrq, card, disable_multi, mq);
        return mmc_start_req(card->host, areq, NULL);
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 271efea..83f03a3 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2059,6 +2059,8 @@ void mmc_rescan(struct work_struct *work)
        if (host->rescan_disable)
                return;

+       mmc_card_set_rescan(host->card);
+


        /*
@@ -2101,6 +2103,7 @@


  out:
+       mmc_card_clr_rescan(host->card);


 }
-----------------------

Thanks,
Per

Re: [PATCH] mmc: block: release host in case of error

[Per Forlin]

On Sun, Nov 20, 2011 at 9:50 PM, Per Forlin <per.lkml@gmail.com> wrote:
> Hi Adrian,
>
> On Fri, Nov 18, 2011 at 10:56 AM, Per Förlin <per.forlin@stericsson.com> wrote:
>> On 11/17/2011 10:18 AM, Adrian Hunter wrote:
>>> On 14/11/11 13:12, Per Forlin wrote:
>>>> Host is claimed as long as there are requests in the block queue
>>>> and all request are completed successfully. If an error occur release
>>>> the host in case someone else needs to claim it, for instance if the card
>>>> is removed during a transfer.
>>>>
>>>> Signed-off-by: Per Forlin <per.forlin@stericsson.com>
>>>> ---
>>>>  drivers/mmc/card/block.c |   37 +++++++++++++++++++++++++++++--------
>>>>  1 files changed, 29 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>>>> index c80bb6d..c21fd2c 100644
>>>> --- a/drivers/mmc/card/block.c
>>>> +++ b/drivers/mmc/card/block.c
>>>> @@ -1158,6 +1158,28 @@ static int mmc_blk_cmd_err(struct mmc_blk_data *md, struct mmc_card *card,
>>>>      return ret;
>>>>  }
>>>>
>>>> +/*
>>>> + * This function should be called to resend a request after failure.
>>>> + * Prepares and starts the request.
>>>> + */
>>>> +static inline struct mmc_async_req *mmc_blk_resend(struct mmc_card *card,
>>>> +                                               struct mmc_queue *mq,
>>>> +                                               struct mmc_queue_req *mqrq,
>>>> +                                               int disable_multi,
>>>> +                                               struct mmc_async_req *areq)
>>>> +{
>>>> +    /*
>>>> +     * Release host after failure in case the host is needed
>>>> +     * by someone else. For instance, if the card is removed the
>>>> +     * worker thread needs to claim the host in order to do mmc_rescan.
>>>> +     */
>>>> +    mmc_release_host(card->host);
>>>> +    mmc_claim_host(card->host);
>>>
>>> Does this work?  Won't the current thread win the race
>>> to claim the host again?
>>>
>> Good question. I've tested it and I haven't seen any cases where current has claimed the host again. Sujit has tested the patch as well.
>> But I can't say that your scenario can't happen. I will study the wake_up and wait_queue code to see if I can find the answer.
>>
>
> mmc_release_host() -> wake_up() -> schedule(). If the waking process
> has higher prio than current it will preempt current on NOSMP. If SMP,
> current and waking process may be on separate CPUs and in that case
> it's difficult to guarantee that the waking process will win the race.
> I'm proposing to add yield() in order to give the waking process
> better chances to win the race.
> Here's a patch:
> --------------------------------
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index c21fd2c..add1c38 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -1173,8 +1173,11 @@ static inline struct mmc_async_req
> *mmc_blk_resend(struct mmc_card *card,
>         * by someone else. For instance, if the card is removed the
>         * worker thread needs to claim the host in order to do mmc_rescan.
>         */
> -       mmc_release_host(card->host);
> -       mmc_claim_host(card->host);
> +       if (mmc_card_rescan(card)) {
> +               mmc_release_host(card->host);
> +               yield();
> +               mmc_claim_host(card->host);
> +       }
>
>        mmc_blk_rw_rq_prep(mqrq, card, disable_multi, mq);
>        return mmc_start_req(card->host, areq, NULL);
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 271efea..83f03a3 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -2059,6 +2059,8 @@ void mmc_rescan(struct work_struct *work)
>        if (host->rescan_disable)
>                return;
>
> +       mmc_card_set_rescan(host->card);
> +
>
>
>        /*
> @@ -2101,6 +2103,7 @@
>
>
>  out:
> +       mmc_card_clr_rescan(host->card);
>
>
>  }
> -----------------------
I'm not sure if this patch-extension is really needed, it may only
make the patch more complex. If the race condition Adrian refers to is
unlikely, there may be a few extra retries before mmc_rescan get the
chance to claim the host.
I'm in favor of skipping my proposed extension and staying with the
original v1 patch.
Adrian, what do you say?

Thanks,
Per

Re: [PATCH] mmc: block: release host in case of error

[Adrian Hunter]

On 24/11/11 20:58, Per Forlin wrote:
> On Sun, Nov 20, 2011 at 9:50 PM, Per Forlin <per.lkml@gmail.com> wrote:
>> Hi Adrian,
>>
>> On Fri, Nov 18, 2011 at 10:56 AM, Per Förlin <per.forlin@stericsson.com> wrote:
>>> On 11/17/2011 10:18 AM, Adrian Hunter wrote:
>>>> On 14/11/11 13:12, Per Forlin wrote:
>>>>> Host is claimed as long as there are requests in the block queue
>>>>> and all request are completed successfully. If an error occur release
>>>>> the host in case someone else needs to claim it, for instance if the card
>>>>> is removed during a transfer.
>>>>>
>>>>> Signed-off-by: Per Forlin <per.forlin@stericsson.com>
>>>>> ---
>>>>>  drivers/mmc/card/block.c |   37 +++++++++++++++++++++++++++++--------
>>>>>  1 files changed, 29 insertions(+), 8 deletions(-)
>>>>>
>>>>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>>>>> index c80bb6d..c21fd2c 100644
>>>>> --- a/drivers/mmc/card/block.c
>>>>> +++ b/drivers/mmc/card/block.c
>>>>> @@ -1158,6 +1158,28 @@ static int mmc_blk_cmd_err(struct mmc_blk_data *md, struct mmc_card *card,
>>>>>      return ret;
>>>>>  }
>>>>>
>>>>> +/*
>>>>> + * This function should be called to resend a request after failure.
>>>>> + * Prepares and starts the request.
>>>>> + */
>>>>> +static inline struct mmc_async_req *mmc_blk_resend(struct mmc_card *card,
>>>>> +                                               struct mmc_queue *mq,
>>>>> +                                               struct mmc_queue_req *mqrq,
>>>>> +                                               int disable_multi,
>>>>> +                                               struct mmc_async_req *areq)
>>>>> +{
>>>>> +    /*
>>>>> +     * Release host after failure in case the host is needed
>>>>> +     * by someone else. For instance, if the card is removed the
>>>>> +     * worker thread needs to claim the host in order to do mmc_rescan.
>>>>> +     */
>>>>> +    mmc_release_host(card->host);
>>>>> +    mmc_claim_host(card->host);
>>>>
>>>> Does this work?  Won't the current thread win the race
>>>> to claim the host again?
>>>>
>>> Good question. I've tested it and I haven't seen any cases where current has claimed the host again. Sujit has tested the patch as well.
>>> But I can't say that your scenario can't happen. I will study the wake_up and wait_queue code to see if I can find the answer.
>>>
>>
>> mmc_release_host() -> wake_up() -> schedule(). If the waking process
>> has higher prio than current it will preempt current on NOSMP. If SMP,
>> current and waking process may be on separate CPUs and in that case
>> it's difficult to guarantee that the waking process will win the race.
>> I'm proposing to add yield() in order to give the waking process
>> better chances to win the race.
>> Here's a patch:
>> --------------------------------
>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>> index c21fd2c..add1c38 100644
>> --- a/drivers/mmc/card/block.c
>> +++ b/drivers/mmc/card/block.c
>> @@ -1173,8 +1173,11 @@ static inline struct mmc_async_req
>> *mmc_blk_resend(struct mmc_card *card,
>>         * by someone else. For instance, if the card is removed the
>>         * worker thread needs to claim the host in order to do mmc_rescan.
>>         */
>> -       mmc_release_host(card->host);
>> -       mmc_claim_host(card->host);
>> +       if (mmc_card_rescan(card)) {
>> +               mmc_release_host(card->host);
>> +               yield();
>> +               mmc_claim_host(card->host);
>> +       }
>>
>>        mmc_blk_rw_rq_prep(mqrq, card, disable_multi, mq);
>>        return mmc_start_req(card->host, areq, NULL);
>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>> index 271efea..83f03a3 100644
>> --- a/drivers/mmc/core/core.c
>> +++ b/drivers/mmc/core/core.c
>> @@ -2059,6 +2059,8 @@ void mmc_rescan(struct work_struct *work)
>>        if (host->rescan_disable)
>>                return;
>>
>> +       mmc_card_set_rescan(host->card);
>> +
>>
>>
>>        /*
>> @@ -2101,6 +2103,7 @@
>>
>>
>>  out:
>> +       mmc_card_clr_rescan(host->card);
>>
>>
>>  }
>> -----------------------
> I'm not sure if this patch-extension is really needed, it may only
> make the patch more complex. If the race condition Adrian refers to is
> unlikely, there may be a few extra retries before mmc_rescan get the
> chance to claim the host.
> I'm in favor of skipping my proposed extension and staying with the
> original v1 patch.
> Adrian, what do you say?

As far as I can see, if mmc block is checking / setting whether the
card has been removed, then mmc_blk_resend would not be needed.

Re: [PATCH] mmc: block: release host in case of error

[Per Forlin]

On Fri, Nov 25, 2011 at 1:03 PM, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 24/11/11 20:58, Per Forlin wrote:
>> On Sun, Nov 20, 2011 at 9:50 PM, Per Forlin <per.lkml@gmail.com> wrote:
>>> Hi Adrian,
>>>
>>> On Fri, Nov 18, 2011 at 10:56 AM, Per Förlin <per.forlin@stericsson.com> wrote:
>>>> On 11/17/2011 10:18 AM, Adrian Hunter wrote:
>>>>> On 14/11/11 13:12, Per Forlin wrote:
>>>>>> Host is claimed as long as there are requests in the block queue
>>>>>> and all request are completed successfully. If an error occur release
>>>>>> the host in case someone else needs to claim it, for instance if the card
>>>>>> is removed during a transfer.
>>>>>>
>>>>>> Signed-off-by: Per Forlin <per.forlin@stericsson.com>
>>>>>> ---
>>>>>>  drivers/mmc/card/block.c |   37 +++++++++++++++++++++++++++++--------
>>>>>>  1 files changed, 29 insertions(+), 8 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>>>>>> index c80bb6d..c21fd2c 100644
>>>>>> --- a/drivers/mmc/card/block.c
>>>>>> +++ b/drivers/mmc/card/block.c
>>>>>> @@ -1158,6 +1158,28 @@ static int mmc_blk_cmd_err(struct mmc_blk_data *md, struct mmc_card *card,
>>>>>>      return ret;
>>>>>>  }
>>>>>>
>>>>>> +/*
>>>>>> + * This function should be called to resend a request after failure.
>>>>>> + * Prepares and starts the request.
>>>>>> + */
>>>>>> +static inline struct mmc_async_req *mmc_blk_resend(struct mmc_card *card,
>>>>>> +                                               struct mmc_queue *mq,
>>>>>> +                                               struct mmc_queue_req *mqrq,
>>>>>> +                                               int disable_multi,
>>>>>> +                                               struct mmc_async_req *areq)
>>>>>> +{
>>>>>> +    /*
>>>>>> +     * Release host after failure in case the host is needed
>>>>>> +     * by someone else. For instance, if the card is removed the
>>>>>> +     * worker thread needs to claim the host in order to do mmc_rescan.
>>>>>> +     */
>>>>>> +    mmc_release_host(card->host);
>>>>>> +    mmc_claim_host(card->host);
>>>>>
>>>>> Does this work?  Won't the current thread win the race
>>>>> to claim the host again?
>>>>>
>>>> Good question. I've tested it and I haven't seen any cases where current has claimed the host again. Sujit has tested the patch as well.
>>>> But I can't say that your scenario can't happen. I will study the wake_up and wait_queue code to see if I can find the answer.
>>>>
>>>
>>> mmc_release_host() -> wake_up() -> schedule(). If the waking process
>>> has higher prio than current it will preempt current on NOSMP. If SMP,
>>> current and waking process may be on separate CPUs and in that case
>>> it's difficult to guarantee that the waking process will win the race.
>>> I'm proposing to add yield() in order to give the waking process
>>> better chances to win the race.
>>> Here's a patch:
>>> --------------------------------
>>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>>> index c21fd2c..add1c38 100644
>>> --- a/drivers/mmc/card/block.c
>>> +++ b/drivers/mmc/card/block.c
>>> @@ -1173,8 +1173,11 @@ static inline struct mmc_async_req
>>> *mmc_blk_resend(struct mmc_card *card,
>>>         * by someone else. For instance, if the card is removed the
>>>         * worker thread needs to claim the host in order to do mmc_rescan.
>>>         */
>>> -       mmc_release_host(card->host);
>>> -       mmc_claim_host(card->host);
>>> +       if (mmc_card_rescan(card)) {
>>> +               mmc_release_host(card->host);
>>> +               yield();
>>> +               mmc_claim_host(card->host);
>>> +       }
>>>
>>>        mmc_blk_rw_rq_prep(mqrq, card, disable_multi, mq);
>>>        return mmc_start_req(card->host, areq, NULL);
>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>> index 271efea..83f03a3 100644
>>> --- a/drivers/mmc/core/core.c
>>> +++ b/drivers/mmc/core/core.c
>>> @@ -2059,6 +2059,8 @@ void mmc_rescan(struct work_struct *work)
>>>        if (host->rescan_disable)
>>>                return;
>>>
>>> +       mmc_card_set_rescan(host->card);
>>> +
>>>
>>>
>>>        /*
>>> @@ -2101,6 +2103,7 @@
>>>
>>>
>>>  out:
>>> +       mmc_card_clr_rescan(host->card);
>>>
>>>
>>>  }
>>> -----------------------
>> I'm not sure if this patch-extension is really needed, it may only
>> make the patch more complex. If the race condition Adrian refers to is
>> unlikely, there may be a few extra retries before mmc_rescan get the
>> chance to claim the host.
>> I'm in favor of skipping my proposed extension and staying with the
>> original v1 patch.
>> Adrian, what do you say?
>
> As far as I can see, if mmc block is checking / setting whether the
> card has been removed, then mmc_blk_resend would not be needed.
>
I agree. The intention of this patch is only top let mmc_rescan claim the host.
Flow: card detect IRQ -> mmc_detect_change -> mmc_rescan -> mmc_claim_host

If doing this check in mmc block instead this patch is not needed.
Let's wait and see what comes out of the patch "mmc: Kill block
requests if card is removed".

Thanks,
Per