Re: [PATCH] sysfs: add per pci device msi[x] irq listing (v3)

Neil Horman
2011-09-19T17:34:03Z

On Mon, Sep 19, 2011 at 10:14:11AM -0700, Greg KH wrote:
> On Mon, Sep 19, 2011 at 11:47:15AM -0400, Neil Horman wrote:
> > So a while back, I wanted to provide a way for irqbalance (and other apps) to
> > definitively map irqs to devices, which, for msi[x] irqs is currently not really
> > possible in user space. My first attempt wen't not so well:
> > https://lkml.org/lkml/2011/4/21/308
> >
> > It was plauged by the same issues that prior attempts were, namely that it
> > violated the one-file-one-value sysfs rule. I wandered off but have recently
> > come back to this. I've got a new implementation here that exports a new
> > subdirectory for every pci device, called msi_irqs. This subdirectory contanis
> > a variable number of numbered subdirectories, in which the number represents an
> > msi irq. Each numbered subdirectory contains attributes for that irq, which
> > currently is only the mode it is operating in (msi vs. msix). I think fits
> > within the constraints sysfs requires, and will allow irqbalance to properly map
> > msi irqs to devices without having to rely on rickety, best guess methods like
> > interface name matching.
> >
> > Change Notes:
> >
> > (v2)
> > Fixed up Documentation to put new sysfs interface descriptions in the right
> > place, as per request by Greg K-H
> >
> > Fixed up oops that resulted from removing pci device. Not 100% sure I did this
> > exactly right, but looking at the crash (triggered by echo 1 >
> > /sys/class/net/eth0/device/remove), it looked as though we were freeing the
> > pci_dev struct prior to all sysfs objects releasing their use of the device. AS
> > such it seemed most appropriate to hold references on the pci_dev for each msi
> > irq sysfs object that we create, and release them on free accordingly. With
> > this change in place, I can remove, and add (via rescan) msi enabled devices
> > ad-nauseum without a panic. Again thanks to Greg K-H
> >
> > (v3)
> > As per Gregs suggestion, I looked further and noted that in fact, yes, it wasn't
> > producing any errors on remove, but only because I had a refcounting problem,
> > and my new sysfs objects were left orphaned with a dangling refcount. I've
> > fixed that, added a release method to the new ktype, which now drops the
> > reference I hold on the pci_dev for us and I've validated that all objects I've
> > created, along with the parent directory and pci device are cleaned up and freed
> > by enabling the kobject dyanic_debug set and observing the appropriate release
> > calls. I can provide the logs if anyone wants to review them specifically.
>
> Wonderful, thanks for doing this, the code now looks fine to me, and if
> the PCI maintainer has no objections to it, feel free to add my:
> Acked-by: Greg Kroah-Hartman <gregkh@suse.de>
> to it.
>
> nice job, thanks for sticking with it.
>
No problem, thanks for your reviews!
Neil

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