On Mon, Jul 20, 2015 at 07:14:05PM -0500, Bjorn Helgaas wrote: > Previously, we allocated pci_ats structures when an IOMMU driver called > pci_enable_ats(). An SR-IOV VF shares the STU setting with its PF, so when > enabling ATS on the VF, we allocated a pci_ats struct for the PF if it > didn't already have one. We held the sriov->lock to serialize threads > concurrently enabling ATS on several VFS so only one would allocate the PF > pci_ats. > > Gregor reported a deadlock here: > > pci_enable_sriov > sriov_enable > virtfn_add > mutex_lock(dev->sriov->lock) # acquire sriov->lock > pci_device_add > device_add > BUS_NOTIFY_ADD_DEVICE notifier chain > iommu_bus_notifier > amd_iommu_add_device # iommu_ops.add_device > init_iommu_group > iommu_group_get_for_dev > iommu_group_add_device > __iommu_attach_device > amd_iommu_attach_device # iommu_ops.attach_device > attach_device > pci_enable_ats > mutex_lock(dev->sriov->lock) # deadlock > > There's no reason to delay allocating the pci_ats struct, and if we > allocate it for each device at enumeration-time, there's no need for > locking in pci_enable_ats(). > > Allocate pci_ats struct during enumeration, when we initialize other > capabilities. > > Note that this implementation requires ATS to be enabled on the PF first, > before on any of the VFs because the PF controls the STU for all the VFs. > > Link: http://permalink.gmane.org/gmane.linux.kernel.iommu/9433 > Reported-by: Gregor Dick <gdick@xxxxxxxxxxxxxx> > Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> > --- > drivers/pci/ats.c | 98 ++++++++++++++++++++--------------------------- > drivers/pci/probe.c | 3 + > drivers/pci/remove.c | 1 > include/linux/pci-ats.h | 2 - > include/linux/pci.h | 9 ++++ > 5 files changed, 56 insertions(+), 57 deletions(-) Looks good to me now. Reviewed-by: Joerg Roedel <jroedel@xxxxxxx> -- 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
