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