Messages in this thread | | | Date | Thu, 24 Jun 2004 02:37:29 -0400 (EDT) | From | Zwane Mwaikambo <> | Subject | Re: [PATCH]2.6.7 MSI-X Update |
| |
On Wed, 23 Jun 2004, Roland Dreier wrote:
> I could imagine hardware where the driver does not know exactly how > many vectors it will use until it starts up. As a hypothetical > example, imagine some storage networking host adapter that supports an > interrupt vector per storage target. The driver does not know how > many vectors it will actually use until it has logged into the storage > fabric; in fact, the driver may want to keep some vectors "in reserve" > in case a new target is added to the fabric later. > > I think it would be better to preserve maximum flexibility for devices > and drivers, and not mandate that every allocated MSI-X vector is > always used.
The MSI subsystem should at most reserve and the driver make a request. There may be a limit per PCI device as specified by the MSI subsystem for some reason or other. Isn't this what we're all saying?
> It seems in the code right now you are able to tell if any MSI-X > vectors are hooked, since you wait for the last vector to be unhooked > to disable MSI-X. I would just have it be a WARN_ON() (or maybe > BUG_ON()) if a driver calls pci_disable_msix() without calling > free_irq for all its MSI-X vectors. > > Right now there is an issue if a driver is unloaded without freeing > all its IRQs -- the device will be left in MSI-X mode and can not be > recovered without rebooting.
This sounds like a case of bad driver bug generally the kernel would oops when the ISR text gets unloaded. What kind of behaviour do you expect here?
> Also, drivers have a problem in their error paths right now with > freeing MSI-X resources. For example, suppose a driver successfully > requests 4 MSI-X vectors. request_irq() is a function call that can > fail, for example if the kernel can't allocate memory. What should > the driver do if its second (out of 4) request_irq() call fails? > There doesn't seem to be any way for it to proceed without leaking > MSI-X resources.
I agree here, the request/free of vectors must be controllable in the driver, this is one place where we may have to allow people to hang themselves.
> Similarly, with the API as it stands in your patch, a driver must be > very careful not to take any action that may fail in between calling > pci_enable_msix() and actually calling request_irq(), or otherwise the > only way to avoid leaking MSI-X resources is to take the very risky > step of calling request_irq() on an error path. This doesn't fit very > well with the structure of lots of device drivers, for example Intel's > very own e1000 driver, which wait until the device is actually opened > to call request_irq().
Could you elaborate further here? Won't a matched pci_disable_msix() free the necessary resources on failure?
> For your second point, I would have pci_disable_msix() always free all > MSI-X vectors that have been allocated... the only parameter that I > expect it would take is a struct pci_dev *.
If the driver is doing this, then we won't have to bother about pci_disable_msix() doing the vector free surely?
Thanks Roland, Zwane - 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/
| |