Re: Missing removal of sysfs attributes in gpiolib

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

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.

thanks,

greg k-h