* [PATCH 1/5] lockdep: Introduce lockdep_unregister_key_nosync()
2025-02-03 17:59 [PATCH 0/5] Make it easier to use nested locking for interrupt descriptors Bart Van Assche
@ 2025-02-03 17:59 ` Bart Van Assche
2025-02-03 17:59 ` [PATCH 2/5] irqdesc: Use dynamic lockdep keys for interrupt descriptors Bart Van Assche
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Bart Van Assche @ 2025-02-03 17:59 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Peter Zijlstra, linux-kernel, Bart Van Assche, Ingo Molnar,
Will Deacon, Waiman Long
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();
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/5] irqdesc: Use dynamic lockdep keys for interrupt descriptors
2025-02-03 17:59 [PATCH 0/5] Make it easier to use nested locking for interrupt descriptors Bart Van Assche
2025-02-03 17:59 ` [PATCH 1/5] lockdep: Introduce lockdep_unregister_key_nosync() Bart Van Assche
@ 2025-02-03 17:59 ` Bart Van Assche
2025-02-03 17:59 ` [PATCH 3/5] irq: Remove irq_set_lockdep_class() and __irq_set_lockdep_class() Bart Van Assche
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Bart Van Assche @ 2025-02-03 17:59 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: Peter Zijlstra, linux-kernel, 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
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 3/5] irq: Remove irq_set_lockdep_class() and __irq_set_lockdep_class()
2025-02-03 17:59 [PATCH 0/5] Make it easier to use nested locking for interrupt descriptors Bart Van Assche
2025-02-03 17:59 ` [PATCH 1/5] lockdep: Introduce lockdep_unregister_key_nosync() Bart Van Assche
2025-02-03 17:59 ` [PATCH 2/5] irqdesc: Use dynamic lockdep keys for interrupt descriptors Bart Van Assche
@ 2025-02-03 17:59 ` Bart Van Assche
2025-02-03 17:59 ` [PATCH 4/5] irq: Remove the IRQ_GC_INIT_NESTED_LOCK flag Bart Van Assche
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Bart Van Assche @ 2025-02-03 17:59 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: Peter Zijlstra, linux-kernel, 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, ®map_irq_lock_class, ®map_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
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 4/5] irq: Remove the IRQ_GC_INIT_NESTED_LOCK flag
2025-02-03 17:59 [PATCH 0/5] Make it easier to use nested locking for interrupt descriptors Bart Van Assche
` (2 preceding siblings ...)
2025-02-03 17:59 ` [PATCH 3/5] irq: Remove irq_set_lockdep_class() and __irq_set_lockdep_class() Bart Van Assche
@ 2025-02-03 17:59 ` Bart Van Assche
2025-02-03 17:59 ` [PATCH 5/5] irq: Simplify gpiochip_add_data() Bart Van Assche
2025-02-03 19:40 ` [PATCH 0/5] Make it easier to use nested locking for interrupt descriptors Thomas Gleixner
5 siblings, 0 replies; 7+ messages in thread
From: Bart Van Assche @ 2025-02-03 17:59 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: Peter Zijlstra, linux-kernel, 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,
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 5/5] irq: Simplify gpiochip_add_data()
2025-02-03 17:59 [PATCH 0/5] Make it easier to use nested locking for interrupt descriptors Bart Van Assche
` (3 preceding siblings ...)
2025-02-03 17:59 ` [PATCH 4/5] irq: Remove the IRQ_GC_INIT_NESTED_LOCK flag Bart Van Assche
@ 2025-02-03 17:59 ` Bart Van Assche
2025-02-03 19:40 ` [PATCH 0/5] Make it easier to use nested locking for interrupt descriptors Thomas Gleixner
5 siblings, 0 replies; 7+ messages in thread
From: Bart Van Assche @ 2025-02-03 17:59 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: Peter Zijlstra, linux-kernel, 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,
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 0/5] Make it easier to use nested locking for interrupt descriptors
2025-02-03 17:59 [PATCH 0/5] Make it easier to use nested locking for interrupt descriptors Bart Van Assche
` (4 preceding siblings ...)
2025-02-03 17:59 ` [PATCH 5/5] irq: Simplify gpiochip_add_data() Bart Van Assche
@ 2025-02-03 19:40 ` Thomas Gleixner
5 siblings, 0 replies; 7+ messages in thread
From: Thomas Gleixner @ 2025-02-03 19:40 UTC (permalink / raw)
To: Bart Van Assche; +Cc: Peter Zijlstra, linux-kernel, Bart Van Assche
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
^ permalink raw reply [flat|nested] 7+ messages in thread