Re: Missing removal of sysfs attributes in gpiolib

[Guenter Roeck <linux@roeck-us.net>]

On Wed, Aug 28, 2013 at 11:53:06AM -0700, Greg Kroah-Hartman wrote:
> On Wed, Aug 28, 2013 at 11:44:33AM -0700, Guenter Roeck wrote:
> > On Wed, Aug 28, 2013 at 11:31:39AM -0700, Greg Kroah-Hartman wrote:
> > > On Wed, Aug 28, 2013 at 10:57:17AM -0700, Guenter Roeck wrote:
> > > > Hi,
> > > > 
> > > > I noticed that gpiolib does not explicitly remove the sysfs attributes it
> > > > created when exporting a gpio pin when the pin is unexported, ie when the
> > > > associated device is removed.
> > > > 
> > > > Are those attributes auto-removed when device_unregister() is called ?
> > > 
> > > Yes they are.
> > > 
> > Does that mean that the explicit attribute removal that is implemented
> > in many drivers can be dropped ? That might save a lot of code.
> 
> Yes, they could be dropped, but I think it's nicer to be explicit about
> this.  Originally, it could be a long time before the kernel would
> remove the sysfs device from the tree, so removing the files could cause
> problems as the callbacks would happen for a device that the driver was
> not expecting to be present.
> 
> Also, if a driver is "unbound" from a device, and the device sticks
> around, the sysfs files are not removed automatically, it's up to the
> driver to remove them, so it's good practice to remove them.
> 
> > > > Sorry if this is a dumb question - I have not noticed this anywhere else,
> > > > and I don't seem to be able to find the code actually performing auto-removal
> > > > of manually created attributes, so I wonder if this is a bug or intentional.
> > > 
> > > Hm, I thought this was listed in the kobject.txt documentation file, but
> > > I don't seem to find it there.
> > > 
> > Would be great if this could be documented somewhere (unless it is and
> > I didn't find it ;). I would volunteer to do it myself, but I have no idea
> > what the conditions for auto-removal are nor how to phrase it, nor where
> > to put it.
> 
> When the kobject is removed from the system, the sysfs files attached to
> it are also removed.  That could go into kobject.txt somewhere, but the
> attributes are what need to be highlighted, and used, so that's a better
> thing to focus on at the moment.
> 
> > > But, ideally you aren't creating individual attributes directly, you are
> > > using attribute groups for the device, right?
> > > 
> > gpiolib does currently create individual attributes as well as attribute
> > groups after registering the device. I was in the process of converting
> > it to use device_create_with_groups() when I noticed that there is no
> > removal, so I wondered.
> 
> The driver core will remove the attributes at the point the device goes
> away, no need to remove them on your own here at all.
> 
> > I take it that attribute groups created with sysfs_create_group()
> > are auto-removed as well, correct ?  
> 
> Yes they are, but no driver, or subsystem, should be making that call,
> they should be using the driver core pointers for attributes instead to
> properly handle the lifespan of the attributes for the device.
> 
A good reason to use device_create_with_groups() instead of creating
attributes manually.

Guenter