* Re: [patch 1/6] hardirq: Make hardirq bits generic [not found] ` <20130917183628.534494408@linutronix.de> @ 2013-09-17 20:00 ` Geert Uytterhoeven 2013-09-17 21:24 ` Thomas Gleixner 0 siblings, 1 reply; 24+ messages in thread From: Geert Uytterhoeven @ 2013-09-17 20:00 UTC (permalink / raw) To: Thomas Gleixner Cc: LKML, Peter Zijlstra, Ingo Molnar, Linux-Arch, Linus Torvalds, Andi Kleen, Peter Anvin, Mike Galbraith, Arjan van de Ven, Frederic Weisbecker, Linux/m68k On Tue, Sep 17, 2013 at 8:53 PM, Thomas Gleixner <tglx@linutronix.de> wrote: > --- linux-2.6.orig/arch/m68k/include/asm/hardirq.h > +++ linux-2.6/arch/m68k/include/asm/hardirq.h > @@ -5,17 +5,6 @@ > #include <linux/cache.h> > #include <asm/irq.h> > > -#define HARDIRQ_BITS 8 > --- linux-2.6.orig/include/linux/preempt_mask.h > +++ linux-2.6/include/linux/preempt_mask.h > @@ -11,36 +11,22 @@ > * - bits 0-7 are the preemption count (max preemption depth: 256) > * - bits 8-15 are the softirq count (max # of softirqs: 256) > * > - * The hardirq count can in theory reach the same as NR_IRQS. > - * In reality, the number of nested IRQS is limited to the stack > - * size as well. For archs with over 1000 IRQS it is not practical > - * to expect that they will all nest. We give a max of 10 bits for > - * hardirq nesting. An arch may choose to give less than 10 bits. > - * m68k expects it to be 8. m68k needs some changes in arch/m68k/kernel/entry.S, cfr. this check in arch/m68k/kernel/ints.c: /* assembly irq entry code relies on this... */ if (HARDIRQ_MASK != 0x00ff0000) { extern void hardirq_mask_is_broken(void); hardirq_mask_is_broken(); } Haven't looked into the details yet... > - * - bits 16-25 are the hardirq count (max # of nested hardirqs: 1024) > - * - bit 26 is the NMI_MASK > - * - bit 27 is the PREEMPT_ACTIVE flag > + * The hardirq count could in theory be the same as the number of > + * interrupts in the system, but we run all interrupt handlers with > + * interrupts disabled, so we cannot have nesting interrupts. Though > + * there are a few palaeontologic drivers which reenable interrupts in > + * the handler, so we need more than one bit here. > * > * PREEMPT_MASK: 0x000000ff > * SOFTIRQ_MASK: 0x0000ff00 > - * HARDIRQ_MASK: 0x03ff0000 > - * NMI_MASK: 0x04000000 > + * HARDIRQ_MASK: 0x000f0000 > + * NMI_MASK: 0x00100000 > */ > #define PREEMPT_BITS 8 > #define SOFTIRQ_BITS 8 > +#define HARDIRQ_BITS 4 > #define NMI_BITS 1 Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [patch 1/6] hardirq: Make hardirq bits generic 2013-09-17 20:00 ` [patch 1/6] hardirq: Make hardirq bits generic Geert Uytterhoeven @ 2013-09-17 21:24 ` Thomas Gleixner 2013-09-18 14:06 ` Thomas Gleixner 0 siblings, 1 reply; 24+ messages in thread From: Thomas Gleixner @ 2013-09-17 21:24 UTC (permalink / raw) To: Geert Uytterhoeven Cc: LKML, Peter Zijlstra, Ingo Molnar, Linux-Arch, Linus Torvalds, Andi Kleen, Peter Anvin, Mike Galbraith, Arjan van de Ven, Frederic Weisbecker, Linux/m68k On Tue, 17 Sep 2013, Geert Uytterhoeven wrote: > On Tue, Sep 17, 2013 at 8:53 PM, Thomas Gleixner <tglx@linutronix.de> wrote: > > --- linux-2.6.orig/arch/m68k/include/asm/hardirq.h > > +++ linux-2.6/arch/m68k/include/asm/hardirq.h > > @@ -5,17 +5,6 @@ > > #include <linux/cache.h> > > #include <asm/irq.h> > > > > -#define HARDIRQ_BITS 8 > > > --- linux-2.6.orig/include/linux/preempt_mask.h > > +++ linux-2.6/include/linux/preempt_mask.h > > @@ -11,36 +11,22 @@ > > * - bits 0-7 are the preemption count (max preemption depth: 256) > > * - bits 8-15 are the softirq count (max # of softirqs: 256) > > * > > - * The hardirq count can in theory reach the same as NR_IRQS. > > - * In reality, the number of nested IRQS is limited to the stack > > - * size as well. For archs with over 1000 IRQS it is not practical > > - * to expect that they will all nest. We give a max of 10 bits for > > - * hardirq nesting. An arch may choose to give less than 10 bits. > > - * m68k expects it to be 8. > > m68k needs some changes in arch/m68k/kernel/entry.S, cfr. this check > in arch/m68k/kernel/ints.c: > > /* assembly irq entry code relies on this... */ > if (HARDIRQ_MASK != 0x00ff0000) { > extern void hardirq_mask_is_broken(void); > hardirq_mask_is_broken(); > } > > Haven't looked into the details yet... Whee. Did not notice that one. Though I can't find anything interesting in the low level entry code... Looks like some more histerical left overs. Thanks, tglx ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [patch 1/6] hardirq: Make hardirq bits generic 2013-09-17 21:24 ` Thomas Gleixner @ 2013-09-18 14:06 ` Thomas Gleixner 2013-09-19 15:14 ` Thomas Gleixner 0 siblings, 1 reply; 24+ messages in thread From: Thomas Gleixner @ 2013-09-18 14:06 UTC (permalink / raw) To: Geert Uytterhoeven Cc: LKML, Peter Zijlstra, Ingo Molnar, Linux-Arch, Linus Torvalds, Andi Kleen, Peter Anvin, Mike Galbraith, Arjan van de Ven, Frederic Weisbecker, Linux/m68k On Tue, 17 Sep 2013, Thomas Gleixner wrote: > On Tue, 17 Sep 2013, Geert Uytterhoeven wrote: > > > On Tue, Sep 17, 2013 at 8:53 PM, Thomas Gleixner <tglx@linutronix.de> wrote: > > > --- linux-2.6.orig/arch/m68k/include/asm/hardirq.h > > > +++ linux-2.6/arch/m68k/include/asm/hardirq.h > > > @@ -5,17 +5,6 @@ > > > #include <linux/cache.h> > > > #include <asm/irq.h> > > > > > > -#define HARDIRQ_BITS 8 > > > > > --- linux-2.6.orig/include/linux/preempt_mask.h > > > +++ linux-2.6/include/linux/preempt_mask.h > > > @@ -11,36 +11,22 @@ > > > * - bits 0-7 are the preemption count (max preemption depth: 256) > > > * - bits 8-15 are the softirq count (max # of softirqs: 256) > > > * > > > - * The hardirq count can in theory reach the same as NR_IRQS. > > > - * In reality, the number of nested IRQS is limited to the stack > > > - * size as well. For archs with over 1000 IRQS it is not practical > > > - * to expect that they will all nest. We give a max of 10 bits for > > > - * hardirq nesting. An arch may choose to give less than 10 bits. > > > - * m68k expects it to be 8. > > > > m68k needs some changes in arch/m68k/kernel/entry.S, cfr. this check > > in arch/m68k/kernel/ints.c: > > > > /* assembly irq entry code relies on this... */ > > if (HARDIRQ_MASK != 0x00ff0000) { > > extern void hardirq_mask_is_broken(void); > > hardirq_mask_is_broken(); > > } > > > > Haven't looked into the details yet... > > Whee. Did not notice that one. Though I can't find anything > interesting in the low level entry code... Looks like some more > histerical left overs. Duh. With brain awake I can see it. The low level entry code is fiddling with preempt_count by adding HARDIRQ_OFFSET to it to keep track of nested interrupts. If the count goes to 0, it invokes do_softirq(). And you have another safety guard: ret_from_last_interrupt: moveq #(~ALLOWINT>>8)&0xff,%d0 andb %sp@(PT_OFF_SR),%d0 jne 2b That's due to the irq priority level stuff, which results in nested interrupts depending on the level of the serviced interrupt, right? And that's why you fiddle yourself with the HARDIRQ bits in the preempt count to prevent the core code from calling do_softirq(). Though this scheme also prevents that other parts of irq_exit() are working correctly, because they depend on the hardirq count being zero, e.g. the nohz code. Needs more thoughts how to fix that w/o wasting precious bits for the HARDIRQ count. Thanks, tglx ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [patch 1/6] hardirq: Make hardirq bits generic 2013-09-18 14:06 ` Thomas Gleixner @ 2013-09-19 15:14 ` Thomas Gleixner 2013-09-19 17:02 ` Andreas Schwab 0 siblings, 1 reply; 24+ messages in thread From: Thomas Gleixner @ 2013-09-19 15:14 UTC (permalink / raw) To: Geert Uytterhoeven Cc: LKML, Peter Zijlstra, Ingo Molnar, Linux-Arch, Linus Torvalds, Andi Kleen, Peter Anvin, Mike Galbraith, Arjan van de Ven, Frederic Weisbecker, Linux/m68k Geert, On Wed, 18 Sep 2013, Thomas Gleixner wrote: > The low level entry code is fiddling with preempt_count by adding > HARDIRQ_OFFSET to it to keep track of nested interrupts. If the count > goes to 0, it invokes do_softirq(). And you have another safety guard: > > ret_from_last_interrupt: > moveq #(~ALLOWINT>>8)&0xff,%d0 > andb %sp@(PT_OFF_SR),%d0 > jne 2b > > That's due to the irq priority level stuff, which results in nested > interrupts depending on the level of the serviced interrupt, right? > And that's why you fiddle yourself with the HARDIRQ bits in the > preempt count to prevent the core code from calling do_softirq(). > > Though this scheme also prevents that other parts of irq_exit() are > working correctly, because they depend on the hardirq count being > zero, e.g. the nohz code. > > Needs more thoughts how to fix that w/o wasting precious bits for the > HARDIRQ count. So after staring a while into the m68k code I came up with the following (untested and uncompiled) solution: Instead of doing the HARDIRQ fiddling and the softirq handling from the low level entry code, I provided an irq_exit_nested() variant which has an argument to tell the core code, that it shouldn't invoke softirq handling and the nohz exit code. That way 4 HARDIRQ bits in preempt_count() should be sufficient and we can move them around as we see fit without being bound by some magic asm code fiddling with it. By modifying do_IRQ to return an indicator whether this is the last irq in the chain, we can also simplify the asm code significantly. Can you have a look with m68k eyes on that please? I'm sure that my vague memory of the 68k ASM tricked me into doing something fundamentally wrong :) Thanks, tglx --- Subject: m68k: Deal with interrupt nesting in the core code From: Thomas Gleixner <tglx@linutronix.de> Date: Wed, 18 Sep 2013 11:56:58 +0200 m68k increments the HARDIRQ part of preempt_count in the low level interrupt entry code, which prevents the core code from restructuring the preempt_count layout. This is done to prevent softirq invocation for nested interrupts. The nesting can happen due to the interrupt priority scheme of the 68k. This patch removes the low level handling and moves it to the interrupt core code by providing an irq_exit_nested() variant. The nested argument to this function tells the core code whether to invoke softirqs or not. Also let do_IRQ() and the other handler variants return the nest indicator so the low level entry code can utilize this to select either immediate return or the expensive work functions. Signed-off-by: Thomas Gleixner <tglx@linutronix.de> --- arch/m68k/include/asm/irq.h | 2 +- arch/m68k/kernel/entry.S | 36 +++++++++--------------------------- arch/m68k/kernel/ints.c | 6 ------ arch/m68k/kernel/irq.c | 7 ++++--- arch/m68k/platform/68000/entry.S | 19 ++++++------------- arch/m68k/platform/68000/ints.c | 7 +++---- arch/m68k/platform/68360/entry.S | 25 ++++++++----------------- arch/m68k/q40/q40ints.c | 12 ++++++------ include/linux/hardirq.h | 1 + kernel/softirq.c | 15 +++++++++++---- 10 files changed, 49 insertions(+), 81 deletions(-) Index: linux-2.6/arch/m68k/include/asm/irq.h =================================================================== --- linux-2.6.orig/arch/m68k/include/asm/irq.h +++ linux-2.6/arch/m68k/include/asm/irq.h @@ -74,7 +74,7 @@ extern unsigned int irq_canonicalize(uns #define irq_canonicalize(irq) (irq) #endif /* !(CONFIG_M68020 || CONFIG_M68030 || CONFIG_M68040 || CONFIG_M68060) */ -asmlinkage void do_IRQ(int irq, struct pt_regs *regs); +asmlinkage int do_IRQ(int irq, struct pt_regs *regs); extern atomic_t irq_err_count; #endif /* _M68K_IRQ_H_ */ Index: linux-2.6/arch/m68k/kernel/entry.S =================================================================== --- linux-2.6.orig/arch/m68k/kernel/entry.S +++ linux-2.6/arch/m68k/kernel/entry.S @@ -274,9 +274,6 @@ do_delayed_trace: ENTRY(auto_inthandler) SAVE_ALL_INT - GET_CURRENT(%d0) - movel %d0,%a1 - addqb #1,%a1@(TINFO_PREEMPT+1) | put exception # in d0 bfextu %sp@(PT_OFF_FORMATVEC){#4,#10},%d0 subw #VEC_SPUR,%d0 @@ -284,34 +281,22 @@ ENTRY(auto_inthandler) movel %sp,%sp@- movel %d0,%sp@- | put vector # on stack auto_irqhandler_fixup = . + 2 - jsr do_IRQ | process the IRQ + jsr do_IRQ | process the IRQ, returns nest level addql #8,%sp | pop parameters off stack ret_from_interrupt: - movel %curptr@(TASK_STACK),%a1 - subqb #1,%a1@(TINFO_PREEMPT+1) - jeq ret_from_last_interrupt -2: RESTORE_ALL - - ALIGN -ret_from_last_interrupt: - moveq #(~ALLOWINT>>8)&0xff,%d0 - andb %sp@(PT_OFF_SR),%d0 - jne 2b - - /* check if we need to do software interrupts */ - tstl irq_stat+CPUSTAT_SOFTIRQ_PENDING + /* + * Only the last interrupt leaving the kernel goes through the + * various exception return checks. + */ + cmpl #0, %d0 jeq .Lret_from_exception - pea ret_from_exception - jra do_softirq + RESTORE_ALL /* Handler for user defined interrupt vectors */ ENTRY(user_inthandler) SAVE_ALL_INT - GET_CURRENT(%d0) - movel %d0,%a1 - addqb #1,%a1@(TINFO_PREEMPT+1) | put exception # in d0 bfextu %sp@(PT_OFF_FORMATVEC){#4,#10},%d0 user_irqvec_fixup = . + 2 @@ -319,13 +304,10 @@ user_irqvec_fixup = . + 2 movel %sp,%sp@- movel %d0,%sp@- | put vector # on stack - jsr do_IRQ | process the IRQ + jsr do_IRQ | process the IRQ, returns nest level addql #8,%sp | pop parameters off stack - movel %curptr@(TASK_STACK),%a1 - subqb #1,%a1@(TINFO_PREEMPT+1) - jeq ret_from_last_interrupt - RESTORE_ALL + jra ret_from_interrupt /* Handler for uninitialized and spurious interrupts */ Index: linux-2.6/arch/m68k/kernel/ints.c =================================================================== --- linux-2.6.orig/arch/m68k/kernel/ints.c +++ linux-2.6/arch/m68k/kernel/ints.c @@ -58,12 +58,6 @@ void __init init_IRQ(void) { int i; - /* assembly irq entry code relies on this... */ - if (HARDIRQ_MASK != 0x00ff0000) { - extern void hardirq_mask_is_broken(void); - hardirq_mask_is_broken(); - } - for (i = IRQ_AUTO_1; i <= IRQ_AUTO_7; i++) irq_set_chip_and_handler(i, &auto_irq_chip, handle_simple_irq); Index: linux-2.6/arch/m68k/kernel/irq.c =================================================================== --- linux-2.6.orig/arch/m68k/kernel/irq.c +++ linux-2.6/arch/m68k/kernel/irq.c @@ -17,18 +17,19 @@ #include <linux/seq_file.h> #include <asm/traps.h> -asmlinkage void do_IRQ(int irq, struct pt_regs *regs) +asmlinkage int do_IRQ(int irq, struct pt_regs *regs) { struct pt_regs *oldregs = set_irq_regs(regs); + int nested = regs->sr & ~ALLOWINT; irq_enter(); generic_handle_irq(irq); - irq_exit(); + irq_exit_nested(nested); set_irq_regs(oldregs); + return nested; } - /* The number of spurious interrupts */ atomic_t irq_err_count; Index: linux-2.6/arch/m68k/platform/68000/entry.S =================================================================== --- linux-2.6.orig/arch/m68k/platform/68000/entry.S +++ linux-2.6/arch/m68k/platform/68000/entry.S @@ -217,20 +217,13 @@ inthandler: bra ret_from_interrupt ret_from_interrupt: - jeq 1f -2: - RESTORE_ALL -1: - moveb %sp@(PT_OFF_SR), %d0 - and #7, %d0 - jhi 2b - - /* check if we need to do software interrupts */ + /* + * Only the last interrupt leaving the kernel goes through the + * various exception return checks. + */ + cmpl #0, %d0 jeq ret_from_exception - - pea ret_from_exception - jra do_softirq - + RESTORE_ALL /* * Handler for uninitialized and spurious interrupts. Index: linux-2.6/arch/m68k/platform/68000/ints.c =================================================================== --- linux-2.6.orig/arch/m68k/platform/68000/ints.c +++ linux-2.6/arch/m68k/platform/68000/ints.c @@ -74,11 +74,9 @@ asmlinkage irqreturn_t inthandler7(void) * into one vector and look in the blasted mask register... * This code is designed to be fast, almost constant time, not clean! */ -void process_int(int vec, struct pt_regs *fp) +int process_int(int vec, struct pt_regs *fp) { - int irq; - int mask; - + int irq, mask, nested =fp->sr & ~ALLOWINT; unsigned long pend = ISR; while (pend) { @@ -128,6 +126,7 @@ void process_int(int vec, struct pt_regs do_IRQ(irq, fp); pend &= ~mask; } + return nested; } static void intc_irq_unmask(struct irq_data *d) Index: linux-2.6/arch/m68k/platform/68360/entry.S =================================================================== --- linux-2.6.orig/arch/m68k/platform/68360/entry.S +++ linux-2.6/arch/m68k/platform/68360/entry.S @@ -132,26 +132,17 @@ inthandler: movel %sp,%sp@- movel %d0,%sp@- /* put vector # on stack*/ - jbsr do_IRQ /* process the IRQ*/ -3: addql #8,%sp /* pop parameters off stack*/ - bra ret_from_interrupt + jbsr do_IRQ /* process the IRQ, returns nest level */ + addql #8,%sp /* pop parameters off stack*/ ret_from_interrupt: - jeq 1f -2: - RESTORE_ALL -1: - moveb %sp@(PT_OFF_SR), %d0 - and #7, %d0 - jhi 2b - /* check if we need to do software interrupts */ - - movel irq_stat+CPUSTAT_SOFTIRQ_PENDING,%d0 + /* + * Only the last interrupt leaving the kernel goes through the + * various exception return checks. + */ + cmpl #0, %d0 jeq ret_from_exception - - pea ret_from_exception - jra do_softirq - + RESTORE_ALL /* * Handler for uninitialized and spurious interrupts. Index: linux-2.6/arch/m68k/q40/q40ints.c =================================================================== --- linux-2.6.orig/arch/m68k/q40/q40ints.c +++ linux-2.6/arch/m68k/q40/q40ints.c @@ -202,10 +202,10 @@ static int aliased_irq=0; /* how many t /* got interrupt, dispatch to ISA or keyboard/timer IRQs */ -static void q40_irq_handler(unsigned int irq, struct pt_regs *fp) +static int q40_irq_handler(unsigned int irq, struct pt_regs *fp) { unsigned mir, mer; - int i; + int i, nested = fp->sr & ~ALLOWINT; //repeat: mir = master_inb(IIRQ_REG); @@ -213,14 +213,14 @@ static void q40_irq_handler(unsigned int if ((mir & Q40_IRQ_EXT_MASK) && (master_inb(EIRQ_REG) & Q40_IRQ6_MASK)) { floppy_hardint(); - return; + return nested; } #endif switch (irq) { case 4: case 6: do_IRQ(Q40_IRQ_SAMPLE, fp); - return; + return nested; } if (mir & Q40_IRQ_FRAME_MASK) { do_IRQ(Q40_IRQ_FRAME, fp); @@ -277,7 +277,7 @@ static void q40_irq_handler(unsigned int #endif } // used to do 'goto repeat;' here, this delayed bh processing too long - return; + return nested; } } if (mer && ccleirq > 0 && !aliased_irq) { @@ -291,7 +291,7 @@ static void q40_irq_handler(unsigned int if (mir & Q40_IRQ_KEYB_MASK) do_IRQ(Q40_IRQ_KEYBOARD, fp); - return; + return nested; } void q40_irq_enable(struct irq_data *data) Index: linux-2.6/include/linux/hardirq.h =================================================================== --- linux-2.6.orig/include/linux/hardirq.h +++ linux-2.6/include/linux/hardirq.h @@ -55,6 +55,7 @@ extern void irq_enter(void); /* * Exit irq context and process softirqs if needed: */ +extern void irq_exit_nested(bool nested); extern void irq_exit(void); #define nmi_enter() \ Index: linux-2.6/kernel/softirq.c =================================================================== --- linux-2.6.orig/kernel/softirq.c +++ linux-2.6/kernel/softirq.c @@ -350,7 +350,7 @@ static inline void tick_irq_exit(void) /* * Exit an interrupt context. Process softirqs if needed and possible: */ -void irq_exit(void) +void irq_exit_nested(bool nested) { #ifndef __ARCH_IRQ_EXIT_IRQS_DISABLED local_irq_disable(); @@ -361,13 +361,20 @@ void irq_exit(void) account_irq_exit_time(current); trace_hardirq_exit(); sub_preempt_count(HARDIRQ_OFFSET); - if (!in_interrupt() && local_softirq_pending()) - invoke_softirq(); - tick_irq_exit(); + if (!nested) { + if (!in_interrupt() && local_softirq_pending()) + invoke_softirq(); + tick_irq_exit(); + } rcu_irq_exit(); } +void irq_exit(void) +{ + irq_exit_nested(false); +} + /* * This function must run with irqs disabled! */ ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [patch 1/6] hardirq: Make hardirq bits generic 2013-09-19 15:14 ` Thomas Gleixner @ 2013-09-19 17:02 ` Andreas Schwab 2013-09-19 18:19 ` Geert Uytterhoeven 0 siblings, 1 reply; 24+ messages in thread From: Andreas Schwab @ 2013-09-19 17:02 UTC (permalink / raw) To: Thomas Gleixner Cc: Geert Uytterhoeven, LKML, Peter Zijlstra, Ingo Molnar, Linux-Arch, Linus Torvalds, Andi Kleen, Peter Anvin, Mike Galbraith, Arjan van de Ven, Frederic Weisbecker, Linux/m68k Thomas Gleixner <tglx@linutronix.de> writes: > + /* > + * Only the last interrupt leaving the kernel goes through the > + * various exception return checks. > + */ > + cmpl #0, %d0 tstl %d0 Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [patch 1/6] hardirq: Make hardirq bits generic 2013-09-19 17:02 ` Andreas Schwab @ 2013-09-19 18:19 ` Geert Uytterhoeven 2013-09-20 9:26 ` Thomas Gleixner 2013-11-04 12:06 ` Thomas Gleixner 0 siblings, 2 replies; 24+ messages in thread From: Geert Uytterhoeven @ 2013-09-19 18:19 UTC (permalink / raw) To: Thomas Gleixner Cc: Andreas Schwab, LKML, Peter Zijlstra, Ingo Molnar, Linux-Arch, Linus Torvalds, Andi Kleen, Peter Anvin, Mike Galbraith, Arjan van de Ven, Frederic Weisbecker, Linux/m68k On Thu, Sep 19, 2013 at 7:02 PM, Andreas Schwab <schwab@linux-m68k.org> wrote: > Thomas Gleixner <tglx@linutronix.de> writes: >> + /* >> + * Only the last interrupt leaving the kernel goes through the >> + * various exception return checks. >> + */ >> + cmpl #0, %d0 > tstl %d0 arch/m68k/kernel/built-in.o: In function `bad_inthandler': (.text+0x2a6): undefined reference to `ret_from_last_interrupt' I came up with the quick (whitespace-damaged-gmail) fix below. Or should we handle the nesting in handle_badint(), too? --- a/arch/m68k/kernel/entry.S +++ b/arch/m68k/kernel/entry.S @@ -313,17 +313,11 @@ user_irqvec_fixup = . + 2 ENTRY(bad_inthandler) SAVE_ALL_INT - GET_CURRENT(%d0) - movel %d0,%a1 - addqb #1,%a1@(TINFO_PREEMPT+1) movel %sp,%sp@- jsr handle_badint addql #4,%sp - movel %curptr@(TASK_STACK),%a1 - subqb #1,%a1@(TINFO_PREEMPT+1) - jeq ret_from_last_interrupt RESTORE_ALL However, the resulting kernel hangs (on ARAnyM) after starting userspace: | INIT: version 2.86 booting I'll have a deeper look when I have some more time... Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [patch 1/6] hardirq: Make hardirq bits generic 2013-09-19 18:19 ` Geert Uytterhoeven @ 2013-09-20 9:26 ` Thomas Gleixner 2013-11-04 12:06 ` Thomas Gleixner 1 sibling, 0 replies; 24+ messages in thread From: Thomas Gleixner @ 2013-09-20 9:26 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Andreas Schwab, LKML, Peter Zijlstra, Ingo Molnar, Linux-Arch, Linus Torvalds, Andi Kleen, Peter Anvin, Mike Galbraith, Arjan van de Ven, Frederic Weisbecker, Linux/m68k On Thu, 19 Sep 2013, Geert Uytterhoeven wrote: > On Thu, Sep 19, 2013 at 7:02 PM, Andreas Schwab <schwab@linux-m68k.org> wrote: > > Thomas Gleixner <tglx@linutronix.de> writes: > >> + /* > >> + * Only the last interrupt leaving the kernel goes through the > >> + * various exception return checks. > >> + */ > >> + cmpl #0, %d0 > > tstl %d0 > > arch/m68k/kernel/built-in.o: In function `bad_inthandler': > (.text+0x2a6): undefined reference to `ret_from_last_interrupt' > > I came up with the quick (whitespace-damaged-gmail) fix below. > Or should we handle the nesting in handle_badint(), too? Hmm, probably yes. If a badint gets interrupted by a good one, you would fail to go through ret_from_exception. > --- a/arch/m68k/kernel/entry.S > +++ b/arch/m68k/kernel/entry.S > @@ -313,17 +313,11 @@ user_irqvec_fixup = . + 2 > > ENTRY(bad_inthandler) > SAVE_ALL_INT > - GET_CURRENT(%d0) > - movel %d0,%a1 > - addqb #1,%a1@(TINFO_PREEMPT+1) > > movel %sp,%sp@- > jsr handle_badint > addql #4,%sp > > - movel %curptr@(TASK_STACK),%a1 > - subqb #1,%a1@(TINFO_PREEMPT+1) > - jeq ret_from_last_interrupt > RESTORE_ALL > > However, the resulting kernel hangs (on ARAnyM) after starting userspace: > > | INIT: version 2.86 booting Hmm. > I'll have a deeper look when I have some more time... Thanks a lot! tglx ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [patch 1/6] hardirq: Make hardirq bits generic 2013-09-19 18:19 ` Geert Uytterhoeven 2013-09-20 9:26 ` Thomas Gleixner @ 2013-11-04 12:06 ` Thomas Gleixner 2013-11-04 19:44 ` Geert Uytterhoeven 1 sibling, 1 reply; 24+ messages in thread From: Thomas Gleixner @ 2013-11-04 12:06 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Andreas Schwab, LKML, Peter Zijlstra, Ingo Molnar, Linux-Arch, Linus Torvalds, Andi Kleen, Peter Anvin, Mike Galbraith, Arjan van de Ven, Frederic Weisbecker, Linux/m68k On Thu, 19 Sep 2013, Geert Uytterhoeven wrote: > However, the resulting kernel hangs (on ARAnyM) after starting userspace: > > | INIT: version 2.86 booting > > I'll have a deeper look when I have some more time... Any chance that you find some more time? :) Thanks, tglx ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [patch 1/6] hardirq: Make hardirq bits generic 2013-11-04 12:06 ` Thomas Gleixner @ 2013-11-04 19:44 ` Geert Uytterhoeven 2013-11-06 17:23 ` Thomas Gleixner 0 siblings, 1 reply; 24+ messages in thread From: Geert Uytterhoeven @ 2013-11-04 19:44 UTC (permalink / raw) To: Thomas Gleixner Cc: Andreas Schwab, LKML, Peter Zijlstra, Ingo Molnar, Linux-Arch, Linus Torvalds, Andi Kleen, Peter Anvin, Mike Galbraith, Arjan van de Ven, Frederic Weisbecker, Linux/m68k Hi Thomas, On Mon, 4 Nov 2013, Thomas Gleixner wrote: > On Thu, 19 Sep 2013, Geert Uytterhoeven wrote: > > However, the resulting kernel hangs (on ARAnyM) after starting userspace: > > > > | INIT: version 2.86 booting > > > > I'll have a deeper look when I have some more time... > > Any chance that you find some more time? :) Sure! But only if you look at "[m68k] IRQ: add handle_polled_irq() for timer based soft interrupt" (http://www.spinics.net/lists/linux-m68k/msg05889.html) first ;-) Below is a patch with some fixups, on top of your two patches. Unfortunately it still hangs somewhere after mounting the root filesystem. Using this debug code for do_IRQ(): diff --git a/arch/m68k/kernel/irq.c b/arch/m68k/kernel/irq.c index aaf7b15fad41..da9687803d98 100644 --- a/arch/m68k/kernel/irq.c +++ b/arch/m68k/kernel/irq.c @@ -22,11 +22,21 @@ asmlinkage int do_IRQ(int irq, struct pt_regs *regs) struct pt_regs *oldregs = set_irq_regs(regs); int nested = regs->sr & ~ALLOWINT; +static int nesting; +const char prefix[] = " "; +unsigned long flags; +local_irq_save(flags); +nesting++; +printk("# %sirq %d nested %d\n", &prefix[16-2*nesting], irq, nested); +local_irq_restore(flags); irq_enter(); generic_handle_irq(irq); irq_exit_nested(nested); set_irq_regs(oldregs); +local_irq_save(flags); +nesting--; +local_irq_restore(flags); return nested; } I get output like # irq 15 nested 0 # irq 15 nested 1024 irq 15 while irq 15 in progress?? # irq 15 nested 1024 # irq 15 nested 1024 # irq 15 nested 1024 # irq 13 nested 1024 # irq 13 nested 1024 # irq 13 nested 1024 # irq 13 nested 1024 # irq 13 nested 1024 # irq 4 nested 0 # irq 13 nested 1024 # irq 4 nested 0 # irq 13 nested 1024 # irq 4 nested 0 # irq 13 nested 1024 # irq 13 nested 1024 # irq 13 nested 1024 # irq 4 nested 0 # irq 13 nested 1024 # irq 13 nested 1024 # irq 13 nested 1024 # irq 13 nested 1024 # irq 13 nested 1024 # irq 13 nested 1024 # irq 13 nested 1024 # irq 4 nested 0 # irq 13 nested 1024 # irq 13 nested 1024 [...] # irq 13 nested 1024 # irq 13 nested 1024 # irq 4 nested 0 # irq 13 nested 1024 # irq 4 nested 0 irq 4 while irq 4 in progress? # irq 13 nested 1024 # irq 4 nested 0 # irq 13 nested 0 and then it stops printing anything. With similar debug code on the old working do_IRQ(), I get - slightly less deep nesting, - do_IRQ() is never re-entered with the same irq number. Also note that the value of "nested" doesn't match the indentation level, which depends on my own bookkeeping using "nesting". Anyone with an idea where it's going wrong? Thanks! >From 209b6ac37811297cd305821c5689dff75226af48 Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven <geert@linux-m68k.org> Date: Sun, 22 Sep 2013 11:31:25 +0200 Subject: [PATCH] m68k/hardirq: Make hardirq bits generic fixups - tstl instead of cmpl #0 (from Andreas) - arch/m68k/kernel/built-in.o: In function `bad_inthandler': (.text+0x2a6): undefined reference to `ret_from_last_interrupt' - Handle nesting in bad_inthandler() and handle_badint(), - As do_IRQ() now returns int, m68k_setup_auto_interrupt() should take a function that returns int, too, - Forgot to update forward declaration of q40_irq_handler(), - Whitespace fixes Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> --- arch/m68k/include/asm/irq.h | 4 ++-- arch/m68k/kernel/entry.S | 10 ++-------- arch/m68k/kernel/ints.c | 9 +++++++-- arch/m68k/platform/68000/entry.S | 2 +- arch/m68k/platform/68000/ints.c | 2 +- arch/m68k/platform/68360/entry.S | 4 ++-- arch/m68k/q40/q40ints.c | 2 +- 7 files changed, 16 insertions(+), 17 deletions(-) diff --git a/arch/m68k/include/asm/irq.h b/arch/m68k/include/asm/irq.h index 8d8e0f835275..fa7f079aeafa 100644 --- a/arch/m68k/include/asm/irq.h +++ b/arch/m68k/include/asm/irq.h @@ -60,8 +60,8 @@ struct irq_desc; extern unsigned int m68k_irq_startup(struct irq_data *data); extern unsigned int m68k_irq_startup_irq(unsigned int irq); extern void m68k_irq_shutdown(struct irq_data *data); -extern void m68k_setup_auto_interrupt(void (*handler)(unsigned int, - struct pt_regs *)); +extern void m68k_setup_auto_interrupt(int (*handler)(unsigned int, + struct pt_regs *)); extern void m68k_setup_user_interrupt(unsigned int vec, unsigned int cnt); extern void m68k_setup_irq_controller(struct irq_chip *, void (*handle)(unsigned int irq, diff --git a/arch/m68k/kernel/entry.S b/arch/m68k/kernel/entry.S index d35c2a22398a..ca355813ce51 100644 --- a/arch/m68k/kernel/entry.S +++ b/arch/m68k/kernel/entry.S @@ -289,7 +289,7 @@ ret_from_interrupt: * Only the last interrupt leaving the kernel goes through the * various exception return checks. */ - cmpl #0, %d0 + tstl %d0 jeq .Lret_from_exception RESTORE_ALL @@ -313,18 +313,12 @@ user_irqvec_fixup = . + 2 ENTRY(bad_inthandler) SAVE_ALL_INT - GET_CURRENT(%d0) - movel %d0,%a1 - addqb #1,%a1@(TINFO_PREEMPT+1) movel %sp,%sp@- jsr handle_badint addql #4,%sp - movel %curptr@(TASK_STACK),%a1 - subqb #1,%a1@(TINFO_PREEMPT+1) - jeq ret_from_last_interrupt - RESTORE_ALL + jra ret_from_interrupt resume: diff --git a/arch/m68k/kernel/ints.c b/arch/m68k/kernel/ints.c index 077d3a70fed1..ec1648b97dc8 100644 --- a/arch/m68k/kernel/ints.c +++ b/arch/m68k/kernel/ints.c @@ -72,7 +72,8 @@ void __init init_IRQ(void) * standard do_IRQ(), it will be called with irq numbers in the range * from IRQ_AUTO_1 - IRQ_AUTO_7. */ -void __init m68k_setup_auto_interrupt(void (*handler)(unsigned int, struct pt_regs *)) +void __init m68k_setup_auto_interrupt(int (*handler)(unsigned int, + struct pt_regs *)) { if (handler) *auto_irqhandler_fixup = (u32)handler; @@ -162,8 +163,12 @@ unsigned int irq_canonicalize(unsigned int irq) EXPORT_SYMBOL(irq_canonicalize); -asmlinkage void handle_badint(struct pt_regs *regs) +asmlinkage int handle_badint(struct pt_regs *regs) { + int nested = regs->sr & ~ALLOWINT; + atomic_inc(&irq_err_count); pr_warn("unexpected interrupt from %u\n", regs->vector); + + return nested; } diff --git a/arch/m68k/platform/68000/entry.S b/arch/m68k/platform/68000/entry.S index afc49235c3c7..b32c6c423c90 100644 --- a/arch/m68k/platform/68000/entry.S +++ b/arch/m68k/platform/68000/entry.S @@ -221,7 +221,7 @@ ret_from_interrupt: * Only the last interrupt leaving the kernel goes through the * various exception return checks. */ - cmpl #0, %d0 + tstl %d0 jeq ret_from_exception RESTORE_ALL diff --git a/arch/m68k/platform/68000/ints.c b/arch/m68k/platform/68000/ints.c index fadd2b9ff0d9..a6c05a60a9e5 100644 --- a/arch/m68k/platform/68000/ints.c +++ b/arch/m68k/platform/68000/ints.c @@ -76,7 +76,7 @@ asmlinkage irqreturn_t inthandler7(void); */ int process_int(int vec, struct pt_regs *fp) { - int irq, mask, nested =fp->sr & ~ALLOWINT; + int irq, mask, nested = fp->sr & ~ALLOWINT; unsigned long pend = ISR; while (pend) { diff --git a/arch/m68k/platform/68360/entry.S b/arch/m68k/platform/68360/entry.S index 795abe505c35..e818794edfa7 100644 --- a/arch/m68k/platform/68360/entry.S +++ b/arch/m68k/platform/68360/entry.S @@ -133,14 +133,14 @@ inthandler: movel %sp,%sp@- movel %d0,%sp@- /* put vector # on stack*/ jbsr do_IRQ /* process the IRQ, returns nest level */ - addql #8,%sp /* pop parameters off stack*/ + addql #8,%sp /* pop parameters off stack*/ ret_from_interrupt: /* * Only the last interrupt leaving the kernel goes through the * various exception return checks. */ - cmpl #0, %d0 + tstl %d0 jeq ret_from_exception RESTORE_ALL diff --git a/arch/m68k/q40/q40ints.c b/arch/m68k/q40/q40ints.c index 179aee3a6498..a7525f189264 100644 --- a/arch/m68k/q40/q40ints.c +++ b/arch/m68k/q40/q40ints.c @@ -33,7 +33,7 @@ * */ -static void q40_irq_handler(unsigned int, struct pt_regs *fp); +static int q40_irq_handler(unsigned int, struct pt_regs *fp); static void q40_irq_enable(struct irq_data *data); static void q40_irq_disable(struct irq_data *data); -- 1.7.9.5 Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [patch 1/6] hardirq: Make hardirq bits generic 2013-11-04 19:44 ` Geert Uytterhoeven @ 2013-11-06 17:23 ` Thomas Gleixner 2013-11-07 14:12 ` Geert Uytterhoeven 0 siblings, 1 reply; 24+ messages in thread From: Thomas Gleixner @ 2013-11-06 17:23 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Andreas Schwab, LKML, Peter Zijlstra, Ingo Molnar, Linux-Arch, Linus Torvalds, Andi Kleen, Peter Anvin, Mike Galbraith, Arjan van de Ven, Frederic Weisbecker, Linux/m68k Geert, On Mon, 4 Nov 2013, Geert Uytterhoeven wrote: > But only if you look at "[m68k] IRQ: add handle_polled_irq() for timer > based soft interrupt" (http://www.spinics.net/lists/linux-m68k/msg05889.html) > first ;-) Done. Thanks for the reminder! > Below is a patch with some fixups, on top of your two patches. > > Unfortunately it still hangs somewhere after mounting the root filesystem. > > Using this debug code for do_IRQ(): > > diff --git a/arch/m68k/kernel/irq.c b/arch/m68k/kernel/irq.c > index aaf7b15fad41..da9687803d98 100644 > --- a/arch/m68k/kernel/irq.c > +++ b/arch/m68k/kernel/irq.c > @@ -22,11 +22,21 @@ asmlinkage int do_IRQ(int irq, struct pt_regs *regs) > struct pt_regs *oldregs = set_irq_regs(regs); > int nested = regs->sr & ~ALLOWINT; > > +static int nesting; > +const char prefix[] = " "; > +unsigned long flags; > +local_irq_save(flags); > +nesting++; > +printk("# %sirq %d nested %d\n", &prefix[16-2*nesting], irq, nested); > +local_irq_restore(flags); > irq_enter(); > generic_handle_irq(irq); > irq_exit_nested(nested); > > set_irq_regs(oldregs); > +local_irq_save(flags); > +nesting--; > +local_irq_restore(flags); > return nested; > } > > I get output like > > # irq 15 nested 0 > # irq 15 nested 1024 > > irq 15 while irq 15 in progress?? Huch, that's odd. > With similar debug code on the old working do_IRQ(), I get > - slightly less deep nesting, > - do_IRQ() is never re-entered with the same irq number. > > Also note that the value of "nested" doesn't match the indentation level, > which depends on my own bookkeeping using "nesting". Well, nested is just an indicator. It's not the nest level. nested = pt->sr & ~ALLOWINT; i.e.: nested = pt->sr & 0x0700; So in the case above nested is 0x400 > Anyone with an idea where it's going wrong? The original code does: add_preempt_count(HARDIRQ_OFFSET); do_IRQ() irq_enter(); add_preempt_count(HARDIRQ_OFFSET); handle_irq(); irq_exit(); local_irq_disable(); sub_preempt_count(HARDIRQ_OFFSET); sub_preempt_count(HARDIRQ_OFFSET); /* Check for nested irq */ if (in_hardirq()) reti(); /* Check for nested irq again */ if (pt->sr & ~ALLOWINT != 0) reti(); do_softirq(); .... ret_from_exception(); With the patches in place it looks like this: do_IRQ() nested = pt->sr & ~ALLOWINT; irq_enter(); add_preempt_count(HARDIRQ_OFFSET); handle_irq(); irq_exit_nested(nested); local_irq_disable(); sub_preempt_count(HARDIRQ_OFFSET); if (!nested && !in_hardirq()) do_softirq() return nested; if (nested) reti(); ret_from_exception(); So all it does essentially is to move the softirq invocation in the non nested case a tad earlier. I'm really puzzled as I can't spot the point where this change makes a real difference. Thanks, tglx ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [patch 1/6] hardirq: Make hardirq bits generic 2013-11-06 17:23 ` Thomas Gleixner @ 2013-11-07 14:12 ` Geert Uytterhoeven 2013-11-07 16:39 ` Thomas Gleixner 0 siblings, 1 reply; 24+ messages in thread From: Geert Uytterhoeven @ 2013-11-07 14:12 UTC (permalink / raw) To: Thomas Gleixner Cc: Andreas Schwab, LKML, Peter Zijlstra, Ingo Molnar, Linux-Arch, Linus Torvalds, Andi Kleen, Peter Anvin, Mike Galbraith, Arjan van de Ven, Frederic Weisbecker, Linux/m68k Hi Thomas, On Wed, Nov 6, 2013 at 6:23 PM, Thomas Gleixner <tglx@linutronix.de> wrote: >> Also note that the value of "nested" doesn't match the indentation level, >> which depends on my own bookkeeping using "nesting". > > Well, nested is just an indicator. It's not the nest level. I know, the only thing that matters is whether it's zero or not. But it should always be zero if there's no nesting, and non-zero if there is, right? So: # irq 13 nested 1024 nested should be 0 here. # irq 4 nested 0 ok # irq 13 nested 1024 ok (two extra spaces in front of "irq"). # irq 4 nested 0 nested should be non-zero here. > nested = pt->sr & ~ALLOWINT; > i.e.: > nested = pt->sr & 0x0700; > > So in the case above nested is 0x400 > >> Anyone with an idea where it's going wrong? > > The original code does: > > add_preempt_count(HARDIRQ_OFFSET); > > do_IRQ() > irq_enter(); > add_preempt_count(HARDIRQ_OFFSET); > > handle_irq(); > > irq_exit(); > local_irq_disable(); > sub_preempt_count(HARDIRQ_OFFSET); > > sub_preempt_count(HARDIRQ_OFFSET); > > /* Check for nested irq */ > if (in_hardirq()) > reti(); > > /* Check for nested irq again */ > if (pt->sr & ~ALLOWINT != 0) > reti(); > > do_softirq(); > .... > ret_from_exception(); > > With the patches in place it looks like this: > > do_IRQ() > nested = pt->sr & ~ALLOWINT; > > irq_enter(); > add_preempt_count(HARDIRQ_OFFSET); > > handle_irq(); > > irq_exit_nested(nested); > local_irq_disable(); > sub_preempt_count(HARDIRQ_OFFSET); > if (!nested && !in_hardirq()) > do_softirq() > > return nested; > > if (nested) > reti(); > > ret_from_exception(); > > So all it does essentially is to move the softirq invocation in the > non nested case a tad earlier. I'm really puzzled as I can't spot the > point where this change makes a real difference. Yes, that's also my understanding. But I can't spot it neither :-( Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [patch 1/6] hardirq: Make hardirq bits generic 2013-11-07 14:12 ` Geert Uytterhoeven @ 2013-11-07 16:39 ` Thomas Gleixner 2013-11-10 8:49 ` Michael Schmitz 0 siblings, 1 reply; 24+ messages in thread From: Thomas Gleixner @ 2013-11-07 16:39 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Andreas Schwab, LKML, Peter Zijlstra, Ingo Molnar, Linux-Arch, Linus Torvalds, Andi Kleen, Peter Anvin, Mike Galbraith, Arjan van de Ven, Frederic Weisbecker, Linux/m68k On Thu, 7 Nov 2013, Geert Uytterhoeven wrote: > Hi Thomas, > > On Wed, Nov 6, 2013 at 6:23 PM, Thomas Gleixner <tglx@linutronix.de> wrote: > >> Also note that the value of "nested" doesn't match the indentation level, > >> which depends on my own bookkeeping using "nesting". > > > > Well, nested is just an indicator. It's not the nest level. > > I know, the only thing that matters is whether it's zero or not. > But it should always be zero if there's no nesting, and non-zero if there > is, right? > > So: > > # irq 13 nested 1024 > > nested should be 0 here. > > # irq 4 nested 0 > > ok > > # irq 13 nested 1024 > > ok (two extra spaces in front of "irq"). > > # irq 4 nested 0 > > nested should be non-zero here. Hmm. The softirq code reenables interrupts unconditionally. So when an interrupt hits there SR on stack has the bits cleared. You could verify that by checking in_serving_softirq() at the entry to do_IRQ(). That could also explain the irq 4 nests in irq 4 issue. You can't observe that on the original code as the softirq invocation and therefor the interrupt enable happens outside of do_IRQ(). Though that does not explain the non nested case where nested is != 0. But it looks like that irq 13 has a higher level than 4: > # irq 13 nested 1024 > > ok (two extra spaces in front of "irq"). So it could actually be the following: irq X arrives, SR I2/1/0 is set to 4 Now before we reach do_IRQ() irq 13 arrives and interrupts irq X as it has a higher level Your nest accounting shows 0, but the SR says nested, which is actually the correct state. Is there an easy to setup/use emulator around on which I could try to dig into that myself? Thanks, tglx ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [patch 1/6] hardirq: Make hardirq bits generic 2013-11-07 16:39 ` Thomas Gleixner @ 2013-11-10 8:49 ` Michael Schmitz 2013-11-10 9:12 ` Geert Uytterhoeven 0 siblings, 1 reply; 24+ messages in thread From: Michael Schmitz @ 2013-11-10 8:49 UTC (permalink / raw) To: Thomas Gleixner; +Cc: LKML, Linux/m68k, Geert Uytterhoeven Thomas, > Is there an easy to setup/use emulator around on which I could try to > dig into that myself? I believe Geert uses ARAnyM for his tests of m68k kernels on emulators - it is reasonably easy to set up and use. I've used it to debug problems we had with the SLUB allocator two years ago. Emulation is for Atari Falcon 040 hardware, things like interrupt priorities ought to be reproduced faithfully. Not that it would hurt to try Geert's last non-booting kernel on true hardware - either send the kernel image or a patch to apply to your current tree please, Geert. Regards, Michael ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [patch 1/6] hardirq: Make hardirq bits generic 2013-11-10 8:49 ` Michael Schmitz @ 2013-11-10 9:12 ` Geert Uytterhoeven 2013-11-11 14:11 ` Thomas Gleixner 0 siblings, 1 reply; 24+ messages in thread From: Geert Uytterhoeven @ 2013-11-10 9:12 UTC (permalink / raw) To: Michael Schmitz; +Cc: Thomas Gleixner, LKML, Linux/m68k On Sun, Nov 10, 2013 at 9:49 AM, Michael Schmitz <schmitz@biophys.uni-duesseldorf.de> wrote: >> Is there an easy to setup/use emulator around on which I could try to >> dig into that myself? > > I believe Geert uses ARAnyM for his tests of m68k kernels on emulators - it > is reasonably easy to set up and use. I've used it to debug problems we had > with the SLUB allocator two years ago. Indeed. ARAnyM is the way to go. > Not that it would hurt to try Geert's last non-booting kernel on true > hardware - either send the kernel image or a patch to apply to your current > tree please, Geert. Patches sent by private email. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [patch 1/6] hardirq: Make hardirq bits generic 2013-11-10 9:12 ` Geert Uytterhoeven @ 2013-11-11 14:11 ` Thomas Gleixner 2013-11-11 19:34 ` Thomas Gleixner 2013-11-11 19:42 ` Andreas Schwab 0 siblings, 2 replies; 24+ messages in thread From: Thomas Gleixner @ 2013-11-11 14:11 UTC (permalink / raw) To: Geert Uytterhoeven; +Cc: Michael Schmitz, LKML, Linux/m68k On Sun, 10 Nov 2013, Geert Uytterhoeven wrote: > On Sun, Nov 10, 2013 at 9:49 AM, Michael Schmitz > <schmitz@biophys.uni-duesseldorf.de> wrote: > >> Is there an easy to setup/use emulator around on which I could try to > >> dig into that myself? > > > > I believe Geert uses ARAnyM for his tests of m68k kernels on emulators - it > > is reasonably easy to set up and use. I've used it to debug problems we had > > with the SLUB allocator two years ago. > > Indeed. ARAnyM is the way to go. Ok. Got it running and looked a bit deeper. I haven't yet found the root cause, but there are quite some fishy things going on. Adding enough debug printks makes the thing boot. Aside of that there seems to be a violation of the 68k interrupt model. The 68k interrupt handling allows only interrupts which have an higher level than the value of the interrupt priority mask in SR. Further the cpu sets the SR priority mask on interrupt entry to the level of the interrupt which is serviced. So now with aranym I can see a different behaviour. I just added the debug patch below to a vanilla 3.12. And I can see ever repeating IRQ 13 flags 0x400 regs->sr 0x400 with a few IRQ 15 flags 0x400 regs->sr 0x400 sprinkeled in. Not what you would expect, right? Thanks, tglx Index: linux-2.6/arch/m68k/kernel/irq.c =================================================================== --- linux-2.6.orig/arch/m68k/kernel/irq.c +++ linux-2.6/arch/m68k/kernel/irq.c @@ -20,6 +20,12 @@ asmlinkage void do_IRQ(int irq, struct pt_regs *regs) { struct pt_regs *oldregs = set_irq_regs(regs); + unsigned long nested = regs->sr & ~ALLOWINT; + unsigned long flags = arch_local_save_flags() & ~ALLOWINT; + + if (nested >= flags) + printk(KERN_ERR "IRQ %d flags 0x%lx regs->sr 0x%lx\n", + irq, flags, nested); irq_enter(); generic_handle_irq(irq); ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [patch 1/6] hardirq: Make hardirq bits generic 2013-11-11 14:11 ` Thomas Gleixner @ 2013-11-11 19:34 ` Thomas Gleixner 2013-11-11 20:52 ` Thomas Gleixner 2013-11-12 14:09 ` [patch 1/6] hardirq: Make hardirq bits generic Geert Uytterhoeven 2013-11-11 19:42 ` Andreas Schwab 1 sibling, 2 replies; 24+ messages in thread From: Thomas Gleixner @ 2013-11-11 19:34 UTC (permalink / raw) To: Geert Uytterhoeven; +Cc: Michael Schmitz, LKML, Linux/m68k On Mon, 11 Nov 2013, Thomas Gleixner wrote: > Not what you would expect, right? Finally found the issue. The patch below fixes the problem here. The little missing detail is, that I zapped GET_CURRENT() assuming blindly that this is only needed for the preempt_count hackery. But in fact the world and some more depends on it which leads to interesting explosions. Thanks, tglx ---- Index: linux-2.6/arch/m68k/kernel/entry.S =================================================================== --- linux-2.6.orig/arch/m68k/kernel/entry.S +++ linux-2.6/arch/m68k/kernel/entry.S @@ -274,6 +274,7 @@ do_delayed_trace: ENTRY(auto_inthandler) SAVE_ALL_INT + GET_CURRENT(%d0) | put exception # in d0 bfextu %sp@(PT_OFF_FORMATVEC){#4,#10},%d0 subw #VEC_SPUR,%d0 @@ -297,6 +298,7 @@ ret_from_interrupt: ENTRY(user_inthandler) SAVE_ALL_INT + GET_CURRENT(%d0) | put exception # in d0 bfextu %sp@(PT_OFF_FORMATVEC){#4,#10},%d0 user_irqvec_fixup = . + 2 @@ -313,6 +315,7 @@ user_irqvec_fixup = . + 2 ENTRY(bad_inthandler) SAVE_ALL_INT + GET_CURRENT(%d0) movel %sp,%sp@- jsr handle_badint ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [patch 1/6] hardirq: Make hardirq bits generic 2013-11-11 19:34 ` Thomas Gleixner @ 2013-11-11 20:52 ` Thomas Gleixner 2013-11-12 6:56 ` Michael Schmitz ` (2 more replies) 2013-11-12 14:09 ` [patch 1/6] hardirq: Make hardirq bits generic Geert Uytterhoeven 1 sibling, 3 replies; 24+ messages in thread From: Thomas Gleixner @ 2013-11-11 20:52 UTC (permalink / raw) To: Geert Uytterhoeven; +Cc: Michael Schmitz, LKML, Linux/m68k On Mon, 11 Nov 2013, Thomas Gleixner wrote: > On Mon, 11 Nov 2013, Thomas Gleixner wrote: > > Not what you would expect, right? > > Finally found the issue. The patch below fixes the problem here. The > little missing detail is, that I zapped GET_CURRENT() assuming blindly > that this is only needed for the preempt_count hackery. But in fact > the world and some more depends on it which leads to interesting > explosions. Some more thoughts on this. The whole nesting check in the exisiting low level entry code and what I tried to resemble with the irq_exit_nested() is pretty pointless. Let's look at auto_inthandler and ret_from_exception ENTRY(auto_inthandler) SAVE_ALL_INT GET_CURRENT(%d0) movel %d0,%a1 addqb #1,%a1@(TINFO_PREEMPT+1) | put exception # in d0 bfextu %sp@(PT_OFF_FORMATVEC){#4,#10},%d0 subw #VEC_SPUR,%d0 movel %sp,%sp@- movel %d0,%sp@- | put vector # on stack auto_irqhandler_fixup = . + 2 jsr do_IRQ | process the IRQ addql #8,%sp | pop parameters off stack ret_from_interrupt: movel %curptr@(TASK_STACK),%a1 subqb #1,%a1@(TINFO_PREEMPT+1) jeq ret_from_last_interrupt 2: RESTORE_ALL ALIGN ret_from_last_interrupt: moveq #(~ALLOWINT>>8)&0xff,%d0 andb %sp@(PT_OFF_SR),%d0 jne 2b /* check if we need to do software interrupts */ tstl irq_stat+CPUSTAT_SOFTIRQ_PENDING jeq .Lret_from_exception pea ret_from_exception jra do_softirq ENTRY(ret_from_exception) .Lret_from_exception: btst #5,%sp@(PT_OFF_SR) | check if returning to kernel bnes 1f | if so, skip resched, signals .... 1: RESTORE_ALL So in every interrupt exit path we check: 1) Whether the hardirq part of preempt_count is zero 2) Whether the interrupt prio mask of SR on stack is zero and if we finally reach ret_from_exception we have the final check: 3) whether we return to kernel or user space. And this final check is the only one which matters, really. If you look at the probability of the first two checks catching anything, then it's pretty low. Most interrupts returns go through ret_from_exception. Yes, I added counters which prove that at least on the aranym, but I doubt that it will make a real difference if you run this on real hardware. So what's the point of having these checks in the hotpath? The patch below against 3.12 vanilla works nicely on the aranym and I don't see a reason why this extra hackery is necessary at all. It's just code bloat in a hotpath. Now the only valid concern might be do_softirq itself, but that's pointless as well. If the softirq is interrupted, then we do not invoke it again. If the nested interrupt happens before irq_exit() actually disables interrupts, then we won't invoke it either as the hardirq part of preempt_count is still not zero. As a side note: The do_softirq calls in the platform/68xxx entry pathes are just copied leftovers as well. Both entry code pathes are not fiddling with the preempt count and both call do_IRQ() which will call irq_exit() at the end which will invoke do_softirq(), so the check for more softirqs in the irq return path is just pointless. Thanks, tglx --- kernel/entry.S | 40 ++++------------------------------------ kernel/ints.c | 6 ------ platform/68000/entry.S | 33 ++++++++------------------------- platform/68360/entry.S | 24 +++--------------------- 4 files changed, 15 insertions(+), 88 deletions(-) Index: linux-2.6/arch/m68k/kernel/entry.S =================================================================== --- linux-2.6.orig/arch/m68k/kernel/entry.S +++ linux-2.6/arch/m68k/kernel/entry.S @@ -45,7 +45,7 @@ .globl system_call, buserr, trap, resume .globl sys_call_table .globl __sys_fork, __sys_clone, __sys_vfork -.globl ret_from_interrupt, bad_interrupt +.globl bad_interrupt .globl auto_irqhandler_fixup .globl user_irqvec_fixup @@ -275,8 +275,6 @@ do_delayed_trace: ENTRY(auto_inthandler) SAVE_ALL_INT GET_CURRENT(%d0) - movel %d0,%a1 - addqb #1,%a1@(TINFO_PREEMPT+1) | put exception # in d0 bfextu %sp@(PT_OFF_FORMATVEC){#4,#10},%d0 subw #VEC_SPUR,%d0 @@ -286,32 +284,13 @@ ENTRY(auto_inthandler) auto_irqhandler_fixup = . + 2 jsr do_IRQ | process the IRQ addql #8,%sp | pop parameters off stack - -ret_from_interrupt: - movel %curptr@(TASK_STACK),%a1 - subqb #1,%a1@(TINFO_PREEMPT+1) - jeq ret_from_last_interrupt -2: RESTORE_ALL - - ALIGN -ret_from_last_interrupt: - moveq #(~ALLOWINT>>8)&0xff,%d0 - andb %sp@(PT_OFF_SR),%d0 - jne 2b - - /* check if we need to do software interrupts */ - tstl irq_stat+CPUSTAT_SOFTIRQ_PENDING - jeq .Lret_from_exception - pea ret_from_exception - jra do_softirq + jra ret_from_exception /* Handler for user defined interrupt vectors */ ENTRY(user_inthandler) SAVE_ALL_INT GET_CURRENT(%d0) - movel %d0,%a1 - addqb #1,%a1@(TINFO_PREEMPT+1) | put exception # in d0 bfextu %sp@(PT_OFF_FORMATVEC){#4,#10},%d0 user_irqvec_fixup = . + 2 @@ -321,29 +300,18 @@ user_irqvec_fixup = . + 2 movel %d0,%sp@- | put vector # on stack jsr do_IRQ | process the IRQ addql #8,%sp | pop parameters off stack - - movel %curptr@(TASK_STACK),%a1 - subqb #1,%a1@(TINFO_PREEMPT+1) - jeq ret_from_last_interrupt - RESTORE_ALL + jra ret_from_exception /* Handler for uninitialized and spurious interrupts */ ENTRY(bad_inthandler) SAVE_ALL_INT GET_CURRENT(%d0) - movel %d0,%a1 - addqb #1,%a1@(TINFO_PREEMPT+1) movel %sp,%sp@- jsr handle_badint addql #4,%sp - - movel %curptr@(TASK_STACK),%a1 - subqb #1,%a1@(TINFO_PREEMPT+1) - jeq ret_from_last_interrupt - RESTORE_ALL - + jra ret_from_exception resume: /* Index: linux-2.6/arch/m68k/kernel/ints.c =================================================================== --- linux-2.6.orig/arch/m68k/kernel/ints.c +++ linux-2.6/arch/m68k/kernel/ints.c @@ -58,12 +58,6 @@ void __init init_IRQ(void) { int i; - /* assembly irq entry code relies on this... */ - if (HARDIRQ_MASK != 0x00ff0000) { - extern void hardirq_mask_is_broken(void); - hardirq_mask_is_broken(); - } - for (i = IRQ_AUTO_1; i <= IRQ_AUTO_7; i++) irq_set_chip_and_handler(i, &auto_irq_chip, handle_simple_irq); Index: linux-2.6/arch/m68k/platform/68000/entry.S =================================================================== --- linux-2.6.orig/arch/m68k/platform/68000/entry.S +++ linux-2.6/arch/m68k/platform/68000/entry.S @@ -27,7 +27,6 @@ .globl ret_from_exception .globl ret_from_signal .globl sys_call_table -.globl ret_from_interrupt .globl bad_interrupt .globl inthandler1 .globl inthandler2 @@ -137,7 +136,7 @@ inthandler1: movel #65,%sp@- /* put vector # on stack*/ jbsr process_int /* process the IRQ*/ 3: addql #8,%sp /* pop parameters off stack*/ - bra ret_from_interrupt + bra ret_from_exception inthandler2: SAVE_ALL_INT @@ -148,7 +147,7 @@ inthandler2: movel #66,%sp@- /* put vector # on stack*/ jbsr process_int /* process the IRQ*/ 3: addql #8,%sp /* pop parameters off stack*/ - bra ret_from_interrupt + bra ret_from_exception inthandler3: SAVE_ALL_INT @@ -159,7 +158,7 @@ inthandler3: movel #67,%sp@- /* put vector # on stack*/ jbsr process_int /* process the IRQ*/ 3: addql #8,%sp /* pop parameters off stack*/ - bra ret_from_interrupt + bra ret_from_exception inthandler4: SAVE_ALL_INT @@ -170,7 +169,7 @@ inthandler4: movel #68,%sp@- /* put vector # on stack*/ jbsr process_int /* process the IRQ*/ 3: addql #8,%sp /* pop parameters off stack*/ - bra ret_from_interrupt + bra ret_from_exception inthandler5: SAVE_ALL_INT @@ -181,7 +180,7 @@ inthandler5: movel #69,%sp@- /* put vector # on stack*/ jbsr process_int /* process the IRQ*/ 3: addql #8,%sp /* pop parameters off stack*/ - bra ret_from_interrupt + bra ret_from_exception inthandler6: SAVE_ALL_INT @@ -192,7 +191,7 @@ inthandler6: movel #70,%sp@- /* put vector # on stack*/ jbsr process_int /* process the IRQ*/ 3: addql #8,%sp /* pop parameters off stack*/ - bra ret_from_interrupt + bra ret_from_exception inthandler7: SAVE_ALL_INT @@ -203,7 +202,7 @@ inthandler7: movel #71,%sp@- /* put vector # on stack*/ jbsr process_int /* process the IRQ*/ 3: addql #8,%sp /* pop parameters off stack*/ - bra ret_from_interrupt + bra ret_from_exception inthandler: SAVE_ALL_INT @@ -214,23 +213,7 @@ inthandler: movel %d0,%sp@- /* put vector # on stack*/ jbsr process_int /* process the IRQ*/ 3: addql #8,%sp /* pop parameters off stack*/ - bra ret_from_interrupt - -ret_from_interrupt: - jeq 1f -2: - RESTORE_ALL -1: - moveb %sp@(PT_OFF_SR), %d0 - and #7, %d0 - jhi 2b - - /* check if we need to do software interrupts */ - jeq ret_from_exception - - pea ret_from_exception - jra do_softirq - + bra ret_from_exception /* * Handler for uninitialized and spurious interrupts. Index: linux-2.6/arch/m68k/platform/68360/entry.S =================================================================== --- linux-2.6.orig/arch/m68k/platform/68360/entry.S +++ linux-2.6/arch/m68k/platform/68360/entry.S @@ -29,7 +29,6 @@ .globl ret_from_exception .globl ret_from_signal .globl sys_call_table -.globl ret_from_interrupt .globl bad_interrupt .globl inthandler @@ -132,26 +131,9 @@ inthandler: movel %sp,%sp@- movel %d0,%sp@- /* put vector # on stack*/ - jbsr do_IRQ /* process the IRQ*/ -3: addql #8,%sp /* pop parameters off stack*/ - bra ret_from_interrupt - -ret_from_interrupt: - jeq 1f -2: - RESTORE_ALL -1: - moveb %sp@(PT_OFF_SR), %d0 - and #7, %d0 - jhi 2b - /* check if we need to do software interrupts */ - - movel irq_stat+CPUSTAT_SOFTIRQ_PENDING,%d0 - jeq ret_from_exception - - pea ret_from_exception - jra do_softirq - + jbsr do_IRQ /* process the IRQ */ + addql #8,%sp /* pop parameters off stack*/ + jra ret_from_exception /* * Handler for uninitialized and spurious interrupts. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [patch 1/6] hardirq: Make hardirq bits generic 2013-11-11 20:52 ` Thomas Gleixner @ 2013-11-12 6:56 ` Michael Schmitz 2013-11-12 8:44 ` schmitz 2013-11-12 15:08 ` Geert Uytterhoeven 2013-11-13 19:42 ` [tip:irq/urgent] m68k: Simplify low level interrupt handling code tip-bot for Thomas Gleixner 2 siblings, 1 reply; 24+ messages in thread From: Michael Schmitz @ 2013-11-12 6:56 UTC (permalink / raw) To: Thomas Gleixner; +Cc: LKML, Linux/m68k, Geert Uytterhoeven Thomas, >> Finally found the issue. The patch below fixes the problem here. The >> little missing detail is, that I zapped GET_CURRENT() assuming blindly >> that this is only needed for the preempt_count hackery. But in fact >> the world and some more depends on it which leads to interesting >> explosions. Thanks for debugging this - I won't speculate on why we attempt to handle softirqs explicitly, as I have only a passing acquaintance with this code, many years back. > > Some more thoughts on this. > > The whole nesting check in the exisiting low level entry code and what > I tried to resemble with the irq_exit_nested() is pretty pointless. > > Let's look at auto_inthandler and ret_from_exception > > ENTRY(auto_inthandler) > SAVE_ALL_INT > GET_CURRENT(%d0) > movel %d0,%a1 > addqb #1,%a1@(TINFO_PREEMPT+1) > | put exception # in d0 > bfextu %sp@(PT_OFF_FORMATVEC){#4,#10},%d0 > subw #VEC_SPUR,%d0 > > movel %sp,%sp@- > movel %d0,%sp@- | put vector # on stack > auto_irqhandler_fixup = . + 2 > jsr do_IRQ | process the IRQ > addql #8,%sp | pop parameters off stack > > ret_from_interrupt: > movel %curptr@(TASK_STACK),%a1 > subqb #1,%a1@(TINFO_PREEMPT+1) > jeq ret_from_last_interrupt > 2: RESTORE_ALL > > ALIGN > ret_from_last_interrupt: > moveq #(~ALLOWINT>>8)&0xff,%d0 > andb %sp@(PT_OFF_SR),%d0 > jne 2b > > /* check if we need to do software interrupts */ > tstl irq_stat+CPUSTAT_SOFTIRQ_PENDING > jeq .Lret_from_exception > pea ret_from_exception > jra do_softirq > > > ENTRY(ret_from_exception) > .Lret_from_exception: > btst #5,%sp@(PT_OFF_SR) | check if returning to kernel > bnes 1f | if so, skip resched, signals > .... > 1: RESTORE_ALL > > So in every interrupt exit path we check: > > 1) Whether the hardirq part of preempt_count is zero > > 2) Whether the interrupt prio mask of SR on stack is zero > > and if we finally reach ret_from_exception we have the final check: > > 3) whether we return to kernel or user space. > > And this final check is the only one which matters, really. > > If you look at the probability of the first two checks catching > anything, then it's pretty low. Most interrupts returns go through > ret_from_exception. Yes, I added counters which prove that at least on > the aranym, but I doubt that it will make a real difference if you run > this on real hardware. I'm happy to try that (indeed, verify your patch works on Atari hardware first) - I trust the patch is relative to m68k git, not your previous patches? Cheers, Michael > > So what's the point of having these checks in the hotpath? The patch > below against 3.12 vanilla works nicely on the aranym and I don't see > a reason why this extra hackery is necessary at all. It's just code > bloat in a hotpath. > > Now the only valid concern might be do_softirq itself, but that's > pointless as well. If the softirq is interrupted, then we do not > invoke it again. If the nested interrupt happens before irq_exit() > actually disables interrupts, then we won't invoke it either as the > hardirq part of preempt_count is still not zero. > > As a side note: The do_softirq calls in the platform/68xxx entry > pathes are just copied leftovers as well. Both entry code pathes are > not fiddling with the preempt count and both call do_IRQ() which will > call irq_exit() at the end which will invoke do_softirq(), so the > check for more softirqs in the irq return path is just pointless. > > Thanks, > > tglx > --- > kernel/entry.S | 40 ++++------------------------------------ > kernel/ints.c | 6 ------ > platform/68000/entry.S | 33 ++++++++------------------------- > platform/68360/entry.S | 24 +++--------------------- > 4 files changed, 15 insertions(+), 88 deletions(-) > > > Index: linux-2.6/arch/m68k/kernel/entry.S > =================================================================== > --- linux-2.6.orig/arch/m68k/kernel/entry.S > +++ linux-2.6/arch/m68k/kernel/entry.S > @@ -45,7 +45,7 @@ > .globl system_call, buserr, trap, resume > .globl sys_call_table > .globl __sys_fork, __sys_clone, __sys_vfork > -.globl ret_from_interrupt, bad_interrupt > +.globl bad_interrupt > .globl auto_irqhandler_fixup > .globl user_irqvec_fixup > > @@ -275,8 +275,6 @@ do_delayed_trace: > ENTRY(auto_inthandler) > SAVE_ALL_INT > GET_CURRENT(%d0) > - movel %d0,%a1 > - addqb #1,%a1@(TINFO_PREEMPT+1) > | put exception # in d0 > bfextu %sp@(PT_OFF_FORMATVEC){#4,#10},%d0 > subw #VEC_SPUR,%d0 > @@ -286,32 +284,13 @@ ENTRY(auto_inthandler) > auto_irqhandler_fixup = . + 2 > jsr do_IRQ | process the IRQ > addql #8,%sp | pop parameters off stack > - > -ret_from_interrupt: > - movel %curptr@(TASK_STACK),%a1 > - subqb #1,%a1@(TINFO_PREEMPT+1) > - jeq ret_from_last_interrupt > -2: RESTORE_ALL > - > - ALIGN > -ret_from_last_interrupt: > - moveq #(~ALLOWINT>>8)&0xff,%d0 > - andb %sp@(PT_OFF_SR),%d0 > - jne 2b > - > - /* check if we need to do software interrupts */ > - tstl irq_stat+CPUSTAT_SOFTIRQ_PENDING > - jeq .Lret_from_exception > - pea ret_from_exception > - jra do_softirq > + jra ret_from_exception > > /* Handler for user defined interrupt vectors */ > > ENTRY(user_inthandler) > SAVE_ALL_INT > GET_CURRENT(%d0) > - movel %d0,%a1 > - addqb #1,%a1@(TINFO_PREEMPT+1) > | put exception # in d0 > bfextu %sp@(PT_OFF_FORMATVEC){#4,#10},%d0 > user_irqvec_fixup = . + 2 > @@ -321,29 +300,18 @@ user_irqvec_fixup = . + 2 > movel %d0,%sp@- | put vector # on stack > jsr do_IRQ | process the IRQ > addql #8,%sp | pop parameters off stack > - > - movel %curptr@(TASK_STACK),%a1 > - subqb #1,%a1@(TINFO_PREEMPT+1) > - jeq ret_from_last_interrupt > - RESTORE_ALL > + jra ret_from_exception > > /* Handler for uninitialized and spurious interrupts */ > > ENTRY(bad_inthandler) > SAVE_ALL_INT > GET_CURRENT(%d0) > - movel %d0,%a1 > - addqb #1,%a1@(TINFO_PREEMPT+1) > > movel %sp,%sp@- > jsr handle_badint > addql #4,%sp > - > - movel %curptr@(TASK_STACK),%a1 > - subqb #1,%a1@(TINFO_PREEMPT+1) > - jeq ret_from_last_interrupt > - RESTORE_ALL > - > + jra ret_from_exception > > resume: > /* > Index: linux-2.6/arch/m68k/kernel/ints.c > =================================================================== > --- linux-2.6.orig/arch/m68k/kernel/ints.c > +++ linux-2.6/arch/m68k/kernel/ints.c > @@ -58,12 +58,6 @@ void __init init_IRQ(void) > { > int i; > > - /* assembly irq entry code relies on this... */ > - if (HARDIRQ_MASK != 0x00ff0000) { > - extern void hardirq_mask_is_broken(void); > - hardirq_mask_is_broken(); > - } > - > for (i = IRQ_AUTO_1; i <= IRQ_AUTO_7; i++) > irq_set_chip_and_handler(i, &auto_irq_chip, handle_simple_irq); > > Index: linux-2.6/arch/m68k/platform/68000/entry.S > =================================================================== > --- linux-2.6.orig/arch/m68k/platform/68000/entry.S > +++ linux-2.6/arch/m68k/platform/68000/entry.S > @@ -27,7 +27,6 @@ > .globl ret_from_exception > .globl ret_from_signal > .globl sys_call_table > -.globl ret_from_interrupt > .globl bad_interrupt > .globl inthandler1 > .globl inthandler2 > @@ -137,7 +136,7 @@ inthandler1: > movel #65,%sp@- /* put vector # on stack*/ > jbsr process_int /* process the IRQ*/ > 3: addql #8,%sp /* pop parameters off stack*/ > - bra ret_from_interrupt > + bra ret_from_exception > > inthandler2: > SAVE_ALL_INT > @@ -148,7 +147,7 @@ inthandler2: > movel #66,%sp@- /* put vector # on stack*/ > jbsr process_int /* process the IRQ*/ > 3: addql #8,%sp /* pop parameters off stack*/ > - bra ret_from_interrupt > + bra ret_from_exception > > inthandler3: > SAVE_ALL_INT > @@ -159,7 +158,7 @@ inthandler3: > movel #67,%sp@- /* put vector # on stack*/ > jbsr process_int /* process the IRQ*/ > 3: addql #8,%sp /* pop parameters off stack*/ > - bra ret_from_interrupt > + bra ret_from_exception > > inthandler4: > SAVE_ALL_INT > @@ -170,7 +169,7 @@ inthandler4: > movel #68,%sp@- /* put vector # on stack*/ > jbsr process_int /* process the IRQ*/ > 3: addql #8,%sp /* pop parameters off stack*/ > - bra ret_from_interrupt > + bra ret_from_exception > > inthandler5: > SAVE_ALL_INT > @@ -181,7 +180,7 @@ inthandler5: > movel #69,%sp@- /* put vector # on stack*/ > jbsr process_int /* process the IRQ*/ > 3: addql #8,%sp /* pop parameters off stack*/ > - bra ret_from_interrupt > + bra ret_from_exception > > inthandler6: > SAVE_ALL_INT > @@ -192,7 +191,7 @@ inthandler6: > movel #70,%sp@- /* put vector # on stack*/ > jbsr process_int /* process the IRQ*/ > 3: addql #8,%sp /* pop parameters off stack*/ > - bra ret_from_interrupt > + bra ret_from_exception > > inthandler7: > SAVE_ALL_INT > @@ -203,7 +202,7 @@ inthandler7: > movel #71,%sp@- /* put vector # on stack*/ > jbsr process_int /* process the IRQ*/ > 3: addql #8,%sp /* pop parameters off stack*/ > - bra ret_from_interrupt > + bra ret_from_exception > > inthandler: > SAVE_ALL_INT > @@ -214,23 +213,7 @@ inthandler: > movel %d0,%sp@- /* put vector # on stack*/ > jbsr process_int /* process the IRQ*/ > 3: addql #8,%sp /* pop parameters off stack*/ > - bra ret_from_interrupt > - > -ret_from_interrupt: > - jeq 1f > -2: > - RESTORE_ALL > -1: > - moveb %sp@(PT_OFF_SR), %d0 > - and #7, %d0 > - jhi 2b > - > - /* check if we need to do software interrupts */ > - jeq ret_from_exception > - > - pea ret_from_exception > - jra do_softirq > - > + bra ret_from_exception > > /* > * Handler for uninitialized and spurious interrupts. > Index: linux-2.6/arch/m68k/platform/68360/entry.S > =================================================================== > --- linux-2.6.orig/arch/m68k/platform/68360/entry.S > +++ linux-2.6/arch/m68k/platform/68360/entry.S > @@ -29,7 +29,6 @@ > .globl ret_from_exception > .globl ret_from_signal > .globl sys_call_table > -.globl ret_from_interrupt > .globl bad_interrupt > .globl inthandler > > @@ -132,26 +131,9 @@ inthandler: > > movel %sp,%sp@- > movel %d0,%sp@- /* put vector # on stack*/ > - jbsr do_IRQ /* process the IRQ*/ > -3: addql #8,%sp /* pop parameters off stack*/ > - bra ret_from_interrupt > - > -ret_from_interrupt: > - jeq 1f > -2: > - RESTORE_ALL > -1: > - moveb %sp@(PT_OFF_SR), %d0 > - and #7, %d0 > - jhi 2b > - /* check if we need to do software interrupts */ > - > - movel irq_stat+CPUSTAT_SOFTIRQ_PENDING,%d0 > - jeq ret_from_exception > - > - pea ret_from_exception > - jra do_softirq > - > + jbsr do_IRQ /* process the IRQ */ > + addql #8,%sp /* pop parameters off stack*/ > + jra ret_from_exception > > /* > * Handler for uninitialized and spurious interrupts. > > > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-m68k" > in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [patch 1/6] hardirq: Make hardirq bits generic 2013-11-12 6:56 ` Michael Schmitz @ 2013-11-12 8:44 ` schmitz 0 siblings, 0 replies; 24+ messages in thread From: schmitz @ 2013-11-12 8:44 UTC (permalink / raw) To: Thomas Gleixner; +Cc: LKML, Linux/m68k, Geert Uytterhoeven Thomas, > >> >> If you look at the probability of the first two checks catching >> anything, then it's pretty low. Most interrupts returns go through >> ret_from_exception. Yes, I added counters which prove that at least on >> the aranym, but I doubt that it will make a real difference if you run >> this on real hardware. > > I'm happy to try that (indeed, verify your patch works on Atari > hardware first) - I trust the patch is relative to m68k git, not your > previous patches? Pleased to report the patch works, as expected, on the actual hardware. I doubt we still need to profile the interrupt return path, Geert? Cheers, Michael ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [patch 1/6] hardirq: Make hardirq bits generic 2013-11-11 20:52 ` Thomas Gleixner 2013-11-12 6:56 ` Michael Schmitz @ 2013-11-12 15:08 ` Geert Uytterhoeven 2013-11-13 19:42 ` [tip:irq/urgent] m68k: Simplify low level interrupt handling code tip-bot for Thomas Gleixner 2 siblings, 0 replies; 24+ messages in thread From: Geert Uytterhoeven @ 2013-11-12 15:08 UTC (permalink / raw) To: Thomas Gleixner; +Cc: Michael Schmitz, LKML, Linux/m68k Hi Thomas, On Mon, Nov 11, 2013 at 9:52 PM, Thomas Gleixner <tglx@linutronix.de> wrote: > Some more thoughts on this. > > The whole nesting check in the exisiting low level entry code and what > I tried to resemble with the irq_exit_nested() is pretty pointless. > > Let's look at auto_inthandler and ret_from_exception > > ENTRY(auto_inthandler) > SAVE_ALL_INT > GET_CURRENT(%d0) > movel %d0,%a1 > addqb #1,%a1@(TINFO_PREEMPT+1) > | put exception # in d0 > bfextu %sp@(PT_OFF_FORMATVEC){#4,#10},%d0 > subw #VEC_SPUR,%d0 > > movel %sp,%sp@- > movel %d0,%sp@- | put vector # on stack > auto_irqhandler_fixup = . + 2 > jsr do_IRQ | process the IRQ > addql #8,%sp | pop parameters off stack > > ret_from_interrupt: > movel %curptr@(TASK_STACK),%a1 > subqb #1,%a1@(TINFO_PREEMPT+1) > jeq ret_from_last_interrupt > 2: RESTORE_ALL > > ALIGN > ret_from_last_interrupt: > moveq #(~ALLOWINT>>8)&0xff,%d0 > andb %sp@(PT_OFF_SR),%d0 > jne 2b > > /* check if we need to do software interrupts */ > tstl irq_stat+CPUSTAT_SOFTIRQ_PENDING > jeq .Lret_from_exception > pea ret_from_exception > jra do_softirq > > > ENTRY(ret_from_exception) > .Lret_from_exception: > btst #5,%sp@(PT_OFF_SR) | check if returning to kernel > bnes 1f | if so, skip resched, signals > .... > 1: RESTORE_ALL > > So in every interrupt exit path we check: > > 1) Whether the hardirq part of preempt_count is zero > > 2) Whether the interrupt prio mask of SR on stack is zero > > and if we finally reach ret_from_exception we have the final check: > > 3) whether we return to kernel or user space. > > And this final check is the only one which matters, really. > > If you look at the probability of the first two checks catching > anything, then it's pretty low. Most interrupts returns go through > ret_from_exception. Yes, I added counters which prove that at least on > the aranym, but I doubt that it will make a real difference if you run > this on real hardware. > > So what's the point of having these checks in the hotpath? The patch Most of this seems to be as old as stone-age. It was rewritten for v2.5.29, but the initial bookkeeping was there in v2.1, and even in some form in v1.3.94. > below against 3.12 vanilla works nicely on the aranym and I don't see > a reason why this extra hackery is necessary at all. It's just code > bloat in a hotpath. > > Now the only valid concern might be do_softirq itself, but that's > pointless as well. If the softirq is interrupted, then we do not > invoke it again. If the nested interrupt happens before irq_exit() > actually disables interrupts, then we won't invoke it either as the > hardirq part of preempt_count is still not zero. > > As a side note: The do_softirq calls in the platform/68xxx entry > pathes are just copied leftovers as well. Both entry code pathes are > not fiddling with the preempt count and both call do_IRQ() which will > call irq_exit() at the end which will invoke do_softirq(), so the > check for more softirqs in the irq return path is just pointless. Your reasoning sounds OK, and it works on ARAnyM, so, thanks again and Acked-by: Geert Uytterhoeven <geert@linux-m68k.org> BTW, do you plan to get the "make hardirq bits generic" series in 3.13? Or do you want me to take this patch through the m68k tree? So far I don't have any plans to send another pull request for 3.13. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 24+ messages in thread
* [tip:irq/urgent] m68k: Simplify low level interrupt handling code 2013-11-11 20:52 ` Thomas Gleixner 2013-11-12 6:56 ` Michael Schmitz 2013-11-12 15:08 ` Geert Uytterhoeven @ 2013-11-13 19:42 ` tip-bot for Thomas Gleixner 2 siblings, 0 replies; 24+ messages in thread From: tip-bot for Thomas Gleixner @ 2013-11-13 19:42 UTC (permalink / raw) To: linux-tip-commits Cc: linux-kernel, hpa, mingo, geert, schmitz, tglx, schwab, linux-m68k Commit-ID: 09f90f6685cd88b6b904c141035d096169958cc4 Gitweb: http://git.kernel.org/tip/09f90f6685cd88b6b904c141035d096169958cc4 Author: Thomas Gleixner <tglx@linutronix.de> AuthorDate: Mon, 11 Nov 2013 21:01:03 +0100 Committer: Thomas Gleixner <tglx@linutronix.de> CommitDate: Wed, 13 Nov 2013 20:21:46 +0100 m68k: Simplify low level interrupt handling code The low level interrupt entry code of m68k contains the following: add_preempt_count(HARDIRQ_OFFSET); do_IRQ(); irq_enter(); add_preempt_count(HARDIRQ_OFFSET); handle_interrupt(); irq_exit(); sub_preempt_count(HARDIRQ_OFFSET); if (in_interrupt()) return; <---- On m68k always taken! if (local_softirq_pending()) do_softirq(); sub_preempt_count(HARDIRQ_OFFSET); if (in_hardirq()) return; if (status_on_stack_has_interrupt_priority_mask > 0) return; if (local_softirq_pending()) do_softirq(); ret_from_exception: if (interrupted_context_is_kernel) return: .... I tried to find a proper explanation for this, but the changelog is sparse and there are no mails explaining it further. But obviously this relates to the interrupt priority levels of the m68k and tries to be extra clever with nested interrupts. Though this cleverness just adds code bloat to the interrupt hotpath. For the common case of non nested interrupts the code runs through two extra conditionals to the only important one, which checks whether the return is to kernel or user space. For the nested case the checks for in_hardirq() and the priority mask value on stack catch only the case where the nested interrupt happens inside the hard irq context of the first interrupt. If the nested interrupt happens while the first interrupt handles soft interrupts, then these extra checks buy nothing. The nested interrupt will fall through to the final kernel/user space return check at ret_from_exception. Changing the code flow in the following way: do_IRQ(); irq_enter(); add_preempt_count(HARDIRQ_OFFSET); handle_interrupt(); irq_exit(); sub_preempt_count(HARDIRQ_OFFSET); if (in_interrupt()) return; if (local_softirq_pending()) do_softirq(); ret_from_exception: if (interrupted_context_is_kernel) return: makes the region protected by the hardirq count slightly smaller and the softirq handling is invoked with a minimal deeper stack. But otherwise it's completely functional equivalent and saves 104 bytes of text in arch/m68k/kernel/entry.o. This modification allows us further to get rid of the limitations which m68k puts on the preempt_count layout, so we can make the preempt count bits completely generic. Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Tested-by: Michael Schmitz <schmitz@biophys.uni-duesseldorf.de> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org> Cc: Linux/m68k <linux-m68k@vger.kernel.org> Cc: Andreas Schwab <schwab@linux-m68k.org> Link: http://lkml.kernel.org/r/alpine.DEB.2.02.1311112052360.30673@ionos.tec.linutronix.de --- arch/m68k/kernel/entry.S | 40 ++++------------------------------------ arch/m68k/kernel/ints.c | 6 ------ arch/m68k/platform/68000/entry.S | 33 ++++++++------------------------- arch/m68k/platform/68360/entry.S | 24 +++--------------------- 4 files changed, 15 insertions(+), 88 deletions(-) diff --git a/arch/m68k/kernel/entry.S b/arch/m68k/kernel/entry.S index a78f564..b54ac7a 100644 --- a/arch/m68k/kernel/entry.S +++ b/arch/m68k/kernel/entry.S @@ -45,7 +45,7 @@ .globl system_call, buserr, trap, resume .globl sys_call_table .globl __sys_fork, __sys_clone, __sys_vfork -.globl ret_from_interrupt, bad_interrupt +.globl bad_interrupt .globl auto_irqhandler_fixup .globl user_irqvec_fixup @@ -275,8 +275,6 @@ do_delayed_trace: ENTRY(auto_inthandler) SAVE_ALL_INT GET_CURRENT(%d0) - movel %d0,%a1 - addqb #1,%a1@(TINFO_PREEMPT+1) | put exception # in d0 bfextu %sp@(PT_OFF_FORMATVEC){#4,#10},%d0 subw #VEC_SPUR,%d0 @@ -286,32 +284,13 @@ ENTRY(auto_inthandler) auto_irqhandler_fixup = . + 2 jsr do_IRQ | process the IRQ addql #8,%sp | pop parameters off stack - -ret_from_interrupt: - movel %curptr@(TASK_STACK),%a1 - subqb #1,%a1@(TINFO_PREEMPT+1) - jeq ret_from_last_interrupt -2: RESTORE_ALL - - ALIGN -ret_from_last_interrupt: - moveq #(~ALLOWINT>>8)&0xff,%d0 - andb %sp@(PT_OFF_SR),%d0 - jne 2b - - /* check if we need to do software interrupts */ - tstl irq_stat+CPUSTAT_SOFTIRQ_PENDING - jeq .Lret_from_exception - pea ret_from_exception - jra do_softirq + jra ret_from_exception /* Handler for user defined interrupt vectors */ ENTRY(user_inthandler) SAVE_ALL_INT GET_CURRENT(%d0) - movel %d0,%a1 - addqb #1,%a1@(TINFO_PREEMPT+1) | put exception # in d0 bfextu %sp@(PT_OFF_FORMATVEC){#4,#10},%d0 user_irqvec_fixup = . + 2 @@ -321,29 +300,18 @@ user_irqvec_fixup = . + 2 movel %d0,%sp@- | put vector # on stack jsr do_IRQ | process the IRQ addql #8,%sp | pop parameters off stack - - movel %curptr@(TASK_STACK),%a1 - subqb #1,%a1@(TINFO_PREEMPT+1) - jeq ret_from_last_interrupt - RESTORE_ALL + jra ret_from_exception /* Handler for uninitialized and spurious interrupts */ ENTRY(bad_inthandler) SAVE_ALL_INT GET_CURRENT(%d0) - movel %d0,%a1 - addqb #1,%a1@(TINFO_PREEMPT+1) movel %sp,%sp@- jsr handle_badint addql #4,%sp - - movel %curptr@(TASK_STACK),%a1 - subqb #1,%a1@(TINFO_PREEMPT+1) - jeq ret_from_last_interrupt - RESTORE_ALL - + jra ret_from_exception resume: /* diff --git a/arch/m68k/kernel/ints.c b/arch/m68k/kernel/ints.c index 4d7da38..077d3a7 100644 --- a/arch/m68k/kernel/ints.c +++ b/arch/m68k/kernel/ints.c @@ -58,12 +58,6 @@ void __init init_IRQ(void) { int i; - /* assembly irq entry code relies on this... */ - if (HARDIRQ_MASK != 0x00ff0000) { - extern void hardirq_mask_is_broken(void); - hardirq_mask_is_broken(); - } - for (i = IRQ_AUTO_1; i <= IRQ_AUTO_7; i++) irq_set_chip_and_handler(i, &auto_irq_chip, handle_simple_irq); diff --git a/arch/m68k/platform/68000/entry.S b/arch/m68k/platform/68000/entry.S index 7f91c2f..23ac054 100644 --- a/arch/m68k/platform/68000/entry.S +++ b/arch/m68k/platform/68000/entry.S @@ -27,7 +27,6 @@ .globl ret_from_exception .globl ret_from_signal .globl sys_call_table -.globl ret_from_interrupt .globl bad_interrupt .globl inthandler1 .globl inthandler2 @@ -137,7 +136,7 @@ inthandler1: movel #65,%sp@- /* put vector # on stack*/ jbsr process_int /* process the IRQ*/ 3: addql #8,%sp /* pop parameters off stack*/ - bra ret_from_interrupt + bra ret_from_exception inthandler2: SAVE_ALL_INT @@ -148,7 +147,7 @@ inthandler2: movel #66,%sp@- /* put vector # on stack*/ jbsr process_int /* process the IRQ*/ 3: addql #8,%sp /* pop parameters off stack*/ - bra ret_from_interrupt + bra ret_from_exception inthandler3: SAVE_ALL_INT @@ -159,7 +158,7 @@ inthandler3: movel #67,%sp@- /* put vector # on stack*/ jbsr process_int /* process the IRQ*/ 3: addql #8,%sp /* pop parameters off stack*/ - bra ret_from_interrupt + bra ret_from_exception inthandler4: SAVE_ALL_INT @@ -170,7 +169,7 @@ inthandler4: movel #68,%sp@- /* put vector # on stack*/ jbsr process_int /* process the IRQ*/ 3: addql #8,%sp /* pop parameters off stack*/ - bra ret_from_interrupt + bra ret_from_exception inthandler5: SAVE_ALL_INT @@ -181,7 +180,7 @@ inthandler5: movel #69,%sp@- /* put vector # on stack*/ jbsr process_int /* process the IRQ*/ 3: addql #8,%sp /* pop parameters off stack*/ - bra ret_from_interrupt + bra ret_from_exception inthandler6: SAVE_ALL_INT @@ -192,7 +191,7 @@ inthandler6: movel #70,%sp@- /* put vector # on stack*/ jbsr process_int /* process the IRQ*/ 3: addql #8,%sp /* pop parameters off stack*/ - bra ret_from_interrupt + bra ret_from_exception inthandler7: SAVE_ALL_INT @@ -203,7 +202,7 @@ inthandler7: movel #71,%sp@- /* put vector # on stack*/ jbsr process_int /* process the IRQ*/ 3: addql #8,%sp /* pop parameters off stack*/ - bra ret_from_interrupt + bra ret_from_exception inthandler: SAVE_ALL_INT @@ -214,23 +213,7 @@ inthandler: movel %d0,%sp@- /* put vector # on stack*/ jbsr process_int /* process the IRQ*/ 3: addql #8,%sp /* pop parameters off stack*/ - bra ret_from_interrupt - -ret_from_interrupt: - jeq 1f -2: - RESTORE_ALL -1: - moveb %sp@(PT_OFF_SR), %d0 - and #7, %d0 - jhi 2b - - /* check if we need to do software interrupts */ - jeq ret_from_exception - - pea ret_from_exception - jra do_softirq - + bra ret_from_exception /* * Handler for uninitialized and spurious interrupts. diff --git a/arch/m68k/platform/68360/entry.S b/arch/m68k/platform/68360/entry.S index 904fd9a..447c33e 100644 --- a/arch/m68k/platform/68360/entry.S +++ b/arch/m68k/platform/68360/entry.S @@ -29,7 +29,6 @@ .globl ret_from_exception .globl ret_from_signal .globl sys_call_table -.globl ret_from_interrupt .globl bad_interrupt .globl inthandler @@ -132,26 +131,9 @@ inthandler: movel %sp,%sp@- movel %d0,%sp@- /* put vector # on stack*/ - jbsr do_IRQ /* process the IRQ*/ -3: addql #8,%sp /* pop parameters off stack*/ - bra ret_from_interrupt - -ret_from_interrupt: - jeq 1f -2: - RESTORE_ALL -1: - moveb %sp@(PT_OFF_SR), %d0 - and #7, %d0 - jhi 2b - /* check if we need to do software interrupts */ - - movel irq_stat+CPUSTAT_SOFTIRQ_PENDING,%d0 - jeq ret_from_exception - - pea ret_from_exception - jra do_softirq - + jbsr do_IRQ /* process the IRQ */ + addql #8,%sp /* pop parameters off stack*/ + jra ret_from_exception /* * Handler for uninitialized and spurious interrupts. ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [patch 1/6] hardirq: Make hardirq bits generic 2013-11-11 19:34 ` Thomas Gleixner 2013-11-11 20:52 ` Thomas Gleixner @ 2013-11-12 14:09 ` Geert Uytterhoeven 1 sibling, 0 replies; 24+ messages in thread From: Geert Uytterhoeven @ 2013-11-12 14:09 UTC (permalink / raw) To: Thomas Gleixner; +Cc: Michael Schmitz, LKML, Linux/m68k Hi Thomas, On Mon, Nov 11, 2013 at 8:34 PM, Thomas Gleixner <tglx@linutronix.de> wrote: > Finally found the issue. The patch below fixes the problem here. The > little missing detail is, that I zapped GET_CURRENT() assuming blindly > that this is only needed for the preempt_count hackery. But in fact > the world and some more depends on it which leads to interesting > explosions. Yes, GET_CURRENT() sets up the current task in %a2. Many thanks for tracking this down! Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [patch 1/6] hardirq: Make hardirq bits generic 2013-11-11 14:11 ` Thomas Gleixner 2013-11-11 19:34 ` Thomas Gleixner @ 2013-11-11 19:42 ` Andreas Schwab 2013-11-12 9:18 ` Thomas Gleixner 1 sibling, 1 reply; 24+ messages in thread From: Andreas Schwab @ 2013-11-11 19:42 UTC (permalink / raw) To: Thomas Gleixner; +Cc: Geert Uytterhoeven, Michael Schmitz, LKML, Linux/m68k Thomas Gleixner <tglx@linutronix.de> writes: > And I can see ever repeating > > IRQ 13 flags 0x400 regs->sr 0x400 > > with a few > > IRQ 15 flags 0x400 regs->sr 0x400 > > sprinkeled in. > > Not what you would expect, right? If you configured for ATARI only, then ALLOWINT filters out the I1 bit, which makes irq level 4 and level 6 look the same (all MFP interrupts are at level 6). Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [patch 1/6] hardirq: Make hardirq bits generic 2013-11-11 19:42 ` Andreas Schwab @ 2013-11-12 9:18 ` Thomas Gleixner 0 siblings, 0 replies; 24+ messages in thread From: Thomas Gleixner @ 2013-11-12 9:18 UTC (permalink / raw) To: Andreas Schwab; +Cc: Geert Uytterhoeven, Michael Schmitz, LKML, Linux/m68k On Mon, 11 Nov 2013, Andreas Schwab wrote: > Thomas Gleixner <tglx@linutronix.de> writes: > > > And I can see ever repeating > > > > IRQ 13 flags 0x400 regs->sr 0x400 > > > > with a few > > > > IRQ 15 flags 0x400 regs->sr 0x400 > > > > sprinkeled in. > > > > Not what you would expect, right? > > If you configured for ATARI only, then ALLOWINT filters out the I1 bit, > which makes irq level 4 and level 6 look the same (all MFP interrupts > are at level 6). Fair enough. ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2013-11-13 19:42 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20130917082838.218329307@infradead.org>
[not found] ` <20130917182350.449685712@linutronix.de>
[not found] ` <20130917183628.534494408@linutronix.de>
2013-09-17 20:00 ` [patch 1/6] hardirq: Make hardirq bits generic Geert Uytterhoeven
2013-09-17 21:24 ` Thomas Gleixner
2013-09-18 14:06 ` Thomas Gleixner
2013-09-19 15:14 ` Thomas Gleixner
2013-09-19 17:02 ` Andreas Schwab
2013-09-19 18:19 ` Geert Uytterhoeven
2013-09-20 9:26 ` Thomas Gleixner
2013-11-04 12:06 ` Thomas Gleixner
2013-11-04 19:44 ` Geert Uytterhoeven
2013-11-06 17:23 ` Thomas Gleixner
2013-11-07 14:12 ` Geert Uytterhoeven
2013-11-07 16:39 ` Thomas Gleixner
2013-11-10 8:49 ` Michael Schmitz
2013-11-10 9:12 ` Geert Uytterhoeven
2013-11-11 14:11 ` Thomas Gleixner
2013-11-11 19:34 ` Thomas Gleixner
2013-11-11 20:52 ` Thomas Gleixner
2013-11-12 6:56 ` Michael Schmitz
2013-11-12 8:44 ` schmitz
2013-11-12 15:08 ` Geert Uytterhoeven
2013-11-13 19:42 ` [tip:irq/urgent] m68k: Simplify low level interrupt handling code tip-bot for Thomas Gleixner
2013-11-12 14:09 ` [patch 1/6] hardirq: Make hardirq bits generic Geert Uytterhoeven
2013-11-11 19:42 ` Andreas Schwab
2013-11-12 9:18 ` Thomas Gleixner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox