Message-ID: <Pine.LNX.4.44L0.1509151505040.1420-100000@iolanthe.rowland.org>
Date:	Tue, 15 Sep 2015 15:18:19 -0400 (EDT)
From:	Alan Stern <stern@...land.harvard.edu>
To:	Thierry Reding <thierry.reding@...il.com>
cc:	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	"Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
	Grygorii Strashko <grygorii.strashko@...com>,
	Tomeu Vizoso <tomeu.vizoso@...labora.com>,
	<linux-pm@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] driver core: Ensure proper suspend/resume ordering

On Tue, 15 Sep 2015, Thierry Reding wrote:

> > There are a few things to watch out for.  Since the dpm_list gets
> > modified during system sleep transitions, we would have to make sure
> > that nothing gets probed during those times.  In principle, that's what
> > the "prepare" stage is meant for, but there's still a race.  As long as
> > no other kernel thread (such as the deferred probing mechanism) tries
> > to probe a device once everything has been frozen, we should be okay.
> > But if not, there will be trouble -- after the ->prepare callback runs, 
> > the device is no longer on the dpm_list and so we don't want this patch 
> > to put it back on that list.
> 
> Perhaps moving to the end of the list needs to be a little smarter. That
> is it could check whether the device has been prepared for suspension or
> not and only move when it hasn't?

Maybe.  But doesn't that mean it won't solve your problem completely?

> Then again, shouldn't the core even prohibit new probes once the suspend
> has been triggered? Sounds like asking for a lot of trouble if it didn't
> ...

The core prohibits new devices from being registered.  It does not 
prohibit probes of existing devices, because they currently do not 
affect the dpm_list.

In general, we rely on subsystems not to do any probing once a device 
is suspended.  It's probably reasonable to ask them not to do any 
probing once a device has gone through the "prepare" stage.

> > There's also an issue about other types of dependencies.  For instance,
> > it's conceivable that device B might be discovered and depend on device
> > A, even before A has been bound to a driver.  (B might be discovered by 
> > A's subsystem rather than A's driver.)  In that case, moving A to the 
> > end of the list would cause B to come before A even though B depends on 
> > A.  Of course, deferred probing already has this problem.
> 
> But that's exactly the problem that I'm seeing.

Not quite.

>  B isn't discovered by
> A's subsystem, but the type of dependency is the same. A in this case
> would be the GPIO controller and B the gpio-keys device. B clearly
> depends on A, but deferred probe currently moves A to the end of the
> list but not A, hence why the problem occurs.

The difference is that in my example, B can be probed before A.  In
your case it can't.  Therefore the patch works for your case but not
for mine.

> That's also a problem that I think this patch solves. By moving every
> device to the end of the list before it is probed we ensure that the
> dpm_list is ordered in the same way as the probe order. For this to work
> the precondition of course is that drivers know about the dependencies
> and will defer probe if necessary.

Do I understand correctly?  You're saying a driver must defer a probe
if the device it's probing depends on another device which hasn't
been bound yet.  That does not sound like a reasonable sort of
requirement -- we might know about the dependency but we shouldn't have
to check whether the prerequisite device has been bound.

> > An easy way to check for this sort of thing would be to verify that a 
> > device about to be probed doesn't have any children.  This wouldn't 
> > catch all the potential dependencies, but it would be a reasonable 
> > start.  Do we currently check that after a device has been unbound, it 
> > doesn't have any remaining children?  We should do that too.
> 
> I think that would cover the other half of the cases where deferred
> probe isn't implemented because drivers are written with the assumption
> that children will be instantiated after their parent has been probed.
> 
> But it won't catch all the cases where there is no parent-child
> relationship between the depender and the dependee.

No, it won't catch all cases.  But the cases it does catch are ones 
where your patch is likely to cause trouble, so it would be good to 
know about them.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists