[PATCH 0/5] Make it easier to use nested locking for interrupt descriptors

[PATCH 0/5] Make it easier to use nested locking for interrupt descriptors

[Bart Van Assche]

Hi Thomas,

The number of irq_set_lockdep_class() calls and IRQ_GC_INIT_NESTED_LOCK
occurrences in the kernel tree shows that nested locking is common for
interrupt descriptors. This patch series makes it easier to use nested
locking by removing the function irq_set_lockdep_class() and also the
IRQ_GC_INIT_NESTED_LOCK flag.

As one can tell from the diffstat, this patch series removes more code
than it adds.

Please consider this patch series for the next merge window.

Thanks,

Bart.

Bart Van Assche (5):
  lockdep: Introduce lockdep_unregister_key_nosync()
  irqdesc: Use dynamic lockdep keys for interrupt descriptors
  irq: Remove irq_set_lockdep_class() and __irq_set_lockdep_class()
  irq: Remove the IRQ_GC_INIT_NESTED_LOCK flag
  irq: Simplify gpiochip_add_data()

 arch/powerpc/sysdev/fsl_msi.c             |  5 ---
 drivers/base/regmap/regmap-irq.c          |  4 ---
 drivers/gpio/gpio-bcm-kona.c              |  8 -----
 drivers/gpio/gpio-brcmstb.c               | 10 ------
 drivers/gpio/gpio-mxc.c                   |  3 +-
 drivers/gpio/gpio-mxs.c                   |  3 +-
 drivers/gpio/gpiolib-devres.c             | 12 +++----
 drivers/gpio/gpiolib.c                    | 30 ++++-------------
 drivers/gpu/drm/msm/msm_mdss.c            |  3 --
 drivers/irqchip/irq-imgpdc.c              |  3 +-
 drivers/irqchip/irq-renesas-intc-irqpin.c | 11 ------
 drivers/irqchip/irq-renesas-irqc.c        |  2 +-
 drivers/mfd/arizona-irq.c                 |  5 ---
 drivers/net/dsa/mv88e6xxx/chip.c          |  8 -----
 drivers/pinctrl/pinctrl-at91-pio4.c       |  8 -----
 drivers/pinctrl/pinctrl-single.c          | 11 ------
 drivers/pinctrl/sunxi/pinctrl-sunxi.c     |  9 -----
 drivers/spmi/spmi-pmic-arb.c              |  5 ---
 drivers/thermal/qcom/lmh.c                |  7 ----
 include/linux/gpio/driver.h               | 41 ++---------------------
 include/linux/irq.h                       |  4 ---
 include/linux/irqdesc.h                   | 14 +++-----
 include/linux/lockdep.h                   |  5 +++
 kernel/irq/generic-chip.c                 | 15 ---------
 kernel/irq/irqdesc.c                      | 30 +++++------------
 kernel/locking/lockdep.c                  | 22 ++++++++++--
 26 files changed, 54 insertions(+), 224 deletions(-)

[PATCH 1/5] lockdep: Introduce lockdep_unregister_key_nosync()

[Bart Van Assche]

Add a variant of lockdep_unregister_key() that doesn't sleep and hence
that may be called from inside RCU callbacks.

Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Will Deacon <will@kernel.org>
Cc: Waiman Long <longman@redhat.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 include/linux/lockdep.h  |  5 +++++
 kernel/locking/lockdep.c | 22 +++++++++++++++++++---
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index 67964dc4db95..afb3f0ec7304 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -117,6 +117,7 @@ do {							\
 } while (0)
 
 extern void lockdep_register_key(struct lock_class_key *key);
+extern void lockdep_unregister_key_nosync(struct lock_class_key *key);
 extern void lockdep_unregister_key(struct lock_class_key *key);
 
 /*
@@ -372,6 +373,10 @@ static inline void lockdep_register_key(struct lock_class_key *key)
 {
 }
 
+static inline void lockdep_unregister_key_nosync(struct lock_class_key *key)
+{
+}
+
 static inline void lockdep_unregister_key(struct lock_class_key *key)
 {
 }
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 4470680f0226..6e0423df9ebe 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -6558,8 +6558,11 @@ void lockdep_reset_lock(struct lockdep_map *lock)
  * Unlike lockdep_register_key(), a search is always done to find a matching
  * key irrespective of debug_locks to avoid potential invalid access to freed
  * memory in lock_class entry.
+ *
+ * Does not call synchronize_rcu(). The caller is responsible for making sure
+ * that memory is only freed after concurrent accesses have finished.
  */
-void lockdep_unregister_key(struct lock_class_key *key)
+void lockdep_unregister_key_nosync(struct lock_class_key *key)
 {
 	struct hlist_head *hash_head = keyhashentry(key);
 	struct lock_class_key *k;
@@ -6568,8 +6571,6 @@ void lockdep_unregister_key(struct lock_class_key *key)
 	bool found = false;
 	bool need_callback = false;
 
-	might_sleep();
-
 	if (WARN_ON_ONCE(static_obj(key)))
 		return;
 
@@ -6594,6 +6595,21 @@ void lockdep_unregister_key(struct lock_class_key *key)
 
 	if (need_callback)
 		call_rcu(&delayed_free.rcu_head, free_zapped_rcu);
+}
+EXPORT_SYMBOL_GPL(lockdep_unregister_key_nosync);
+
+/*
+ * Unregister a dynamically allocated key.
+ *
+ * Unlike lockdep_register_key(), a search is always done to find a matching
+ * key irrespective of debug_locks to avoid potential invalid access to freed
+ * memory in lock_class entry.
+ */
+void lockdep_unregister_key(struct lock_class_key *key)
+{
+	might_sleep();
+
+	lockdep_unregister_key_nosync(key);
 
 	/* Wait until is_dynamic_key() has finished accessing k->hash_entry. */
 	synchronize_rcu();

[PATCH 2/5] irqdesc: Use dynamic lockdep keys for interrupt descriptors

[Bart Van Assche]

Having to call irq_set_lockdep_class() if nested locking is expected is
cumbersome. Hence, prepare for removing irq_set_lockdep_class() by
associating a dynamic key with the synchronization objects in interrupt
descriptors.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 include/linux/irqdesc.h |  4 ++++
 kernel/irq/irqdesc.c    | 22 +++++++++-------------
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h
index fd091c35d572..5f4bd476fcc8 100644
--- a/include/linux/irqdesc.h
+++ b/include/linux/irqdesc.h
@@ -46,6 +46,7 @@ struct irqstat {
  * @threads_handled:	stats field for deferred spurious detection of threaded handlers
  * @threads_handled_last: comparator field for deferred spurious detection of threaded handlers
  * @lock:		locking for SMP
+ * @lock_key:		Lockdep class for @lock
  * @affinity_hint:	hint to user space for preferred irq affinity
  * @affinity_notify:	context for notification of affinity changes
  * @pending_mask:	pending rebalanced interrupts
@@ -60,6 +61,7 @@ struct irqstat {
  * @rcu:		rcu head for delayed free
  * @kobj:		kobject used to represent this struct in sysfs
  * @request_mutex:	mutex to protect request/free before locking desc->lock
+ * @request_key:	Lockdep class for @request_mutex
  * @dir:		/proc/irq/ procfs entry
  * @debugfs_file:	dentry for the debugfs file
  * @name:		flow handler name for /proc/interrupts output
@@ -81,6 +83,7 @@ struct irq_desc {
 	atomic_t		threads_handled;
 	int			threads_handled_last;
 	raw_spinlock_t		lock;
+	struct lock_class_key	lock_key;
 	struct cpumask		*percpu_enabled;
 	const struct cpumask	*percpu_affinity;
 #ifdef CONFIG_SMP
@@ -111,6 +114,7 @@ struct irq_desc {
 	struct kobject		kobj;
 #endif
 	struct mutex		request_mutex;
+	struct lock_class_key	request_key;
 	int			parent_irq;
 	struct module		*owner;
 	const char		*name;
diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index 287830739783..261305a213fd 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -19,11 +19,6 @@
 
 #include "internals.h"
 
-/*
- * lockdep: we want to handle all irq_desc locks as a single lock-class:
- */
-static struct lock_class_key irq_desc_lock_class;
-
 #if defined(CONFIG_SMP)
 static int __init irq_affinity_setup(char *str)
 {
@@ -222,8 +217,11 @@ static int init_desc(struct irq_desc *desc, int irq, int node,
 	}
 
 	raw_spin_lock_init(&desc->lock);
-	lockdep_set_class(&desc->lock, &irq_desc_lock_class);
+	lockdep_register_key(&desc->lock_key);
+	lockdep_set_class(&desc->lock, &desc->lock_key);
 	mutex_init(&desc->request_mutex);
+	lockdep_register_key(&desc->request_key);
+	lockdep_set_class(&desc->lock, &desc->request_key);
 	init_waitqueue_head(&desc->wait_for_threads);
 	desc_set_defaults(irq, desc, node, affinity, owner);
 	irqd_set(&desc->irq_data, flags);
@@ -484,13 +482,17 @@ static struct irq_desc *alloc_desc(int irq, int node, unsigned int flags,
 	return desc;
 }
 
+/* Called from RCU context. Hence, must not sleep. */
 static void irq_kobj_release(struct kobject *kobj)
 {
 	struct irq_desc *desc = container_of(kobj, struct irq_desc, kobj);
 
 	free_masks(desc);
 	free_percpu(desc->kstat_irqs);
-	kfree(desc);
+	lockdep_unregister_key_nosync(&desc->request_key);
+	lockdep_unregister_key_nosync(&desc->lock_key);
+	init_rcu_head(&desc->rcu);
+	kfree_rcu(desc, rcu);
 }
 
 static void delayed_free_desc(struct rcu_head *rhp)
@@ -1064,12 +1066,6 @@ unsigned int kstat_irqs_usr(unsigned int irq)
 void __irq_set_lockdep_class(unsigned int irq, struct lock_class_key *lock_class,
 			     struct lock_class_key *request_class)
 {
-	struct irq_desc *desc = irq_to_desc(irq);
-
-	if (desc) {
-		lockdep_set_class(&desc->lock, lock_class);
-		lockdep_set_class(&desc->request_mutex, request_class);
-	}
 }
 EXPORT_SYMBOL_GPL(__irq_set_lockdep_class);
 #endif

[PATCH 3/5] irq: Remove irq_set_lockdep_class() and __irq_set_lockdep_class()

[Bart Van Assche]

The previous patch made __irq_set_lockdep_class() empty. Hence remove this
function and all its callers.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 arch/powerpc/sysdev/fsl_msi.c             |  5 -----
 drivers/base/regmap/regmap-irq.c          |  4 ----
 drivers/gpio/gpio-bcm-kona.c              |  8 --------
 drivers/gpio/gpio-brcmstb.c               | 10 ----------
 drivers/gpio/gpiolib.c                    |  6 ------
 drivers/gpu/drm/msm/msm_mdss.c            |  3 ---
 drivers/irqchip/irq-renesas-intc-irqpin.c | 11 -----------
 drivers/mfd/arizona-irq.c                 |  5 -----
 drivers/net/dsa/mv88e6xxx/chip.c          |  8 --------
 drivers/pinctrl/pinctrl-at91-pio4.c       |  8 --------
 drivers/pinctrl/pinctrl-single.c          | 11 -----------
 drivers/pinctrl/sunxi/pinctrl-sunxi.c     |  9 ---------
 drivers/spmi/spmi-pmic-arb.c              |  5 -----
 drivers/thermal/qcom/lmh.c                |  7 -------
 include/linux/gpio/driver.h               | 14 --------------
 include/linux/irqdesc.h                   | 10 ----------
 kernel/irq/generic-chip.c                 | 15 ---------------
 kernel/irq/irqdesc.c                      |  8 --------
 18 files changed, 147 deletions(-)

diff --git a/arch/powerpc/sysdev/fsl_msi.c b/arch/powerpc/sysdev/fsl_msi.c
index 1aa0cb097c9c..680ab348e636 100644
--- a/arch/powerpc/sysdev/fsl_msi.c
+++ b/arch/powerpc/sysdev/fsl_msi.c
@@ -345,9 +345,6 @@ static void fsl_of_msi_remove(struct platform_device *ofdev)
 	kfree(msi);
 }
 
-static struct lock_class_key fsl_msi_irq_class;
-static struct lock_class_key fsl_msi_irq_request_class;
-
 static int fsl_msi_setup_hwirq(struct fsl_msi *msi, struct platform_device *dev,
 			       int offset, int irq_index)
 {
@@ -366,8 +363,6 @@ static int fsl_msi_setup_hwirq(struct fsl_msi *msi, struct platform_device *dev,
 		dev_err(&dev->dev, "No memory for MSI cascade data\n");
 		return -ENOMEM;
 	}
-	irq_set_lockdep_class(virt_msir, &fsl_msi_irq_class,
-			      &fsl_msi_irq_request_class);
 	cascade_data->index = offset;
 	cascade_data->msi_data = msi;
 	cascade_data->virq = virt_msir;
diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c
index 0bcd81389a29..33ec28e3a802 100644
--- a/drivers/base/regmap/regmap-irq.c
+++ b/drivers/base/regmap/regmap-irq.c
@@ -511,16 +511,12 @@ static irqreturn_t regmap_irq_thread(int irq, void *d)
 		return IRQ_NONE;
 }
 
-static struct lock_class_key regmap_irq_lock_class;
-static struct lock_class_key regmap_irq_request_class;
-
 static int regmap_irq_map(struct irq_domain *h, unsigned int virq,
 			  irq_hw_number_t hw)
 {
 	struct regmap_irq_chip_data *data = h->host_data;
 
 	irq_set_chip_data(virq, data);
-	irq_set_lockdep_class(virq, &regmap_irq_lock_class, &regmap_irq_request_class);
 	irq_set_chip(virq, &data->irq_chip);
 	irq_set_nested_thread(virq, 1);
 	irq_set_parent(virq, data->irq);
diff --git a/drivers/gpio/gpio-bcm-kona.c b/drivers/gpio/gpio-bcm-kona.c
index 5321ef98f442..624a121a64d8 100644
--- a/drivers/gpio/gpio-bcm-kona.c
+++ b/drivers/gpio/gpio-bcm-kona.c
@@ -503,13 +503,6 @@ static struct of_device_id const bcm_kona_gpio_of_match[] = {
 	{}
 };
 
-/*
- * This lock class tells lockdep that GPIO irqs are in a different
- * category than their parents, so it won't report false recursion.
- */
-static struct lock_class_key gpio_lock_class;
-static struct lock_class_key gpio_request_class;
-
 static int bcm_kona_gpio_irq_map(struct irq_domain *d, unsigned int irq,
 				 irq_hw_number_t hwirq)
 {
@@ -518,7 +511,6 @@ static int bcm_kona_gpio_irq_map(struct irq_domain *d, unsigned int irq,
 	ret = irq_set_chip_data(irq, d->host_data);
 	if (ret < 0)
 		return ret;
-	irq_set_lockdep_class(irq, &gpio_lock_class, &gpio_request_class);
 	irq_set_chip_and_handler(irq, &bcm_gpio_irq_chip, handle_simple_irq);
 	irq_set_noprobe(irq);
 
diff --git a/drivers/gpio/gpio-brcmstb.c b/drivers/gpio/gpio-brcmstb.c
index 491b529d25f8..25d7107a74cb 100644
--- a/drivers/gpio/gpio-brcmstb.c
+++ b/drivers/gpio/gpio-brcmstb.c
@@ -309,14 +309,6 @@ static struct brcmstb_gpio_bank *brcmstb_gpio_hwirq_to_bank(
 	return NULL;
 }
 
-/*
- * This lock class tells lockdep that GPIO irqs are in a different
- * category than their parents, so it won't report false recursion.
- */
-static struct lock_class_key brcmstb_gpio_irq_lock_class;
-static struct lock_class_key brcmstb_gpio_irq_request_class;
-
-
 static int brcmstb_gpio_irq_map(struct irq_domain *d, unsigned int irq,
 		irq_hw_number_t hwirq)
 {
@@ -334,8 +326,6 @@ static int brcmstb_gpio_irq_map(struct irq_domain *d, unsigned int irq,
 	ret = irq_set_chip_data(irq, &bank->gc);
 	if (ret < 0)
 		return ret;
-	irq_set_lockdep_class(irq, &brcmstb_gpio_irq_lock_class,
-			      &brcmstb_gpio_irq_request_class);
 	irq_set_chip_and_handler(irq, &priv->irq_chip, handle_level_irq);
 	irq_set_noprobe(irq);
 	return 0;
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 679ed764cb14..940d3f980457 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1523,7 +1523,6 @@ static int gpiochip_hierarchy_irq_domain_alloc(struct irq_domain *d,
 
 	chip_dbg(gc, "alloc_irqs_parent for %d parent hwirq %d\n",
 		  irq, parent_hwirq);
-	irq_set_lockdep_class(irq, gc->irq.lock_key, gc->irq.request_key);
 	ret = irq_domain_alloc_irqs_parent(d, irq, 1, &gpio_parent_fwspec);
 	/*
 	 * If the parent irqdomain is msi, the interrupts have already
@@ -1715,11 +1714,6 @@ static int gpiochip_irq_map(struct irq_domain *d, unsigned int irq,
 		return -ENXIO;
 
 	irq_set_chip_data(irq, gc);
-	/*
-	 * This lock class tells lockdep that GPIO irqs are in a different
-	 * category than their parents, so it won't report false recursion.
-	 */
-	irq_set_lockdep_class(irq, gc->irq.lock_key, gc->irq.request_key);
 	irq_set_chip_and_handler(irq, gc->irq.chip, gc->irq.handler);
 	/* Chips that use nested thread handlers have them marked */
 	if (gc->irq.threaded)
diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
index dcb49fd30402..3c6f2e83d0f5 100644
--- a/drivers/gpu/drm/msm/msm_mdss.c
+++ b/drivers/gpu/drm/msm/msm_mdss.c
@@ -125,14 +125,11 @@ static struct irq_chip msm_mdss_irq_chip = {
 	.irq_unmask = msm_mdss_irq_unmask,
 };
 
-static struct lock_class_key msm_mdss_lock_key, msm_mdss_request_key;
-
 static int msm_mdss_irqdomain_map(struct irq_domain *domain,
 		unsigned int irq, irq_hw_number_t hwirq)
 {
 	struct msm_mdss *msm_mdss = domain->host_data;
 
-	irq_set_lockdep_class(irq, &msm_mdss_lock_key, &msm_mdss_request_key);
 	irq_set_chip_and_handler(irq, &msm_mdss_irq_chip, handle_level_irq);
 
 	return irq_set_chip_data(irq, msm_mdss);
diff --git a/drivers/irqchip/irq-renesas-intc-irqpin.c b/drivers/irqchip/irq-renesas-intc-irqpin.c
index 954419f2460d..1c57a34bb541 100644
--- a/drivers/irqchip/irq-renesas-intc-irqpin.c
+++ b/drivers/irqchip/irq-renesas-intc-irqpin.c
@@ -315,15 +315,6 @@ static irqreturn_t intc_irqpin_shared_irq_handler(int irq, void *dev_id)
 	return status;
 }
 
-/*
- * This lock class tells lockdep that INTC External IRQ Pin irqs are in a
- * different category than their parents, so it won't report false recursion.
- */
-static struct lock_class_key intc_irqpin_irq_lock_class;
-
-/* And this is for the request mutex */
-static struct lock_class_key intc_irqpin_irq_request_class;
-
 static int intc_irqpin_irq_domain_map(struct irq_domain *h, unsigned int virq,
 				      irq_hw_number_t hw)
 {
@@ -334,8 +325,6 @@ static int intc_irqpin_irq_domain_map(struct irq_domain *h, unsigned int virq,
 
 	intc_irqpin_dbg(&p->irq[hw], "map");
 	irq_set_chip_data(virq, h->host_data);
-	irq_set_lockdep_class(virq, &intc_irqpin_irq_lock_class,
-			      &intc_irqpin_irq_request_class);
 	irq_set_chip_and_handler(virq, &p->irq_chip, handle_level_irq);
 	return 0;
 }
diff --git a/drivers/mfd/arizona-irq.c b/drivers/mfd/arizona-irq.c
index d919ae9691e2..962d9ffab5cd 100644
--- a/drivers/mfd/arizona-irq.c
+++ b/drivers/mfd/arizona-irq.c
@@ -180,17 +180,12 @@ static struct irq_chip arizona_irq_chip = {
 	.irq_set_wake		= arizona_irq_set_wake,
 };
 
-static struct lock_class_key arizona_irq_lock_class;
-static struct lock_class_key arizona_irq_request_class;
-
 static int arizona_irq_map(struct irq_domain *h, unsigned int virq,
 			      irq_hw_number_t hw)
 {
 	struct arizona *data = h->host_data;
 
 	irq_set_chip_data(virq, data);
-	irq_set_lockdep_class(virq, &arizona_irq_lock_class,
-		&arizona_irq_request_class);
 	irq_set_chip_and_handler(virq, &arizona_irq_chip, handle_simple_irq);
 	irq_set_nested_thread(virq, 1);
 	irq_set_noprobe(virq);
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 68d1e891752b..08236d7e845e 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -343,20 +343,12 @@ static int mv88e6xxx_g1_irq_setup_common(struct mv88e6xxx_chip *chip)
 
 static int mv88e6xxx_g1_irq_setup(struct mv88e6xxx_chip *chip)
 {
-	static struct lock_class_key lock_key;
-	static struct lock_class_key request_key;
 	int err;
 
 	err = mv88e6xxx_g1_irq_setup_common(chip);
 	if (err)
 		return err;
 
-	/* These lock classes tells lockdep that global 1 irqs are in
-	 * a different category than their parent GPIO, so it won't
-	 * report false recursion.
-	 */
-	irq_set_lockdep_class(chip->irq, &lock_key, &request_key);
-
 	snprintf(chip->irq_name, sizeof(chip->irq_name),
 		 "mv88e6xxx-%s", dev_name(chip->dev));
 
diff --git a/drivers/pinctrl/pinctrl-at91-pio4.c b/drivers/pinctrl/pinctrl-at91-pio4.c
index 8b01d312305a..ec939fb50759 100644
--- a/drivers/pinctrl/pinctrl-at91-pio4.c
+++ b/drivers/pinctrl/pinctrl-at91-pio4.c
@@ -1065,13 +1065,6 @@ static const struct of_device_id atmel_pctrl_of_match[] = {
 	}
 };
 
-/*
- * This lock class allows to tell lockdep that parent IRQ and children IRQ do
- * not share the same class so it does not raise false positive
- */
-static struct lock_class_key atmel_lock_key;
-static struct lock_class_key atmel_request_key;
-
 static int atmel_pinctrl_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -1218,7 +1211,6 @@ static int atmel_pinctrl_probe(struct platform_device *pdev)
 		irq_set_chip_and_handler(irq, &atmel_gpio_irq_chip,
 					 handle_simple_irq);
 		irq_set_chip_data(irq, atmel_pioctrl);
-		irq_set_lockdep_class(irq, &atmel_lock_key, &atmel_request_key);
 		dev_dbg(dev,
 			"atmel gpio irq domain: hwirq: %d, linux irq: %d\n",
 			i, irq);
diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index 5be14dc979e2..bd82eef6ccc4 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -216,16 +216,6 @@ static enum pin_config_param pcs_bias[] = {
 	PIN_CONFIG_BIAS_PULL_UP,
 };
 
-/*
- * This lock class tells lockdep that irqchip core that this single
- * pinctrl can be in a different category than its parents, so it won't
- * report false recursion.
- */
-static struct lock_class_key pcs_lock_class;
-
-/* Class for the IRQ request mutex */
-static struct lock_class_key pcs_request_class;
-
 /*
  * REVISIT: Reads and writes could eventually use regmap or something
  * generic. But at least on omaps, some mux registers are performance
@@ -1556,7 +1546,6 @@ static int pcs_irqdomain_map(struct irq_domain *d, unsigned int irq,
 	irq_set_chip_data(irq, pcs_soc);
 	irq_set_chip_and_handler(irq, &pcs->chip,
 				 handle_level_irq);
-	irq_set_lockdep_class(irq, &pcs_lock_class, &pcs_request_class);
 	irq_set_noprobe(irq);
 
 	return 0;
diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
index bde67ee31417..0db3a3a4066a 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
@@ -35,13 +35,6 @@
 #include "../core.h"
 #include "pinctrl-sunxi.h"
 
-/*
- * These lock classes tell lockdep that GPIO IRQs are in a different
- * category than their parents, so it won't report false recursion.
- */
-static struct lock_class_key sunxi_pinctrl_irq_lock_class;
-static struct lock_class_key sunxi_pinctrl_irq_request_class;
-
 static struct irq_chip sunxi_pinctrl_edge_irq_chip;
 static struct irq_chip sunxi_pinctrl_level_irq_chip;
 
@@ -1639,8 +1632,6 @@ int sunxi_pinctrl_init_with_variant(struct platform_device *pdev,
 	for (i = 0; i < (pctl->desc->irq_banks * IRQ_PER_BANK); i++) {
 		int irqno = irq_create_mapping(pctl->domain, i);
 
-		irq_set_lockdep_class(irqno, &sunxi_pinctrl_irq_lock_class,
-				      &sunxi_pinctrl_irq_request_class);
 		irq_set_chip_and_handler(irqno, &sunxi_pinctrl_edge_irq_chip,
 					 handle_edge_irq);
 		irq_set_chip_data(irqno, pctl);
diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
index 5c058db21821..11af89f45210 100644
--- a/drivers/spmi/spmi-pmic-arb.c
+++ b/drivers/spmi/spmi-pmic-arb.c
@@ -939,8 +939,6 @@ static int qpnpint_irq_domain_translate(struct irq_domain *d,
 	return 0;
 }
 
-static struct lock_class_key qpnpint_irq_lock_class, qpnpint_irq_request_class;
-
 static void qpnpint_irq_domain_map(struct spmi_pmic_arb_bus *bus,
 				   struct irq_domain *domain, unsigned int virq,
 				   irq_hw_number_t hwirq, unsigned int type)
@@ -955,9 +953,6 @@ static void qpnpint_irq_domain_map(struct spmi_pmic_arb_bus *bus,
 	else
 		handler = handle_level_irq;
 
-
-	irq_set_lockdep_class(virq, &qpnpint_irq_lock_class,
-			      &qpnpint_irq_request_class);
 	irq_domain_set_info(domain, virq, hwirq, &pmic_arb_irqchip, bus,
 			    handler, NULL, NULL);
 }
diff --git a/drivers/thermal/qcom/lmh.c b/drivers/thermal/qcom/lmh.c
index d2d49264cf83..5225b3621a56 100644
--- a/drivers/thermal/qcom/lmh.c
+++ b/drivers/thermal/qcom/lmh.c
@@ -73,14 +73,7 @@ static struct irq_chip lmh_irq_chip = {
 static int lmh_irq_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
 {
 	struct lmh_hw_data *lmh_data = d->host_data;
-	static struct lock_class_key lmh_lock_key;
-	static struct lock_class_key lmh_request_key;
 
-	/*
-	 * This lock class tells lockdep that GPIO irqs are in a different
-	 * category than their parents, so it won't report false recursion.
-	 */
-	irq_set_lockdep_class(irq, &lmh_lock_key, &lmh_request_key);
 	irq_set_chip_and_handler(irq, &lmh_irq_chip, handle_simple_irq);
 	irq_set_chip_data(irq, lmh_data);
 
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index 2dd7cb9cc270..087a295a5480 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -157,20 +157,6 @@ struct gpio_irq_chip {
 	 */
 	unsigned int default_type;
 
-	/**
-	 * @lock_key:
-	 *
-	 * Per GPIO IRQ chip lockdep class for IRQ lock.
-	 */
-	struct lock_class_key *lock_key;
-
-	/**
-	 * @request_key:
-	 *
-	 * Per GPIO IRQ chip lockdep class for IRQ request.
-	 */
-	struct lock_class_key *request_key;
-
 	/**
 	 * @parent_handler:
 	 *
diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h
index 5f4bd476fcc8..c16e8e1c1079 100644
--- a/include/linux/irqdesc.h
+++ b/include/linux/irqdesc.h
@@ -257,14 +257,4 @@ static inline bool irq_is_percpu_devid(unsigned int irq)
 	return irq_check_status_bit(irq, IRQ_PER_CPU_DEVID);
 }
 
-void __irq_set_lockdep_class(unsigned int irq, struct lock_class_key *lock_class,
-			     struct lock_class_key *request_class);
-static inline void
-irq_set_lockdep_class(unsigned int irq, struct lock_class_key *lock_class,
-		      struct lock_class_key *request_class)
-{
-	if (IS_ENABLED(CONFIG_LOCKDEP))
-		__irq_set_lockdep_class(irq, lock_class, request_class);
-}
-
 #endif
diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
index c4a8bca5f2b0..91c359d9133e 100644
--- a/kernel/irq/generic-chip.c
+++ b/kernel/irq/generic-chip.c
@@ -441,13 +441,6 @@ irq_get_domain_generic_chip(struct irq_domain *d, unsigned int hw_irq)
 }
 EXPORT_SYMBOL_GPL(irq_get_domain_generic_chip);
 
-/*
- * Separate lockdep classes for interrupt chip which can nest irq_desc
- * lock and request mutex.
- */
-static struct lock_class_key irq_nested_lock_class;
-static struct lock_class_key irq_nested_request_class;
-
 /*
  * irq_map_generic_chip - Map a generic chip for an irq domain
  */
@@ -487,10 +480,6 @@ int irq_map_generic_chip(struct irq_domain *d, unsigned int virq,
 	/* Mark the interrupt as installed */
 	set_bit(idx, &gc->installed);
 
-	if (dgc->gc_flags & IRQ_GC_INIT_NESTED_LOCK)
-		irq_set_lockdep_class(virq, &irq_nested_lock_class,
-				      &irq_nested_request_class);
-
 	if (chip->irq_calc_mask)
 		chip->irq_calc_mask(data);
 	else
@@ -558,10 +547,6 @@ void irq_setup_generic_chip(struct irq_chip_generic *gc, u32 msk,
 		if (!(msk & 0x01))
 			continue;
 
-		if (flags & IRQ_GC_INIT_NESTED_LOCK)
-			irq_set_lockdep_class(i, &irq_nested_lock_class,
-					      &irq_nested_request_class);
-
 		if (!(flags & IRQ_GC_NO_MASK)) {
 			struct irq_data *d = irq_get_irq_data(i);
 
diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index 261305a213fd..14b845c2b29e 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -1061,11 +1061,3 @@ unsigned int kstat_irqs_usr(unsigned int irq)
 	rcu_read_unlock();
 	return sum;
 }
-
-#ifdef CONFIG_LOCKDEP
-void __irq_set_lockdep_class(unsigned int irq, struct lock_class_key *lock_class,
-			     struct lock_class_key *request_class)
-{
-}
-EXPORT_SYMBOL_GPL(__irq_set_lockdep_class);
-#endif

[PATCH 4/5] irq: Remove the IRQ_GC_INIT_NESTED_LOCK flag

[Bart Van Assche]

Remove the IRQ_GC_INIT_NESTED_LOCK flag since the previous patch removed
all code that tests this flag.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/gpio/gpio-mxc.c            | 3 +--
 drivers/gpio/gpio-mxs.c            | 3 +--
 drivers/irqchip/irq-imgpdc.c       | 3 +--
 drivers/irqchip/irq-renesas-irqc.c | 2 +-
 include/linux/irq.h                | 4 ----
 5 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
index 619b6fb9d833..403e76d283a9 100644
--- a/drivers/gpio/gpio-mxc.c
+++ b/drivers/gpio/gpio-mxc.c
@@ -369,8 +369,7 @@ static int mxc_gpio_init_gc(struct mxc_gpio_port *port, int irq_base)
 	ct->regs.ack = GPIO_ISR;
 	ct->regs.mask = GPIO_IMR;
 
-	rv = devm_irq_setup_generic_chip(port->dev, gc, IRQ_MSK(32),
-					 IRQ_GC_INIT_NESTED_LOCK,
+	rv = devm_irq_setup_generic_chip(port->dev, gc, IRQ_MSK(32), 0,
 					 IRQ_NOREQUEST, 0);
 
 	return rv;
diff --git a/drivers/gpio/gpio-mxs.c b/drivers/gpio/gpio-mxs.c
index 024ad077e98d..81f48727bff9 100644
--- a/drivers/gpio/gpio-mxs.c
+++ b/drivers/gpio/gpio-mxs.c
@@ -221,8 +221,7 @@ static int mxs_gpio_init_gc(struct mxs_gpio_port *port, int irq_base)
 	ct->regs.disable = PINCTRL_IRQEN(port) + MXS_CLR;
 	ct->handler = handle_level_irq;
 
-	rv = devm_irq_setup_generic_chip(port->dev, gc, IRQ_MSK(32),
-					 IRQ_GC_INIT_NESTED_LOCK,
+	rv = devm_irq_setup_generic_chip(port->dev, gc, IRQ_MSK(32), 0,
 					 IRQ_NOREQUEST, 0);
 
 	return rv;
diff --git a/drivers/irqchip/irq-imgpdc.c b/drivers/irqchip/irq-imgpdc.c
index 85f80bac0961..c6bb50af2bb3 100644
--- a/drivers/irqchip/irq-imgpdc.c
+++ b/drivers/irqchip/irq-imgpdc.c
@@ -385,8 +385,7 @@ static int pdc_intc_probe(struct platform_device *pdev)
 	 * The second one for syswake irqs (edge and level chip types)
 	 */
 	ret = irq_alloc_domain_generic_chips(priv->domain, 8, 2, "pdc",
-					     handle_level_irq, 0, 0,
-					     IRQ_GC_INIT_NESTED_LOCK);
+					     handle_level_irq, 0, 0, 0);
 	if (ret)
 		goto err_generic;
 
diff --git a/drivers/irqchip/irq-renesas-irqc.c b/drivers/irqchip/irq-renesas-irqc.c
index cbce8ffc7de4..7ecd51e90daa 100644
--- a/drivers/irqchip/irq-renesas-irqc.c
+++ b/drivers/irqchip/irq-renesas-irqc.c
@@ -178,7 +178,7 @@ static int irqc_probe(struct platform_device *pdev)
 
 	ret = irq_alloc_domain_generic_chips(p->irq_domain, p->number_of_irqs,
 					     1, "irqc", handle_level_irq,
-					     0, 0, IRQ_GC_INIT_NESTED_LOCK);
+					     0, 0, 0);
 	if (ret) {
 		dev_err(dev, "cannot allocate generic chip\n");
 		goto err_remove_domain;
diff --git a/include/linux/irq.h b/include/linux/irq.h
index 8daa17f0107a..c297fdeb61c6 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -1073,16 +1073,12 @@ struct irq_chip_generic {
 /**
  * enum irq_gc_flags - Initialization flags for generic irq chips
  * @IRQ_GC_INIT_MASK_CACHE:	Initialize the mask_cache by reading mask reg
- * @IRQ_GC_INIT_NESTED_LOCK:	Set the lock class of the irqs to nested for
- *				irq chips which need to call irq_set_wake() on
- *				the parent irq. Usually GPIO implementations
  * @IRQ_GC_MASK_CACHE_PER_TYPE:	Mask cache is chip type private
  * @IRQ_GC_NO_MASK:		Do not calculate irq_data->mask
  * @IRQ_GC_BE_IO:		Use big-endian register accesses (default: LE)
  */
 enum irq_gc_flags {
 	IRQ_GC_INIT_MASK_CACHE		= 1 << 0,
-	IRQ_GC_INIT_NESTED_LOCK		= 1 << 1,
 	IRQ_GC_MASK_CACHE_PER_TYPE	= 1 << 2,
 	IRQ_GC_NO_MASK			= 1 << 3,
 	IRQ_GC_BE_IO			= 1 << 4,

[PATCH 5/5] irq: Simplify gpiochip_add_data()

[Bart Van Assche]

Because of patch "irqdesc: Use dynamic lockdep keys for interrupt
descriptors", the 'lock_key' and 'request_key' members of
gpiochip_add_data_with_key() and devm_gpiochip_add_data_with_key() are no
longer used. Hence, remove the gpiochip_add_data() and
devm_gpiochip_add_data() macros, remove the 'lock_key' and 'request_key'
arguments and remove the _with_key suffix from the two aforementioned
functions.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/gpio/gpiolib-devres.c | 12 ++++--------
 drivers/gpio/gpiolib.c        | 24 ++++++------------------
 include/linux/gpio/driver.h   | 27 +++------------------------
 3 files changed, 13 insertions(+), 50 deletions(-)

diff --git a/drivers/gpio/gpiolib-devres.c b/drivers/gpio/gpiolib-devres.c
index 08205f355ceb..70f0c60c0b1c 100644
--- a/drivers/gpio/gpiolib-devres.c
+++ b/drivers/gpio/gpiolib-devres.c
@@ -366,12 +366,10 @@ static void devm_gpio_chip_release(void *data)
 }
 
 /**
- * devm_gpiochip_add_data_with_key() - Resource managed gpiochip_add_data_with_key()
+ * devm_gpiochip_add_data() - Resource managed gpiochip_add_data()
  * @dev: pointer to the device that gpio_chip belongs to.
  * @gc: the GPIO chip to register
  * @data: driver-private data associated with this chip
- * @lock_key: lockdep class for IRQ lock
- * @request_key: lockdep class for IRQ request
  *
  * Context: potentially before irqs will work
  *
@@ -382,16 +380,14 @@ static void devm_gpio_chip_release(void *data)
  * gc->base is invalid or already associated with a different chip.
  * Otherwise it returns zero as a success code.
  */
-int devm_gpiochip_add_data_with_key(struct device *dev, struct gpio_chip *gc, void *data,
-				    struct lock_class_key *lock_key,
-				    struct lock_class_key *request_key)
+int devm_gpiochip_add_data(struct device *dev, struct gpio_chip *gc, void *data)
 {
 	int ret;
 
-	ret = gpiochip_add_data_with_key(gc, data, lock_key, request_key);
+	ret = gpiochip_add_data(gc, data);
 	if (ret < 0)
 		return ret;
 
 	return devm_add_action_or_reset(dev, devm_gpio_chip_release, gc);
 }
-EXPORT_SYMBOL_GPL(devm_gpiochip_add_data_with_key);
+EXPORT_SYMBOL_GPL(devm_gpiochip_add_data);
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 940d3f980457..2aac9660b86b 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -93,9 +93,7 @@ static LIST_HEAD(gpio_machine_hogs);
 const char *const gpio_suffixes[] = { "gpios", "gpio", NULL };
 
 static void gpiochip_free_hogs(struct gpio_chip *gc);
-static int gpiochip_add_irqchip(struct gpio_chip *gc,
-				struct lock_class_key *lock_key,
-				struct lock_class_key *request_key);
+static int gpiochip_add_irqchip(struct gpio_chip *gc);
 static void gpiochip_irqchip_remove(struct gpio_chip *gc);
 static int gpiochip_irqchip_init_hw(struct gpio_chip *gc);
 static int gpiochip_irqchip_init_valid_mask(struct gpio_chip *gc);
@@ -916,9 +914,7 @@ int gpiochip_get_ngpios(struct gpio_chip *gc, struct device *dev)
 }
 EXPORT_SYMBOL_GPL(gpiochip_get_ngpios);
 
-int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
-			       struct lock_class_key *lock_key,
-			       struct lock_class_key *request_key)
+int gpiochip_add_data(struct gpio_chip *gc, void *data)
 {
 	struct gpio_device *gdev;
 	unsigned int desc_index;
@@ -1085,7 +1081,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
 	if (ret)
 		goto err_remove_irqchip_mask;
 
-	ret = gpiochip_add_irqchip(gc, lock_key, request_key);
+	ret = gpiochip_add_irqchip(gc);
 	if (ret)
 		goto err_remove_irqchip_mask;
 
@@ -1148,7 +1144,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
 	}
 	return ret;
 }
-EXPORT_SYMBOL_GPL(gpiochip_add_data_with_key);
+EXPORT_SYMBOL_GPL(gpiochip_add_data);
 
 /**
  * gpiochip_remove() - unregister a gpio_chip
@@ -1930,15 +1926,11 @@ static int gpiochip_irqchip_add_allocated_domain(struct gpio_chip *gc,
 /**
  * gpiochip_add_irqchip() - adds an IRQ chip to a GPIO chip
  * @gc: the GPIO chip to add the IRQ chip to
- * @lock_key: lockdep class for IRQ lock
- * @request_key: lockdep class for IRQ request
  *
  * Returns:
  * 0 on success, or a negative errno on failure.
  */
-static int gpiochip_add_irqchip(struct gpio_chip *gc,
-				struct lock_class_key *lock_key,
-				struct lock_class_key *request_key)
+static int gpiochip_add_irqchip(struct gpio_chip *gc)
 {
 	struct fwnode_handle *fwnode = dev_fwnode(&gc->gpiodev->dev);
 	struct irq_chip *irqchip = gc->irq.chip;
@@ -1967,8 +1959,6 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc,
 		type = IRQ_TYPE_NONE;
 
 	gc->irq.default_type = type;
-	gc->irq.lock_key = lock_key;
-	gc->irq.request_key = request_key;
 
 	/* If a parent irqdomain is provided, let's build a hierarchy */
 	if (gpiochip_hierarchy_is_hierarchical(gc)) {
@@ -2083,9 +2073,7 @@ EXPORT_SYMBOL_GPL(gpiochip_irqchip_add_domain);
 
 #else /* CONFIG_GPIOLIB_IRQCHIP */
 
-static inline int gpiochip_add_irqchip(struct gpio_chip *gc,
-				       struct lock_class_key *lock_key,
-				       struct lock_class_key *request_key)
+static inline int gpiochip_add_irqchip(struct gpio_chip *gc)
 {
 	return 0;
 }
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index 087a295a5480..ef39e60a0849 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -572,9 +572,7 @@ DEFINE_CLASS(_gpiochip_for_each_data,
 	for_each_requested_gpio_in_range(chip, i, 0, chip->ngpio, label)
 
 /* add/remove chips */
-int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
-			       struct lock_class_key *lock_key,
-			       struct lock_class_key *request_key);
+int gpiochip_add_data(struct gpio_chip *gc, void *data);
 
 /**
  * gpiochip_add_data() - register a gpio_chip
@@ -599,29 +597,10 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
  * gc->base is invalid or already associated with a different chip.
  * Otherwise it returns zero as a success code.
  */
-#ifdef CONFIG_LOCKDEP
-#define gpiochip_add_data(gc, data) ({		\
-		static struct lock_class_key lock_key;	\
-		static struct lock_class_key request_key;	  \
-		gpiochip_add_data_with_key(gc, data, &lock_key, \
-					   &request_key);	  \
-	})
-#define devm_gpiochip_add_data(dev, gc, data) ({ \
-		static struct lock_class_key lock_key;	\
-		static struct lock_class_key request_key;	  \
-		devm_gpiochip_add_data_with_key(dev, gc, data, &lock_key, \
-					   &request_key);	  \
-	})
-#else
-#define gpiochip_add_data(gc, data) gpiochip_add_data_with_key(gc, data, NULL, NULL)
-#define devm_gpiochip_add_data(dev, gc, data) \
-	devm_gpiochip_add_data_with_key(dev, gc, data, NULL, NULL)
-#endif /* CONFIG_LOCKDEP */
 
 void gpiochip_remove(struct gpio_chip *gc);
-int devm_gpiochip_add_data_with_key(struct device *dev, struct gpio_chip *gc,
-				    void *data, struct lock_class_key *lock_key,
-				    struct lock_class_key *request_key);
+int devm_gpiochip_add_data(struct device *dev, struct gpio_chip *gc,
+			   void *data);
 
 struct gpio_device *gpio_device_find(const void *data,
 				int (*match)(struct gpio_chip *gc,

Re: [PATCH 0/5] Make it easier to use nested locking for interrupt descriptors

[Thomas Gleixner]

Bart!

On Mon, Feb 03 2025 at 09:59, Bart Van Assche wrote:
> The number of irq_set_lockdep_class() calls and IRQ_GC_INIT_NESTED_LOCK
> occurrences in the kernel tree shows that nested locking is common for
> interrupt descriptors.

I can see why you think it's cumbersome to nest interrupts, but calling
it common is pretty exaggerated. If my mental arithmetic is not
completely off, then this affects less than 5% of the interrupt
chip/handler implementations.

The restriction is there on purpose because nesting interrupts is the
exception and not the common case. If there is no real reason, i.e. a
hardware constraint, then interrupt nesting is a bug by definition.

> This patch series makes it easier to use nested
> locking by removing the function irq_set_lockdep_class() and also the
> IRQ_GC_INIT_NESTED_LOCK flag.
>
> As one can tell from the diffstat, this patch series removes more code
> than it adds.

Sure, but with that it removes a debug mechanism for the common case,
which is not nested. Line count is not a measure for quality, neither in
one nor in the other direction.

So if you want to go there, then you have to come up with a proper
explanation why removing the 'yell, if not annotated nesting'
functionality is not a problem and the 'simplificaiton' is actually
worth it.

Thanks,

        tglx