automatic sysfs attribute creation

automatic sysfs attribute creation

[Alexander Stein]

Hello,

I've read Greg's blog post about sysfs attributes at http://www.kroah.com/log/blog/2013/06/26/how-to-create-a-sysfs-file-correctly/. We've used attribute groups for the whole time, but the post made me wondering whether our drivers might be affected by that too. Unfortunately the blog mentions only struct class, struct bus, struct device and so on. I couldn't find anything about USB interfaces or struct net_device. At least the latter has const struct attribute_group *sysfs_groups[4]; itself. So I tried setting sysfs_groups[0] in the probe function of my usb_driver before calling register_candev (which in return set the attribute groups in struct device later). So IMHO the sysfs files should be created before the new CAN interface is announced to userspace.
But I get the exact problem Greg describes: A udev rules shall set an attribute during ACTION="add". This USB device create 2 CAN interfaces and only on one device the attribute is set correctly. _Unless_: the rule file is named 75-usbcan.rules in /etc/udev/rules.d or has a higher number at the beginning. This indicates a race condition to me.
Am I doing anything wrong here? The described way should have prevented this.
BTW: Is there something similar for the USB device driver?

Regards and thanks in advance
Alexander

Re: automatic sysfs attribute creation

[Greg KH]

On Wed, Aug 21, 2013 at 05:17:15PM +0200, Alexander Stein wrote:
> Hello,
> 
> I've read Greg's blog post about sysfs attributes at
> http://www.kroah.com/log/blog/2013/06/26/how-to-create-a-sysfs-file-correctly/.
> We've used attribute groups for the whole time, but the post made me
> wondering whether our drivers might be affected by that too.

It depends on the driver type.

> Unfortunately the blog mentions only struct class, struct bus, struct
> device and so on. I couldn't find anything about USB interfaces or
> struct net_device. At least the latter has const struct
> attribute_group *sysfs_groups[4]; itself. So I tried setting
> sysfs_groups[0] in the probe function of my usb_driver before calling
> register_candev (which in return set the attribute groups in struct
> device later). So IMHO the sysfs files should be created before the
> new CAN interface is announced to userspace.

Yes, they "should".

> But I get the exact problem Greg describes: A udev rules shall set an
> attribute during ACTION="add". This USB device create 2 CAN
> interfaces and only on one device the attribute is set correctly.
> _Unless_: the rule file is named 75-usbcan.rules in /etc/udev/rules.d
> or has a higher number at the beginning. This indicates a race
> condition to me.  Am I doing anything wrong here? The described way
> should have prevented this.
> BTW: Is there something similar for the USB device driver?

The big issue is that for things like USB interfaces, where the device
is already created and announced to userspace, the driver binding
happens in the kernel afterward.  So, in the probe function for the USB
driver, if you want to attach to this device, and add sysfs files to it,
it will be "too late".

Which in a way makes sense, as the USB device is "owned" by the USB
core, not the driver.  For a network device, it doesn't make sense, and
I'm working on fixing this, right now there's not a way for you to
resolve this, it needs to be done in the network core, and it's on my
TODO list to resolve.

But then there is the issue of "platform" devices.  They get announced
to userspace before the driver is bound to them, but there's really only
ever one driver that can bind to it, so their sysfs files are also
created "late".  I don't know what to do about this example (which is
the same thing for the USB devices), but I'm open to ideas.

Right now I'm working on fixing up all of the attribute code in the
drivers (part of it will show up in 3.12), then I can move to resolving
the network driver issue you have pointed out, and then worry about
platform devices (and USB and the rest).  So it will be a while, unless
people wish to help out.

thanks,

greg k-h