On Wed, Jun 05, 2013 at 10:56:38PM +0200, Sebastian Andrzej Siewior wrote: > On Mon, May 13, 2013 at 11:05:48AM +0200, Alexander Gordeev wrote: > > Note, although the existing 'msi_desc::multiple' field might seem > > redundant, in fact in does not. In general case the number of MSIs a > > PCI device is initialized with is not necessarily the closest power- > > of-two value of the number of MSIs the device will send. Thus, in > > theory it would not be always possible to derive the former from the > > latter and we need to keep them both, to stress this corner case. > > Besides, since 'msi_desc::multiple' is a bitfield, throwing it out > > would not save us any space. > > The last paragraph makes me curious. The only place where 'multiple' is set is > in do_setup_msi_irqs() and this uses the next power of two for it. And since a > device is not enabled twice, it is not overridden. > So it should be possible to compute 'multiple' out of 'nvec' but it saves > cycles not do to so. I agree to keep 'multiple' but your argument does not > seem to make sense. > While nitpicking, 'nvec' might deserve a better comment than 'number of > messages' since it holds the number of allocated interrupts. :) Sebastian, I re-read my comment few times and I admit it might be confusing. You are right - 'multiple' is set by rounding up only. The part '...not necessarily the closest power-of-two value...' implied an abstract PCI device rather than the described code, but the wording is less than perfect, indeed. In fact, at the moment of writing I kept in mind a follow-up patch that could help with aforementioned devices. That would be a new interface: int pci_enable_msi_block_partial(struct pci_dev *dev, unsigned int nvec_use, unsigned int nvec_init); In this case 'nvec_use' would go to 'msi_desc::nvec_used' and 'nvec_init' would translate to 'msi_desc::multiple' in case 'nvec_init' is not zero. In case 'nvec_init' is zero, 'msi_desc::multiple' would be initialized with the maximum possible value for the device (the way it is done now for pci_enable_msi_block_auto() interface). So, for the AHCI device (Bjorn mentioned) such a call would conserve on 10 of 16 vectors: pci_enable_msi_block_partial(pdev, 6, 0); What I am not sure is whether we need to read out the maximum possible number of vectors like pci_enable_msi_block_auto() does: int pci_enable_msi_block_partial(struct pci_dev *dev, unsigned int nvec_use, unsigned int nvec_init, unsigned int *maxvec); I can not think of any use of 'maxvec' with this interface, but the second variant completes the whole picture about a device... > Sebastian -- Regards, Alexander Gordeev agordeev@xxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
