| Message ID | 79a2aeb6a06563c5f09463d53d2e0146e44c92be.1382103786.git.agordeev@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
Hello, Alexander. On Fri, Oct 18, 2013 at 07:12:06PM +0200, Alexander Gordeev wrote: > @@ -744,23 +744,6 @@ static int msix_capability_init(struct pci_dev *dev, > > return 0; > > -out_avail: > - if (ret < 0) { > - /* > - * If we had some success, report the number of irqs > - * we succeeded in setting up. > - */ > - struct msi_desc *entry; > - int avail = 0; > - > - list_for_each_entry(entry, &dev->msi_list, list) { > - if (entry->irq != 0) > - avail++; > - } > - if (avail != 0) > - ret = avail; > - } Hmmm... so, before this, the function would have returned the partial number of irqs that arch set up successfully. Is it okay to lose that information? If so, can you please elaborate a bit more on why it's okay in the description? Thanks.
On Wed, Nov 20, 2013 at 11:07:16AM -0500, Tejun Heo wrote: > Hmmm... so, before this, the function would have returned the partial > number of irqs that arch set up successfully. Is it okay to lose that > information? If so, can you please elaborate a bit more on why it's > okay in the description? Well, this is embarrassing. A quick check into x86 shows the partially initialized MSI-Xs are possible - something I should have noticed. Self-Nack. > tejun
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c index b43f391..bbe3d3d 100644 --- a/drivers/pci/msi.c +++ b/drivers/pci/msi.c @@ -719,7 +719,7 @@ static int msix_capability_init(struct pci_dev *dev, ret = arch_setup_msi_irqs(dev, nvec, PCI_CAP_ID_MSIX); if (ret) - goto out_avail; + goto out_free; /* * Some devices require MSI-X to be enabled before we can touch the @@ -744,23 +744,6 @@ static int msix_capability_init(struct pci_dev *dev, return 0; -out_avail: - if (ret < 0) { - /* - * If we had some success, report the number of irqs - * we succeeded in setting up. - */ - struct msi_desc *entry; - int avail = 0; - - list_for_each_entry(entry, &dev->msi_list, list) { - if (entry->irq != 0) - avail++; - } - if (avail != 0) - ret = avail; - } - out_free: free_msi_irqs(dev);
If an architecture failed to allocate requested number of MSI-Xs we should not mislead device drivers and make them retry based on how many MSI-Xs were possibly allocated by the architecture in the process, before the failure. It is rude for an architecture to leave any leftovers anyway. Signed-off-by: Alexander Gordeev <agordeev@redhat.com> --- drivers/pci/msi.c | 19 +------------------ 1 files changed, 1 insertions(+), 18 deletions(-)