On Fri, Oct 11, 2013 at 04:29:39PM -0400, Mark Lord wrote:
> > static int xx_alloc_msix_irqs(struct xx_dev *dev, int nvec)
> > {
> > nvec = roundup_pow_of_two(nvec); /* assume 0 > nvec <= 16 */
> >
> > xx_disable_all_irqs(dev);
> >
> > pci_lock_msi(dev->pdev);
> >
> > rc = pci_get_msix_limit(dev->pdev, nvec);
> > if (rc < 0)
> > goto err;
> >
> > nvec = min(nvec, rc); /* if limit is more than requested */
> > nvec = rounddown_pow_of_two(nvec); /* (a) */
> >
> > xx_prep_for_msix_vectors(dev, nvec);
> >
> > rc = pci_enable_msix(dev->pdev, dev->irqs, nvec); /* (b) */
> > if (rc < 0)
> > goto err;
> >
> > pci_unlock_msi(dev->pdev);
> >
> > dev->num_vectors = nvec; /* (b) */
> > return 0;
> >
> > err:
> > pci_unlock_msi(dev->pdev);
> >
> > kerr(dev->name, "pci_enable_msix() failed, err=%d", rc);
> > dev->num_vectors = 0;
> > return rc;
> > }
>
> That would still need a loop, to handle the natural race between
> the calls to pci_get_msix_limit() and pci_enable_msix() -- the driver and device
> can and should fall back to a smaller number of vectors when pci_enable_msix() fails.
Could you please explain why the value returned by pci_get_msix_limit()
might change before pci_enable_msix() returned, while both protected by
pci_lock_msi()?
Anyway, although the loop-free code (IMHO) reads better, pci_lock_msi()
it is not a part of the original proposal and the more I think about it
the less I like it.
--
Regards,
Alexander Gordeev
agordeev@xxxxxxxxxx
