[PATCH][PCI]: Introduce pci_find_capability_cached and make MSI use it

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

 



Hi,

	While using the preemptirqsoff ftrace tracer I noticed that
everytime handle_edge_irq is called it needs to mask and unmask MSI, and
that leads to a series of very expensive calls to pci_find_capability,
as can be seen here, with preemption disabled:

          <idle>-0     [03]   422.558652: unmask_msi_irq <-handle_edge_irq
          <idle>-0     [03]   422.558653: msi_set_mask_bits <-unmask_msi_irq
          <idle>-0     [03]   422.558653: msi_set_enable <-msi_set_mask_bits
          <idle>-0     [03]   422.558654: pci_find_capability <-msi_set_enable
          <idle>-0     [03]   422.558655: __pci_bus_find_cap_start <-pci_find_capability
          <idle>-0     [03]   422.558655: pci_bus_read_config_word <-__pci_bus_find_cap_start
          <idle>-0     [03]   422.558656: _spin_lock_irqsave <-pci_bus_read_config_word
          <idle>-0     [03]   422.558656: add_preempt_count <-_spin_lock_irqsave
          <idle>-0     [03]   422.558657: pci_read <-pci_bus_read_config_word
          <idle>-0     [03]   422.558657: raw_pci_read <-pci_read
          <idle>-0     [03]   422.558658: pci_conf1_read <-raw_pci_read
          <idle>-0     [03]   422.558658: _spin_lock_irqsave <-pci_conf1_read
          <idle>-0     [03]   422.558659: add_preempt_count <-_spin_lock_irqsave

	  BZZT! 37us

          <idle>-0     [03]   422.558696: _spin_unlock_irqrestore <-pci_conf1_read
          <idle>-0     [03]   422.558697: sub_preempt_count <-_spin_unlock_irqrestore
          <idle>-0     [03]   422.558697: preempt_schedule <-_spin_unlock_irqrestore
          <idle>-0     [03]   422.558698: _spin_unlock_irqrestore <-pci_bus_read_config_word
          <idle>-0     [03]   422.558698: sub_preempt_count <-_spin_unlock_irqrestore
          <idle>-0     [03]   422.558699: preempt_schedule <-_spin_unlock_irqrestore
          <idle>-0     [03]   422.558699: __pci_find_next_cap <-pci_find_capability
          <idle>-0     [03]   422.558700: __pci_find_next_cap_ttl <-__pci_find_next_cap
          <idle>-0     [03]   422.558700: pci_bus_read_config_byte <-__pci_find_next_cap_ttl
          <idle>-0     [03]   422.558701: _spin_lock_irqsave <-pci_bus_read_config_byte
          <idle>-0     [03]   422.558701: add_preempt_count <-_spin_lock_irqsave
          <idle>-0     [03]   422.558702: pci_read <-pci_bus_read_config_byte
          <idle>-0     [03]   422.558702: raw_pci_read <-pci_read
          <idle>-0     [03]   422.558703: pci_conf1_read <-raw_pci_read
          <idle>-0     [03]   422.558703: _spin_lock_irqsave <-pci_conf1_read
          <idle>-0     [03]   422.558704: add_preempt_count <-_spin_lock_irqsave

	  BZZT! 38us

          <idle>-0     [03]   422.558742: _spin_unlock_irqrestore <-pci_conf1_read
          <idle>-0     [03]   422.558743: sub_preempt_count <-_spin_unlock_irqrestore
          <idle>-0     [03]   422.558743: preempt_schedule <-_spin_unlock_irqrestore
          <idle>-0     [03]   422.558744: _spin_unlock_irqrestore <-pci_bus_read_config_byte
          <idle>-0     [03]   422.558744: sub_preempt_count <-_spin_unlock_irqrestore
          <idle>-0     [03]   422.558745: preempt_schedule <-_spin_unlock_irqrestore
          <idle>-0     [03]   422.558745: pci_bus_read_config_byte <-__pci_find_next_cap_ttl
          <idle>-0     [03]   422.558746: _spin_lock_irqsave <-pci_bus_read_config_byte
          <idle>-0     [03]   422.558746: add_preempt_count <-_spin_lock_irqsave
          <idle>-0     [03]   422.558747: pci_read <-pci_bus_read_config_byte
          <idle>-0     [03]   422.558747: raw_pci_read <-pci_read
          <idle>-0     [03]   422.558748: pci_conf1_read <-raw_pci_read
          <idle>-0     [03]   422.558748: _spin_lock_irqsave <-pci_conf1_read
          <idle>-0     [03]   422.558749: add_preempt_count <-_spin_lock_irqsave

	  BZZT! 39us

          <idle>-0     [03]   422.558788: _spin_unlock_irqrestore <-pci_conf1_read
          <idle>-0     [03]   422.558789: sub_preempt_count <-_spin_unlock_irqrestore
          <idle>-0     [03]   422.558789: preempt_schedule <-_spin_unlock_irqrestore
          <idle>-0     [03]   422.558790: _spin_unlock_irqrestore <-pci_bus_read_config_byte
          <idle>-0     [03]   422.558790: sub_preempt_count <-_spin_unlock_irqrestore
          <idle>-0     [03]   422.558791: preempt_schedule <-_spin_unlock_irqrestore
          <idle>-0     [03]   422.558791: pci_bus_read_config_byte <-__pci_find_next_cap_ttl
          <idle>-0     [03]   422.558792: _spin_lock_irqsave <-pci_bus_read_config_byte
          <idle>-0     [03]   422.558792: add_preempt_count <-_spin_lock_irqsave
          <idle>-0     [03]   422.558793: pci_read <-pci_bus_read_config_byte
          <idle>-0     [03]   422.558793: raw_pci_read <-pci_read
          <idle>-0     [03]   422.558794: pci_conf1_read <-raw_pci_read
          <idle>-0     [03]   422.558794: _spin_lock_irqsave <-pci_conf1_read
          <idle>-0     [03]   422.558795: add_preempt_count <-_spin_lock_irqsave

	  BZZT! 39us

          <idle>-0     [03]   422.558834: _spin_unlock_irqrestore <-pci_conf1_read
          <idle>-0     [03]   422.558834: sub_preempt_count <-_spin_unlock_irqrestore

	  <SNIP many more such BZZT!s>

	So I implemented pci_find_capability_cached and made MSI use it
for good measure, please consider applying.

Signed-off-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>

diff --git a/Makefile b/Makefile
index 14f34b4..d79fdac 100644
--- a/Makefile
+++ b/Makefile
@@ -1,7 +1,7 @@
 VERSION = 2
 PATCHLEVEL = 6
 SUBLEVEL = 25
-EXTRAVERSION =
+EXTRAVERSION = .pci_cached
 NAME = Funky Weasel is Jiggy wit it
 
 # *DOCUMENTATION*
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 8c61304..297f185 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -75,7 +75,7 @@ static void msi_set_enable(struct pci_dev *dev, int enable)
 	int pos;
 	u16 control;
 
-	pos = pci_find_capability(dev, PCI_CAP_ID_MSI);
+	pos = pci_find_capability_cached(dev, PCI_CAP_ID_MSI);
 	if (pos) {
 		pci_read_config_word(dev, pos + PCI_MSI_FLAGS, &control);
 		control &= ~PCI_MSI_FLAGS_ENABLE;
@@ -90,7 +90,7 @@ static void msix_set_enable(struct pci_dev *dev, int enable)
 	int pos;
 	u16 control;
 
-	pos = pci_find_capability(dev, PCI_CAP_ID_MSIX);
+	pos = pci_find_capability_cached(dev, PCI_CAP_ID_MSIX);
 	if (pos) {
 		pci_read_config_word(dev, pos + PCI_MSIX_FLAGS, &control);
 		control &= ~PCI_MSIX_FLAGS_ENABLE;
@@ -352,7 +352,7 @@ static int msi_capability_init(struct pci_dev *dev)
 
 	msi_set_enable(dev, 0);	/* Ensure msi is disabled as I set it up */
 
-   	pos = pci_find_capability(dev, PCI_CAP_ID_MSI);
+   	pos = pci_find_capability_cached(dev, PCI_CAP_ID_MSI);
 	pci_read_config_word(dev, msi_control_reg(pos), &control);
 	/* MSI Entry Initialization */
 	entry = alloc_msi_entry();
@@ -426,7 +426,7 @@ static int msix_capability_init(struct pci_dev *dev,
 
 	msix_set_enable(dev, 0);/* Ensure msix is disabled as I set it up */
 
-   	pos = pci_find_capability(dev, PCI_CAP_ID_MSIX);
+   	pos = pci_find_capability_cached(dev, PCI_CAP_ID_MSIX);
 	/* Request & Map MSI-X table region */
  	pci_read_config_word(dev, msi_control_reg(pos), &control);
 	nr_entries = multi_msix_capable(control);
@@ -533,7 +533,7 @@ static int pci_msi_check_device(struct pci_dev* dev, int nvec, int type)
 	if (ret)
 		return ret;
 
-	if (!pci_find_capability(dev, type))
+	if (!pci_find_capability_cached(dev, type))
 		return -EINVAL;
 
 	return 0;
@@ -667,7 +667,7 @@ int pci_enable_msix(struct pci_dev* dev, struct msix_entry *entries, int nvec)
 	if (status)
 		return status;
 
-	pos = pci_find_capability(dev, PCI_CAP_ID_MSIX);
+	pos = pci_find_capability_cached(dev, PCI_CAP_ID_MSIX);
 	pci_read_config_word(dev, msi_control_reg(pos), &control);
 	nr_entries = multi_msix_capable(control);
 	if (nvec > nr_entries)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index e4548ab..659c93c 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -171,6 +171,35 @@ int pci_find_capability(struct pci_dev *dev, int cap)
 }
 
 /**
+ * pci_find_capability_cached - query for devices' capabilities, cached version
+ * @dev: PCI device to query
+ * @cap: capability code
+ *
+ * Tell if a device supports a given PCI capability.
+ * Returns the address of the requested capability structure within the
+ * device's PCI configuration space or 0 in case the device does not
+ * support it.  Possible values for @cap:
+ *
+ *  %PCI_CAP_ID_PM           Power Management 
+ *  %PCI_CAP_ID_AGP          Accelerated Graphics Port 
+ *  %PCI_CAP_ID_VPD          Vital Product Data 
+ *  %PCI_CAP_ID_SLOTID       Slot Identification 
+ *  %PCI_CAP_ID_MSI          Message Signalled Interrupts
+ *  %PCI_CAP_ID_CHSWP        CompactPCI HotSwap 
+ *  %PCI_CAP_ID_PCIX         PCI-X
+ *  %PCI_CAP_ID_EXP          PCI Express
+ */
+int pci_find_capability_cached(struct pci_dev *dev, int cap)
+{
+	const int idx = cap - 1;
+
+	if (dev->cached_capabilities[idx] == -1)
+		dev->cached_capabilities[idx] = pci_find_capability(dev, cap);
+
+	return dev->cached_capabilities[idx];
+}
+
+/**
  * pci_bus_find_capability - query for devices' capabilities 
  * @bus:   the PCI bus to query
  * @devfn: PCI device to query
@@ -1680,6 +1709,7 @@ EXPORT_SYMBOL(pcim_enable_device);
 EXPORT_SYMBOL(pcim_pin_device);
 EXPORT_SYMBOL(pci_disable_device);
 EXPORT_SYMBOL(pci_find_capability);
+EXPORT_SYMBOL(pci_find_capability_cached);
 EXPORT_SYMBOL(pci_bus_find_capability);
 EXPORT_SYMBOL(pci_release_regions);
 EXPORT_SYMBOL(pci_request_regions);
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 4a55bf3..59338e7 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -885,6 +885,7 @@ static void pci_release_bus_bridge_dev(struct device *dev)
 
 struct pci_dev *alloc_pci_dev(void)
 {
+	int i;
 	struct pci_dev *dev;
 
 	dev = kzalloc(sizeof(struct pci_dev), GFP_KERNEL);
@@ -893,6 +894,9 @@ struct pci_dev *alloc_pci_dev(void)
 
 	INIT_LIST_HEAD(&dev->bus_list);
 
+	for (i = 0; i < ARRAY_SIZE(dev->cached_capabilities); ++i)
+		dev->cached_capabilities[i] = -1;
+
 	pci_msi_init_pci_dev(dev);
 
 	return dev;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 6d0f93d..b7c4cfb 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -197,6 +197,7 @@ struct pci_dev {
 	unsigned int	msix_enabled:1;
 	unsigned int	is_managed:1;
 	unsigned int	is_pcie:1;
+	int		cached_capabilities[PCI_CAP_LIST_NR_ENTRIES]; /* See pci_find_capability_cached */
 	pci_dev_flags_t dev_flags;
 	atomic_t	enable_cnt;	/* pci_enable_device has been called */
 
@@ -516,6 +517,7 @@ struct pci_dev __deprecated *pci_find_slot(unsigned int bus,
 #endif /* CONFIG_PCI_LEGACY */
 
 int pci_find_capability(struct pci_dev *dev, int cap);
+int pci_find_capability_cached(struct pci_dev *dev, int cap);
 int pci_find_next_capability(struct pci_dev *dev, u8 pos, int cap);
 int pci_find_ext_capability(struct pci_dev *dev, int cap);
 int pci_find_ht_capability(struct pci_dev *dev, int ht_cap);
@@ -874,6 +876,11 @@ static inline int pci_find_capability(struct pci_dev *dev, int cap)
 	return 0;
 }
 
+static inline int pci_find_capability_cached(struct pci_dev *dev, int cap)
+{
+	return 0;
+}
+
 static inline int pci_find_next_capability(struct pci_dev *dev, u8 post,
 					   int cap)
 {
diff --git a/include/linux/pci_regs.h b/include/linux/pci_regs.h
index c0c1223..60c64fb 100644
--- a/include/linux/pci_regs.h
+++ b/include/linux/pci_regs.h
@@ -210,6 +210,7 @@
 #define  PCI_CAP_ID_AGP3	0x0E	/* AGP Target PCI-PCI bridge */
 #define  PCI_CAP_ID_EXP 	0x10	/* PCI Express */
 #define  PCI_CAP_ID_MSIX	0x11	/* MSI-X */
+#define PCI_CAP_LIST_NR_ENTRIES PCI_CAP_ID_MSIX
 #define PCI_CAP_LIST_NEXT	1	/* Next capability in the list */
 #define PCI_CAP_FLAGS		2	/* Capability defined flags (16 bits) */
 #define PCI_CAP_SIZEOF		4
--
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