On Wed, Sep 18, 2013 at 09:22:31AM -0500, Tejun Heo wrote:

Hello,

On Wed, Sep 18, 2013 at 11:48:00AM +0200, Alexander Gordeev wrote:

quoted

On Wed, Sep 18, 2013 at 12:30:23AM +1000, Michael Ellerman wrote:

quoted

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")?

Hmmm... do we need to treat this any differently?  If the platform
can't allocate full range of requested MSIs, just failing should be
enough regardless of why such allocation can't be met, no?

quoted

quoted

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 ;)

LOL. :)

quoted

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.

Yeah, I mean, this type of interface is a trap.  People have to
actively resist to avoid doing silly stuff which is a lot to ask.

I really think you're overstating the complexity here.

Functions typically return a boolean   -> nothing to see here
This function returns a tristate value -> brain explosion!

quoted

	/*
	 * 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;

I really think what we should do is

* Determine the number of MSIs the controller wants.  Don't worry
  about quotas or limits or anything.  Just determine the number
  necessary to enable enhanced interrupt handling.
	
* Try allocating that number of MSIs.  If it fails, then just revert
  to single interrupt mode.  It's not the end of the world and mostly
  guaranteed to work.  Let's please not even try to do partial
  multiple interrupts.  I really don't think it's worth the risk or
  complexity.

It will potentially break existing setups on our hardware.

Can I make that any clearer?

cheers