Re: [PATCH 0/2] Race fixes in sdhci

[Mark Brown <broonie@opensource.wolfsonmicro.com>]

On Wed, Apr 27, 2011 at 04:41:27PM -0400, Chris Ball wrote:

> No, I think Mark's saying there's a race of:

> * the successful completion interrupt fires, and

> * the host timer fires to signify timeout due to *lack* of an interrupt
>   (via sdhci_timeout_timer()).  i.e., the completion interrupt fires
>   very close to the timeout period.

> Both cases call tasklet_schedule(&host->finish_tasklet), and if you
> manage to schedule a tasklet while it's already running, it runs again
> after it completes -- but during the first run we set host->mrq->cmd to
> NULL, so then it oopses on the second run.

Yes, exactly - that's what I saw through code inspection.  I haven't
actually seen this directly and analysed enough to understand a
particular race, I just looked at the fixes that we are carrying in our
tree and looked at the code sufficiently to determine that the fixes
addressed an issue I could identify.

> We could consider taking Mark's first patch and also adding to the top:

Ben's patch.

>         /*
>          * If we get scheduled twice concurrently, this tasklet will
>          * be run again afterwards but without any active request.
>          */
>         if (!host->mrq)
>                 return;

> .. and pushing to .39 with a stable@ tag.

Yes, that's a broader way of writing Dimitris' patch.  The issue isn't
concurrent *schedules* - they will just fall out into a single queue for
the task - it's that the tasklet can be rescheduled while it's running.