Forget I mentioned it. It isn't a problem if the subsystem uses its own
mutex to provide the mutual exclusion. Things become difficult only
if the subsystem tries to use dev->sem.
I saw something recently suggesting such a thing is already present in
Andrew Morton's queue. Great minds think alike... :-)
Agreed.
You know, things may not be quite as bad as I had thought. I was under
the impression that the driver core did put_device(dev->parent) during
device_release(). But that's not the case; the call is made during
device_del().
This makes things very different. Failure to drop references to a device
during remove() won't cause any lingering references to the device's
parent. The effects of one badly-behaved driver won't propagate
indefinitely far up the device tree.
On the other hand, although devices behave this way, kobjects do not. The
call to kobject_put(kobj->parent) is in kobject_cleanup(), not in
kobject_del() or unlink(). So this still needs to be fixed. It may be
related to the sysfs <-> kobject link you have been trying to break.
Dmitry, in thinking things over some more I realized there's going to be a
problem with the autosuspend support in USB. It has to do with the way a
driver needs to prevent (or block) suspends from occurring while it is
actively using a device.
To understand this, you need to know that USB adds a pm_mutex to the
device structure. It gets used for synchronizing all suspend- or
resume-related activities. In particular, the driver's suspend() method
is called with usbdev->pm_mutex held.
The next idea is that a driver needs to synchronize its remove() method
with other methods such as read() -- we mustn't allow read() to try and
refer to the device after the driver has been unbound. Let's say the
driver has unbind_mutex embedded in its private data structure for this
purpose.
Now consider what read() has to do. It needs to block suspends from
occurring while it runs, and it needs to do an autoresume if the device
was already suspended. So read() will look like this:
mutex_lock(&private->unbind_mutex);
if (private->gone) {
ret = -ENODEV;
goto done;
}
if (private->suspended)
autoresume(private->usbdev);
...
done:
mutex_unlock(&private->unbind_mutex);
return ret;
Meanwhile, the suspend() method needs to block while read() is running.
So it will look like this:
private->suspended = 1;
mutex_lock(&private->unbind_mutex);
/* Now the driver has quiesced */
mutex_unlock(&private->unbind_mutex);
Here's the problem: The autoresume() call inside read() tries to acquire
usbdev->pm_mutex while holding private->unbind_mutex. The suspend()
method does the reverse, a locking-order violation.
So far I haven't figured out any way to make this work. Do you have a
suggestion? (Don't say read() should hold pm_mutex as well as
unbind_mutex; that's no good -- although the reason is rather obscure.)
Alan Stern
-
