* [PATCH 0/2] irqchip: gic*: Fix unsafe locking reported by lockdep
@ 2015-01-09 17:55 Marc Zyngier
2015-01-09 17:55 ` [PATCH 1/2] irqchip: gicv3-its: " Marc Zyngier
2015-01-09 17:55 ` [PATCH 2/2] irqchip: gic: " Marc Zyngier
0 siblings, 2 replies; 3+ messages in thread
From: Marc Zyngier @ 2015-01-09 17:55 UTC (permalink / raw)
To: linux-arm-kernel
Will Deacon reported some locking issues in the GICv3 ITS driver
(lockdep being quite angry...). After fixing those, I thought I'd
check the GIC driver. Good thing I did, because lockdep wasn't happy
either.
After these two patches, lockdep did stay quiet. Can't be completely
bad...
Marc Zyngier (2):
irqchip: gicv3-its: Fix unsafe locking reported by lockdep
irqchip: gic: Fix unsafe locking reported by lockdep
drivers/irqchip/irq-gic-v3-its.c | 23 ++++++++++++++---------
drivers/irqchip/irq-gic.c | 20 ++++++++++++--------
2 files changed, 26 insertions(+), 17 deletions(-)
--
2.1.4
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH 1/2] irqchip: gicv3-its: Fix unsafe locking reported by lockdep
2015-01-09 17:55 [PATCH 0/2] irqchip: gic*: Fix unsafe locking reported by lockdep Marc Zyngier
@ 2015-01-09 17:55 ` Marc Zyngier
2015-01-09 17:55 ` [PATCH 2/2] irqchip: gic: " Marc Zyngier
1 sibling, 0 replies; 3+ messages in thread
From: Marc Zyngier @ 2015-01-09 17:55 UTC (permalink / raw)
To: linux-arm-kernel
When compiled with CONFIG_LOCKDEP, the kernel shouts badly, saying
that my locking is unsafe. I'm afraid the kernel is right:
CPU0 CPU1
---- ----
lock(&its->lock);
local_irq_disable();
lock(&irq_desc_lock_class);
lock(&its->lock);
<Interrupt>
lock(&irq_desc_lock_class);
*** DEADLOCK ***
The fix is to always take its->lock with interrupts disabled.
Reported-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
drivers/irqchip/irq-gic-v3-its.c | 23 ++++++++++++++---------
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 2a2fb2e..c5c1a3c 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -415,13 +415,14 @@ static void its_send_single_command(struct its_node *its,
{
struct its_cmd_block *cmd, *sync_cmd, *next_cmd;
struct its_collection *sync_col;
+ unsigned long flags;
- raw_spin_lock(&its->lock);
+ raw_spin_lock_irqsave(&its->lock, flags);
cmd = its_allocate_entry(its);
if (!cmd) { /* We're soooooo screewed... */
pr_err_ratelimited("ITS can't allocate, dropping command\n");
- raw_spin_unlock(&its->lock);
+ raw_spin_unlock_irqrestore(&its->lock, flags);
return;
}
sync_col = builder(cmd, desc);
@@ -441,7 +442,7 @@ static void its_send_single_command(struct its_node *its,
post:
next_cmd = its_post_commands(its);
- raw_spin_unlock(&its->lock);
+ raw_spin_unlock_irqrestore(&its->lock, flags);
its_wait_for_range_completion(its, cmd, next_cmd);
}
@@ -1019,8 +1020,9 @@ static void its_cpu_init_collection(void)
static struct its_device *its_find_device(struct its_node *its, u32 dev_id)
{
struct its_device *its_dev = NULL, *tmp;
+ unsigned long flags;
- raw_spin_lock(&its->lock);
+ raw_spin_lock_irqsave(&its->lock, flags);
list_for_each_entry(tmp, &its->its_device_list, entry) {
if (tmp->device_id == dev_id) {
@@ -1029,7 +1031,7 @@ static struct its_device *its_find_device(struct its_node *its, u32 dev_id)
}
}
- raw_spin_unlock(&its->lock);
+ raw_spin_unlock_irqrestore(&its->lock, flags);
return its_dev;
}
@@ -1039,6 +1041,7 @@ static struct its_device *its_create_device(struct its_node *its, u32 dev_id,
{
struct its_device *dev;
unsigned long *lpi_map;
+ unsigned long flags;
void *itt;
int lpi_base;
int nr_lpis;
@@ -1074,9 +1077,9 @@ static struct its_device *its_create_device(struct its_node *its, u32 dev_id,
dev->device_id = dev_id;
INIT_LIST_HEAD(&dev->entry);
- raw_spin_lock(&its->lock);
+ raw_spin_lock_irqsave(&its->lock, flags);
list_add(&dev->entry, &its->its_device_list);
- raw_spin_unlock(&its->lock);
+ raw_spin_unlock_irqrestore(&its->lock, flags);
/* Bind the device to the first possible CPU */
cpu = cpumask_first(cpu_online_mask);
@@ -1090,9 +1093,11 @@ static struct its_device *its_create_device(struct its_node *its, u32 dev_id,
static void its_free_device(struct its_device *its_dev)
{
- raw_spin_lock(&its_dev->its->lock);
+ unsigned long flags;
+
+ raw_spin_lock_irqsave(&its_dev->its->lock, flags);
list_del(&its_dev->entry);
- raw_spin_unlock(&its_dev->its->lock);
+ raw_spin_unlock_irqrestore(&its_dev->its->lock, flags);
kfree(its_dev->itt);
kfree(its_dev);
}
--
2.1.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH 2/2] irqchip: gic: Fix unsafe locking reported by lockdep
2015-01-09 17:55 [PATCH 0/2] irqchip: gic*: Fix unsafe locking reported by lockdep Marc Zyngier
2015-01-09 17:55 ` [PATCH 1/2] irqchip: gicv3-its: " Marc Zyngier
@ 2015-01-09 17:55 ` Marc Zyngier
1 sibling, 0 replies; 3+ messages in thread
From: Marc Zyngier @ 2015-01-09 17:55 UTC (permalink / raw)
To: linux-arm-kernel
When compiled with CONFIG_LOCKDEP, the kernel shouts badly, saying
that the locking in the GIC code is unsafe. I'm afraid the kernel
is right:
CPU0
----
lock(irq_controller_lock);
<Interrupt>
lock(irq_controller_lock);
*** DEADLOCK ***
This can happen while enabling, disabling, setting the type
or the affinity of an interrupt.
The fix is to take the interrupt_controller_lock with interrupts
disabled in these cases.
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
drivers/irqchip/irq-gic.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index d617ee5..7e93aeb 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -154,23 +154,25 @@ static inline unsigned int gic_irq(struct irq_data *d)
static void gic_mask_irq(struct irq_data *d)
{
u32 mask = 1 << (gic_irq(d) % 32);
+ unsigned long flags;
- raw_spin_lock(&irq_controller_lock);
+ raw_spin_lock_irqsave(&irq_controller_lock, flags);
writel_relaxed(mask, gic_dist_base(d) + GIC_DIST_ENABLE_CLEAR + (gic_irq(d) / 32) * 4);
if (gic_arch_extn.irq_mask)
gic_arch_extn.irq_mask(d);
- raw_spin_unlock(&irq_controller_lock);
+ raw_spin_unlock_irqrestore(&irq_controller_lock, flags);
}
static void gic_unmask_irq(struct irq_data *d)
{
u32 mask = 1 << (gic_irq(d) % 32);
+ unsigned long flags;
- raw_spin_lock(&irq_controller_lock);
+ raw_spin_lock_irqsave(&irq_controller_lock, flags);
if (gic_arch_extn.irq_unmask)
gic_arch_extn.irq_unmask(d);
writel_relaxed(mask, gic_dist_base(d) + GIC_DIST_ENABLE_SET + (gic_irq(d) / 32) * 4);
- raw_spin_unlock(&irq_controller_lock);
+ raw_spin_unlock_irqrestore(&irq_controller_lock, flags);
}
static void gic_eoi_irq(struct irq_data *d)
@@ -188,6 +190,7 @@ static int gic_set_type(struct irq_data *d, unsigned int type)
{
void __iomem *base = gic_dist_base(d);
unsigned int gicirq = gic_irq(d);
+ unsigned long flags;
/* Interrupt configuration for SGIs can't be changed */
if (gicirq < 16)
@@ -196,14 +199,14 @@ static int gic_set_type(struct irq_data *d, unsigned int type)
if (type != IRQ_TYPE_LEVEL_HIGH && type != IRQ_TYPE_EDGE_RISING)
return -EINVAL;
- raw_spin_lock(&irq_controller_lock);
+ raw_spin_lock_irqsave(&irq_controller_lock, flags);
if (gic_arch_extn.irq_set_type)
gic_arch_extn.irq_set_type(d, type);
gic_configure_irq(gicirq, type, base, NULL);
- raw_spin_unlock(&irq_controller_lock);
+ raw_spin_unlock_irqrestore(&irq_controller_lock, flags);
return 0;
}
@@ -224,6 +227,7 @@ static int gic_set_affinity(struct irq_data *d, const struct cpumask *mask_val,
void __iomem *reg = gic_dist_base(d) + GIC_DIST_TARGET + (gic_irq(d) & ~3);
unsigned int cpu, shift = (gic_irq(d) % 4) * 8;
u32 val, mask, bit;
+ unsigned long flags;
if (!force)
cpu = cpumask_any_and(mask_val, cpu_online_mask);
@@ -233,12 +237,12 @@ static int gic_set_affinity(struct irq_data *d, const struct cpumask *mask_val,
if (cpu >= NR_GIC_CPU_IF || cpu >= nr_cpu_ids)
return -EINVAL;
- raw_spin_lock(&irq_controller_lock);
+ raw_spin_lock_irqsave(&irq_controller_lock, flags);
mask = 0xff << shift;
bit = gic_cpu_map[cpu] << shift;
val = readl_relaxed(reg) & ~mask;
writel_relaxed(val | bit, reg);
- raw_spin_unlock(&irq_controller_lock);
+ raw_spin_unlock_irqrestore(&irq_controller_lock, flags);
return IRQ_SET_MASK_OK;
}
--
2.1.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-01-09 17:55 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-09 17:55 [PATCH 0/2] irqchip: gic*: Fix unsafe locking reported by lockdep Marc Zyngier
2015-01-09 17:55 ` [PATCH 1/2] irqchip: gicv3-its: " Marc Zyngier
2015-01-09 17:55 ` [PATCH 2/2] irqchip: gic: " Marc Zyngier
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.