Messages in this thread | | | Date | Fri, 18 Jul 2008 09:45:17 -0500 | From | Eric Sandeen <> | Subject | Re: [PATCH] i386: Execute stack overflow warning on interrupt stack v2 |
| |
Andi Kleen wrote: > i386: Execute stack overflow warning on interrupt stack v2 > > [Found the problem that caused the warnings on some configs now] > > Previously the reporting printk would run on the process stack, which risks > overflow an already low stack. Instead execute it on the interrupt stack. > This makes it more likely for the printk to make it actually out. > > It adds one not taken test/branch more to the interrupt path when > stack overflow checking is enabled. We could avoid that by duplicating > more code, but that seemed not worth it. > > Based on an observation by Eric Sandeen. > > v2: Fix warnings in some configs
Did this patch get lost? I'd sure like to see it go in, the stack overflow warning + heavy dump_stack on the original stack makes the warning do much more harm than good.
Thanks, -Eric
> Signed-off-by: Andi Kleen <andi@firstfloor.org> > > --- > arch/x86/kernel/irq_32.c | 51 +++++++++++++++++++++++++++++++---------------- > 1 file changed, 34 insertions(+), 17 deletions(-) > > Index: linux/arch/x86/kernel/irq_32.c > =================================================================== > --- linux.orig/arch/x86/kernel/irq_32.c > +++ linux/arch/x86/kernel/irq_32.c > @@ -61,6 +61,26 @@ static union irq_ctx *hardirq_ctx[NR_CPU > static union irq_ctx *softirq_ctx[NR_CPUS] __read_mostly; > #endif > > +static void stack_overflow(void) > +{ > + printk("low stack detected by irq handler\n"); > + dump_stack(); > +} > + > +static inline void call_on_stack2(void *func, void *stack, > + unsigned long arg1, unsigned long arg2) > +{ > + unsigned long bx; > + asm volatile( > + " xchgl %%ebx,%%esp \n" > + " call *%%edi \n" > + " movl %%ebx,%%esp \n" > + : "=a" (arg1), "=d" (arg2), "=b" (bx) > + : "0" (arg1), "1" (arg2), "2" (stack), > + "D" (func) > + : "memory", "cc", "ecx"); > +} > + > /* > * do_IRQ handles all normal device IRQ's (the special > * SMP cross-CPU interrupts have their own specific > @@ -76,6 +96,7 @@ unsigned int do_IRQ(struct pt_regs *regs > union irq_ctx *curctx, *irqctx; > u32 *isp; > #endif > + int overflow = 0; > > if (unlikely((unsigned)irq >= NR_IRQS)) { > printk(KERN_EMERG "%s: cannot handle IRQ %d\n", > @@ -92,11 +113,8 @@ unsigned int do_IRQ(struct pt_regs *regs > > __asm__ __volatile__("andl %%esp,%0" : > "=r" (sp) : "0" (THREAD_SIZE - 1)); > - if (unlikely(sp < (sizeof(struct thread_info) + STACK_WARN))) { > - printk("do_IRQ: stack overflow: %ld\n", > - sp - sizeof(struct thread_info)); > - dump_stack(); > - } > + if (unlikely(sp < (sizeof(struct thread_info) + STACK_WARN))) > + overflow = 1; > } > #endif > > @@ -112,8 +130,6 @@ unsigned int do_IRQ(struct pt_regs *regs > * current stack (which is the irq stack already after all) > */ > if (curctx != irqctx) { > - int arg1, arg2, bx; > - > /* build the stack frame on the IRQ stack */ > isp = (u32*) ((char*)irqctx + sizeof(*irqctx)); > irqctx->tinfo.task = curctx->tinfo.task; > @@ -127,18 +143,19 @@ unsigned int do_IRQ(struct pt_regs *regs > (irqctx->tinfo.preempt_count & ~SOFTIRQ_MASK) | > (curctx->tinfo.preempt_count & SOFTIRQ_MASK); > > - asm volatile( > - " xchgl %%ebx,%%esp \n" > - " call *%%edi \n" > - " movl %%ebx,%%esp \n" > - : "=a" (arg1), "=d" (arg2), "=b" (bx) > - : "0" (irq), "1" (desc), "2" (isp), > - "D" (desc->handle_irq) > - : "memory", "cc", "ecx" > - ); > + /* Execute warning on interrupt stack */ > + if (unlikely(overflow)) > + call_on_stack2(stack_overflow, isp, 0, 0); > + > + call_on_stack2(desc->handle_irq, isp, irq, (unsigned long)desc); > } else > #endif > + { > + /* AK: Slightly bogus here */ > + if (overflow) > + stack_overflow(); > desc->handle_irq(irq, desc); > + } > > irq_exit(); > set_irq_regs(old_regs); > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ >
| |