On Wed, 24 Apr 2013 14:10:38 -0600 Bjorn Helgaas <bhelgaas@xxxxxxxxxx> wrote: > On Tue, Apr 23, 2013 at 1:51 PM, Greg Rose <gregory.v.rose@xxxxxxxxx> > wrote: > > On Mon, 22 Apr 2013 14:50:33 -0700 > > Alexander Duyck <alexander.h.duyck@xxxxxxxxx> wrote: > > > >> On 04/22/2013 01:09 PM, Bjorn Helgaas wrote: > >> > On Sat, Apr 20, 2013 at 9:31 PM, Jeff Kirsher > >> > <jeffrey.t.kirsher@xxxxxxxxx> wrote: > >> >> On Sat, 2013-04-20 at 02:49 -0700, Jeff Kirsher wrote: > >> >>> From: Alexander Duyck <alexander.h.duyck@xxxxxxxxx> > >> >>> > >> >>> This function is meant to add a helper function that will > >> >>> determine if a PF has any VFs that are currently assigned to a > >> >>> guest. We currently have been implementing this function per > >> >>> driver, and going forward I would like to avoid that by making > >> >>> this function generic and using this helper. > >> >>> > >> >>> Signed-off-by: Alexander Duyck <alexander.h.duyck@xxxxxxxxx> > >> >>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@xxxxxxxxx> > >> >> Adding linux-pci mailing list and Bjorn to the CC. > >> >> > >> >> Bjorn- David Miller needs a signoff by PCI maintainer. > >> >> > >> >>> --- > >> >>> drivers/pci/iov.c | 41 > >> >>> +++++++++++++++++++++++++++++++++++++++++ include/linux/pci.h | > >> >>> 5 +++++ 2 files changed, 46 insertions(+) > >> >>> > >> >>> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c > >> >>> index ee599f2..fd99720 100644 > >> >>> --- a/drivers/pci/iov.c > >> >>> +++ b/drivers/pci/iov.c > >> >>> @@ -729,6 +729,47 @@ int pci_num_vf(struct pci_dev *dev) > >> >>> EXPORT_SYMBOL_GPL(pci_num_vf); > >> >>> > >> >>> /** > >> >>> + * pci_vfs_assigned - returns number of VFs are assigned to a > >> >>> guest > >> >>> + * @dev: the PCI device > >> >>> + * > >> >>> + * Returns number of VFs belonging to this device that are > >> >>> assigned to a guest. > >> >>> + * If device is not a physical function returns -ENODEV. > >> >>> + */ > >> >>> +int pci_vfs_assigned(struct pci_dev *dev) > >> > I guess the idea here is to replace be_find_vfs(), > >> > igb_vfs_are_assigned(), ixgbe_vfs_are_assigned(), etc. It does > >> > seem good to reduce duplicated code. > >> > >> The general idea was just to remove duplicate code. As is we have > >> a couple more drivers on the way that would end up needing a > >> similar function. > >> > >> > I'm trying to figure out why this is safe -- there's no explicit > >> > synchronization between the iteration through PCI devices looking > >> > for matching VFs and the device assignment/deassignment paths > >> > that set or clear PCI_DEV_FLAGS_ASSIGNED, so on the face of it, > >> > it looks like things could change between calling > >> > pci_vfs_assigned() and using the result to make a decision. > >> > > >> > Most of the calls would be in .remove() functions, so maybe > >> > there's some sort of synchronization in that path that makes > >> > this safe. > >> > > >> > Bjorn > >> > >> I'm assuming this will be used in regions that are somehow > >> protected since the main spots where this might be called would be > >> probe, remove, or when updating the number of VFs. From what I > >> can tell in the Xen case there is a driver stub that is loaded > >> that sets the flag so that is covered by probe/remove. I don't > >> know about the KVM case. > > > > KVM should be fine. Setting/clearing the flag occurs while a > > device is being assigned to or removed from a VM - presumably > > device assignment is already safe against race conditions. I'd > > find it hard to believe that it's not. Code is > > in ../virt/assigned_dev.c and ../virt/iommu.c. > > That's not a very convincing argument :) It's been a long time since I worked on that code. Sorry, it's the best I've got, my memory gets real hazy on code that I haven't touched in a year or two. But then that's part of my argument - if it were subject to race conditions it seems like someone would have run into it in the last couple of years. But my apologies for the less than convincing argument! :) - Greg -- 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
