Re: [RESEND PATCH V3] mmc: core: Detect card removal on I/O error

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

 



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


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

  Powered by Linux