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
