On Wed, Sep 18, 2013 at 12:30:23AM +1000, Michael Ellerman wrote: > How about no? > > We have a small number of MSIs available, limited by hardware & > firmware, if we don't impose a quota then the first device that probes > will get most/all of the MSIs and other devices miss out. Out of curiosity - how pSeries has had done it without quotas before 448e2ca ("powerpc/pseries: Implement a quota system for MSIs")? > Anyway I don't see what problem you're trying to solve? I agree the > -ve/0/+ve return value pattern is ugly, but it's hardly the end of the > world. Well, the interface recently has been re-classified from "ugly" to "unnecessarily complex and actively harmful" in Tejun's words ;) Indeed, I checked most of the drivers and it is incredible how people are creative in misusing the interface: from innocent pci_disable_msix() calls when if pci_enable_msix() failed to assuming MSI-Xs were enabled if pci_enable_msix() returned a positive value (apparently untested). Roughly third of the drivers just do not care and bail out once pci_enable_msix() has not succeeded. Not sure how many of these are mandated by the hardware. Another quite common pattern is a call to pci_enable_msix() to figure out the number of MSI-Xs available and a repeated call of pci_enable_msix() to enable those MSI-Xs, this time. The last pattern makes most of sense to me and could be updated with a more clear sequence - a call to (bit modified) pci_msix_table_size() followed by a call to pci_enable_msix(). I think this pattern can effectively supersede the currently recommended "loop" practice. But as pSeries quota is still required I propose to introduce a new interface pci_get_msix_limit() that combines pci_msix_table_size() and (also new) arch_get_msix_limit(). The latter would check the quota thing in case of pSeries and none in case of all other architectures. The recommended practice would be: /* * Retrieving 'nvec' by means other than pci_msix_table_size() */ rc = pci_get_msix_limit(pdev); if (rc < 0) return rc; /* * nvec = min(rc, nvec); */ for (i = 0; i < nvec; i++) msix_entry[i].entry = i; rc = pci_enable_msix(pdev, msix_entry, nvec); if (rc) return rc; Thoughts? > cheers -- Regards, Alexander Gordeev agordeev@xxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html