On Tue, Nov 5, 2013 at 11:55 AM, Bjorn Helgaas <bhelgaas@xxxxxxxxxx> wrote: > On Sat, Nov 2, 2013 at 9:50 AM, Greg Kroah-Hartman > <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: >> On Fri, Nov 01, 2013 at 05:40:02PM -0600, Bjorn Helgaas wrote: >>> On Tue, Oct 29, 2013 at 3:46 PM, Greg Kroah-Hartman >>> <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: >>> > From: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> >>> > >>> > The PCI MSI sysfs code is a mess with kobjects for things that don't >>> > really need to be kobjects. This patch creates attributes dynamically >>> > for the MSI interrupts instead of using kobjects. >>> > >>> > Note, this does not delete the existing sysfs MSI code, but puts the >>> > attributes under a "msi_irqs_2" directory for testing / example. >>> > >>> > Also note, this removes a directory from the current MSI interrupt sysfs >>> > code: >>> > >>> > old MSI kobjects: >>> > pci_device >>> > └── msi_irqs >>> > └── 40 >>> > └── mode >>> > >>> > new MSI attributes: >>> > pci_device >>> > └── msi_irqs_2 >>> > └── 40 >>> > >>> > As there was only one file "mode" with the kobject model, the interrupt >>> > number is now a file that returns the "mode" of the interrupt (msi vs. >>> > msix). >>> > >>> > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> >>> > --- >>> > >>> > Bjorn, I can make up a patch that rips out the existing kobject code >>> > here, but I figured this patch would make things easier to follow >>> > instead of having to dig through the removed logic at the same time. >>> > >>> > I'll clean up the error handling path for the create attribute logic as >>> > well, this was just a proof of concept that this could be done. >>> > >>> > Do you think that anyone cares about the current mode files in sysfs to >>> > move things in this manner? >>> >>> I like this a lot better than trying to fix all the holes in the >>> current kobject code. >> >> Great. >> >>> I have no idea who, if anybody, cares about the "mode" files. I >>> assume there's a way to create the "mode" files with attributes, too? >>> If so, we could replicate the existing structure with one patch, and >>> simplify it with a second patch, so it would be easier to revert the >>> directory change while keeping the fix. >> >> No, we can't create a 2-level deep attribute at the moment, only one >> level, like the patch does. >> >> Based on Neil's comments, I think we should be fine with this as-is as >> no one is messing with these files directly (which implies that we could >> possibly just remove them entirely to save us the overall pain...) > > Hmmm. https://bugzilla.redhat.com/show_bug.cgi?id=744012 suggests > that irqbalance might be reading these files. I looked at the current irqbalance on github [1], and I *think* it never reads the "mode" files. It reads the entries in the "msi_irqs" directory, which you're proposing to change from directories to files, but I think it only uses the names. It looks like it should be safe at least for irqbalance to make this a one-level attribute. It's possible we'll break somebody's scripts, but I'm willing to try making this change because it really makes the refcounting much simpler. Bjorn [1] https://github.com/Irqbalance/irqbalance/blob/master/classify.c#L357 -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html