Date:	Mon, 19 Sep 2011 13:33:53 -0400
From:	Neil Horman <nhorman@...driver.com>
To:	Greg KH <gregkh@...e.de>
Cc:	linux-kernel@...r.kernel.org,
	Jesse Barnes <jbarnes@...tuousgeek.org>,
	linux-pci@...r.kernel.org
Subject: Re: [PATCH] sysfs: add per pci device msi[x] irq listing (v3)

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@...e.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@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists