On 03/02/12 15:23, Ulf Hansson wrote: > Adrian Hunter wrote: >> On 03/02/12 14:16, Ulf Hansson wrote: >>> Adrian Hunter wrote: >>>> On 03/02/12 11:33, Ulf Hansson wrote: >>>>> To prevent I/O as soon as possible at card removal, a new detect work >>>>> is re-scheduled without a delay to let a rescan remove the card device >>>>> as soon a possible. >>>>> >>>>> Additionally, MMC_CAP2_DETECT_ON_ERR can now be used to handle "slowly" >>>>> removed cards that a scheduled detect work did not detect as removed. >>>>> To prevent further I/O requests for these lingering removed cards, >>>>> check if card has been removed and >>>>> then schedule a detect work to properly remove it. >>>>> >>>>> Signed-off-by: Ulf Hansson <ulf.hansson@xxxxxxxxxxxxxx> --- >>>>> >>>>> Changes in v3: - Check for card is NULL and minor code >>>>> simplifications. >>>>> >>>>> Changes in v2: - Updated according to review comments. - >>>>> Merging two patches for this feature into one. >>>>> >>>>> --- drivers/mmc/core/core.c | 24 +++++++++++++++++++++--- >>>>> include/linux/mmc/host.h | 1 + 2 files changed, 22 >>>>> insertions(+), 3 deletions(-) >>>>> >>>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index >>>>> bec0bf2..9645e8c 100644 --- a/drivers/mmc/core/core.c +++ >>>>> b/drivers/mmc/core/core.c @@ -2077,18 +2077,36 @@ 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); + + if (!card) + return 1; + >>>>> + ret = mmc_card_removed(card); /* * The card will be >>>>> considered unchanged unless we have been asked to * detect a >>>>> change or host requires polling to provide card detection. */ - >>>>> if (card && !host->detect_change && !(host->caps & >>>>> MMC_CAP_NEEDS_POLL)) - return mmc_card_removed(card); + >>>>> if (!host->detect_change && !(host->caps & MMC_CAP_NEEDS_POLL) >>>>> && + !(host->caps2 & MMC_CAP2_DETECT_ON_ERR)) + >>>>> return ret; >>>>> >>>>> host->detect_change = 0; + if (!ret) { + ret = >>>>> _mmc_detect_card_removed(host); + if (ret) { >>>> Please make this >>>> >>>> if (ret && (host->caps2 & MMC_CAP2_DETECT_ON_ERR)) >>>> >>>> because if rescan is already running this will needlessly queue another >>>> one. >>>> >>> It wont queue another one. It will cancel a work which likely has >>> been scheduled to be executed within several ms later. Then it will >>> re-schedule a new one without any delay since there is no need to >>> wait, when we already know that the card has been removed. >> >> It won't cancel a rescan that is already running (waiting to claim >> the host for example) but it will queue another one. Hence the >> comment. >> > > Do you think this is case that we need to bother about? It should be a > rare case and in worst case we only schedule a second not needed rescan, > which I believe should not be a problem. > > I discussed this with Jaehoon Chung, previously regarding the return value > of cancel_delayed_work(&host->detect). See below copied comments: > > I change accordingly if you like, please let me know. Again. :-) Yes please. > > ---------- > >>>> 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 >> > > --------------------- > -- 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