lkml.org 
[lkml]   [2017]   [Mar]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] pci/sriov: Add an option to probe VFs or not before enabling SR-IOV
    On Mon, 20 Mar 2017 18:34:23 -0500
    Bodong Wang <bodong@mellanox.com> wrote:

    > On 3/20/2017 6:07 PM, Gavin Shan wrote:
    > > On Mon, Mar 20, 2017 at 05:14:34PM +0200, bodong@mellanox.com wrote:
    > >> From: Bodong Wang <bodong@mellanox.com>
    > >>
    > >> Sometimes it is not desirable to probe the virtual functions after
    > >> SRIOV is enabled. This can save host side resource usage by VF
    > >> instances which would be eventually probed to VMs.

    What resources would not be released when the VF driver is unbound?

    > >> Added a new PCI sysfs interface "sriov_probe_vfs" to control that
    > > >from PF, all current callers still retain the same functionality.
    > >> To modify it, echo 0/n/N (disable probe) or 1/y/Y (enable probe) to
    > >>
    > >> /sys/bus/pci/devices/<DOMAIN:BUS:DEVICE.FUNCTION>/sriov_probe_vfs
    > >>
    > >> Note that, the choice must be made before enabling VFs. The change
    > >> will not take effect if VFs are already enabled. Simply, one can set
    > >> sriov_numvfs to 0, choose whether to probe or not, and then resume
    > >> sriov_numvfs.
    > >>
    > > Bodong, I'm not sure if there is a requirement to load driver for the
    > > specified number of VFs? That indicates no driver will be loaded for
    > > other VFs. If so, this interface might serve the purpose as well.
    > Gavin, thanks for the review. That is indeed an interesting suggestion.
    > Theoretically, we can change that probe_vfs from boolean to integer.
    > And use it as a counter to probe the first N VFs(if N < total_vfs).
    > Let's see if there are any objections.

    Is it just me or does this seem like a confusing user interface, ie. to
    get binary on/off behavior a user now needs to 'cat total_vfs >
    sriov_probe_vfs'. It's not very intuitive, what's the use case for it?


    > >
    > >
    > > +#ifdef CONFIG_PCI_IOV
    > > + if (!pci_dev->is_virtfn ||
    > > + (pci_dev->is_virtfn && pci_dev->physfn->sriov->probe_vfs)) {
    > > +#endif
    > > + error = __pci_device_probe(drv, pci_dev);
    > > + if (error) {
    > > + pcibios_free_irq(pci_dev);
    > > + pci_dev_put(pci_dev);
    > > + }
    > > +#ifdef CONFIG_PCI_IOV
    > > }
    > > +#endif
    > >
    > > I think it's reasonable to have a inline function for this check:
    > It's doable, but what's the benefit?

    Way cleaner.

    > >
    > > #ifdef CONFIG_PCI_IOV
    > > static inline bool pci_device_can_probe(struct pci_dev *pdev)
    > > {
    > > return (!pdev->is_virtfn || pdev->physfn->sriov->probe_vfs);
    > should be return (!pdev->is_virtfn || (pci_dev->is_virtfn &&
    > pci_dev->physfn->sriov->probe_vfs));
    >
    > We want to probe that device if 1) it's a PF 2) it'a VF and probe_vfs is set
    > > }
    > > #else
    > > static inline bool pci_device_can_probe(struct pci_dev *pdev)
    > > {
    > > return true;
    > > }
    > This function will be a waste if CONFIG_PCI_IOV is not defined.
    > > #endif
    >
    > > Thanks,
    > > Gavin
    > >
    >

    \
     
     \ /
      Last update: 2017-03-21 06:06    [W:5.670 / U:0.012 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site