Re: [PATCH v2 10/11] PCI: Stop caching ATS Invalidate Queue Depth

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/27/2015 06:27 PM, Bjorn Helgaas wrote:
Hi Don,

On Mon, Jul 27, 2015 at 10:00:53AM -0400, Don Dutile wrote:
On 07/20/2015 08:15 PM, Bjorn Helgaas wrote:
Stop caching the Invalidate Queue Depth in struct pci_dev.
pci_ats_queue_depth() is typically called only once per device, and it
returns a fixed value per-device, so callers who need the value frequently
can cache it themselves.

Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
---
  drivers/pci/ats.c   |    9 ++++-----
  include/linux/pci.h |    1 -
  2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
index 67524a7..bdb1383 100644
--- a/drivers/pci/ats.c
+++ b/drivers/pci/ats.c
@@ -20,16 +20,12 @@
  void pci_ats_init(struct pci_dev *dev)
  {
  	int pos;
-	u16 cap;

  	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ATS);
  	if (!pos)
  		return;

  	dev->ats_cap = pos;
-	pci_read_config_word(dev, dev->ats_cap + PCI_ATS_CAP, &cap);
-	dev->ats_qdep = PCI_ATS_CAP_QDEP(cap) ? PCI_ATS_CAP_QDEP(cap) :
-					    PCI_ATS_MAX_QDEP;
  }

  /**
@@ -131,13 +127,16 @@ EXPORT_SYMBOL_GPL(pci_restore_ats_state);
   */
  int pci_ats_queue_depth(struct pci_dev *dev)
  {
+	u16 cap;
+
  	if (!dev->ats_cap)
  		return -EINVAL;

  	if (dev->is_virtfn)
  		return 0;

hmmm, isn't one of the fixes in this patch set to
change the caching of ats queue depth ?  won't above
make it 0 for all VF's ?  is that correct, or desired (virtual) value?

Per spec (SR-IOV r1.1., sec 3.7.4), the ATS queue depth register on a VF
always contains zero.

Here's the v4.1 code:

     int pci_ats_queue_depth(struct pci_dev *dev)
     {
         if (dev->is_virtfn)
                 return 0;

         if (dev->ats)
                 return dev->ats->qdep;

	...


In v4.1, pci_ats_queue_depth() always returned 0 for a VF.  For VFs, we
didn't look at the dev->ats->qdep cache.  So I don't think this changes
anything for the caller.

A previous patch changed this path so we return -EINVAL instead of 0 for
VFs that don't support ATS.  I think the previous behavior there was wrong,
but I doubt anybody noticed.

Bjorn

ok. thanks for the sanity check.

-	return dev->ats_qdep;
+	pci_read_config_word(dev, dev->ats_cap + PCI_ATS_CAP, &cap);
+	return PCI_ATS_CAP_QDEP(cap) ? PCI_ATS_CAP_QDEP(cap) : PCI_ATS_MAX_QDEP;
  }
  EXPORT_SYMBOL_GPL(pci_ats_queue_depth);

diff --git a/include/linux/pci.h b/include/linux/pci.h
index 307f96a..4b484fd 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -378,7 +378,6 @@ struct pci_dev {
  	};
  	u16		ats_cap;	/* ATS Capability offset */
  	u8		ats_stu;	/* ATS Smallest Translation Unit */
-	u8		ats_qdep;	/* ATS Invalidate Queue Depth */
  	atomic_t	ats_ref_cnt;	/* number of VFs with ATS enabled */
  #endif
  	phys_addr_t rom; /* Physical address of ROM if it's not from the BAR */

--
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



--
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



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux