On Fri, Oct 06, 2006 at 01:23:26PM -0400, Alan Stern wrote: > On Thu, 5 Oct 2006, Greg KH wrote: > > > > It's not safe for a driver to create an attribute file in > > > a sysfs directory unless the driver also owns the directory! > > > > <snip example> > > > > Yeah, this has been known for some time without any solution :( > > > > Although it isn't usually as bad as you think, as the module is still > > pinned in memory when the file is open, so the code doesn't go away. > > But yeah, the pointer back to the usb_device could be stale and things > > still go boom that way. > > > > > Here's an alternative approach. Define struct sub_device as a sort of > > > lightweight struct device. It won't have most of the supporting > > > infrastructure, not even an embedded kobject; it's only purpose will be to > > > hold attribute files. These files will show up in the directory of the > > > sub_device's parent device. > > > > > > That way C can create and destroy the sub_device as needed, typically by > > > embedding it within the private data structure. Even though the attribute > > > files will show up in the original device directory, the callbacks will > > > receive a pointer to the sub_device structure, and so C will have total > > > control. > > > > But how will the removal of the sub_device help with the fact that > > userspace could still have the file open? > > > > I like the general sound of the idea, just don't know how it would solve > > the private data issue from going stale. > > Here's what would happen. C gets a disconnect call and does > dev_set_drvdata(dev, NULL), so it no longer owns the device. But it still > owns the sub_device. The attributes are unregistered, but there can be > open file references in userspace. They keep the sub_device's refcount > above 0, so the private data structure (which uses the sub_device's > embedded kref) remains in memory. As opposed to the current situation, > where open references to an attribute pin the device but not the private > data structure. > > Now suppose userspace accesses the show or set methods for an attribute. > The callback is invoked, and it is passed a pointer to the sub_device -- > not the device! C can use container_of() to get at the private data > structure, and then it can determine that it has been unbound from the > device. (For instance, a flag in the private data structure may be > cleared, or C can test whether dev_get_drvdata(dev) still gives the > address of the private data structure.) Knowing that, the attribute > method can do something sensible. > > (Synchronizing attribute method calls with disconnect remains an issue. > Perhaps struct sub_device should include a mutex for that purpose.) > > By contrast, if the attribute method has only a pointer to the device then > it can't do anything. dev_get_drvdata(dev) might return NULL, or the > device may have been bound to another driver entirely. There's no way to > go from dev to the private data structure, which might already have been > deallocated! C has no way of knowing what's going on. > > Basically, you can think of a sub_device as a regular struct device > without its own directory in sysfs -- it shares use of its parent's > directory.
Ok, I'm sold, this looks sane, but implementing it might be another matter... Good luck :) greg k-h ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys -- and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
