Skip to content

Commit 935acd3

Browse files
torvaldstorvalds
committed
Merge branch 'irq-urgent-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
Pull irq fix from Thomas Gleixner: "Fix the fallout from reworking the locking and resource management in request/free_irq()" * 'irq-urgent-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip: genirq: Keep chip buslock across irq_request/release_resources()
2 parents 31ba04d + 19d39a3 commit 935acd3

File tree

1 file changed

+53
-10
lines changed

1 file changed

+53
-10
lines changed

‎kernel/irq/manage.c

Lines changed: 53 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1090,6 +1090,16 @@ setup_irq_thread(struct irqaction *new, unsigned int irq, bool secondary)
10901090
/*
10911091
* Internal function to register an irqaction - typically used to
10921092
* allocate special interrupts that are part of the architecture.
1093+
*
1094+
* Locking rules:
1095+
*
1096+
* desc->request_mutex Provides serialization against a concurrent free_irq()
1097+
* chip_bus_lock Provides serialization for slow bus operations
1098+
* desc->lock Provides serialization against hard interrupts
1099+
*
1100+
* chip_bus_lock and desc->lock are sufficient for all other management and
1101+
* interrupt related functions. desc->request_mutex solely serializes
1102+
* request/free_irq().
10931103
*/
10941104
static int
10951105
__setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new)
@@ -1167,20 +1177,35 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new)
11671177
if (desc->irq_data.chip->flags & IRQCHIP_ONESHOT_SAFE)
11681178
new->flags &= ~IRQF_ONESHOT;
11691179

1180+
/*
1181+
* Protects against a concurrent __free_irq() call which might wait
1182+
* for synchronize_irq() to complete without holding the optional
1183+
* chip bus lock and desc->lock.
1184+
*/
11701185
mutex_lock(&desc->request_mutex);
1186+
1187+
/*
1188+
* Acquire bus lock as the irq_request_resources() callback below
1189+
* might rely on the serialization or the magic power management
1190+
* functions which are abusing the irq_bus_lock() callback,
1191+
*/
1192+
chip_bus_lock(desc);
1193+
1194+
/* First installed action requests resources. */
11711195
if (!desc->action) {
11721196
ret = irq_request_resources(desc);
11731197
if (ret) {
11741198
pr_err("Failed to request resources for %s (irq %d) on irqchip %s\n",
11751199
new->name, irq, desc->irq_data.chip->name);
1176-
goto out_mutex;
1200+
goto out_bus_unlock;
11771201
}
11781202
}
11791203

1180-
chip_bus_lock(desc);
1181-
11821204
/*
11831205
* The following block of code has to be executed atomically
1206+
* protected against a concurrent interrupt and any of the other
1207+
* management calls which are not serialized via
1208+
* desc->request_mutex or the optional bus lock.
11841209
*/
11851210
raw_spin_lock_irqsave(&desc->lock, flags);
11861211
old_ptr = &desc->action;
@@ -1286,10 +1311,8 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new)
12861311
ret = __irq_set_trigger(desc,
12871312
new->flags & IRQF_TRIGGER_MASK);
12881313

1289-
if (ret) {
1290-
irq_release_resources(desc);
1314+
if (ret)
12911315
goto out_unlock;
1292-
}
12931316
}
12941317

12951318
desc->istate &= ~(IRQS_AUTODETECT | IRQS_SPURIOUS_DISABLED | \
@@ -1385,12 +1408,10 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new)
13851408
out_unlock:
13861409
raw_spin_unlock_irqrestore(&desc->lock, flags);
13871410

1388-
chip_bus_sync_unlock(desc);
1389-
13901411
if (!desc->action)
13911412
irq_release_resources(desc);
1392-
1393-
out_mutex:
1413+
out_bus_unlock:
1414+
chip_bus_sync_unlock(desc);
13941415
mutex_unlock(&desc->request_mutex);
13951416

13961417
out_thread:
@@ -1472,6 +1493,7 @@ static struct irqaction *__free_irq(unsigned int irq, void *dev_id)
14721493
WARN(1, "Trying to free already-free IRQ %d\n", irq);
14731494
raw_spin_unlock_irqrestore(&desc->lock, flags);
14741495
chip_bus_sync_unlock(desc);
1496+
mutex_unlock(&desc->request_mutex);
14751497
return NULL;
14761498
}
14771499

@@ -1498,6 +1520,20 @@ static struct irqaction *__free_irq(unsigned int irq, void *dev_id)
14981520
#endif
14991521

15001522
raw_spin_unlock_irqrestore(&desc->lock, flags);
1523+
/*
1524+
* Drop bus_lock here so the changes which were done in the chip
1525+
* callbacks above are synced out to the irq chips which hang
1526+
* behind a slow bus (I2C, SPI) before calling synchronize_irq().
1527+
*
1528+
* Aside of that the bus_lock can also be taken from the threaded
1529+
* handler in irq_finalize_oneshot() which results in a deadlock
1530+
* because synchronize_irq() would wait forever for the thread to
1531+
* complete, which is blocked on the bus lock.
1532+
*
1533+
* The still held desc->request_mutex() protects against a
1534+
* concurrent request_irq() of this irq so the release of resources
1535+
* and timing data is properly serialized.
1536+
*/
15011537
chip_bus_sync_unlock(desc);
15021538

15031539
unregister_handler_proc(irq, action);
@@ -1530,8 +1566,15 @@ static struct irqaction *__free_irq(unsigned int irq, void *dev_id)
15301566
}
15311567
}
15321568

1569+
/* Last action releases resources */
15331570
if (!desc->action) {
1571+
/*
1572+
* Reaquire bus lock as irq_release_resources() might
1573+
* require it to deallocate resources over the slow bus.
1574+
*/
1575+
chip_bus_lock(desc);
15341576
irq_release_resources(desc);
1577+
chip_bus_sync_unlock(desc);
15351578
irq_remove_timings(desc);
15361579
}
15371580

0 commit comments

Comments
 (0)