* [PATCH 0/2] Improve handling of card removal
@ 2012-01-19 16:39 Ulf Hansson
2012-01-19 16:39 ` [PATCH 1/2] mmc: core: Prevent I/O as soon as possible at " Ulf Hansson
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: Ulf Hansson @ 2012-01-19 16:39 UTC (permalink / raw)
To: linux-mmc, Chris Ball
Cc: Adrian Hunter, Per Forlin, Ulf Hansson, Johan Rudholm, Lee Jones
These patches is based upon the patch recently pushed to the mmc mailing list:
mmc: core: Force a "detect" to handle non-properly removed cards
According to Adrian Hunters comment about adding a CAP2 flag to enable
this feature has been done.
Patch 2 depends on patch 1; but patch 1 can also be discussed separately.
Ulf Hansson (2):
mmc: core: Prevent I/O as soon as possible at card removal
mmc: core: Detect card removal on I/O error
drivers/mmc/core/core.c | 19 ++++++++++++++++---
include/linux/mmc/host.h | 1 +
2 files changed, 17 insertions(+), 3 deletions(-)
--
1.7.5.4
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/2] mmc: core: Prevent I/O as soon as possible at card removal
2012-01-19 16:39 [PATCH 0/2] Improve handling of card removal Ulf Hansson
@ 2012-01-19 16:39 ` Ulf Hansson
2012-01-20 11:55 ` Adrian Hunter
2012-01-19 16:39 ` [PATCH 2/2] mmc: core: Detect card removal on I/O error Ulf Hansson
2012-01-20 11:55 ` [PATCH 0/2] Improve handling of card removal Adrian Hunter
2 siblings, 1 reply; 16+ messages in thread
From: Ulf Hansson @ 2012-01-19 16:39 UTC (permalink / raw)
To: linux-mmc, Chris Ball
Cc: Adrian Hunter, Per Forlin, Ulf Hansson, Johan Rudholm, Lee Jones
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.hansson@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;
+ ret = mmc_card_removed(card);
+ 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);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/2] mmc: core: Detect card removal on I/O error
2012-01-19 16:39 [PATCH 0/2] Improve handling of card removal Ulf Hansson
2012-01-19 16:39 ` [PATCH 1/2] mmc: core: Prevent I/O as soon as possible at " Ulf Hansson
@ 2012-01-19 16:39 ` Ulf Hansson
2012-01-20 11:55 ` [PATCH 0/2] Improve handling of card removal Adrian Hunter
2 siblings, 0 replies; 16+ messages in thread
From: Ulf Hansson @ 2012-01-19 16:39 UTC (permalink / raw)
To: linux-mmc, Chris Ball
Cc: Adrian Hunter, Per Forlin, Ulf Hansson, Johan Rudholm, Lee Jones
A card can be "slowly" removed which means that when a
card detect irq has triggered a scheduled detect work to
handle the card removal, the card will still be present
in the card slot and thus not removed by the rescan sequence.
To prevent further I/O request for these lingering "removed"
cards, use MMC_CAP2_DETECT_ON_ERR which will check if card is
still present from mmc_detect_card_removed function.
Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
---
drivers/mmc/core/core.c | 3 ++-
include/linux/mmc/host.h | 1 +
2 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 265dfd8..e5d1c3b 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2084,7 +2084,8 @@ int mmc_detect_card_removed(struct mmc_host *host)
* 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))
+ if (card && !host->detect_change && !(host->caps & MMC_CAP_NEEDS_POLL)
+ && !(host->caps2 & MMC_CAP2_DETECT_ON_ERR))
return mmc_card_removed(card);
ret = mmc_card_removed(card);
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
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] mmc: core: Prevent I/O as soon as possible at card removal
2012-01-19 16:39 ` [PATCH 1/2] mmc: core: Prevent I/O as soon as possible at " Ulf Hansson
@ 2012-01-20 11:55 ` Adrian Hunter
2012-01-31 12:54 ` Ulf Hansson
0 siblings, 1 reply; 16+ messages in thread
From: Adrian Hunter @ 2012-01-20 11:55 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, Chris Ball, Per Forlin, Johan Rudholm, Lee Jones
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.hansson@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.
> + 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?
> + mmc_detect_change(host, 0);
> + }
> + }
>
> - return _mmc_detect_card_removed(host);
> + return ret;
> }
> EXPORT_SYMBOL(mmc_detect_card_removed);
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/2] Improve handling of card removal
2012-01-19 16:39 [PATCH 0/2] Improve handling of card removal Ulf Hansson
2012-01-19 16:39 ` [PATCH 1/2] mmc: core: Prevent I/O as soon as possible at " Ulf Hansson
2012-01-19 16:39 ` [PATCH 2/2] mmc: core: Detect card removal on I/O error Ulf Hansson
@ 2012-01-20 11:55 ` Adrian Hunter
2012-01-31 13:00 ` Ulf Hansson
2 siblings, 1 reply; 16+ messages in thread
From: Adrian Hunter @ 2012-01-20 11:55 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, Chris Ball, Per Forlin, Johan Rudholm, Lee Jones
On 19/01/12 18:39, Ulf Hansson wrote:
> These patches is based upon the patch recently pushed to the mmc mailing list:
> mmc: core: Force a "detect" to handle non-properly removed cards
>
> According to Adrian Hunters comment about adding a CAP2 flag to enable
> this feature has been done.
>
> Patch 2 depends on patch 1; but patch 1 can also be discussed separately.
There should only be 1 patch otherwise you are breaking bisectablility for
people not setting MMC_CAP2_DETECT_ON_ERR
>
> Ulf Hansson (2):
> mmc: core: Prevent I/O as soon as possible at card removal
> mmc: core: Detect card removal on I/O error
>
> drivers/mmc/core/core.c | 19 ++++++++++++++++---
> include/linux/mmc/host.h | 1 +
> 2 files changed, 17 insertions(+), 3 deletions(-)
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] mmc: core: Prevent I/O as soon as possible at card removal
2012-01-20 11:55 ` Adrian Hunter
@ 2012-01-31 12:54 ` Ulf Hansson
2012-01-31 13:56 ` Adrian Hunter
0 siblings, 1 reply; 16+ messages in thread
From: Ulf Hansson @ 2012-01-31 12:54 UTC (permalink / raw)
To: Adrian Hunter; +Cc: linux-mmc, Chris Ball, Per FORLIN, Johan RUDHOLM, Lee Jones
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.hansson@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..."
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.
>
>> + mmc_detect_change(host, 0);
>> + }
>> + }
>>
>> - return _mmc_detect_card_removed(host);
>> + return ret;
>> }
>> EXPORT_SYMBOL(mmc_detect_card_removed);
>>
>
>
Br
Ulf Hansson
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/2] Improve handling of card removal
2012-01-20 11:55 ` [PATCH 0/2] Improve handling of card removal Adrian Hunter
@ 2012-01-31 13:00 ` Ulf Hansson
2012-01-31 13:56 ` Adrian Hunter
0 siblings, 1 reply; 16+ messages in thread
From: Ulf Hansson @ 2012-01-31 13:00 UTC (permalink / raw)
To: Adrian Hunter; +Cc: linux-mmc, Chris Ball, Per FORLIN, Johan RUDHOLM, Lee Jones
Adrian Hunter wrote:
> On 19/01/12 18:39, Ulf Hansson wrote:
>> These patches is based upon the patch recently pushed to the mmc mailing list:
>> mmc: core: Force a "detect" to handle non-properly removed cards
>>
>> According to Adrian Hunters comment about adding a CAP2 flag to enable
>> this feature has been done.
>>
>> Patch 2 depends on patch 1; but patch 1 can also be discussed separately.
>
> There should only be 1 patch otherwise you are breaking bisectablility for
> people not setting MMC_CAP2_DETECT_ON_ERR
The first patch "Prevent I/O as soon as possible..." does only make sure
that I/O is prevented as soon as possible but still within the timeout
for the scheduled detect work. Once the detect work (mmc_rescan) has run
the "detect_change" flag is preventing any further I/O errors from
directly trying to detect a card removal.
The second patch introduces the MMC_CAP2_DETECT_ON_ERR, as you suggested.
>
>> Ulf Hansson (2):
>> mmc: core: Prevent I/O as soon as possible at card removal
>> mmc: core: Detect card removal on I/O error
>>
>> drivers/mmc/core/core.c | 19 ++++++++++++++++---
>> include/linux/mmc/host.h | 1 +
>> 2 files changed, 17 insertions(+), 3 deletions(-)
>>
>
>
Br
Ulf Hansson
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] mmc: core: Prevent I/O as soon as possible at card removal
2012-01-31 12:54 ` Ulf Hansson
@ 2012-01-31 13:56 ` Adrian Hunter
2012-01-31 15:23 ` Ulf Hansson
0 siblings, 1 reply; 16+ messages in thread
From: Adrian Hunter @ 2012-01-31 13:56 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, Chris Ball, Per FORLIN, Johan RUDHOLM, Lee Jones
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.hansson@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
>
> 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.
>
>>
>>> + mmc_detect_change(host, 0);
>>> + }
>>> + }
>>>
>>> - return _mmc_detect_card_removed(host);
>>> + return ret;
>>> }
>>> EXPORT_SYMBOL(mmc_detect_card_removed);
>>>
>>
>>
>
> Br
> Ulf Hansson
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/2] Improve handling of card removal
2012-01-31 13:00 ` Ulf Hansson
@ 2012-01-31 13:56 ` Adrian Hunter
2012-01-31 15:40 ` Ulf Hansson
0 siblings, 1 reply; 16+ messages in thread
From: Adrian Hunter @ 2012-01-31 13:56 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, Chris Ball, Per FORLIN, Johan RUDHOLM, Lee Jones
On 31/01/12 15:00, Ulf Hansson wrote:
> Adrian Hunter wrote:
>> On 19/01/12 18:39, Ulf Hansson wrote:
>>> These patches is based upon the patch recently pushed to the mmc mailing
>>> list:
>>> mmc: core: Force a "detect" to handle non-properly removed cards
>>>
>>> According to Adrian Hunters comment about adding a CAP2 flag to enable
>>> this feature has been done.
>>>
>>> Patch 2 depends on patch 1; but patch 1 can also be discussed separately.
>>
>> There should only be 1 patch otherwise you are breaking bisectablility for
>> people not setting MMC_CAP2_DETECT_ON_ERR
>
> The first patch "Prevent I/O as soon as possible..." does only make sure
> that I/O is prevented as soon as possible but still within the timeout for
> the scheduled detect work. Once the detect work (mmc_rescan) has run the
> "detect_change" flag is preventing any further I/O errors from directly
> trying to detect a card removal.
>
> The second patch introduces the MMC_CAP2_DETECT_ON_ERR, as you suggested.
For drivers not using MMC_CAP2_DETECT_ON_ERR, the first patch introduces a
change in behaviour and the second patch removes it again. Anyone doing a
git bisect that lands between the patches will see that change in behaviour.
There is no reason for that - just make it one patch.
>
>>
>>> Ulf Hansson (2):
>>> mmc: core: Prevent I/O as soon as possible at card removal
>>> mmc: core: Detect card removal on I/O error
>>>
>>> drivers/mmc/core/core.c | 19 ++++++++++++++++---
>>> include/linux/mmc/host.h | 1 +
>>> 2 files changed, 17 insertions(+), 3 deletions(-)
>>>
>>
>>
>
> Br
> Ulf Hansson
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] mmc: core: Prevent I/O as soon as possible at card removal
2012-01-31 13:56 ` Adrian Hunter
@ 2012-01-31 15:23 ` Ulf Hansson
2012-02-01 3:02 ` Jaehoon Chung
0 siblings, 1 reply; 16+ messages in thread
From: Ulf Hansson @ 2012-01-31 15:23 UTC (permalink / raw)
To: Adrian Hunter; +Cc: linux-mmc, Chris Ball, Per FORLIN, Johan RUDHOLM, Lee Jones
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.hansson@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.
>>
>>>> + 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 majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/2] Improve handling of card removal
2012-01-31 13:56 ` Adrian Hunter
@ 2012-01-31 15:40 ` Ulf Hansson
2012-02-01 9:37 ` Adrian Hunter
0 siblings, 1 reply; 16+ messages in thread
From: Ulf Hansson @ 2012-01-31 15:40 UTC (permalink / raw)
To: Adrian Hunter; +Cc: linux-mmc, Chris Ball, Per FORLIN, Johan RUDHOLM, Lee Jones
Adrian Hunter wrote:
> On 31/01/12 15:00, Ulf Hansson wrote:
>> Adrian Hunter wrote:
>>> On 19/01/12 18:39, Ulf Hansson wrote:
>>>> These patches is based upon the patch recently pushed to the mmc mailing
>>>> list:
>>>> mmc: core: Force a "detect" to handle non-properly removed cards
>>>>
>>>> According to Adrian Hunters comment about adding a CAP2 flag to enable
>>>> this feature has been done.
>>>>
>>>> Patch 2 depends on patch 1; but patch 1 can also be discussed separately.
>>> There should only be 1 patch otherwise you are breaking bisectablility for
>>> people not setting MMC_CAP2_DETECT_ON_ERR
>> The first patch "Prevent I/O as soon as possible..." does only make sure
>> that I/O is prevented as soon as possible but still within the timeout for
>> the scheduled detect work. Once the detect work (mmc_rescan) has run the
>> "detect_change" flag is preventing any further I/O errors from directly
>> trying to detect a card removal.
>>
>> The second patch introduces the MMC_CAP2_DETECT_ON_ERR, as you suggested.
>
> For drivers not using MMC_CAP2_DETECT_ON_ERR, the first patch introduces a
> change in behaviour and the second patch removes it again. Anyone doing a
> git bisect that lands between the patches will see that change in behaviour.
> There is no reason for that - just make it one patch.
The first patch is trying to "Prevent I/O as soon as possible at card
removal". It changes some behavior for how we handle card removal, yes.
A change I believe you would like to have no matter if the second patch
is accepted or not.
The second patch, takes the card removal handling to the next level. If
MMC_CAP2_DETECT_ON_ERR is used, card removal will be checked for every
I/O error until the card has been detected to be removed. It does not
remove the behavior for the first patch it only makes it possible to
enable the option of checking for card removal at I/O errors as well.
>
>>>> Ulf Hansson (2):
>>>> mmc: core: Prevent I/O as soon as possible at card removal
>>>> mmc: core: Detect card removal on I/O error
>>>>
>>>> drivers/mmc/core/core.c | 19 ++++++++++++++++---
>>>> include/linux/mmc/host.h | 1 +
>>>> 2 files changed, 17 insertions(+), 3 deletions(-)
>>>>
>>>
>> Br
>> Ulf Hansson
>>
>>
>
> --
> 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
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] mmc: core: Prevent I/O as soon as possible at card removal
2012-01-31 15:23 ` Ulf Hansson
@ 2012-02-01 3:02 ` Jaehoon Chung
2012-02-01 9:40 ` Ulf Hansson
0 siblings, 1 reply; 16+ messages in thread
From: Jaehoon Chung @ 2012-02-01 3:02 UTC (permalink / raw)
To: Ulf Hansson
Cc: Adrian Hunter, linux-mmc, Chris Ball, Per FORLIN, Johan RUDHOLM,
Lee Jones
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.hansson@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?
>>>
>>>>> + 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 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
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/2] Improve handling of card removal
2012-01-31 15:40 ` Ulf Hansson
@ 2012-02-01 9:37 ` Adrian Hunter
2012-02-01 9:44 ` Ulf Hansson
0 siblings, 1 reply; 16+ messages in thread
From: Adrian Hunter @ 2012-02-01 9:37 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, Chris Ball, Per FORLIN, Johan RUDHOLM, Lee Jones
On 31/01/12 17:40, Ulf Hansson wrote:
> Adrian Hunter wrote:
>> On 31/01/12 15:00, Ulf Hansson wrote:
>>> Adrian Hunter wrote:
>>>> On 19/01/12 18:39, Ulf Hansson wrote:
>>>>> These patches is based upon the patch recently pushed to the mmc mailing
>>>>> list:
>>>>> mmc: core: Force a "detect" to handle non-properly removed cards
>>>>>
>>>>> According to Adrian Hunters comment about adding a CAP2 flag to enable
>>>>> this feature has been done.
>>>>>
>>>>> Patch 2 depends on patch 1; but patch 1 can also be discussed separately.
>>>> There should only be 1 patch otherwise you are breaking bisectablility for
>>>> people not setting MMC_CAP2_DETECT_ON_ERR
>>> The first patch "Prevent I/O as soon as possible..." does only make sure
>>> that I/O is prevented as soon as possible but still within the timeout for
>>> the scheduled detect work. Once the detect work (mmc_rescan) has run the
>>> "detect_change" flag is preventing any further I/O errors from directly
>>> trying to detect a card removal.
>>>
>>> The second patch introduces the MMC_CAP2_DETECT_ON_ERR, as you suggested.
>>
>> For drivers not using MMC_CAP2_DETECT_ON_ERR, the first patch introduces a
>> change in behaviour and the second patch removes it again. Anyone doing a
>> git bisect that lands between the patches will see that change in behaviour.
>> There is no reason for that - just make it one patch.
>
> The first patch is trying to "Prevent I/O as soon as possible at card removal". It changes some behavior for how we handle card removal, yes. A change I believe you would like to have no matter if the second patch is accepted or not.
>
> The second patch, takes the card removal handling to the next level. If MMC_CAP2_DETECT_ON_ERR is used, card removal will be checked for every I/O error until the card has been detected to be removed. It does not remove the behavior for the first patch it only makes it possible to enable the option of checking for card removal at I/O errors as well.
Make the patch like this and there is no problem:
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index bec0bf2..a847311 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2077,18 +2077,30 @@ 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);
/*
* 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))
+ if (card && !host->detect_change && !(host->caps & MMC_CAP_NEEDS_POLL)
+ && !(host->caps2 & MMC_CAP2_DETECT_ON_ERR))
return mmc_card_removed(card);
host->detect_change = 0;
- return _mmc_detect_card_removed(host);
+ ret = _mmc_detect_card_removed(host);
+ if (ret && (host->caps2 & MMC_CAP2_DETECT_ON_ERR)) {
+ /*
+ * 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 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;
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] mmc: core: Prevent I/O as soon as possible at card removal
2012-02-01 3:02 ` Jaehoon Chung
@ 2012-02-01 9:40 ` Ulf Hansson
2012-02-02 4:00 ` Jaehoon Chung
0 siblings, 1 reply; 16+ messages in thread
From: Ulf Hansson @ 2012-02-01 9:40 UTC (permalink / raw)
To: Jaehoon Chung
Cc: Adrian Hunter, linux-mmc, Chris Ball, Per FORLIN, Johan RUDHOLM,
Lee Jones
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.hansson@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!
>
>>>>>> + 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 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
>>
>
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/2] Improve handling of card removal
2012-02-01 9:37 ` Adrian Hunter
@ 2012-02-01 9:44 ` Ulf Hansson
0 siblings, 0 replies; 16+ messages in thread
From: Ulf Hansson @ 2012-02-01 9:44 UTC (permalink / raw)
To: Adrian Hunter; +Cc: linux-mmc, Chris Ball, Per FORLIN, Johan RUDHOLM, Lee Jones
Adrian Hunter wrote:
> On 31/01/12 17:40, Ulf Hansson wrote:
>> Adrian Hunter wrote:
>>> On 31/01/12 15:00, Ulf Hansson wrote:
>>>> Adrian Hunter wrote:
>>>>> On 19/01/12 18:39, Ulf Hansson wrote:
>>>>>> These patches is based upon the patch recently pushed to the mmc mailing
>>>>>> list:
>>>>>> mmc: core: Force a "detect" to handle non-properly removed cards
>>>>>>
>>>>>> According to Adrian Hunters comment about adding a CAP2 flag to enable
>>>>>> this feature has been done.
>>>>>>
>>>>>> Patch 2 depends on patch 1; but patch 1 can also be discussed separately.
>>>>> There should only be 1 patch otherwise you are breaking bisectablility for
>>>>> people not setting MMC_CAP2_DETECT_ON_ERR
>>>> The first patch "Prevent I/O as soon as possible..." does only make sure
>>>> that I/O is prevented as soon as possible but still within the timeout for
>>>> the scheduled detect work. Once the detect work (mmc_rescan) has run the
>>>> "detect_change" flag is preventing any further I/O errors from directly
>>>> trying to detect a card removal.
>>>>
>>>> The second patch introduces the MMC_CAP2_DETECT_ON_ERR, as you suggested.
>>> For drivers not using MMC_CAP2_DETECT_ON_ERR, the first patch introduces a
>>> change in behaviour and the second patch removes it again. Anyone doing a
>>> git bisect that lands between the patches will see that change in behaviour.
>>> There is no reason for that - just make it one patch.
>> The first patch is trying to "Prevent I/O as soon as possible at card removal". It changes some behavior for how we handle card removal, yes. A change I believe you would like to have no matter if the second patch is accepted or not.
>>
>> The second patch, takes the card removal handling to the next level. If MMC_CAP2_DETECT_ON_ERR is used, card removal will be checked for every I/O error until the card has been detected to be removed. It does not remove the behavior for the first patch it only makes it possible to enable the option of checking for card removal at I/O errors as well.
>
> Make the patch like this and there is no problem:
>
OK, I will change the patch(es) according to your proposal and create
one instead of two.
Thanks.
>
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index bec0bf2..a847311 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -2077,18 +2077,30 @@ 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);
> /*
> * 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))
> + if (card && !host->detect_change && !(host->caps & MMC_CAP_NEEDS_POLL)
> + && !(host->caps2 & MMC_CAP2_DETECT_ON_ERR))
> return mmc_card_removed(card);
>
> host->detect_change = 0;
>
> - return _mmc_detect_card_removed(host);
> + ret = _mmc_detect_card_removed(host);
> + if (ret && (host->caps2 & MMC_CAP2_DETECT_ON_ERR)) {
> + /*
> + * 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 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;
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] mmc: core: Prevent I/O as soon as possible at card removal
2012-02-01 9:40 ` Ulf Hansson
@ 2012-02-02 4:00 ` Jaehoon Chung
0 siblings, 0 replies; 16+ messages in thread
From: Jaehoon Chung @ 2012-02-02 4:00 UTC (permalink / raw)
To: Ulf Hansson
Cc: Jaehoon Chung, Adrian Hunter, linux-mmc, Chris Ball, Per FORLIN,
Johan RUDHOLM, Lee Jones
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.hansson@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 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
>>>
>>
>>
>>
>
> --
> 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
>
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2012-02-02 4:00 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-19 16:39 [PATCH 0/2] Improve handling of card removal Ulf Hansson
2012-01-19 16:39 ` [PATCH 1/2] mmc: core: Prevent I/O as soon as possible at " Ulf Hansson
2012-01-20 11:55 ` Adrian Hunter
2012-01-31 12:54 ` Ulf Hansson
2012-01-31 13:56 ` Adrian Hunter
2012-01-31 15:23 ` Ulf Hansson
2012-02-01 3:02 ` Jaehoon Chung
2012-02-01 9:40 ` Ulf Hansson
2012-02-02 4:00 ` Jaehoon Chung
2012-01-19 16:39 ` [PATCH 2/2] mmc: core: Detect card removal on I/O error Ulf Hansson
2012-01-20 11:55 ` [PATCH 0/2] Improve handling of card removal Adrian Hunter
2012-01-31 13:00 ` Ulf Hansson
2012-01-31 13:56 ` Adrian Hunter
2012-01-31 15:40 ` Ulf Hansson
2012-02-01 9:37 ` Adrian Hunter
2012-02-01 9:44 ` Ulf Hansson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.