| Message ID | 4A4094A4.1070603@jp.fujitsu.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Tue, Jun 23, 2009 at 05:39:00PM +0900, Hidetoshi Seto wrote: > In current code it continues setup even if alloc_msi_entry for MSI-X > is failed due to lack of memory. It means arch_setup_msi_irqs() might > be called with msi_desc entries less than its argument nvec. Yeah. To be honest, this is silly. 32-byte GFP_KERNEL allocations basically don't fail. > This patch fixes the error route to return the number of entries can be > allocated (or -ENOMEM), instead of continuing the setup. It will let > drivers to retry pci_enable_msix() with less number of entries. If we're running out of memory while doing pci_enable_msix(), should we even be trying again? Something else is bound to run out of memory later (creating irqs consumes an awful lot of memory). Might as well return -ENOMEM and have done with it. > Note that there is no iounmap in msi_free_irqs() if no msi_disc is > allocated. Yes, that's unfortunate. > Signed-off-by: Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com> > --- > drivers/pci/msi.c | 10 ++++++++-- > 1 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c > index e176316..e784526 100644 > --- a/drivers/pci/msi.c > +++ b/drivers/pci/msi.c > @@ -463,8 +463,14 @@ static int msix_capability_init(struct pci_dev *dev, > > for (i = 0; i < nvec; i++) { > entry = alloc_msi_entry(dev); > - if (!entry) > - break; > + if (!entry) { > + if (!i) { > + iounmap(base); > + return -ENOMEM; > + } > + msi_free_irqs(dev); > + return i; > + } > > j = entries[i].entry; > entry->msi_attrib.is_msix = 1; > -- > 1.6.3.3 > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c index e176316..e784526 100644 --- a/drivers/pci/msi.c +++ b/drivers/pci/msi.c @@ -463,8 +463,14 @@ static int msix_capability_init(struct pci_dev *dev, for (i = 0; i < nvec; i++) { entry = alloc_msi_entry(dev); - if (!entry) - break; + if (!entry) { + if (!i) { + iounmap(base); + return -ENOMEM; + } + msi_free_irqs(dev); + return i; + } j = entries[i].entry; entry->msi_attrib.is_msix = 1;
In current code it continues setup even if alloc_msi_entry for MSI-X is failed due to lack of memory. It means arch_setup_msi_irqs() might be called with msi_desc entries less than its argument nvec. At least x86's arch_setup_msi_irqs() uses list_for_each_entry() for dev->msi_list that suspected to have entries same numbers as nvec, and it doesn't check the number of allocated vectors and passed arg nvec. Therefore it will result in success of pci_enable_msix(), with less vectors allocated than requested. This patch fixes the error route to return the number of entries can be allocated (or -ENOMEM), instead of continuing the setup. It will let drivers to retry pci_enable_msix() with less number of entries. Note that there is no iounmap in msi_free_irqs() if no msi_disc is allocated. Signed-off-by: Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com> --- drivers/pci/msi.c | 10 ++++++++-- 1 files changed, 8 insertions(+), 2 deletions(-)