Re: device_remove_file and disconnect

[Greg KH <greg@kroah.com>]

On Thu, Jun 30, 2005 at 09:26:43AM +0200, mat@mut38-1-82-67-62-65.fbx.proxad.net wrote:
> Hi,
> 
> On Wed, Jun 29, 2005 at 03:42:35PM -0700, Greg KH wrote:
> > On Wed, Jun 29, 2005 at 10:17:59PM +0200, matthieu castet wrote:
> > > Hi,
> > > 
> > > Greg KH wrote:
> > > >On Wed, Jun 29, 2005 at 06:59:00PM +0200, matthieu castet wrote:
> > > >
> > > >>Hi,
> > > >>
> > > >>I have a question about sysfs interface.
> > > >>
> > > >>If you open a sysfs file created by a module, then remove it (rmmoding 
> > > >>the module that create this sysfs file), then try to read the opened 
> > > >>file, you often get strange result (segdefault or oppps).
> > > >
> > > >
> > > >What file did you do this for?  The module count should be incremented
> > > >if you do this, to prevent the module from being unloaded.
> > > >
> > > Ok, but if we unplug a device, then disconnect will be called even if we 
> > > opened a sysfs file.
> > 
> > Yes but the device structure will still be in memory, so you will be ok.
> > 
> disconnect method isn't supposed to clear the device structure in memory ?

It all depends on the bus you are talking about :)

> > > Couldn't be a race between the moment we read our private data and check 
> > > it is valid and the moment we use it :
> > > 
> > > Process A (read/write sysfs file) 		Process B (disconnect)
> > > recover our private data from struct device
> > > check it is valid
> > > 						free our private data
> > > do operation on private data
> > 
> > No, you should not be freeing your private data on your own.  You should
> > do that in the device release function.
> But that's what I do !

What type of driver?

> I free it in  device release function = usb_disconnect in my case =
> Process B. Process A is call by sysfs and don't seem serialized with
> Process B.
> Is there another place where I could free my private data later : I don't
> think so.

For usb, no, you are correct.  I was referring to the driver core when
you were mentioning the sysfs stuff, sorry.

Hm, in thinking about it, it might make more sense to rework the usb
core to handle this better.  Possibly add a release() callback to the
driver when the device is actually being freed.  Wouldn't be that hard
to do so, and might cut down on some of the common locking errors.

> The problem is that device_remove_file don't block if the sysfs file is
> opened. So we must serialized A and B with mutex.

Yes.

> > Again, any specific place in the kernel that you see not doing this?
> I believe some drivers expected that sysfs read/write callback are always
> called when the device is plugged so they don't check if
> to_usb_interface/usb_get_intfdata return valid pointer.

Then they should be fixed.  Any specific examples?

> Also I always see driver free their privatre data in device disconnect,
> so if read/write from sysfs aren't serialized with device disconnect
> there are still a possible race like I show in my example.

Yes, you are correct.  Again, any specific drivers you see with this
problem?

thanks,

greg k-h
-
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/