Re: [patch 1/6] hardirq: Make hardirq bits generic

Re: [patch 1/6] hardirq: Make hardirq bits generic

[Geert Uytterhoeven]

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

Re: [patch 1/6] hardirq: Make hardirq bits generic

[Thomas Gleixner]

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

Re: [patch 1/6] hardirq: Make hardirq bits generic

[Thomas Gleixner]

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

Re: [patch 1/6] hardirq: Make hardirq bits generic

[Thomas Gleixner]

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!
  */

Re: [patch 1/6] hardirq: Make hardirq bits generic

[Andreas Schwab]

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."

Re: [patch 1/6] hardirq: Make hardirq bits generic

[Geert Uytterhoeven]

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

Re: [patch 1/6] hardirq: Make hardirq bits generic

[Thomas Gleixner]

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

Re: [patch 1/6] hardirq: Make hardirq bits generic

[Thomas Gleixner]

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

Re: [patch 1/6] hardirq: Make hardirq bits generic

[Geert Uytterhoeven]

	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

Re: [patch 1/6] hardirq: Make hardirq bits generic

[Thomas Gleixner]

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

Re: [patch 1/6] hardirq: Make hardirq bits generic

[Geert Uytterhoeven]

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

Re: [patch 1/6] hardirq: Make hardirq bits generic

[Thomas Gleixner]

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

Re: [patch 1/6] hardirq: Make hardirq bits generic

[Michael Schmitz]

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

Re: [patch 1/6] hardirq: Make hardirq bits generic

[Geert Uytterhoeven]

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

Re: [patch 1/6] hardirq: Make hardirq bits generic

[Thomas Gleixner]

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);

Re: [patch 1/6] hardirq: Make hardirq bits generic

[Thomas Gleixner]

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

Re: [patch 1/6] hardirq: Make hardirq bits generic

[Andreas Schwab]

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."

Re: [patch 1/6] hardirq: Make hardirq bits generic

[Thomas Gleixner]

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.

Re: [patch 1/6] hardirq: Make hardirq bits generic

[Michael Schmitz]

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

Re: [patch 1/6] hardirq: Make hardirq bits generic

[schmitz]

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

Re: [patch 1/6] hardirq: Make hardirq bits generic

[Thomas Gleixner]

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.

Re: [patch 1/6] hardirq: Make hardirq bits generic

[Geert Uytterhoeven]

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

Re: [patch 1/6] hardirq: Make hardirq bits generic

[Geert Uytterhoeven]

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

[tip:irq/urgent] m68k: Simplify low level interrupt handling code

[tip-bot for Thomas Gleixner]

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.