[PATCH] Fix PREEMPT_ACTIVE definition

[PATCH] Fix PREEMPT_ACTIVE definition

[Paul Mackerras]

When the generic IRQ stuff went in, it seems that HARDIRQ_BITS got
bumped from 9 (for ppc64) up to 12.  Consequently, the PREEMPT_ACTIVE
bit is now within HARDIRQ_MASK, and I get in_interrupt() falsely
returning true when PREEMPT_ACTIVE is set, and thus a BUG_ON tripping
in arch/ppc64/mm/tlb.c.

The patch below fixes this by changing PREEMPT_ACTIVE to 0x10000000.
I have changed the PREEMPT_ACTIVE definitions for each of the
architectures that define CONFIG_GENERIC_HARDIRQS (i386, ppc, ppc64,
x86_64) and fixed the comment in include/linux/hardirq.h.  We could
perhaps move the PREEMPT_ACTIVE definition to include/linux/hardirq.h
- I don't know why it is still per-arch.

Signed-off-by: Paul Mackerras <paulus@samba.org>

diff -urN linux-2.5/include/asm-i386/thread_info.h akpm/include/asm-i386/thread_info.h
--- linux-2.5/include/asm-i386/thread_info.h	2004-08-24 07:22:48.000000000 +1000
+++ akpm/include/asm-i386/thread_info.h	2004-10-20 16:45:48.035497384 +1000
@@ -51,7 +51,7 @@
 
 #endif
 
-#define PREEMPT_ACTIVE		0x4000000
+#define PREEMPT_ACTIVE		0x10000000
 #ifdef CONFIG_4KSTACKS
 #define THREAD_SIZE            (4096)
 #else
diff -urN linux-2.5/include/asm-ppc/thread_info.h akpm/include/asm-ppc/thread_info.h
--- linux-2.5/include/asm-ppc/thread_info.h	2004-08-24 07:22:48.000000000 +1000
+++ akpm/include/asm-ppc/thread_info.h	2004-10-20 16:45:50.920591472 +1000
@@ -65,7 +65,7 @@
  */
 #define THREAD_SIZE		8192	/* 2 pages */
 
-#define PREEMPT_ACTIVE		0x4000000
+#define PREEMPT_ACTIVE		0x10000000
 
 /*
  * thread information flag bit numbers
diff -urN linux-2.5/include/asm-ppc64/thread_info.h test/include/asm-ppc64/thread_info.h
--- linux-2.5/include/asm-ppc64/thread_info.h	2004-06-18 19:06:50.000000000 +1000
+++ test/include/asm-ppc64/thread_info.h	2004-10-20 16:45:50.930589952 +1000
@@ -82,7 +82,7 @@
 
 #endif /* __ASSEMBLY__ */
 
-#define PREEMPT_ACTIVE		0x4000000
+#define PREEMPT_ACTIVE		0x10000000
 
 /*
  * thread information flag bit numbers
diff -urN linux-2.5/include/asm-x86_64/thread_info.h test/include/asm-x86_64/thread_info.h
--- linux-2.5/include/asm-x86_64/thread_info.h	2004-04-13 09:25:10.000000000 +1000
+++ test/include/asm-x86_64/thread_info.h	2004-10-20 16:45:51.005578552 +1000
@@ -125,7 +125,7 @@
 /* work to do on any return to user space */
 #define _TIF_ALLWORK_MASK 0x0000FFFF	
 
-#define PREEMPT_ACTIVE     0x4000000
+#define PREEMPT_ACTIVE     0x10000000
 
 /*
  * Thread-synchronous status.
diff -urN linux-2.5/include/linux/hardirq.h test/include/linux/hardirq.h
--- linux-2.5/include/linux/hardirq.h	2004-10-20 08:16:49.000000000 +1000
+++ test/include/linux/hardirq.h	2004-10-20 16:45:06.096615640 +1000
@@ -14,7 +14,7 @@
  * - bits 8-15 are the softirq count (max # of softirqs: 256)
  * - bits 16-27 are the hardirq count (max # of hardirqs: 4096)
  *
- * - ( bit 26 is the PREEMPT_ACTIVE flag. )
+ * - ( bit 28 is the PREEMPT_ACTIVE flag. )
  *
  * PREEMPT_MASK: 0x000000ff
  * SOFTIRQ_MASK: 0x0000ff00

Re: [PATCH] Fix PREEMPT_ACTIVE definition

[Ingo Molnar]

* Paul Mackerras <paulus@samba.org> wrote:

> When the generic IRQ stuff went in, it seems that HARDIRQ_BITS got
> bumped from 9 (for ppc64) up to 12.  Consequently, the PREEMPT_ACTIVE
> bit is now within HARDIRQ_MASK, and I get in_interrupt() falsely
> returning true when PREEMPT_ACTIVE is set, and thus a BUG_ON tripping
> in arch/ppc64/mm/tlb.c.

indeed! The reason why this problem didnt trigger on the other
architectures is that the in_atomic() test is separate and excludes
PREEMPT_ACTIVE.

> The patch below fixes this by changing PREEMPT_ACTIVE to 0x10000000. I
> have changed the PREEMPT_ACTIVE definitions for each of the
> architectures that define CONFIG_GENERIC_HARDIRQS (i386, ppc, ppc64,
> x86_64) and fixed the comment in include/linux/hardirq.h.  We could
> perhaps move the PREEMPT_ACTIVE definition to include/linux/hardirq.h
> - I don't know why it is still per-arch.
> 
> Signed-off-by: Paul Mackerras <paulus@samba.org>

Acked-by: Ingo Molnar <mingo@elte.hu>

	Ingo

New consolidate irqs vs . probe_irq_*()

[Benjamin Herrenschmidt]

Ingo, while we are at it,

Do you have any objection if I send a patch making the whole probe_irq_*
stuff optional on a CONFIG_ option ? (turning it into nops like we used
to have on ppc until now, if the option isn't set).

I really don't want to mess with that racy mecanism that makes sense for
ISA only afaik, and it seems some drivers are trying to use it now that
it's there (/me looks toward yenta_socket) and I'm afraid of the
consequences since I cannot see how that thing can work properly in the
first place ;)

Ben.

Re: New consolidate irqs vs . probe_irq_*()

[Ingo Molnar]

* Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> Do you have any objection if I send a patch making the whole
> probe_irq_* stuff optional on a CONFIG_ option ? (turning it into nops
> like we used to have on ppc until now, if the option isn't set).
> 
> I really don't want to mess with that racy mecanism that makes sense
> for ISA only afaik, and it seems some drivers are trying to use it now
> that it's there (/me looks toward yenta_socket) and I'm afraid of the
> consequences since I cannot see how that thing can work properly in
> the first place ;)

yeah. I've put it into a separate autoprobe.c file specifically for that
reason, you can exclude it in the Makefile and can provide your own
architecture version. Or should we make the no-autoprobing choice
generic perhaps?

	Ingo

Re: New consolidate irqs vs . probe_irq_*()

[Benjamin Herrenschmidt]

On Wed, 2004-10-20 at 18:48, Ingo Molnar wrote:

> yeah. I've put it into a separate autoprobe.c file specifically for that
> reason, you can exclude it in the Makefile and can provide your own
> architecture version. Or should we make the no-autoprobing choice
> generic perhaps?

I like this later option... How may archs actually had autoprobing
implemented and actually used ?

Well, I'll do some grep'ing around tonight as I do the NO_IRQ stuff
and see what makes more sense. I don't think an arch that didn't have
autoprobing needs it now, besides, it's not exactly a reliable
mecanism...

Ben.

Re: New consolidate irqs vs . probe_irq_*()

[Ingo Molnar]

* Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> On Wed, 2004-10-20 at 18:48, Ingo Molnar wrote:
> 
> > yeah. I've put it into a separate autoprobe.c file specifically for that
> > reason, you can exclude it in the Makefile and can provide your own
> > architecture version. Or should we make the no-autoprobing choice
> > generic perhaps?
> 
> I like this later option... How may archs actually had autoprobing
> implemented and actually used ?
> 
> Well, I'll do some grep'ing around tonight as I do the NO_IRQ stuff
> and see what makes more sense. I don't think an arch that didn't have
> autoprobing needs it now, besides, it's not exactly a reliable
> mecanism...

btw., auto-detection of interrupt sources is not _necessarily_ broken. 
Especially with level-triggered interrupts (where no active irq source
can get lost) it can be doable and reliable. (Here i dont mean the
probe_irq_on/off interface, but the concept of auto-detection.)

Alan has a very neat hack that fixes laptop BIOS breakage in associating
a screaming interrupt with the driver that responds to it with
IRQ_HANDLED, by probing through all the SHIRQ handlers and checking
whether the return code is IRQ_HANDLED. (this patch was in -mm but has
not been integrated into the generic irq subsystem yet.) This patch
turned a broken-under-Linux into a working laptop so we cannot ignore
it.

In theory, as long as all drivers involved are shared-irq-capable, and
all interrupts are level-triggered and get repeated if unhandled, we can
always do this kinds of driver-feedback-based irq vector discovery.

Think about the positive effects: in theory we could even boot into a
box without _any_ BIOS interrupt info whatsoever, assuming only a few
architecture basics like the identity of the timer interrupt.
Furthermore, if the BIOS reports _bad_ interrupt information, we can
detect & redirect that interrupt to the driver that responds to it.

this is not a concept that would be too useful for the server space, but
it sure could be useful for the produce-and-forget PC mass-market. We
are playing a constant and mostly losing catchup game with BIOS quirks.

what can never work fully reliably is of course what the feature was
used for primarily: ISA :-) One-time edge-triggered interrupts that get
lost are nasty ...

so i thought autodetect.c could survive in the generic code - maybe we
can make something really nice out of it, based on Alan's patch.

	Ingo

Re: New consolidate irqs vs . probe_irq_*()

[Benjamin Herrenschmidt]

On Wed, 2004-10-20 at 19:20, Ingo Molnar wrote:
>
> .../...
>
> this is not a concept that would be too useful for the server space, but
> it sure could be useful for the produce-and-forget PC mass-market. We
> are playing a constant and mostly losing catchup game with BIOS quirks.
> 
> what can never work fully reliably is of course what the feature was
> used for primarily: ISA :-) One-time edge-triggered interrupts that get
> lost are nasty ...
> 
> so i thought autodetect.c could survive in the generic code - maybe we
> can make something really nice out of it, based on Alan's patch.

Hrm... Okay, maybe, but I'd rather keep it disabled for now on ppc and
ppc64, there is simply no case there where the interrupt informations
are incorrect :) And the way PCMCIA uses it is a bit nasty imho, besides
the current interface passing in a mask isn't really useable when your
max interrupt number goes beyond the size of a long.

Note that the reason why it dies on ppc atm is that it calls the irq
desc functions like shutdown() without testing if they exist in the
first place...

Looking at the code, it seems that I need to add startup() & shutdown()
callbacks to all my irq controllers so they do the equivalent of enable
and disable at least ...

Ben. 

Re: New consolidate irqs vs . probe_irq_*()

[Chris Wedgwood]

On Wed, Oct 20, 2004 at 10:48:38AM +0200, Ingo Molnar wrote:

> yeah. I've put it into a separate autoprobe.c file specifically for
> that reason, you can exclude it in the Makefile and can provide your
> own architecture version. Or should we make the no-autoprobing
> choice generic perhaps?

UML could also use that since it doesn't need autoprobe.c strictly
speaking.

Re: New consolidate irqs vs . probe_irq_*()

[Russell King]

On Wed, Oct 20, 2004 at 06:42:26PM +1000, Benjamin Herrenschmidt wrote:
> I really don't want to mess with that racy mecanism that makes sense for
> ISA only afaik, and it seems some drivers are trying to use it now that
> it's there (/me looks toward yenta_socket) and I'm afraid of the
> consequences since I cannot see how that thing can work properly in the
> first place ;)

yenta_socket has always used it.  Its rather fundamental to the way
that the PCMCIA core has worked for the last I don't know how many
years.

Nothing new.  Maybe something in PPC64 land broke recently?

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: New consolidate irqs vs . probe_irq_*()

[Benjamin Herrenschmidt]

> yenta_socket has always used it.  Its rather fundamental to the way
> that the PCMCIA core has worked for the last I don't know how many
> years.
> 
> Nothing new.  Maybe something in PPC64 land broke recently?

No, what happened is that until the big irq unification, ppc and ppc64
probe_* were no-ops. Probing of "ISA" irqs is a big no-no on most non
x86 architectures.

Ben.

Re: New consolidate irqs vs . probe_irq_*()

[Russell King]

On Wed, Oct 20, 2004 at 08:50:56PM +1000, Benjamin Herrenschmidt wrote:
> > yenta_socket has always used it.  Its rather fundamental to the way
> > that the PCMCIA core has worked for the last I don't know how many
> > years.
> > 
> > Nothing new.  Maybe something in PPC64 land broke recently?
> 
> No, what happened is that until the big irq unification, ppc and ppc64
> probe_* were no-ops. Probing of "ISA" irqs is a big no-no on most non
> x86 architectures.

Well, I've no plans to rewrite that bit of PCMCIA anytime soon,
especially as my time is very precious over the next two months
or so.

Remember that PCMCIA effectively has its own IRQ router which requires
the PCMCIA code to know which IRQs are physically connected and which
aren't.  Unfortunately, there's no way to get that information as far
as I know except by the published method in the code.

So even if I had time to look at this, I doubt anything would change.
I think it's a necessary evil.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: New consolidate irqs vs . probe_irq_*()

[Benjamin Herrenschmidt]

On Wed, 2004-10-20 at 21:01, Russell King wrote:
> On Wed, Oct 20, 2004 at 08:50:56PM +1000, Benjamin Herrenschmidt wrote:
> > > yenta_socket has always used it.  Its rather fundamental to the way
> > > that the PCMCIA core has worked for the last I don't know how many
> > > years.
> > > 
> > > Nothing new.  Maybe something in PPC64 land broke recently?
> > 
> > No, what happened is that until the big irq unification, ppc and ppc64
> > probe_* were no-ops. Probing of "ISA" irqs is a big no-no on most non
> > x86 architectures.
> 
> Well, I've no plans to rewrite that bit of PCMCIA anytime soon,
> especially as my time is very precious over the next two months
> or so.

I'm not asking you to do so :)

> Remember that PCMCIA effectively has its own IRQ router which requires
> the PCMCIA code to know which IRQs are physically connected and which
> aren't.  Unfortunately, there's no way to get that information as far
> as I know except by the published method in the code.

It's fine if the probe_* thing does nothing, though we could imagine a
specific arch callback at one point...

> So even if I had time to look at this, I doubt anything would change.
> I think it's a necessary evil.
-- 
Benjamin Herrenschmidt <benh@kernel.crashing.org>

Re: New consolidate irqs vs . probe_irq_*()

[Paul Mackerras]

Russell King writes:

> Remember that PCMCIA effectively has its own IRQ router which requires
> the PCMCIA code to know which IRQs are physically connected and which
> aren't.  Unfortunately, there's no way to get that information as far
> as I know except by the published method in the code.

On my powerbook, the pcmcia/cardbus controller has one interrupt,
which is used both for card status changes and for card functional
interrupts.  It doesn't have an ISA bus and it doesn't have an 8259
interrupt controller, and interrupts 0-15 aren't anything like what
they might be on a PC.  This is why (as Ben says) there is no point
probing for interrupts, and why on ppc (or at least on powermacs) the
probe functions are no-ops.

Paul.

Re: New consolidate irqs vs . probe_irq_*()

[Russell King]

On Wed, Oct 20, 2004 at 09:53:33PM +1000, Paul Mackerras wrote:
> Russell King writes:
> 
> > Remember that PCMCIA effectively has its own IRQ router which requires
> > the PCMCIA code to know which IRQs are physically connected and which
> > aren't.  Unfortunately, there's no way to get that information as far
> > as I know except by the published method in the code.
> 
> On my powerbook, the pcmcia/cardbus controller has one interrupt,
> which is used both for card status changes and for card functional
> interrupts.  It doesn't have an ISA bus and it doesn't have an 8259
> interrupt controller, and interrupts 0-15 aren't anything like what
> they might be on a PC.  This is why (as Ben says) there is no point
> probing for interrupts, and why on ppc (or at least on powermacs) the
> probe functions are no-ops.

Correct - and Ben's comments seem to imply that yenta is wrong for doing
so.  I'm making the point that it isn't.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core