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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Adrian,

On Fri, Nov 18, 2011 at 10:56 AM, Per Förlin <per.forlin@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx>
>>> ---
>>>  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
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux