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

Reply via email to