|
|
Re: [RESEND PATCH V3] mmc: core: Detect card removal on I/O error |
Adrian Hunter wrote:
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.
Fixed a v4 patch then. Thanks.
----------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
|
|