lkml.org 
[lkml]   [2013]   [Sep]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2 2/6] PCI/MSI: Factor out pci_get_msi_cap() interface
    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@redhat.com


    \
     
     \ /
      Last update: 2013-09-18 13:01    [W:4.408 / U:0.092 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site