Re: [PATCH] PCI: pci_bus_add_device() should succeed even ifcreating sysfs files fails

[Artem Savkov]

On Wed, Apr 17, 2013 at 10:42:12AM -0600, Bjorn Helgaas wrote:
> On Wed, Apr 17, 2013 at 9:54 AM, Bjorn Helgaas <bhelgaas@xxxxxxxxxx> wrote:
> > On Wed, Apr 17, 2013 at 5:06 AM, Artem Savkov <artem.savkov@xxxxxxxxx> wrote:
> >> Introduced in cc8f7f9e4e79a0940af6b4b6fdfbcf18a03aa9f4
> >> ("PCI: Fix __must_check annotation on pci_create_sysfs_dev_files()")
> >>
> >> pci_bus_add_device() should not fail to add device if
> >> pci_create_sysfs_dev_files() call fails, as sysfs files are not critical.
> >>
> >> pci_sysfs_init() is a "late_initcall" and is called after acpi_init, so
> >> pci_bus_add_device() fails to create devices during boot resulting in a
> >> bug during acpi_init. It is safe not to create sysfs_dev_files in
> >> pci_bus_add_device() as they are later created in pci_sysfs_ini().
> >> ...
> 
> > Sorry for the breakage.  Thanks for the report, analysis, and fix.
> > I'll fix this up
> 
> It's not clear to me what the best fix is, so I just dropped my original patch.
> 
> It's clearly wrong to have __must_check on the
> pci_create_sysfs_dev_files() definition, because it doesn't do
> anything there.
> 
> We could simply drop the __must_check altogether, as you suggest, but
> I'd rather structure things to avoid the failure in the first place
> rather than ignoring an error.
> 
> It would be ideal if we could get rid of the pci_sysfs_init()
> late_initcall and just make the pci_create_sysfs_dev_files() from
> pci_bus_add_device() work all the time, even at boot-time.  I didn't
> look hard enough to figure out why this needs to be a late_initcall.
> In any case, we can figure this out later, after we merge the current
> stuff for v3.10.

It looks like late_initcall is either due to "it belongs there" or maybe
pci_create_sysfs_dev_files() was initially only called from
pci_sysfs_init() and pci subsystem should have been initialized before
this for devices to be there to add to sysfs. I've tried changing
late_initcall to subsys_initcall and didn't see any immediate problems.

Anyway even with things properly aligned it still sounds like a bad idea
to fail pci_bus_add_device() call just because creating sysfs files
fails. Some kind of warning should be enough.

-- 
Regards,
Artem
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/