Messages in this thread | | | Date | Thu, 19 Apr 2007 14:39:40 -0400 | From | "Dmitry Torokhov" <> | Subject | Re: [PATCH RFD] alternative kobject release wait mechanism |
| |
On 4/19/07, Alan Stern <stern@rowland.harvard.edu> wrote: > On Wed, 18 Apr 2007, Dmitry Torokhov wrote: > > > I am still do not understand why this is needed. Would it not be > > simplier just to use a reference to struct device instead of embedding > > it in a larger structure if their lifetimes are different and one does > > not have a subsystem that takes care of releasing logic. > > > > > > Pretty much drivers have 2 options: > > > > struct my_device { > > void *private_data; > > struct device dev; > > }; > > Actually people use dev_[gs]et_drvdata() instead of a separate > private_data pointer. That way there's no need for the my_device > container. >
No, drvdata belongs to driver that is bound to a device. We are talking about private data created and managed by driver that provides device.
> > In this case ->release must live in a subsystem code; individual > > drivers kfree(my_dev->private) and do any additional cleanup after > > calling device_unregister(&my_dev->dev); > > That doesn't sound right. Generally the call to device_unregister() and > the release method live in the same module. Maybe you meant to say > individual drivers kfree(my_dev->private_data) and do any additional > cleanup in their remove() routine.
Again, if we are talking about driver bound to a device then devices ->release() is irrelevant. If we are talking about driver that created device then driver's ->remove() is irrelevant.
> > This approach seems dangerous. Suppose there's mutex embedded in > my_dev->private_data, and suppose some other thread is blocked waiting on > that mutex when remove() is called. That other thread will then oops when > my_dev->private_data is deallocated.
What other thread? I suppose it is module-local thread or subsystem-local thread. Let's that particular subsystem take care of it's own data and use its own ->release() when it is ready. What I mean is there is no need to perform clean-up at once; every layer can do its own cleanup.
> > > Second option: > > > > struct my_device { > > type member1; > > type member2; > > > > struct device *dev; > > }; > > > > dev is coming from _device_create(). Driver core takes care of > > releasing dev structure; driver does cleanup of my_device. > > Lots of drivers create devices dynamically without using device_create(). > > More to the point, how does the driver clean up my_device? It probably > has a reference count somewhere in my_device, especially if my_device is > shared with other threads or other drivers. We then face exactly the same > problem: What happens if the driver's module is unloaded before all the > references to my_device are gone? >
This is up to subsystem to ensure that it does not access dead devices.
> > With current sysfs orphaning attributes upon removal request there is > > no issue of accessing driver-private data through references obtained > > via ether embedded or referenced dev structure so everything is fine. > > Not so. There are other pathways besides sysfs which can cause a driver > to access its data structures. >
Which ones? These needs to be identified and treated with "immediate disconnect" that you advocated earlier. Once active users of device's services are gone you can zap it.
-- Dmitry - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |