Re: refcounting drivers' data structures used in sysfs buffers

Alan Stern
2007-03-09T19:32:09Z

On Fri, 9 Mar 2007, Dmitry Torokhov wrote:

> On 3/9/07, Oliver Neukum <oneukum@suse.de> wrote:
> > Am Freitag, 9. März 2007 18:02 schrieb Dmitry Torokhov:
> >
> > > I think we already have all refcounting that is needed. What is
> > > missing is subsystem-provided ->release() hooks for drivers to release
> > > driver-specific resources when a device finally goes away.
> >
> > This is an interesting idea. Is it nice to pass through release()
> > but not open() ?
> >
>
> Not sure if I follow... Generally speaking open is not a mandatory
> operation; however every object in driver model has a release method.
> What I am saying is that certain drivers need to have their disconnect
> method split in 2 parts - one that shuts down the device and second is
> releases resources that might be accesses through sysfs (and other
> kernel parts). That second part will have to be called from
> subsystem's core ->release() method se we need a release() hook.

Dmitry, you're not viewing this correctly.

Adding a new release() callback would solve the problem by creating
another. Drivers need to release their data as soon as possible after
they unbind from a device, not when the device itself goes away. Think
about what would happen if you tried to rmmod a driver. The rmmod process
would block until the device was unregistered.

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.

It might be better to keep your earlier patch and fix the deadlock you
mentioned earlier, the one that occurs when unbinding a driver through
sysfs. How exactly does that deadlock work?

Alan Stern