Re: [PATCH] mmc: omap_hsmmc: fix runtime suspend crash: add host checks in callbacks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jul 10, 2012 at 7:47 PM, Kevin Hilman <khilman@xxxxxx> wrote:
> "S, Venkatraman" <svenkatr@xxxxxx> writes:
>
>> On Sat, Jul 7, 2012 at 5:56 AM, Kevin Hilman <khilman@xxxxxx> wrote:
>>> Due to the way the driver core takes runtime PM references during
>>> probe, a driver's runtime PM callbacks may not be called until probe
>>> returns.  During probe, drvdata is set to the 'host' pointer but if
>>> probe fails, drvdata is set to NULL.
>>>
>>> The runtime PM callbacks currently blindly dereference this host
>>> pointer, but if probe fails, and the runtime PM callbacks are called
>>> after probe returns, a NULL pointer dereference will result.
>>>
>> Pardon my ignorance, but why would runtime suspend be called for a
>> device whose probe has failed ? AFAIK, MMC stack wouldn't make the
>> _enable()/disable() calls, which call runtime PM APIs, unless the
>> probe is successful.  Is there a stacktrace for the offending caller ?
>
> First thing to note: there are several error conditions *after*
> pm_runtime_enable() and the first _get_sync() are called.
>
> So here's what can happen:
>
> The driver core does a _get_noresume() before calling the driver's
> probe, so the runtime PM usecount is already 1 when the driver is
> called.  The _get_sync() in _probe enables the device and increases the
> usecount to 2.  The _put_sync() at the end of ->probe() will decrease
> the usecount to 1, but since the usecount is still non-zero, the
> driver's callbacks are still not called.  It's not until the driver core
> does its _put_sync (to balance the _get_noresume() that the usecount
> goes to zero and the driver's callbacks are called.
>
Thanks for the detailed explanation.
But pm_runtime_disable() is called in the error path, so shouldn't
it prevent the driver callbacks from being called ? (The docuementation
talks about disable_depth field, but it doesn't sound right that runtime
pm would be enabled before _probe() is called. So disable_depth should
be 1 when pm_runtime_disable is called in _probe error path, right ?)

> When probe succeeds, this all works fine.  However, if probe fails the
> host pointer is set to NULL, but the runtime PM callbacks are still
> called and attempt to dereference a NULL pointer.
>
>>> Fix this by simply checking to be sure the host pointer is non-NULL.
>>>
>>> Signed-off-by: Kevin Hilman <khilman@xxxxxx>
>>> ---
>>> Applies to v3.5-rc5.  Fix is needed for v3.5-rc.
>>>
>> Can you please let me know which commit introduced this problem
>> in the first place ? There were not many changes in MMC PM code
>> in this merge window.
>
> I guess this problem is probably not a regression and has been around
> for some time.  It has not been seen because probe has not failed.
>
> In using recent l-o master though, with Russell's dmaengine changes
> merged, probe is failing for me becasue it can't allocate a DMA channel,
> and then I'm seeing this problem.
>
> Kevin

Ok. I'll let Chris decide on this - DMA engine is not yet in Linus's tree
though.
--
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


[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux