Hello everybody,
1) arm/hal.c : rthal_irq_enable/disable/end()
the only arch that calls Linux'es enable_irq() and disable_irq() directly.
The later ones use spinlock with the interrupts off only for the Linux domain,
so that under some circumstances - Linux domain gets preempted being in
enable/disable/setup/free_irq
(THIS_IRQ) by the rt-ISR (of THIS_IRQ) -
the deadlock may happen.
rthal_irq_end() -> enable_irq() -> spin_lock_irqsave(lock,flags) -> deadlock (*)
wait.. does that only may happen for inter-domain shared interrupts (THIS_IRQ must be used
in both domains)? No indeed :)
# cat /proc/interrupts may lead to the same deadlock!
this routine is called for i = 0..NR_IRQS and used in fs/proc.c (arch dependent part, e.g. i386/kernel/irq.c)
int show_interrupts(struct seq_file *p, void *v)
{
int i = *(loff_t *) v, j;
struct irqaction * action;
unsigned long flags;
if (i == 0) {
seq_printf(p, " ");
for_each_cpu(j)
seq_printf(p, "CPU%d ",j);
seq_putc(p, '\n');
}
if (i < NR_IRQS) {
spin_lock_irqsave(&irq_desc[i].lock, flags); <--- LOCK
[Oops!!!] We are preempted here by the rt-ISR that's registered for "irq" only in the primary domain
action = ""> if (!action) <--- i.e. there is no ISR in the linux domain, thus - nothing to be printed out
goto skip;
...
skip:
spin_unlock_irqrestore(&irq_desc[i].lock, flags);
(*) takes place and BUM! :o)
So, I guess, it's better to implement rthal_irq_enable/disable/end() the same way as other archs do.
2) This one concern Linux and not Xeno.
[so one may skip-skip-skip it :]
we know that any ISR is always called with a given local IRQ line disabled.
e.g.
__do_IRQ()
{
.ack -> disables the IRQ line
// all the ISRs for this IRQ line are called
.end -> enables it.
}
If an ISR wants to defer IRQ line enabling until later (e.g. bottom half), it may
call disable_irq_nosync() or even disable_irq() (with care as specified in comments).
void disable_irq_nosync(unsigned int irq)
{
irq_desc_t *desc = irq_desc + irq;
unsigned long flags;
spin_lock_irqsave(&desc->lock, flags);
if (!desc->depth++) { <--- inc (depth)
desc->status |= IRQ_DISABLED; <--- DISABLED is set!
desc->handler->disable(irq);
}
spin_unlock_irqrestore(&desc->lock, flags);
}
In this case, .end must _not_ re-enable the IRQ line! In order to do so, it checks for IRQ_DISABLED flag.
e.g.
here is a correct implementation: i386/kernel/i8259.c
static void end_8259A_irq (unsigned int irq)
{
if (!(irq_desc[irq].status & (IRQ_DISABLED|IRQ_INPROGRESS)) && <--- i.e. was disabled in ISR
irq_desc[irq].action)
enable_8259A_irq(irq);
}
end_8259A_irq() is almost the same as enable_8259A_irq() with the only difference that
the later one always re-enables the IRQ line unconditionaly. That's why
any implementation of .end() (which re-enables the IRQ line) must always check for IRQ_DISABLED
and never be the same routine as .enable!
Otherwise, disable_irq_nosync() being called in ISR works not as expected (IRQ line enabling is not defered).
e.g.
arch/ppc/syslib/gt64260_pic.c
struct hw_interrupt_type gt64260_pic = {
.typename = " gt64260_pic ",
.enable = gt64260_unmask_irq,
.disable = gt64260_mask_irq,
.ack = gt64260_mask_irq,
.end = gt64260_unmask_irq, <--- the same as .enable (i.e. doesn't check for IRQ_DISABLED)
};
There are a few other sub-archs that do the same.
Error?
yet another example :
static struct hw_interrupt_type xilinx_intc = {
.typename = "Xilinx Interrupt Controller",
.enable = xilinx_intc_enable,
.disable = xilinx_intc_disable,
.ack = xilinx_intc_disable_and_ack,
.end = xilinx_intc_end, <--- OK! This is correct.
};
but let's look at the implementation :
static void
xilinx_intc_end(unsigned int irq)
{
unsigned long mask = (0x00000001 << (irq & 31));
pr_debug("end: %d\n", irq);
if (!(irq_desc[irq].status & (IRQ_DISABLED | IRQ_INPROGRESS))) {
intc_out_be32(intc + SIE, mask);
/* ack level sensitive intr */ <--- never gets executed if ISR has called disable_irq()!
if (irq_desc[irq].status & IRQ_LEVEL)
intc_out_be32(intc + IAR, mask);
}
}
so
ISR have not called disable_irq() : enable + ack.
-//- did call disable_irq() : only enable (e.g. enable_irq() -> handler->enable() in bottom half)
Could it be such an arch feature? It looks to me that it's logically wrong anyway.
Heh... if one is still reading these lines, thanks for your time indeed! :o)
--
Best regards,
Dmitry Adamushko
the deadlock may happen.
rthal_irq_end() -> enable_irq() -> spin_lock_irqsave(lock,flags) -> deadlock (*)
wait.. does that only may happen for inter-domain shared interrupts (THIS_IRQ must be used
in both domains)? No indeed :)
# cat /proc/interrupts may lead to the same deadlock!
this routine is called for i = 0..NR_IRQS and used in fs/proc.c (arch dependent part, e.g. i386/kernel/irq.c)
int show_interrupts(struct seq_file *p, void *v)
{
int i = *(loff_t *) v, j;
struct irqaction * action;
unsigned long flags;
if (i == 0) {
seq_printf(p, " ");
for_each_cpu(j)
seq_printf(p, "CPU%d ",j);
seq_putc(p, '\n');
}
if (i < NR_IRQS) {
spin_lock_irqsave(&irq_desc[i].lock, flags); <--- LOCK
[Oops!!!] We are preempted here by the rt-ISR that's registered for "irq" only in the primary domain
action = ""> if (!action) <--- i.e. there is no ISR in the linux domain, thus - nothing to be printed out
goto skip;
...
skip:
spin_unlock_irqrestore(&irq_desc[i].lock, flags);
(*) takes place and BUM! :o)
So, I guess, it's better to implement rthal_irq_enable/disable/end() the same way as other archs do.
2) This one concern Linux and not Xeno.
[so one may skip-skip-skip it :]
we know that any ISR is always called with a given local IRQ line disabled.
e.g.
__do_IRQ()
{
.ack -> disables the IRQ line
// all the ISRs for this IRQ line are called
.end -> enables it.
}
If an ISR wants to defer IRQ line enabling until later (e.g. bottom half), it may
call disable_irq_nosync() or even disable_irq() (with care as specified in comments).
void disable_irq_nosync(unsigned int irq)
{
irq_desc_t *desc = irq_desc + irq;
unsigned long flags;
spin_lock_irqsave(&desc->lock, flags);
if (!desc->depth++) { <--- inc (depth)
desc->status |= IRQ_DISABLED; <--- DISABLED is set!
desc->handler->disable(irq);
}
spin_unlock_irqrestore(&desc->lock, flags);
}
In this case, .end must _not_ re-enable the IRQ line! In order to do so, it checks for IRQ_DISABLED flag.
e.g.
here is a correct implementation: i386/kernel/i8259.c
static void end_8259A_irq (unsigned int irq)
{
if (!(irq_desc[irq].status & (IRQ_DISABLED|IRQ_INPROGRESS)) && <--- i.e. was disabled in ISR
irq_desc[irq].action)
enable_8259A_irq(irq);
}
end_8259A_irq() is almost the same as enable_8259A_irq() with the only difference that
the later one always re-enables the IRQ line unconditionaly. That's why
any implementation of .end() (which re-enables the IRQ line) must always check for IRQ_DISABLED
and never be the same routine as .enable!
Otherwise, disable_irq_nosync() being called in ISR works not as expected (IRQ line enabling is not defered).
e.g.
arch/ppc/syslib/gt64260_pic.c
struct hw_interrupt_type gt64260_pic = {
.typename = " gt64260_pic ",
.enable = gt64260_unmask_irq,
.disable = gt64260_mask_irq,
.ack = gt64260_mask_irq,
.end = gt64260_unmask_irq, <--- the same as .enable (i.e. doesn't check for IRQ_DISABLED)
};
There are a few other sub-archs that do the same.
Error?
yet another example :
static struct hw_interrupt_type xilinx_intc = {
.typename = "Xilinx Interrupt Controller",
.enable = xilinx_intc_enable,
.disable = xilinx_intc_disable,
.ack = xilinx_intc_disable_and_ack,
.end = xilinx_intc_end, <--- OK! This is correct.
};
but let's look at the implementation :
static void
xilinx_intc_end(unsigned int irq)
{
unsigned long mask = (0x00000001 << (irq & 31));
pr_debug("end: %d\n", irq);
if (!(irq_desc[irq].status & (IRQ_DISABLED | IRQ_INPROGRESS))) {
intc_out_be32(intc + SIE, mask);
/* ack level sensitive intr */ <--- never gets executed if ISR has called disable_irq()!
if (irq_desc[irq].status & IRQ_LEVEL)
intc_out_be32(intc + IAR, mask);
}
}
so
ISR have not called disable_irq() : enable + ack.
-//- did call disable_irq() : only enable (e.g. enable_irq() -> handler->enable() in bottom half)
Could it be such an arch feature? It looks to me that it's logically wrong anyway.
Heh... if one is still reading these lines, thanks for your time indeed! :o)
--
Best regards,
Dmitry Adamushko
_______________________________________________ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
