On Fri, 9 Mar 2007, Oliver Neukum wrote: > Am Freitag, 9. März 2007 21:08 schrieb Alan Stern: > > After some more thought, I basically agree with what Oliver wrote > > originally. sysfs_dirent is indeed the logical place to store the kref > > pointer. However it needs to be used during open and release, not during > > OK. > > > read, write, and poll. Another point, which Oliver didn't think of, is > > that the kref pointer needs to be passed to the driver as an argument in > > the show() and store() method calls. > > Why? What's wrong with simply calling kref_get/put?
It's the same old problem: the race between unbind and sysfs I/O. What good does holding a reference to the private data structure do if the show/store method gets called after the driver has been unbound from the device? dev_get_drvdata() will no longer provide a valid pointer to the private data, so the method will have no way to access it. Hence the method needs another argument. (BTW, the sysfs core would actually need more than a kref. It would also need a pointer to a release routine -- the kref contains only the atomic counter. The more you think about it, the more complicated this approach becomes.) > > Finally, there's added complexity in each driver which wants to use the > > new facility. The module_exit routine will need to be smart enough to > > block until all the private data structures have been released. > > usb-storage does something like that now; it's kind of ugly (although it > > could be improved if appropriate support were added to the core kernel). > > If we up the module count for every bound device, all device attributes > should be gone before we ever get that far. Not quite right. However, since every open sysfs file holds a module reference, if the driver's module_exit has been called then there can be no open sysfs files, hence no private data still pinned. Thus this isn't a problem at all. But never mind all the above. I'm going to post another message on this thread in which I argue that Oliver's original approach was a good one and should not have been reverted. The specific problem identified by Hugh Dickins can be fixed in the way Dmitry first suggested, by doing the real operation from a workqueue routine. Alan Stern - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
