On Wed, Nov 10, 2010 at 6:25 PM, Linus Walleij
<[email protected]> wrote:
> Not that one, because the only place where ungate is called
> is immediately before the request or set_ios(). So the request
> or set_ios() will complete

How can you assume that ? Nothing prevents a context switch after
ungate, and before the actual request.

Here is what I had in mind:

Thread 1
=======

calling mmc_host_clk_gate_delayed()
...
spin_unlock_irqrestore(&host->clk_lock, flags);

About to execute mmc_gate_clock(host)

context switch to Thread 2
=====================

calling mmc_start_request()
...
executes mmc_host_clk_ungate();

back to Thread 1
=============

executes mmc_gate_clock(host);


back to Thread 2
==============

/* clock is gated now */
host->ops->request(host, mrq);


>, and immediately after that
> this gating will be triggered.
>
> So the real bug is that if we get this race we don't get
> the 8 MCI cycles of delay that we want.
>
> But I guess I can replace all spinlocks with a mutex instead
> and still hold it across the gate operation, since all
> gate/ungate calls should be coming from process context?
>
> That simplifies things.
>
> I'll see if this works...
>
>>> +               spin_lock_irqsave(&host->clk_lock, flags);
>>> +               pr_debug("%s: gated MCI clock\n",
>>> +                        mmc_hostname(host));
>>> +       }
>>> +       host->clk_pending_gate = false;
>>
>> What is clk_pending_gate used for (I can only see it being assigned values) ?
>
> Hm, a development artifact from patchset v4 2009-06-18...
> It's replaced with host->clk_gated instead.
>
> I'll remove it.
>
>> (PS sorry for the belated posting of these questions)
>
> No problem, I'll fix.
>
> Chris, do you want an incremental patch or shall I spin an all-new
> v10 patch?
>
> Yours,
> Linus Walleij
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to