Port-arm archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[PATCH] fix IRQ reentrancy
Current locks up hard when under load like compiling a kernel. Following
patch fixes this on yesterdays current. Compilation of kernel on OMAP
2420 fails for some other reason but the lockups don't happen anymore.
Please apply.
-Mikko
Author: Imre Deak <imre.deak%teleca.com@localhost>
Date: Wed Jun 18 13:35:12 2008 +0300
ARM: IRQ: fix IRQ reentrancy
- Fix the case when an IRQ with a higher IPL preempts another
IRQ handler.
Currently the blocked PIC mask is global and the blocked IRQ
mask is per PIC, so that each pending PIC/IRQ is marked in
these masks regardless of their IPL. After the higher IPL
preempts the lower one and the corresponding handlers are run,
processed but still unblocked lower IPL IRQs might be cleared
from the blocked masks without actually getting unblocked, thus
future IRQs will be lost.
The patch changes the blocked PIC/IRQ masks into a per IPL
array, so IRQs can be unblocked on a per IPL manner.
- Remove the unused irq_count from pic_deliver_irqs. This value
is anyway the same as irq_base.
- Fix the case when an IRQ is triggered during the execution of
another IRQ's handler on the same IPL / PIC. This could happen
if the new IRQ is in a group that has already been processed
in pic_deliver_irqs and not rechecked again.
To fix this do the IPL unmarking (and if necessary that of the PIC)
before IRQs are reenabled, so that any new IRQs on the same IPL / PIC
will be marked correctly. Start over to check the first IRQ group
if there has been any new IRQs during the previous round.
- Change the progress KASSERT to one which checks if the IPL was indeed
unmarked as the result of processing this PIC / IPL.
- Don't recalculate pending IRQs in the delivery loop. There won't
be any new one until IRQ unblocking, which happens only after the
whole IPL is processed.
- Add a pending flag to interrupts sources so we can KASSERT on
IRQ handlers not beeing reentered.
- In case of chained IRQ handlers, like the OMAP gpio IRQ,
pic_handle_intr is called to mark pending interrupts on the
chained PIC. This should be done with interrupts disabled,
to protect global data like pic_pending_ipls.
- Remove extra whitespace.
diff --git a/sys/arch/arm/pic/pic.c b/sys/arch/arm/pic/pic.c
index aa5118c..a793b10 100644
--- a/sys/arch/arm/pic/pic.c
+++ b/sys/arch/arm/pic/pic.c
@@ -58,7 +58,7 @@ struct pic_softc *pic_list[PIC_MAXPICS];
#if PIC_MAXPICS > 32
#error PIC_MAXPICS > 32 not supported
#endif
-volatile uint32_t pic_blocked_pics;
+volatile uint32_t pic_blocked_pics_by_ipl[NIPL];
volatile uint32_t pic_pending_pics;
volatile uint32_t pic_pending_ipls;
volatile uint32_t pic_pending_ipl_refcnt[NIPL];
@@ -79,9 +79,12 @@ int
pic_handle_intr(void *arg)
{
struct pic_softc * const pic = arg;
+ register_t psw;
int rv;
+ psw = disable_interrupts(I32_bit);
rv = (*pic->pic_ops->pic_find_pending_irqs)(pic);
+ restore_interrupts(psw);
return rv > 0;
}
@@ -122,7 +125,7 @@ pic_mark_pending_sources(struct pic_softc *pic, size_t
irq_base,
return ipl_mask;
KASSERT((irq_base & 31) == 0);
-
+
(*pic->pic_ops->pic_block_irqs)(pic, irq_base, pending);
*ipending |= pending;
@@ -182,28 +185,33 @@ pic_dispatch(struct intrsource *is, void *frame)
is->is_ev.ev_count++;
}
+static inline
+void pic_unmark_ipl(struct pic_softc *pic, uint32_t ipl_mask)
+{
+ KASSERT(pic->pic_pending_ipls & ipl_mask);
+ pic->pic_pending_ipls &= ~ipl_mask;
+ if (pic->pic_pending_ipls == 0)
+ pic_pending_pics &= ~__BIT(pic->pic_id);
+}
+
void
pic_deliver_irqs(struct pic_softc *pic, int ipl, void *frame)
{
const uint32_t ipl_mask = __BIT(ipl);
struct intrsource *is;
uint32_t *ipending = pic->pic_pending_irqs;
- uint32_t *iblocked = pic->pic_blocked_irqs;
+ uint32_t *iblocked = &pic->pic_blocked_irqs_by_ipl[ipl][0];
size_t irq_base;
-#if PIC_MAXSOURCES > 32
- size_t irq_count;
-#endif
uint32_t pending_irqs;
uint32_t blocked_irqs;
int irq;
- bool progress = false;
-
- KASSERT(pic->pic_pending_ipls & ipl_mask);
irq_base = 0;
-#if PIC_MAXSOURCES > 32
- irq_count = 0;
-#endif
+
+ /* Do it this early, so that when we reenable interrupts for
+ * the handlers, new PICs / IPLs will be marked correctly.
+ */
+ pic_unmark_ipl(pic, ipl_mask);
for (;;) {
pending_irqs = pic_find_pending_irqs_by_ipl(pic, irq_base,
@@ -212,91 +220,102 @@ pic_deliver_irqs(struct pic_softc *pic, int ipl, void
*frame)
KASSERT((pending_irqs & ~(*ipending)) == 0);
if (pending_irqs == 0) {
#if PIC_MAXSOURCES > 32
- irq_count += 32;
- if (__predict_true(irq_count >= pic->pic_maxsources))
- break;
irq_base += 32;
- ipending++;
- iblocked++;
- if (irq_base >= pic->pic_maxsources) {
+ if (__predict_false(irq_base >= pic->pic_maxsources)) {
+ /* Now that we went through once all groups,
+ * check for any new pending IRQs in the same
+ * IPL and PIC. Note that this loop is bounded
+ * since processed IRQs remain blocked, thus
+ * we consider each of them only once in this
+ * function.
+ */
+ if ((pic->pic_pending_ipls & ipl_mask) == 0)
+ break;
+ pic_unmark_ipl(pic, ipl_mask);
+ irq_base = 0;
ipending = pic->pic_pending_irqs;
- iblocked = pic->pic_blocked_irqs;
+ iblocked =
+ &pic->pic_blocked_irqs_by_ipl[ipl][0];
+ } else {
+ ipending++;
+ iblocked++;
}
- continue;
#else
- break;
+ if ((pic->pic_pending_ipls & ipl_mask) == 0)
+ break;
+ pic_unmark_ipl(pic, ipl_mask);
#endif
+ continue;
}
- progress = true;
blocked_irqs = pending_irqs;
do {
irq = ffs(pending_irqs) - 1;
KASSERT(irq >= 0);
*ipending &= ~__BIT(irq);
+ pending_irqs &= ~__BIT(irq);
is = pic->pic_sources[irq_base + irq];
if (is != NULL) {
+ KASSERT(!is->is_pending);
+ is->is_pending = 1;
cpsie(I32_bit);
pic_dispatch(is, frame);
cpsid(I32_bit);
+ is->is_pending = 0;
} else {
KASSERT(0);
blocked_irqs &= ~__BIT(irq);
}
- pending_irqs = pic_find_pending_irqs_by_ipl(pic,
- irq_base, *ipending, ipl);
} while (pending_irqs);
if (blocked_irqs) {
*iblocked |= blocked_irqs;
- pic_blocked_pics |= __BIT(pic->pic_id);
+ pic_blocked_pics_by_ipl[ipl] |= __BIT(pic->pic_id);
}
}
- KASSERT(progress);
- /*
- * Since interrupts are disabled, we don't have to be too careful
- * about these.
- */
- pic->pic_pending_ipls &= ~ipl_mask;
- if (pic->pic_pending_ipls == 0)
- pic_pending_pics &= ~__BIT(pic->pic_id);
+ KASSERT((pic->pic_pending_ipls & ipl_mask) == 0);
}
static void
-pic_list_unblock_irqs(void)
+pic_list_unblock_irqs(int ipl)
{
- uint32_t blocked = pic_blocked_pics;
+ uint32_t blocked_pics = pic_blocked_pics_by_ipl[ipl];
- pic_blocked_pics = 0;
+ pic_blocked_pics_by_ipl[ipl] = 0;
for (;;) {
struct pic_softc *pic;
+ int progress;
#if PIC_MAXSOURCES > 32
uint32_t *iblocked;
size_t irq_base;
#endif
- int pic_id = ffs(blocked);
+ int pic_id = ffs(blocked_pics);
if (pic_id-- == 0)
return;
pic = pic_list[pic_id];
KASSERT(pic != NULL);
+ progress = 0;
#if PIC_MAXSOURCES > 32
- for (irq_base = 0, iblocked = pic->pic_blocked_irqs;
+ for (irq_base = 0,
+ iblocked = &pic->pic_blocked_irqs_by_ipl[ipl][0];
irq_base < pic->pic_maxsources;
irq_base += 32, iblocked++) {
if (*iblocked) {
(*pic->pic_ops->pic_unblock_irqs)(pic,
irq_base, *iblocked);
*iblocked = 0;
+ progress = 1;
}
}
- blocked &= ~__BIT(pic_id);
+ KASSERT(progress);
#else
- KASSERT(pic->pic_blocked_irqs[0] != 0);
+ KASSERT(pic->pic_blocked_irqs_by_ipl[ipl][0] != 0);
(*pic->pic_ops->pic_unblock_irqs)(pic,
- 0, pic->pic_blocked_irqs[0]);
+ 0, pic->pic_blocked_irqs_by_ipl[ipl][0]);
#endif
+ blocked_pics &= ~__BIT(pic_id);
}
}
@@ -349,7 +368,7 @@ pic_do_pending_ints(register_t psw, int newipl, void *frame)
ci->ci_cpl = ipl;
pic_list_deliver_irqs(psw, ipl, frame);
- pic_list_unblock_irqs();
+ pic_list_unblock_irqs(ipl);
}
}
if (ci->ci_cpl != newipl)
@@ -434,7 +453,7 @@ pic_establish_intr(struct pic_softc *pic, int irq, int ipl,
int type,
is->is_type = type;
is->is_func = func;
is->is_arg = arg;
-
+
if (pic->pic_ops->pic_source_name)
(*pic->pic_ops->pic_source_name)(pic, irq, is->is_source,
sizeof(is->is_source));
@@ -459,7 +478,7 @@ pic_establish_intr(struct pic_softc *pic, int irq, int ipl,
int type,
/*
* Move up all the sources by one.
- */
+ */
if (ipl < NIPL) {
off = pic_ipl_offset[ipl+1];
memmove(&pic__iplsources[off+1], &pic__iplsources[off],
@@ -486,7 +505,7 @@ pic_establish_intr(struct pic_softc *pic, int irq, int ipl,
int type,
(*pic->pic_ops->pic_unblock_irqs)(pic, is->is_irq & ~0x1f,
__BIT(is->is_irq & 0x1f));
-
+
/* We're done. */
return is;
}
diff --git a/sys/arch/arm/pic/picvar.h b/sys/arch/arm/pic/picvar.h
index 1cf1fab..f7b8aca 100644
--- a/sys/arch/arm/pic/picvar.h
+++ b/sys/arch/arm/pic/picvar.h
@@ -75,6 +75,7 @@ struct intrsource {
uint8_t is_ipl; /* IPL_xxx */
uint8_t is_irq; /* local to pic */
uint8_t is_iplidx;
+ int is_pending;
struct evcnt is_ev;
char is_source[16];
};
@@ -83,7 +84,7 @@ struct pic_softc {
const struct pic_ops *pic_ops;
struct intrsource **pic_sources;
uint32_t pic_pending_irqs[(PIC_MAXSOURCES + 31) / 32];
- uint32_t pic_blocked_irqs[(PIC_MAXSOURCES + 31) / 32];
+ uint32_t pic_blocked_irqs_by_ipl[NIPL][(PIC_MAXSOURCES + 31) / 32];
uint32_t pic_pending_ipls;
size_t pic_maxsources;
uint8_t pic_id;
Home |
Main Index |
Thread Index |
Old Index