| Message ID | 20150721001454.28145.35617.stgit@bhelgaas-glaptop2.roam.corp.google.com |
|---|---|
| State | Accepted |
| Headers | show |
On Mon, Jul 20, 2015 at 07:14:54PM -0500, Bjorn Helgaas wrote: > There's no need to BUG() if we enable ATS when it's already enabled. We > don't need to BUG() when disabling ATS on a device that doesn't support ATS > or if it's already disabled. If ATS is enabled, certainly we found an ATS > capability in the past, so it should still be there now. > > Clean up these error paths. > > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> > --- > drivers/pci/ats.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c > index 0b5b0ed..67524a7 100644 > --- a/drivers/pci/ats.c > +++ b/drivers/pci/ats.c > @@ -44,11 +44,12 @@ int pci_enable_ats(struct pci_dev *dev, int ps) > u16 ctrl; > struct pci_dev *pdev; > > - BUG_ON(dev->ats_cap && dev->ats_enabled); > - > if (!dev->ats_cap) > return -EINVAL; > > + if (pci_ats_enabled(dev)) > + return -EINVAL; > + Better return -EBUSY here? It would be good to find out if we are trying to enable ATS while it is already enabled. A WARN_ON around this would also be good, so that we spot bugs in the iommu drivers early when they use this interface. Joerg -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c index 0b5b0ed..67524a7 100644 --- a/drivers/pci/ats.c +++ b/drivers/pci/ats.c @@ -44,11 +44,12 @@ int pci_enable_ats(struct pci_dev *dev, int ps) u16 ctrl; struct pci_dev *pdev; - BUG_ON(dev->ats_cap && dev->ats_enabled); - if (!dev->ats_cap) return -EINVAL; + if (pci_ats_enabled(dev)) + return -EINVAL; + if (ps < PCI_ATS_MIN_STU) return -EINVAL; @@ -83,7 +84,8 @@ void pci_disable_ats(struct pci_dev *dev) struct pci_dev *pdev; u16 ctrl; - BUG_ON(!dev->ats_cap || !dev->ats_enabled); + if (!pci_ats_enabled(dev)) + return; if (atomic_read(&dev->ats_ref_cnt)) return; /* VFs still enabled */ @@ -107,8 +109,6 @@ void pci_restore_ats_state(struct pci_dev *dev) if (!pci_ats_enabled(dev)) return; - if (!pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ATS)) - BUG(); ctrl = PCI_ATS_CTRL_ENABLE; if (!dev->is_virtfn)
There's no need to BUG() if we enable ATS when it's already enabled. We don't need to BUG() when disabling ATS on a device that doesn't support ATS or if it's already disabled. If ATS is enabled, certainly we found an ATS capability in the past, so it should still be there now. Clean up these error paths. Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> --- drivers/pci/ats.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html