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

Greg KH
2011-09-19T17:14:46Z

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.

greg k-h