Message-ID: <20130828193353.GA8518@roeck-us.net>
Date:	Wed, 28 Aug 2013 12:33:53 -0700
From:	Guenter Roeck <linux@...ck-us.net>
To:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:	linux-kernel@...r.kernel.org,
	Grant Likely <grant.likely@...aro.org>,
	Linus Walleij <linus.walleij@...aro.org>
Subject: Re: Missing removal of sysfs attributes in gpiolib

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists