Re: [RFC] [PATCH] Device removal callback

[Ben Collins]

On Sun, Mar 09, 2003 at 04:11:02PM -0800, Greg KH wrote:
> On Sun, Mar 09, 2003 at 01:14:13PM -0500, Ben Collins wrote:
> > 
> > So I added a new callback to the device stucture called remove. This
> > callback is done when device_del is about to remove a device from the
> > tree. I've used this internally to make sure I can walk the list of
> > children myself, and also do some other cleanups.
> 
> But don't you really want to remove the children before you remove the
> parent?  If you do this patch, then the remove() function will have to
> clean up the children first, right?  Can we handle the core recursion
> with the current locks properly?

Actually, with this patch, the dev->remove(dev) is called before the
driver model does any cleanup. So you can cleanup children at that
point, and the parent device is still sane.

The reason for this is I would like to be able to unregister a node's
device from several places without worrying about other things that need
to be done. One call.

> Yes, for USB we still have a list of a device's children, as we need
> them for various things, and the current driver model only has a parent
> pointer, not a child pointer (which is good, as for USB we can have
> multiple children).  So in the function where we know a USB device is
> disconnected, we walk our list of children and disconnect them in a
> depth-first order.  With this patch I don't see how it helps me push
> code into the driver core.

I haven't looked into USB in depth, but consider this. Without the
patch, to cleanup a device:

void ieee1394_remove_node(struct node_entry *ne)
{
	...

	list_for_each(..., &ne->device.children) {
		device_unregister(list_to_dev(lh));
	}

	device_unregister(&ne->device);
}

Then to remove a device, this function must always be called, so that
the unit-directories get removed. What happens if the PCI bus gets
yanked out from underneath us? How does the OHCI card's callbacks get me
back down to this point? Without a lot of extra infrastructure, the
nodes and unit directories get left hanging.

Instead I now do this, with the patch.

void ieee1394_remove_node(struct device *dev)
{
	list_for_each(..., &ne->device.children) {
		device_unregister(list_to_dev(lh));
	}
}

...
	/* Where the dev is created */
	...
	ne->device.remove = ieee1394_remove_node;
	device_register(&ne->device);

Now, no matter where it's called from, doing...

	device_unregister(&ne->device);

...will make sure my remove callback is executed, so the children
devices get unregistered aswell. I extend this to the host device
and I have a recursive remocal scheme that is safe no matter where my
devices get unregistered. Whole lot simpler that adding in a lot of
failsafe's and checks.

-- 
Debian     - http://www.debian.org/
Linux 1394 - http://www.linux1394.org/
Subversion - http://subversion.tigris.org/
Deqo       - http://www.deqo.com/