On 02/01/2012 06:40 PM, Ulf Hansson wrote:
> Jaehoon Chung wrote:
>> Hi Ulf.
>> On 02/01/2012 12:23 AM, Ulf Hansson wrote:
>>
>>> Adrian Hunter wrote:
>>>> On 31/01/12 14:54, Ulf Hansson wrote:
>>>>> Adrian Hunter wrote:
>>>>>> On 19/01/12 18:39, Ulf Hansson wrote:
>>>>>>> Once the card has been detected to be removed by the
>>>>>>> mmc_detect_card_removed function, schedule a new detect work
>>>>>>> immediately and without a delay to let a rescan remove the
>>>>>>> card device as soon a possible. This will sooner prevent
>>>>>>> further I/O requests.
>>>>>>>
>>>>>>> Signed-off-by: Ulf Hansson <ulf.hans...@stericsson.com>
>>>>>>> ---
>>>>>>> drivers/mmc/core/core.c | 16 ++++++++++++++--
>>>>>>> 1 files changed, 14 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>>>>>> index bec0bf2..265dfd8 100644
>>>>>>> --- a/drivers/mmc/core/core.c
>>>>>>> +++ b/drivers/mmc/core/core.c
>>>>>>> @@ -2077,6 +2077,7 @@ int _mmc_detect_card_removed(struct mmc_host
>>>>>>> *host)
>>>>>>> int mmc_detect_card_removed(struct mmc_host *host)
>>>>>>> {
>>>>>>> struct mmc_card *card = host->card;
>>>>>>> + int ret;
>>>>>>>
>>>>>>> WARN_ON(!host->claimed);
>>>>>>> /*
>>>>>>> @@ -2086,9 +2087,20 @@ int mmc_detect_card_removed(struct mmc_host
>>>>>>> *host)
>>>>>>> if (card && !host->detect_change && !(host->caps &
>>>>>>> MMC_CAP_NEEDS_POLL))
>>>>>>> return mmc_card_removed(card);
>>>>>>>
>>>>>>> - host->detect_change = 0;
>>>>>> That line should not be removed. It is not related to your change.
>>>>> I think it is. Since my patch is trying to make it possible to "prevent
>>>>> I/O as soon as possible..."
>>>> No, the value of detect_change does not affect the outcome
>>>> if MMC_CAP2_DETECT_ON_ERR is set i.e.:
>>>>
>>>> if (card && !host->detect_change && !(host->caps & MMC_CAP_NEEDS_POLL)
>>>> && !(host->caps2 & MMC_CAP2_DETECT_ON_ERR))
>>>>
>>>> is always false if (host->caps2 & MMC_CAP2_DETECT_ON_ERR) is true
>>> You are right! But MMC_CAP2_DETECT_ON_ERR is introduced in the second
>>> patch, so we should not consider that in this patch I think!?
>>>
>>>>> Clearing the detect_change flag here will prevent the I/O layer from
>>>>> doing further tests to see if the card is removed by using
>>>>> "mmc_detect_card_removed -> _mmc_detect_card_removed" due to the upper if
>>>>> sentence.
>>>>>
>>>>> I think this flag should only be cleared from the mmc_rescan function.
>>>>>
>>>>>>> + ret = mmc_card_removed(card);
>>>>>> Calling mmc_card_removed() is not needed here since
>>>>>> _mmc_detect_card_removed() does it anyway.
>>>>>>
>>>>>>> + if (!ret) {
>>>>>>> + ret = _mmc_detect_card_removed(host);
>>>>>>> + if (ret) {
>>>>>>> + /*
>>>>>>> + * Schedule a detect work as soon as possible to let a
>>>>>>> + * rescan handle the card removal.
>>>>>>> + */
>>>>>>> + cancel_delayed_work(&host->detect);
>>>>>> Why cancel the detect work?
>>>>> To "prevent I/O as soon as possible...".
>>>>>
>>>>> The detect work could have been scheduled to be run at several ms later.
>>>>> There is no need to wait for it since we already now that card will be
>>>>> removed when the rescan function will execute.
>>
>> if (cancel_delayed_work(&host->detect))
>> mmc_detect_change(host, 0);
>> isn't?
>
> Good comment. That will mean that patch 2 will have to be updated as well to
> something like below.
>
> if (cancel_delayed_work(&host->detect) || (host->caps2 &
> MMC_CAP2_DETECT_ON_ERR))
> mmc_detect_change(host, 0);
>
> What do you think?
>
> Could we skip this entirely and leave it as is without checking the return
> value of cancel_delayed_work? That will only mean that in some very rare
> cases (since rescan is clearing the detect_change flag) one additional detect
> work will be triggered which shall not cause any problems I believe. But I
> happily change to what you propose if you prefer!
Right...maybe..don't cause any problems..i also think. It's rare case. :)
Best Regards,
Jaehoon Chung
>
>>
>>>>>>> + mmc_detect_change(host, 0);
>>>>>>> + }
>>>>>>> + }
>>>>>>>
>>>>>>> - return _mmc_detect_card_removed(host);
>>>>>>> + return ret;
>>>>>>> }
>>>>>>> EXPORT_SYMBOL(mmc_detect_card_removed);
>>>>>>>
>>>>> Br
>>>>> Ulf Hansson
>>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>>> the body of a message to majord...@vger.kernel.org
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>> the body of a message to majord...@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>>
>>
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
