On Wed, Nov 10, 2010 at 6:25 PM, Linus Walleij
<linus.walleij@xxxxxxxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
