[PATCH v4 0/2] PCI: Suppress lockdep warning & make pci_call_probe() more efficient

[PATCH v4 0/2] PCI: Suppress lockdep warning & make pci_call_probe() more efficient

[Waiman Long]

Patch 1 suppress the false positive lockdep warning when there is a
nested pci_call_probe() call. Patch 2 makes pci_call_probe() more
efficient when the current has been affined to the right node already.

Waiman Long (2):
  PCI: Suppress false positive lockdep warning in pci_call_probe()
  PCI: Call local_pci_probe() directly in pci_call_probe() if affined to
    the right node

 drivers/pci/pci-driver.c | 40 +++++++++++++++++++++++++++++++++-------
 1 file changed, 33 insertions(+), 7 deletions(-)

-- 
2.54.0

[PATCH v4 1/2] PCI: Suppress false positive lockdep warning in pci_call_probe()

[Waiman Long]

local_pci_probe() and hence pci_call_probe() can be called recursively,
e.g. when vmd_probe() calls .probe() for devices in the new hierarchy
below VMD or a PF .probe() enables VFs and calls .probe() for them. If
the recursive calls are done indirectly via workqueue kworker, a lockdep
recursive warning like the following can be produced.

  ============================================
  WARNING: possible recursive locking detected
  7.1.0-rc6-test+ #1 Not tainted
  --------------------------------------------
  kworker/52:1/1593 is trying to acquire lock:
  ffffc9001498f708 ((work_completion)(&arg.work)){+.+.}-{0:0}, at: start_flush_work+0x3e9/0x9a0

  but task is already holding lock:
  ffffc9001498fd10 ((work_completion)(&arg.work)){+.+.}-{0:0}, at: process_one_work+0xd4c/0x1390

  other info that might help us debug this:
   Possible unsafe locking scenario:

         CPU0
         ----
    lock((work_completion)(&arg.work));
    lock((work_completion)(&arg.work));

   *** DEADLOCK ***

This is a false positive warning due to the limitation on the number
of distinct lockdep keys allowed. The same lockdep key is used when the
INIT_WORK_ONSTACK() macro is called in pci_call_probe(). So when a work
function queued by pci_call_probe() calls into pci_call_probe() again
and another work function is queued and flushed, the lockdep warning
will be displayed. This can be suppressed by registering a dynamic key
and used it whenever the current task is a wq kworker.

Dynamic lockdep keys are limited finite resources and they should be
used only when really necessary.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 drivers/pci/pci-driver.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index e3f59001785a..bc8c0f061072 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -394,8 +394,21 @@ static int pci_call_probe(struct pci_driver *drv, struct pci_dev *dev,
 		error = local_pci_probe(&ddi);
 	} else {
 		struct pci_probe_arg arg = { .ddi = &ddi };
+		struct lock_class_key key;
+
+		/*
+		 * A nested pci_call_probe() via a work func will produce a
+		 * false positive lockdep recursive locking warning. Use
+		 * lockdep_register_key() to provision a dynamic key to
+		 * suppress this warning when the current task is a wq kworker.
+		 */
+		if (current->flags & PF_WQ_WORKER) {
+			lockdep_register_key(&key);
+			INIT_WORK_ONSTACK_KEY(&arg.work, local_pci_probe_callback, &key);
+		} else {
+			INIT_WORK_ONSTACK(&arg.work, local_pci_probe_callback);
+		}
 
-		INIT_WORK_ONSTACK(&arg.work, local_pci_probe_callback);
 		/*
 		 * The target election and the enqueue of the work must be within
 		 * the same RCU read side section so that when the workqueue pool
@@ -422,6 +435,8 @@ static int pci_call_probe(struct pci_driver *drv, struct pci_dev *dev,
 		}
 
 		destroy_work_on_stack(&arg.work);
+		if (current->flags & PF_WQ_WORKER)
+			lockdep_unregister_key(&key);
 	}
 
 	dev->is_probed = 0;
-- 
2.54.0

[PATCH v4 2/2] PCI: Call local_pci_probe() directly in pci_call_probe() if affined to the right node

[Waiman Long]

Currently pci_call_probe() will unconditionally queue a work function
to perform the actual device probing whenever the device is associated
with a valid NUMA node. The use of work function originally comes from
commit 873392ca514f ("PCI: work_on_cpu: use in drivers/pci/pci-driver.c")
to execute the device probing and allocate memory on the right node
where the device bus is attached to.

This is inefficient to schedule another work function in the same or a
neigboring CPU and wait for its completion when the current task is a
work function itself queued by pci_call_probe() or async_schedule_dev()
that has been affined to the right node already. It will also reduce
the amount of parallelism as noted in [1].

Fix that by calling local_pci_probe() directly if the current task has
been affined to the right NUMA node. Now lockdep dynamic key will only
be used if the current task is a wq kworker but is not affined to the
right node.

[1] https://lore.kernel.org/linux-pci/20251227113326.964-1-guojinhui.liam@bytedance.com/

Signed-off-by: Waiman Long <longman@redhat.com>
---
 drivers/pci/pci-driver.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index bc8c0f061072..70e7b944c097 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -375,6 +375,8 @@ static int pci_call_probe(struct pci_driver *drv, struct pci_dev *dev,
 {
 	int error, node, cpu;
 	struct drv_dev_and_id ddi = { drv, dev, id };
+	bool node_invalid, affine_to_node;
+	const struct cpumask *node_cpus;
 
 	/*
 	 * Execute driver initialization on node where the device is
@@ -383,14 +385,24 @@ static int pci_call_probe(struct pci_driver *drv, struct pci_dev *dev,
 	 */
 	node = dev_to_node(&dev->dev);
 	dev->is_probed = 1;
+	node_invalid = node < 0 || node >= MAX_NUMNODES || !node_online(node);
+
+	if (node_invalid) {
+		node_cpus = cpu_online_mask;
+		affine_to_node = false;
+	} else {
+		node_cpus = cpumask_of_node(node);
+		/* Check if the current task is affined to the right node */
+		affine_to_node = cpumask_subset(current->cpus_ptr, node_cpus);
+	}
 
 	cpu_hotplug_disable();
 	/*
-	 * Prevent nesting work_on_cpu() for the case where a Virtual Function
-	 * device is probed from work_on_cpu() of the Physical device.
+	 * Prevent nesting queue_work_on() for the case where a Virtual Function
+	 * device is probed from queue_work_on() of the Physical function or
+	 * when the current task is affined to the right node.
 	 */
-	if (node < 0 || node >= MAX_NUMNODES || !node_online(node) ||
-	    pci_physfn_is_probed(dev)) {
+	if (node_invalid || affine_to_node || pci_physfn_is_probed(dev)) {
 		error = local_pci_probe(&ddi);
 	} else {
 		struct pci_probe_arg arg = { .ddi = &ddi };
@@ -417,8 +429,7 @@ static int pci_call_probe(struct pci_driver *drv, struct pci_dev *dev,
 		 * targets.
 		 */
 		rcu_read_lock();
-		cpu = cpumask_any_and(cpumask_of_node(node),
-				      housekeeping_cpumask(HK_TYPE_DOMAIN));
+		cpu = cpumask_any_and(node_cpus, housekeeping_cpumask(HK_TYPE_DOMAIN));
 
 		if (cpu < nr_cpu_ids) {
 			struct workqueue_struct *wq = pci_probe_wq;
-- 
2.54.0