Re: refcounting drivers' data structures used in sysfs buffers

Alan Stern
2007-03-09T20:08:34Z

On Fri, 9 Mar 2007, Alan Stern wrote:

> Oliver, your idea won't work either. Think about what would happen if
> someone did
>
> rmmod driver_module </sys/devices/.../attribute_file
>
> The rmmod process would never actually read the attribute, so until it
> exited the private data structure would have a positive refcount. But
> rmmod can't exit until the driver has been unloaded from memory, and it
> can't be unloaded while its data structure is still allocated. Thus we
> would end up with deadlock; rmmod would hang forever.

I take this back. Redirecting stdin to the attribute file would increase
the module's refcount and cause rmmod to exit immediately with an error.

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
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.

Implementing this will be difficult. One possibility is to change the
definition of sysfs_ops, adding the new struct kref * argument to the
prototypes. This will involve changing _lots_ of source files, adding an
unused argument to many functions, which isn't attractive.

The other possibility is to test at runtime whether the kref pointer is
NULL, and if it is, don't pass it. This would work, but it isn't
type-safe.

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).

Alan Stern