| Message ID | 20240220060718.823229-3-apatel@ventanamicro.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Linux RISC-V AIA Support | expand |
On Tue, Feb 20 2024 at 11:37, Anup Patel wrote: > Now that PLIC driver is probed as a regular platform driver, the lock > dependency validator complains about the safety of handler->enable_lock > usage: > > [ 0.956775] Possible interrupt unsafe locking scenario: > > [ 0.956998] CPU0 CPU1 > [ 0.957247] ---- ---- > [ 0.957439] lock(&handler->enable_lock); > [ 0.957607] local_irq_disable(); > [ 0.957793] lock(&irq_desc_lock_class); > [ 0.958021] lock(&handler->enable_lock); > [ 0.958246] <Interrupt> > [ 0.958342] lock(&irq_desc_lock_class); > [ 0.958501] > *** DEADLOCK *** > > To address above, let's use raw_spin_lock_irqsave/unlock_irqrestore() > instead of raw_spin_lock/unlock(). s/let's//
On Tue, Feb 20, 2024 at 3:41 PM Thomas Gleixner <tglx@linutronix.de> wrote: > > On Tue, Feb 20 2024 at 11:37, Anup Patel wrote: > > Now that PLIC driver is probed as a regular platform driver, the lock > > dependency validator complains about the safety of handler->enable_lock > > usage: > > > > [ 0.956775] Possible interrupt unsafe locking scenario: > > > > [ 0.956998] CPU0 CPU1 > > [ 0.957247] ---- ---- > > [ 0.957439] lock(&handler->enable_lock); > > [ 0.957607] local_irq_disable(); > > [ 0.957793] lock(&irq_desc_lock_class); > > [ 0.958021] lock(&handler->enable_lock); > > [ 0.958246] <Interrupt> > > [ 0.958342] lock(&irq_desc_lock_class); > > [ 0.958501] > > *** DEADLOCK *** > > > > To address above, let's use raw_spin_lock_irqsave/unlock_irqrestore() > > instead of raw_spin_lock/unlock(). > > s/let's// Okay, I will update. Regards, Anup
diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c index 48483a1a41dd..e91077ff171f 100644 --- a/drivers/irqchip/irq-sifive-plic.c +++ b/drivers/irqchip/irq-sifive-plic.c @@ -103,9 +103,11 @@ static void __plic_toggle(void __iomem *enable_base, int hwirq, int enable) static void plic_toggle(struct plic_handler *handler, int hwirq, int enable) { - raw_spin_lock(&handler->enable_lock); + unsigned long flags; + + raw_spin_lock_irqsave(&handler->enable_lock, flags); __plic_toggle(handler->enable_base, hwirq, enable); - raw_spin_unlock(&handler->enable_lock); + raw_spin_unlock_irqrestore(&handler->enable_lock, flags); } static inline void plic_irq_toggle(const struct cpumask *mask, @@ -236,6 +238,7 @@ static int plic_irq_set_type(struct irq_data *d, unsigned int type) static int plic_irq_suspend(void) { unsigned int i, cpu; + unsigned long flags; u32 __iomem *reg; struct plic_priv *priv; @@ -253,12 +256,12 @@ static int plic_irq_suspend(void) if (!handler->present) continue; - raw_spin_lock(&handler->enable_lock); + raw_spin_lock_irqsave(&handler->enable_lock, flags); for (i = 0; i < DIV_ROUND_UP(priv->nr_irqs, 32); i++) { reg = handler->enable_base + i * sizeof(u32); handler->enable_save[i] = readl(reg); } - raw_spin_unlock(&handler->enable_lock); + raw_spin_unlock_irqrestore(&handler->enable_lock, flags); } return 0; @@ -267,6 +270,7 @@ static int plic_irq_suspend(void) static void plic_irq_resume(void) { unsigned int i, index, cpu; + unsigned long flags; u32 __iomem *reg; struct plic_priv *priv; @@ -284,12 +288,12 @@ static void plic_irq_resume(void) if (!handler->present) continue; - raw_spin_lock(&handler->enable_lock); + raw_spin_lock_irqsave(&handler->enable_lock, flags); for (i = 0; i < DIV_ROUND_UP(priv->nr_irqs, 32); i++) { reg = handler->enable_base + i * sizeof(u32); writel(handler->enable_save[i], reg); } - raw_spin_unlock(&handler->enable_lock); + raw_spin_unlock_irqrestore(&handler->enable_lock, flags); } }
Now that PLIC driver is probed as a regular platform driver, the lock dependency validator complains about the safety of handler->enable_lock usage: [ 0.956775] Possible interrupt unsafe locking scenario: [ 0.956998] CPU0 CPU1 [ 0.957247] ---- ---- [ 0.957439] lock(&handler->enable_lock); [ 0.957607] local_irq_disable(); [ 0.957793] lock(&irq_desc_lock_class); [ 0.958021] lock(&handler->enable_lock); [ 0.958246] <Interrupt> [ 0.958342] lock(&irq_desc_lock_class); [ 0.958501] *** DEADLOCK *** To address above, let's use raw_spin_lock_irqsave/unlock_irqrestore() instead of raw_spin_lock/unlock(). Signed-off-by: Anup Patel <apatel@ventanamicro.com> --- drivers/irqchip/irq-sifive-plic.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)