On Sun, 2008-07-06 at 20:41 -0600, Matthew Wilcox wrote: > On Mon, Jul 07, 2008 at 12:05:24PM +1000, Michael Ellerman wrote: > > On Sat, 2008-07-05 at 09:34 -0400, Matthew Wilcox wrote: > > > This first part simply changes the msi_attrib data structure to store > > > how many vectors have been allocated. In order to do this, I shrink the > > > 'type' from 5 bits to 2 and rename it to _type to catch any unsuspecting > > > users. > > > > Please don't, it significantly uglifies the code IMHO. Just add a new > > field for the size, I'd rather call it qsize to match the register. > > Uglifies the code? Seriously? Other than the _ addition (which really > I just did to be sure I didn't miss a case), how is MSI_ATTRIB uglier > than PCI_CAP_ID_MSI? Yeah seriously :) The _ is part of it, but MSI_ATTRIB is uglier than PCI_CAP_ID_MSI exactly because it's not PCI_CAP_ID_MSI, which exists and is well defined and is used in the rest of the code. > I'd like to rename the register definition from QSIZE. It's _not_ a > queue. I don't know where this misunderstanding came from, but I > certainly don't want to spread it any further. I didn't say it was a queue, but a Q ;) But I agree it's not a good name, the spec calls it "multiple message enable", nvec would match the existing code best, or log_nvec. > > If you're worried about bloating msi_desc, there's several fields in > > there that are per-device not per-desc, so we could do another patch to > > move them into pci_dev or something hanging off it, eg. > > pci_dev->msi_info rather than storing them in every desc. > > Might be worth it anyway for devices with lots of MSI-X interrupts. Eventually yeah, last I looked we didn't have any drivers using more than a few MSI-X, but at some point it will happen. > I think the MSI-X implementation is a bit poorly written anyway. If we > had an array of msi_desc for each device, we could avoid the list_head > in the msi_desc, for example. That'd save two pointers (8 or 16 bytes), > plus the overhead of allocating each one individually. Yeah that would be nice. > I also think that MSI-X could be improved by changing the interface to > do away with this msix_entry list passed in -- just allocate the irqs > consecutively. It would be nice, but as I said the other day we have at least one driver (s2io) which asks for non-consecutive entries. That doesn't effect the irq allocation, but you need some way for the driver to express it. cheers -- 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