On Tue, 2009-02-10 at 13:46 -0700, Matthew Wilcox wrote:
> On Mon, Jan 19, 2009 at 12:07:30PM +1100, Michael Ellerman wrote:
> > There is code in msix_capability_init() which, when the requested number
> > of MSI-X couldn't be allocated, calculates how many MSI-X /could/ be
> > allocated and returns that to the driver. That allows the driver to then
> > make a second request, with a number of MSIs that should succeed.
> >
> > The current code requires the arch code to setup as many msi_descs as it
> > can, and then return to the generic code. On some platforms the arch
> > code may already know how many MSI-X it can allocate, before it sets up
> > any of the msi_descs.
>
> Would it be fair to say that those platforms are the same ones which
> implement arch_msi_check_device(), which already works for this case?
Yes and yes but ..
We can do some checking in arch_msi_check_device() and return a
"could-be-allocated" value from there[1].
But we still want to be able to return a "could-be-allocated" value from
arch_setup_msi_irqs(), without going through the business of setting up
msi_descs.
Simply because when we do the firmware call to actually allocate the
MSIs, it might give us less than we ask for. At that point we can just
return that number.
We /could/ try to allocate in arch_msi_check_device(), see what we get,
deallocate, and return the number. But it's a) ugly as hell, and b)
still possible that when we come back to do the real allocation we get a
different number.
> > @@ -43,7 +43,8 @@ int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
> > list_for_each_entry(entry, &dev->msi_list, list) {
> > ret = arch_setup_msi_irq(dev, entry);
> > if (ret)
> > - return ret;
> > + /* Ensure the error code is always < 0 */
> > + return min(ret, -ret);
> > }
> >
> > return 0;
>
> I really don't like this. If we're returning an error, we should return
> a sensible error value. Something like:
>
> if (ret < 0)
> return ret;
> if (ret > 0)
> return -ENOSPC;
Fair enough. My reasoning was that negating it at least left some chance
that you could work out where it came from. But I'm not fussed, and I
admit it's a bit ugly.
cheers
[1]: Which we (powerpc) actually don't do at the moment, but I have a
patch on the way for that.
--
Michael Ellerman
OzLabs, IBM Australia Development Lab
wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)
We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person
Attachment:
signature.asc
Description: This is a digitally signed message part
