incompatible pci sysfs change since 3.12 (5136b2da770d)

incompatible pci sysfs change since 3.12 (5136b2da770d)

[Jeff Epler]

Hi.

I am an author of LinuxCNC, a GPL'd CNC control for Linux.

Recently we added support for userspace PCI drivers.  This worked with
kernel 3.2 but doesn't with kernel 3.16.  The software fails early with
    Failed to open "/sys/devices/pci0000:00/.../enable" (Permission denied)

This appears to be because our software relies on the documented
"enable" sysfs file for pci devices (Documentation/filesystem/sysfs-pci.txt)
which was (unintentionally?) changed to "enabled" in the above-named
patch:

5136b2da770d PCI: convert bus code to use dev_groups
...
+static DEVICE_ATTR_RW(enabled);
...
-   __ATTR(enable, 0600, is_enabled_show, is_enabled_store),

Are we in the LinuxCNC project wrong in thinking that stuff in /sys (and
not /sys/debug) is supposed to be a durable API/interface for userspace
to the kernel?  (It must be a low-usage API if it went unnoticed for a
year :-/)

We'll have to work around it by modifying our software (since we'd like
to work with the kernels people already have) in any case.

Even if it is not going to be changed compatibly with older kernels, it
seems like the documentation should be updated!

Jeff

Re: incompatible pci sysfs change since 3.12 (5136b2da770d)

[Greg Kroah-Hartman]

On Wed, Oct 29, 2014 at 07:22:58PM -0500, Jeff Epler wrote:
> Hi.
> 
> I am an author of LinuxCNC, a GPL'd CNC control for Linux.
> 
> Recently we added support for userspace PCI drivers.  This worked with
> kernel 3.2 but doesn't with kernel 3.16.  The software fails early with
>     Failed to open "/sys/devices/pci0000:00/.../enable" (Permission denied)
> 
> This appears to be because our software relies on the documented
> "enable" sysfs file for pci devices (Documentation/filesystem/sysfs-pci.txt)
> which was (unintentionally?) changed to "enabled" in the above-named
> patch:
> 
> 5136b2da770d PCI: convert bus code to use dev_groups
> ...
> +static DEVICE_ATTR_RW(enabled);
> ...
> -   __ATTR(enable, 0600, is_enabled_show, is_enabled_store),
> 
> Are we in the LinuxCNC project wrong in thinking that stuff in /sys (and
> not /sys/debug) is supposed to be a durable API/interface for userspace
> to the kernel?  (It must be a low-usage API if it went unnoticed for a
> year :-/)

Ugh, that's my fault, I made a typo and should not have renamed the
sysfs file, very sorry about that.

I'll work on making up a patch to fix this and get it into the stable
kernels so that you don't have to have a work-around for very long.

> We'll have to work around it by modifying our software (since we'd like
> to work with the kernels people already have) in any case.
> 
> Even if it is not going to be changed compatibly with older kernels, it
> seems like the documentation should be updated!

I'll fix it up, this was just a bug, my apologies.

greg k-h

Re: incompatible pci sysfs change since 3.12 (5136b2da770d)

[Jeff Epler]

On Wed, Oct 29, 2014 at 05:36:57PM -0700, Greg Kroah-Hartman wrote:
> Ugh, that's my fault, I made a typo and should not have renamed the
> sysfs file, very sorry about that.
> 
> I'll work on making up a patch to fix this and get it into the stable
> kernels so that you don't have to have a work-around for very long.

Thank you!  I appreciate your quick response.

Jeff

Re: incompatible pci sysfs change since 3.12 (5136b2da770d)

[Greg Kroah-Hartman]

On Wed, Oct 29, 2014 at 08:16:15PM -0500, Jeff Epler wrote:
> On Wed, Oct 29, 2014 at 05:36:57PM -0700, Greg Kroah-Hartman wrote:
> > Ugh, that's my fault, I made a typo and should not have renamed the
> > sysfs file, very sorry about that.
> > 
> > I'll work on making up a patch to fix this and get it into the stable
> > kernels so that you don't have to have a work-around for very long.
> 
> Thank you!  I appreciate your quick response.

I've now sent a patch that should fix this.  If you could verify it, I'd
appreciate it.

thanks,

greg k-h

Re: incompatible pci sysfs change since 3.12 (5136b2da770d)

[Jeff Epler]

On Thu, Oct 30, 2014 at 09:30:53AM -0700, Greg Kroah-Hartman wrote:
> I've now sent a patch that should fix this.  If you could verify it, I'd
> appreciate it.

Will test.  Thank you.  The patch sure looks plausible.

Jeff