diff mbox

[2/4] mmc: core: allow non-blocking form of mmc_claim_host

Message ID 20150224024223.22719.95786.stgit@notabene.brown (mailing list archive)
State New, archived
Headers show

Commit Message

NeilBrown Feb. 24, 2015, 2:42 a.m. UTC
Change the handling for the 'abort' flag so that if
it is set, but we can claim the host, then do the claim,
rather than aborting.

When the abort is async this just means that a race between aborting
an allowing a claim is resolved slightly differently.  Any
code must already be able to handle 'abort' being set just as the host
is claimed.

This allows extra functionality.  If __mmc_claim_host() is called
with an 'abort' pointer which is initialized to '1', it will effect a
non-blocking 'claim'.

Signed-off-by: NeilBrown <neil@brown.name>
---
 drivers/mmc/core/core.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)



--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Ulf Hansson March 23, 2015, 9:58 a.m. UTC | #1
On 24 February 2015 at 03:42, NeilBrown <neilb@suse.de> wrote:
> Change the handling for the 'abort' flag so that if
> it is set, but we can claim the host, then do the claim,
> rather than aborting.
>
> When the abort is async this just means that a race between aborting
> an allowing a claim is resolved slightly differently.  Any
> code must already be able to handle 'abort' being set just as the host
> is claimed.
>
> This allows extra functionality.  If __mmc_claim_host() is called
> with an 'abort' pointer which is initialized to '1', it will effect a
> non-blocking 'claim'.
>
> Signed-off-by: NeilBrown <neil@brown.name>
> ---
>  drivers/mmc/core/core.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 23f10f72e5f3..541c8903dc6b 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -912,10 +912,11 @@ int __mmc_claim_host(struct mmc_host *host, atomic_t *abort)
>                 spin_lock_irqsave(&host->lock, flags);
>         }
>         set_current_state(TASK_RUNNING);
> -       if (!stop) {
> +       if (!host->claimed || host->claimer == current) {

This seems risky in that regards that it will change the behaviour for
the sdio_irq_thread(). Did you check that?

I guess we could change the sdio_irq_thread() to read it's
sdio_irq_thread_abort value before trying to claim the host?

>                 host->claimed = 1;
>                 host->claimer = current;
>                 host->claim_cnt += 1;
> +               stop = 0;
>         } else
>                 wake_up(&host->wq);
>         spin_unlock_irqrestore(&host->lock, flags);
>
>

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ulf Hansson March 23, 2015, 3:19 p.m. UTC | #2
On 23 March 2015 at 10:58, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 24 February 2015 at 03:42, NeilBrown <neilb@suse.de> wrote:
>> Change the handling for the 'abort' flag so that if
>> it is set, but we can claim the host, then do the claim,
>> rather than aborting.
>>
>> When the abort is async this just means that a race between aborting
>> an allowing a claim is resolved slightly differently.  Any
>> code must already be able to handle 'abort' being set just as the host
>> is claimed.
>>
>> This allows extra functionality.  If __mmc_claim_host() is called
>> with an 'abort' pointer which is initialized to '1', it will effect a
>> non-blocking 'claim'.
>>
>> Signed-off-by: NeilBrown <neil@brown.name>
>> ---
>>  drivers/mmc/core/core.c |    3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>> index 23f10f72e5f3..541c8903dc6b 100644
>> --- a/drivers/mmc/core/core.c
>> +++ b/drivers/mmc/core/core.c
>> @@ -912,10 +912,11 @@ int __mmc_claim_host(struct mmc_host *host, atomic_t *abort)
>>                 spin_lock_irqsave(&host->lock, flags);
>>         }
>>         set_current_state(TASK_RUNNING);
>> -       if (!stop) {
>> +       if (!host->claimed || host->claimer == current) {
>
> This seems risky in that regards that it will change the behaviour for
> the sdio_irq_thread(). Did you check that?
>
> I guess we could change the sdio_irq_thread() to read it's
> sdio_irq_thread_abort value before trying to claim the host?

Ah, that won't work. Since we may be spinning to claim the host and
the abort value may change dynamically during this time.

Another option would be to re-invent mmc_try_claim_host(). Which we
removed in commit:
b83e867026ca (mmc: core: remove dead function mmc_try_claim_host)

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 23f10f72e5f3..541c8903dc6b 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -912,10 +912,11 @@  int __mmc_claim_host(struct mmc_host *host, atomic_t *abort)
 		spin_lock_irqsave(&host->lock, flags);
 	}
 	set_current_state(TASK_RUNNING);
-	if (!stop) {
+	if (!host->claimed || host->claimer == current) {
 		host->claimed = 1;
 		host->claimer = current;
 		host->claim_cnt += 1;
+		stop = 0;
 	} else
 		wake_up(&host->wq);
 	spin_unlock_irqrestore(&host->lock, flags);