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

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

[Ulf Hansson]

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@stericsson.com>
---

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) {
+			/*
+			 * Schedule a detect work as soon as possible to let a
+			 * rescan handle the card removal.
+			 */
+			cancel_delayed_work(&host->detect);
+			mmc_detect_change(host, 0);
+		}
+	}
 
-	return _mmc_detect_card_removed(host);
+	return ret;
 }
 EXPORT_SYMBOL(mmc_detect_card_removed);
 
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index dd13e05..368a2b9 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -257,6 +257,7 @@ struct mmc_host {
 #define MMC_CAP2_HS200_1_2V_SDR	(1 << 6)        /* can support */
 #define MMC_CAP2_HS200		(MMC_CAP2_HS200_1_8V_SDR | \
 				 MMC_CAP2_HS200_1_2V_SDR)
+#define MMC_CAP2_DETECT_ON_ERR	(1 << 7)	/* On I/O err check card removal */
 
 	mmc_pm_flag_t		pm_caps;	/* supported pm features */
 	unsigned int        power_notify_type;
-- 
1.7.5.4

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

[Namjae Jeon]

2012/2/3 Ulf Hansson <ulf.hansson@stericsson.com>:
> 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@stericsson.com>
Reviewed-by: Namjae Jeon <linkinjeon@gmail.com>

Hi. Ulf.
Looks good to me.
Thanks.
> ---

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

[Jaehoon Chung]

On 02/03/2012 06:33 PM, 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@stericsson.com>
> ---
> 
> 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);

this function called mmc_detect_card_removed(card->host) in block.c.
then card isn't NULL..isn't? 
Just wondering... :)

Best Regards,
Jaehoon Chung

>  	/*
>  	 * 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) {
> +			/*
> +			 * Schedule a detect work as soon as possible to let a
> +			 * rescan handle the card removal.
> +			 */
> +			cancel_delayed_work(&host->detect);
> +			mmc_detect_change(host, 0);
> +		}
> +	}
>  
> -	return _mmc_detect_card_removed(host);
> +	return ret;
>  }
>  EXPORT_SYMBOL(mmc_detect_card_removed);
>  
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index dd13e05..368a2b9 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -257,6 +257,7 @@ struct mmc_host {
>  #define MMC_CAP2_HS200_1_2V_SDR	(1 << 6)        /* can support */
>  #define MMC_CAP2_HS200		(MMC_CAP2_HS200_1_8V_SDR | \
>  				 MMC_CAP2_HS200_1_2V_SDR)
> +#define MMC_CAP2_DETECT_ON_ERR	(1 << 7)	/* On I/O err check card removal */
>  
>  	mmc_pm_flag_t		pm_caps;	/* supported pm features */
>  	unsigned int        power_notify_type;

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

[Namjae Jeon]

2012/2/3 Jaehoon Chung <jh80.chung@samsung.com>:
> On 02/03/2012 06:33 PM, 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@stericsson.com>
>> ---
>>
>> 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);
>
> this function called mmc_detect_card_removed(card->host) in block.c.
> then card isn't NULL..isn't?
> Just wondering... :)
If we think this function is used anywhere else next time, it is reasonable.
And old code also checked null.
>
> Best Regards,
> Jaehoon Chung
>
>>       /*
>>        * 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) {
>> +                     /*
>> +                      * Schedule a detect work as soon as possible to let a
>> +                      * rescan handle the card removal.
>> +                      */
>> +                     cancel_delayed_work(&host->detect);
>> +                     mmc_detect_change(host, 0);
>> +             }
>> +     }
>>
>> -     return _mmc_detect_card_removed(host);
>> +     return ret;
>>  }
>>  EXPORT_SYMBOL(mmc_detect_card_removed);
>>
>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>> index dd13e05..368a2b9 100644
>> --- a/include/linux/mmc/host.h
>> +++ b/include/linux/mmc/host.h
>> @@ -257,6 +257,7 @@ struct mmc_host {
>>  #define MMC_CAP2_HS200_1_2V_SDR      (1 << 6)        /* can support */
>>  #define MMC_CAP2_HS200               (MMC_CAP2_HS200_1_8V_SDR | \
>>                                MMC_CAP2_HS200_1_2V_SDR)
>> +#define MMC_CAP2_DETECT_ON_ERR       (1 << 7)        /* On I/O err check card removal */
>>
>>       mmc_pm_flag_t           pm_caps;        /* supported pm features */
>>       unsigned int        power_notify_type;
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Ulf Hansson]

Namjae Jeon wrote:
> 2012/2/3 Jaehoon Chung <jh80.chung@samsung.com>:
>> On 02/03/2012 06:33 PM, 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@stericsson.com>
>>> ---
>>>
>>> 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);
>> this function called mmc_detect_card_removed(card->host) in block.c.
>> then card isn't NULL..isn't?
>> Just wondering... :)

Yes, if called from block device thread (block.c) card is always present.

> If we think this function is used anywhere else next time, it is reasonable.
> And old code also checked null.

That is why I also agree that this should be checked. It is an exported 
function and who know if it is going to used from some other context in 
future.

>> Best Regards,
>> Jaehoon Chung
>>
>>>       /*
>>>        * 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) {
>>> +                     /*
>>> +                      * Schedule a detect work as soon as possible to let a
>>> +                      * rescan handle the card removal.
>>> +                      */
>>> +                     cancel_delayed_work(&host->detect);
>>> +                     mmc_detect_change(host, 0);
>>> +             }
>>> +     }
>>>
>>> -     return _mmc_detect_card_removed(host);
>>> +     return ret;
>>>  }
>>>  EXPORT_SYMBOL(mmc_detect_card_removed);
>>>
>>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>>> index dd13e05..368a2b9 100644
>>> --- a/include/linux/mmc/host.h
>>> +++ b/include/linux/mmc/host.h
>>> @@ -257,6 +257,7 @@ struct mmc_host {
>>>  #define MMC_CAP2_HS200_1_2V_SDR      (1 << 6)        /* can support */
>>>  #define MMC_CAP2_HS200               (MMC_CAP2_HS200_1_8V_SDR | \
>>>                                MMC_CAP2_HS200_1_2V_SDR)
>>> +#define MMC_CAP2_DETECT_ON_ERR       (1 << 7)        /* On I/O err check card removal */
>>>
>>>       mmc_pm_flag_t           pm_caps;        /* supported pm features */
>>>       unsigned int        power_notify_type;
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Adrian Hunter]

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@stericsson.com>
> ---
> 
> 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.

> +			/*
> +			 * Schedule a detect work as soon as possible to let a
> +			 * rescan handle the card removal.
> +			 */
> +			cancel_delayed_work(&host->detect);
> +			mmc_detect_change(host, 0);
> +		}
> +	}
>  
> -	return _mmc_detect_card_removed(host);
> +	return ret;
>  }
>  EXPORT_SYMBOL(mmc_detect_card_removed);
>  
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index dd13e05..368a2b9 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -257,6 +257,7 @@ struct mmc_host {
>  #define MMC_CAP2_HS200_1_2V_SDR	(1 << 6)        /* can support */
>  #define MMC_CAP2_HS200		(MMC_CAP2_HS200_1_8V_SDR | \
>  				 MMC_CAP2_HS200_1_2V_SDR)
> +#define MMC_CAP2_DETECT_ON_ERR	(1 << 7)	/* On I/O err check card removal */
>  
>  	mmc_pm_flag_t		pm_caps;	/* supported pm features */
>  	unsigned int        power_notify_type;

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

[Jae hoon Chung]

2012/2/3 Ulf Hansson <ulf.hansson@stericsson.com>:
> Namjae Jeon wrote:
>>
>> 2012/2/3 Jaehoon Chung <jh80.chung@samsung.com>:
>>>
>>> On 02/03/2012 06:33 PM, 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@stericsson.com>
>>>> ---
>>>>
>>>> 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);
>>>
>>> this function called mmc_detect_card_removed(card->host) in block.c.
>>> then card isn't NULL..isn't?
>>> Just wondering... :)
>
>
> Yes, if called from block device thread (block.c) card is always present.
>
>
>> If we think this function is used anywhere else next time, it is
>> reasonable.
>> And old code also checked null.
>
>
> That is why I also agree that this should be checked. It is an exported
> function and who know if it is going to used from some other context in
> future.
I agree this...thanks for Mr.Jeon and Ulf's reply..
>
>
>>> Best Regards,
>>> Jaehoon Chung
>>>
>>>>      /*
>>>>       * 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) {
>>>> +                     /*
>>>> +                      * Schedule a detect work as soon as possible to
>>>> let a
>>>> +                      * rescan handle the card removal.
>>>> +                      */
>>>> +                     cancel_delayed_work(&host->detect);
>>>> +                     mmc_detect_change(host, 0);
>>>> +             }
>>>> +     }
>>>>
>>>> -     return _mmc_detect_card_removed(host);
>>>> +     return ret;
>>>>  }
>>>>  EXPORT_SYMBOL(mmc_detect_card_removed);
>>>>
>>>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>>>> index dd13e05..368a2b9 100644
>>>> --- a/include/linux/mmc/host.h
>>>> +++ b/include/linux/mmc/host.h
>>>> @@ -257,6 +257,7 @@ struct mmc_host {
>>>>  #define MMC_CAP2_HS200_1_2V_SDR      (1 << 6)        /* can support */
>>>>  #define MMC_CAP2_HS200               (MMC_CAP2_HS200_1_8V_SDR | \
>>>>                               MMC_CAP2_HS200_1_2V_SDR)
>>>> +#define MMC_CAP2_DETECT_ON_ERR       (1 << 7)        /* On I/O err
>>>> check card removal */
>>>>
>>>>      mmc_pm_flag_t           pm_caps;        /* supported pm features */
>>>>      unsigned int        power_notify_type;
>>>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>> the body of a message to majordomo@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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Ulf Hansson]

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@stericsson.com>
>> ---
>>
>> 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.


>> +			/*
>> +			 * Schedule a detect work as soon as possible to let a
>> +			 * rescan handle the card removal.
>> +			 */
>> +			cancel_delayed_work(&host->detect);
>> +			mmc_detect_change(host, 0);
>> +		}
>> +	}
>>  
>> -	return _mmc_detect_card_removed(host);
>> +	return ret;
>>  }
>>  EXPORT_SYMBOL(mmc_detect_card_removed);
>>  
>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>> index dd13e05..368a2b9 100644
>> --- a/include/linux/mmc/host.h
>> +++ b/include/linux/mmc/host.h
>> @@ -257,6 +257,7 @@ struct mmc_host {
>>  #define MMC_CAP2_HS200_1_2V_SDR	(1 << 6)        /* can support */
>>  #define MMC_CAP2_HS200		(MMC_CAP2_HS200_1_8V_SDR | \
>>  				 MMC_CAP2_HS200_1_2V_SDR)
>> +#define MMC_CAP2_DETECT_ON_ERR	(1 << 7)	/* On I/O err check card removal */
>>  
>>  	mmc_pm_flag_t		pm_caps;	/* supported pm features */
>>  	unsigned int        power_notify_type;
> 
> 

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

[Adrian Hunter]

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@stericsson.com>
>>> ---
>>>
>>> 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.

> 
> 
>>> +            /*
>>> +             * Schedule a detect work as soon as possible to let a
>>> +             * rescan handle the card removal.
>>> +             */
>>> +            cancel_delayed_work(&host->detect);
>>> +            mmc_detect_change(host, 0);
>>> +        }
>>> +    }
>>>  
>>> -    return _mmc_detect_card_removed(host);
>>> +    return ret;
>>>  }
>>>  EXPORT_SYMBOL(mmc_detect_card_removed);
>>>  
>>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>>> index dd13e05..368a2b9 100644
>>> --- a/include/linux/mmc/host.h
>>> +++ b/include/linux/mmc/host.h
>>> @@ -257,6 +257,7 @@ struct mmc_host {
>>>  #define MMC_CAP2_HS200_1_2V_SDR    (1 << 6)        /* can support */
>>>  #define MMC_CAP2_HS200        (MMC_CAP2_HS200_1_8V_SDR | \
>>>                   MMC_CAP2_HS200_1_2V_SDR)
>>> +#define MMC_CAP2_DETECT_ON_ERR    (1 << 7)    /* On I/O err check card
>>> removal */
>>>  
>>>      mmc_pm_flag_t        pm_caps;    /* supported pm features */
>>>      unsigned int        power_notify_type;
>>
>>
> 
> 

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

[Ulf Hansson]

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@stericsson.com> ---
>>>> 
>>>> 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. :-)

----------

>>> 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
> 

---------------------

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

[Adrian Hunter]

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@stericsson.com> ---
>>>>>
>>>>> 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
>>
> 
> ---------------------
> 

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

[Ulf Hansson]

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@stericsson.com> ---
>>>>>>
>>>>>> 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
>>>
>> ---------------------
>>
> 
>