2004-09-08 12:08:59

by Ingo Molnar

[permalink] [raw]
Subject: [patch] generic-hardirqs.patch, 2.6.9-rc1-bk14


the attached patch moves generic hardirq handling bits to
kernel/hardirq.c. It is not a replacement for any of the existing IRQ
functions, so architectures can use their existing hardirq code in an
unmodified form. It is a library of generic functions that an
architecture can make use of optionally.

I've fully converted x86's irq.c to use the new functions, and Scott
Wood has done the same for ppc/ppc64 as well. (the arch-ppc* changes are
not included in this patch because i couldnt test them myself in the
current port of this patch - but the generic bits were tested on ppc.)

i have test-compiled and test-booted the patch on x86 and x64, on SMP &&
PREEMPT and !SMP && !PREEMPT kernels. x64 needed only a single change (a
setup_irq() prototype) to work fine, which makes me believe that the
patch will not break other architectures either.

(a more complex version of this patch has been tested for weeks as part
of the voluntary-preempt patches as well.)

Ingo


Attachments:
(No filename) (973.00 B)
generic-hardirqs.patch (31.24 kB)
Download all attachments

2004-09-08 12:29:45

by La Monte H.P. Yarroll

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs.patch, 2.6.9-rc1-bk14

Ingo Molnar wrote:

>the attached patch moves generic hardirq handling bits to
>kernel/hardirq.c. It is not a replacement for any of the existing IRQ
>functions, so architectures can use their existing hardirq code in an
>unmodified form. It is a library of generic functions that an
>architecture can make use of optionally.
>
>I've fully converted x86's irq.c to use the new functions, and Scott
>Wood has done the same for ppc/ppc64 as well. (the arch-ppc* changes are
>not included in this patch because i couldnt test them myself in the
>current port of this patch - but the generic bits were tested on ppc.)
>
>

In the interests of full provinence, the TimeSys patches are based on
work by Andrey Panin.

>i have test-compiled and test-booted the patch on x86 and x64, on SMP &&
>PREEMPT and !SMP && !PREEMPT kernels. x64 needed only a single change (a
>setup_irq() prototype) to work fine, which makes me believe that the
>patch will not break other architectures either.
>
>(a more complex version of this patch has been tested for weeks as part
>of the voluntary-preempt patches as well.)
>
> Ingo
>
>
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell's sig

2004-09-08 12:36:21

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs.patch, 2.6.9-rc1-bk14

On Wed, Sep 08, 2004 at 02:06:13PM +0200, Ingo Molnar wrote:
>
> the attached patch moves generic hardirq handling bits to
> kernel/hardirq.c. It is not a replacement for any of the existing IRQ
> functions, so architectures can use their existing hardirq code in an
> unmodified form. It is a library of generic functions that an
> architecture can make use of optionally.
>
> I've fully converted x86's irq.c to use the new functions, and Scott
> Wood has done the same for ppc/ppc64 as well. (the arch-ppc* changes are
> not included in this patch because i couldnt test them myself in the
> current port of this patch - but the generic bits were tested on ppc.)
>
> i have test-compiled and test-booted the patch on x86 and x64, on SMP &&
> PREEMPT and !SMP && !PREEMPT kernels. x64 needed only a single change (a
> setup_irq() prototype) to work fine, which makes me believe that the
> patch will not break other architectures either.
>
> (a more complex version of this patch has been tested for weeks as part
> of the voluntary-preempt patches as well.)


> +++ linux/include/asm-x86_64/hardirq.h
> @@ -25,8 +25,8 @@
> * - ( bit 26 is the PREEMPT_ACTIVE flag. )
> *
> * PREEMPT_MASK: 0x000000ff
> - * HARDIRQ_MASK: 0x0000ff00
> - * SOFTIRQ_MASK: 0x00ff0000
> + * SOFTIRQ_MASK: 0x0000ff00
> + * HARDIRQ_MASK: 0x00ff0000
> */
>
> #define PREEMPT_BITS 8
> @@ -99,4 +99,6 @@ do { \
> extern void synchronize_irq(unsigned int irq);
> #endif /* CONFIG_SMP */
>
> +extern int setup_irq(unsigned int irq, struct irqaction * new);

This doesn't apply anymore because most of <asm/hardirq.h> moved
to linux/hardirq.h in -mm and a the patch is on it's way to Linus.

> @@ -71,10 +74,21 @@ extern irq_desc_t irq_desc [NR_IRQS];
>
> #include <asm/hw_irq.h> /* the arch dependent stuff */
>
> -extern int setup_irq(unsigned int , struct irqaction * );
> +
> +extern asmlinkage int generic_handle_IRQ_event(unsigned int irq, struct pt_regs *regs, struct irqaction *action);
> +extern void generic_synchronize_irq(unsigned int irq);
> +extern int generic_setup_irq(unsigned int irq, struct irqaction * new);
> +extern void generic_free_irq(unsigned int irq, void *dev_id);
> +extern void generic_disable_irq_nosync(unsigned int irq);
> +extern void generic_disable_irq(unsigned int irq);
> +extern void generic_enable_irq(unsigned int irq);
> +extern void generic_note_interrupt(int irq, irq_desc_t *desc, int action_ret);

Please don't introduce the generic_ names just to have every arch (in your
previous patches, that is) provide a wrapper with normal names again.


> --- linux/kernel/Makefile.orig
> +++ linux/kernel/Makefile
> @@ -3,7 +3,7 @@
> #
>
> obj-y = sched.o fork.o exec_domain.o panic.o printk.o profile.o \
> - exit.o itimer.o time.o softirq.o resource.o \
> + exit.o itimer.o time.o softirq.o hardirq.o resource.o \

And make hardirq.o dependent on some symbols the architectures set.
Else arches that don't use it carry tons of useless baggage around (and
in fact I'm pretty sure it wouldn't even compie for many)

2004-09-08 12:44:32

by Ingo Molnar

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs.patch, 2.6.9-rc1-bk14


* Christoph Hellwig <[email protected]> wrote:

> > @@ -99,4 +99,6 @@ do { \
> > extern void synchronize_irq(unsigned int irq);
> > #endif /* CONFIG_SMP */
> >
> > +extern int setup_irq(unsigned int irq, struct irqaction * new);
>
> This doesn't apply anymore because most of <asm/hardirq.h> moved to
> linux/hardirq.h in -mm and a the patch is on it's way to Linus.

(yeah. not a big issue - can happen in any order. this patch is against
BK-curr.)

> > +extern asmlinkage int generic_handle_IRQ_event(unsigned int irq, struct pt_regs *regs, struct irqaction *action);
> > +extern void generic_synchronize_irq(unsigned int irq);
> > +extern int generic_setup_irq(unsigned int irq, struct irqaction * new);
> > +extern void generic_free_irq(unsigned int irq, void *dev_id);
> > +extern void generic_disable_irq_nosync(unsigned int irq);
> > +extern void generic_disable_irq(unsigned int irq);
> > +extern void generic_enable_irq(unsigned int irq);
> > +extern void generic_note_interrupt(int irq, irq_desc_t *desc, int action_ret);
>
> Please don't introduce the generic_ names just to have every arch (in
> your previous patches, that is) provide a wrapper with normal names
> again.

some of the architectures dont want to (and cannot) use the generic
functions for one reason or another. So the proper approach i believe is
to provide these generic functions the architectures can plug in. I can
do an asm-generic/hardirq.h that adds all the definitions, for
architectures that dont need any special IRQ logic.

> > obj-y = sched.o fork.o exec_domain.o panic.o printk.o profile.o \
> > - exit.o itimer.o time.o softirq.o resource.o \
> > + exit.o itimer.o time.o softirq.o hardirq.o resource.o \
>
> And make hardirq.o dependent on some symbols the architectures set.
> Else arches that don't use it carry tons of useless baggage around
> (and in fact I'm pretty sure it wouldn't even compie for many)

it compiles fine on x86, x64, ppc and ppc64. Why do you think it wont
compile on others?

wrt. unused generic functions - why dont we drop them link-time?

Ingo

2004-09-08 12:57:16

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs.patch, 2.6.9-rc1-bk14

On Wed, Sep 08, 2004 at 02:45:47PM +0200, Ingo Molnar wrote:
> some of the architectures dont want to (and cannot) use the generic
> functions for one reason or another. So the proper approach i believe is
> to provide these generic functions the architectures can plug in. I can
> do an asm-generic/hardirq.h that adds all the definitions, for
> architectures that dont need any special IRQ logic.

Some architectures definitly can't use it. That's why the prototypes for
them arch in arch-headers. No need to introduce totally useless wrappers.
The asm-generic one sounds like a good idea, but I'd wait with that one
until the consolidation is mostly finished, aka all architectures that
currently use more or less a copy of the i386 irq.c are migrated over so
we can see it's scope.

> > > obj-y = sched.o fork.o exec_domain.o panic.o printk.o profile.o \
> > > - exit.o itimer.o time.o softirq.o resource.o \
> > > + exit.o itimer.o time.o softirq.o hardirq.o resource.o \
> >
> > And make hardirq.o dependent on some symbols the architectures set.
> > Else arches that don't use it carry tons of useless baggage around
> > (and in fact I'm pretty sure it wouldn't even compie for many)
>
> it compiles fine on x86, x64, ppc and ppc64. Why do you think it wont
> compile on others?

linux/irq.h is despite it's name _not_ a public header but a misnamed
asm-generic/hw_irq.h. There's quite a few architectures with a completely
different interrupt architecture and for tose it won't compile.

> wrt. unused generic functions - why dont we drop them link-time?

make explicit what you can do easily instead of relying on the compiler.
It allows to get rid of your horrible generic_ hacks, cuts down compile
time and makes explicit to anyone looking at the code and Kconfig which
architectures use this.

2004-09-08 13:05:36

by William Lee Irwin III

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs.patch, 2.6.9-rc1-bk14

* Christoph Hellwig <[email protected]> wrote:
>> And make hardirq.o dependent on some symbols the architectures set.
>> Else arches that don't use it carry tons of useless baggage around
>> (and in fact I'm pretty sure it wouldn't even compie for many)

On Wed, Sep 08, 2004 at 02:45:47PM +0200, Ingo Molnar wrote:
> it compiles fine on x86, x64, ppc and ppc64. Why do you think it wont
> compile on others?
> wrt. unused generic functions - why dont we drop them link-time?

It may be time for a __weak define to abbreviate __attribute__((weak));
we seem to use it in enough places.


-- wli

2004-09-08 13:09:13

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs.patch, 2.6.9-rc1-bk14


> wrt. unused generic functions - why dont we drop them link-time?

actually if nothing uses ANY function of a .o file then yes the entire
.o gets dropped.

(we should make the kernel use -ffunction-sections some day to make it
on a per function instead of on a per .o file basis)


Attachments:
signature.asc (189.00 B)
This is a digitally signed message part

2004-09-08 13:09:12

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs.patch, 2.6.9-rc1-bk14

On Wed, Sep 08, 2004 at 05:57:55AM -0700, William Lee Irwin III wrote:
> * Christoph Hellwig <[email protected]> wrote:
> >> And make hardirq.o dependent on some symbols the architectures set.
> >> Else arches that don't use it carry tons of useless baggage around
> >> (and in fact I'm pretty sure it wouldn't even compie for many)
>
> On Wed, Sep 08, 2004 at 02:45:47PM +0200, Ingo Molnar wrote:
> > it compiles fine on x86, x64, ppc and ppc64. Why do you think it wont
> > compile on others?
> > wrt. unused generic functions - why dont we drop them link-time?
>
> It may be time for a __weak define to abbreviate __attribute__((weak));
> we seem to use it in enough places.

Personally I'm extremly unhappy with that week model for things like
this. There's no reason why architectures could implement irq handling
as inlines. Or in case of s390 not at all.

2004-09-08 13:17:55

by Ingo Molnar

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs.patch, 2.6.9-rc1-bk14


* Christoph Hellwig <[email protected]> wrote:

> > i disagree. It's the same as the VFS model: we have generic_block_bmap()
> > which a filesystem might or might not make use of. It's still around
> > even if no filesystem makes use of it but do we care? I'd prefer fixing
> > our linking logic to get rid of unused functions than complicating code
> > and the architecture with conditionals.
>
> Completley different model. VFS supports lots of filesystem
> implementation with one interface. IRQ code is a a single
> implementation for each architecture.

not at all different model. 90% of the important drivers (no,
drivers/s390 doesnt count) are shared between multiple architectures
using the same interface: request_irq()/free_irq() and a handler with an
enumerated irq vector.

> > is there any architecture that cannot make use of kernel/hardirq.c _at
> > all_?
>
> s390 doesn't need it at all because it doesn't have the concept of hardirqs.
>
> At least arm{,26}, m68k{,nommu} and parisc and sparc{,64} use extremly
> different models for irq handling

it could be a bit like nommu - a noirq model.

i agree with enabling an architecture to exclude _all_ of hardirq.c, but
specifying per-function is excessive - if an architecture can make use
of some of them then weak symbols will get rid of the rest.

Ingo

2004-09-08 13:17:19

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs.patch, 2.6.9-rc1-bk14

On Wed, Sep 08, 2004 at 03:05:52PM +0200, Ingo Molnar wrote:
> i disagree. It's the same as the VFS model: we have generic_block_bmap()
> which a filesystem might or might not make use of. It's still around
> even if no filesystem makes use of it but do we care? I'd prefer fixing
> our linking logic to get rid of unused functions than complicating code
> and the architecture with conditionals.

Completley different model. VFS supports lots of filesystem implementation
with one interface. IRQ code is a a single implementation for each
architecture.

> is there any architecture that cannot make use of kernel/hardirq.c _at
> all_?

s390 doesn't need it at all because it doesn't have the concept of hardirqs.

At least arm{,26}, m68k{,nommu} and parisc and sparc{,64} use extremly
different models for irq handling

2004-09-08 13:21:58

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs.patch, 2.6.9-rc1-bk14

On Wed, Sep 08, 2004 at 03:17:20PM +0200, Ingo Molnar wrote:
> not at all different model. 90% of the important drivers (no,
> drivers/s390 doesnt count) are shared between multiple architectures
> using the same interface: request_irq()/free_irq() and a handler with an
> enumerated irq vector.

Sure, but that's not the level we're talking about. The function we talk
about compare to the vfs_* routines (when looking at the arches with
i386-style generic irq code)(

> > s390 doesn't need it at all because it doesn't have the concept of hardirqs.
> >
> > At least arm{,26}, m68k{,nommu} and parisc and sparc{,64} use extremly
> > different models for irq handling
>
> it could be a bit like nommu - a noirq model.
>
> i agree with enabling an architecture to exclude _all_ of hardirq.c, but
> specifying per-function is excessive - if an architecture can make use
> of some of them then weak symbols will get rid of the rest.

I never wanted to exclude individual functions. But when you look at
arch/*/kernel/irq.c I don't see a reason for doing it at all. It makes
sense to make this an all or nothing switch.

2004-09-08 13:33:11

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs.patch, 2.6.9-rc1-bk14

On Wed, 8 Sep 2004, William Lee Irwin III wrote:

> * Christoph Hellwig <[email protected]> wrote:
> >> And make hardirq.o dependent on some symbols the architectures set.
> >> Else arches that don't use it carry tons of useless baggage around
> >> (and in fact I'm pretty sure it wouldn't even compie for many)
>
> On Wed, Sep 08, 2004 at 02:45:47PM +0200, Ingo Molnar wrote:
> > it compiles fine on x86, x64, ppc and ppc64. Why do you think it wont
> > compile on others?
> > wrt. unused generic functions - why dont we drop them link-time?
>
> It may be time for a __weak define to abbreviate __attribute__((weak));
> we seem to use it in enough places.

Hmm, whenever i've brought up weak functions in a patch it's never well
received. Frankly i prefer it to littering the architectures with similar
functions.

Zwane

2004-09-08 13:29:46

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs.patch, 2.6.9-rc1-bk14

On Wed, Sep 08, 2004 at 08:29:05AM -0400, La Monte H.P. Yarroll wrote:
> In the interests of full provinence, the TimeSys patches are based on
> work by Andrey Panin.

Btw, Andrey's patches got all the things right I complained about :)

2004-09-08 13:38:05

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs.patch, 2.6.9-rc1-bk14

On Wed, Sep 08, 2004 at 09:34:03AM -0400, Zwane Mwaikambo wrote:
> Hmm, whenever i've brought up weak functions in a patch it's never well
> received. Frankly i prefer it to littering the architectures with similar
> functions.

That's what we have asm-generic for.

2004-09-08 13:48:28

by Ingo Molnar

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs.patch, 2.6.9-rc1-bk14


* Christoph Hellwig <[email protected]> wrote:

> On Wed, Sep 08, 2004 at 08:29:05AM -0400, La Monte H.P. Yarroll wrote:
> > In the interests of full provinence, the TimeSys patches are based on
> > work by Andrey Panin.
>
> Btw, Andrey's patches got all the things right I complained about :)

do you mean the irq and softirq threading patches? Unfortunately, while
they were rather clean the generic bits were also quite substantially
broken semantically on SMP so while i carried them for a while in the
voluntary-preempt patch i had to drop and redo them all from scratch.
Scott Wood then sent ppc bits which might be based on the other code.
Anyway, it seems everyone wants roughly the same thing, it only has to
actually happen now ;-) Once we have kernel/hardirq.c then the
irq-threading patches become nicely local and maintainable.

Ingo

2004-09-08 13:45:48

by Ingo Molnar

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs.patch, 2.6.9-rc1-bk14


* Christoph Hellwig <[email protected]> wrote:

> > > s390 doesn't need it at all because it doesn't have the concept of hardirqs.
> > >
> > > At least arm{,26}, m68k{,nommu} and parisc and sparc{,64} use extremly
> > > different models for irq handling
> >
> > it could be a bit like nommu - a noirq model.
> >
> > i agree with enabling an architecture to exclude _all_ of hardirq.c, but
> > specifying per-function is excessive - if an architecture can make use
> > of some of them then weak symbols will get rid of the rest.
>
> I never wanted to exclude individual functions. But when you look at
> arch/*/kernel/irq.c I don't see a reason for doing it at all. It
> makes sense to make this an all or nothing switch.

ok, agreed. New patch attached. An architecture has to set
GENERIC_HARDIRQS in Kconfig to get it compiled by default. x86 does this
for now.

Ingo


Attachments:
(No filename) (879.00 B)
generic-hardirqs-2.6.9-rc1-bk14-A5 (30.33 kB)
Download all attachments

2004-09-08 13:52:35

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs.patch, 2.6.9-rc1-bk14

On Wed, Sep 08, 2004 at 03:40:32PM +0200, Ingo Molnar wrote:
>
> * Christoph Hellwig <[email protected]> wrote:
>
> > On Wed, Sep 08, 2004 at 08:29:05AM -0400, La Monte H.P. Yarroll wrote:
> > > In the interests of full provinence, the TimeSys patches are based on
> > > work by Andrey Panin.
> >
> > Btw, Andrey's patches got all the things right I complained about :)
>
> do you mean the irq and softirq threading patches?

No, irq.c consolidation.

2004-09-08 13:57:13

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs.patch, 2.6.9-rc1-bk14

On Wed, 8 Sep 2004, Christoph Hellwig wrote:

> On Wed, Sep 08, 2004 at 09:34:03AM -0400, Zwane Mwaikambo wrote:
> > Hmm, whenever i've brought up weak functions in a patch it's never well
> > received. Frankly i prefer it to littering the architectures with similar
> > functions.
>
> That's what we have asm-generic for.

So you have an inline function in a header and include it everywhere? How
is that better?

Zwane

2004-09-08 13:29:45

by William Lee Irwin III

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs.patch, 2.6.9-rc1-bk14

On Wed, Sep 08, 2004 at 05:57:55AM -0700, William Lee Irwin III wrote:
>> It may be time for a __weak define to abbreviate __attribute__((weak));
>> we seem to use it in enough places.

On Wed, Sep 08, 2004 at 02:01:46PM +0100, Christoph Hellwig wrote:
> Personally I'm extremly unhappy with that week model for things like
> this. There's no reason why architectures could implement irq handling
> as inlines. Or in case of s390 not at all.

If there's a question of "not at all" that probably rules out weak
symbols as a method of handling it.


-- wli

2004-09-08 14:17:41

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs.patch, 2.6.9-rc1-bk14

On Wed, 2004-09-08 at 15:47, Zwane Mwaikambo wrote:
> On Wed, 8 Sep 2004, Christoph Hellwig wrote:
>
> > On Wed, Sep 08, 2004 at 09:34:03AM -0400, Zwane Mwaikambo wrote:
> > > Hmm, whenever i've brought up weak functions in a patch it's never well
> > > received. Frankly i prefer it to littering the architectures with similar
> > > functions.
> >
> > That's what we have asm-generic for.
>
> So you have an inline function in a header and include it everywhere? How
> is that better?

the thing is, now with the config option, you don't need the generic_
wrapper thing, just use the full real name. And the prototypes can be
generic for the arch's that want it.


Attachments:
signature.asc (189.00 B)
This is a digitally signed message part

2004-09-08 13:26:38

by Ingo Molnar

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs.patch, 2.6.9-rc1-bk14


* Christoph Hellwig <[email protected]> wrote:

> Personally I'm extremly unhappy with that week model for things like
> this. There's no reason why architectures could implement irq
> handling as inlines. Or in case of s390 not at all.

s390 is a special case i agree - no IRQ handling is a special case. We
can exclude kernel/hardirq.o on s390 and all virtual-guest platforms.

Ingo

2004-09-08 13:26:37

by Ingo Molnar

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs.patch, 2.6.9-rc1-bk14


* Christoph Hellwig <[email protected]> wrote:

> > wrt. unused generic functions - why dont we drop them link-time?
>
> make explicit what you can do easily instead of relying on the
> compiler. It allows to get rid of your horrible generic_ hacks, cuts
> down compile time and makes explicit to anyone looking at the code and
> Kconfig which architectures use this.

i disagree. It's the same as the VFS model: we have generic_block_bmap()
which a filesystem might or might not make use of. It's still around
even if no filesystem makes use of it but do we care? I'd prefer fixing
our linking logic to get rid of unused functions than complicating code
and the architecture with conditionals.

is there any architecture that cannot make use of kernel/hardirq.c _at
all_?

Ingo

2004-09-08 18:26:45

by Ingo Molnar

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs.patch, 2.6.9-rc1-bk14


* Arjan van de Ven <[email protected]> wrote:

> the thing is, now with the config option, you don't need the generic_
> wrapper thing, just use the full real name. And the prototypes can be
> generic for the arch's that want it.

latest patch attached - this should compile on every architecture. It's
basically what Christoph suggested first time around :-) Compiles/boots
on x86. Any other observations?

Ingo


Attachments:
(No filename) (415.00 B)
generic-hardirqs-2.6.9-rc1-bk14-A6 (27.65 kB)
Download all attachments

2004-09-08 18:38:14

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs.patch, 2.6.9-rc1-bk14

On Wed, 8 Sep 2004, Ingo Molnar wrote:

> latest patch attached - this should compile on every architecture. It's
> basically what Christoph suggested first time around :-) Compiles/boots
> on x86. Any other observations?

Apart from very nice? =) It's needed doing for a long time now.

Zwane

2004-09-08 21:16:00

by Ingo Molnar

[permalink] [raw]
Subject: [patch] generic-hardirqs-2.6.9-rc1-mm4.patch


* Ingo Molnar <[email protected]> wrote:

> latest patch attached - this should compile on every architecture.
> It's basically what Christoph suggested first time around :-)
> Compiles/boots on x86. Any other observations?

i've attached generic-hardirqs-2.6.9-rc1-mm4.patch which is a merge
against -mm4. x86 and x64 compiles & boots fine. Since there are zero
changes to non-x86 architectures it should build fine on all platforms.

Ingo


Attachments:
(No filename) (439.00 B)
generic-hardirqs-2.6.9-rc1-mm4.patch (27.26 kB)
Download all attachments

2004-09-09 17:01:28

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs-2.6.9-rc1-mm4.patch

On Wed, Sep 08, 2004 at 11:14:15PM +0200, Ingo Molnar wrote:
>
> * Ingo Molnar <[email protected]> wrote:
>
> > latest patch attached - this should compile on every architecture.
> > It's basically what Christoph suggested first time around :-)
> > Compiles/boots on x86. Any other observations?
>
> i've attached generic-hardirqs-2.6.9-rc1-mm4.patch which is a merge
> against -mm4. x86 and x64 compiles & boots fine. Since there are zero
> changes to non-x86 architectures it should build fine on all platforms.

Looks good to me. I no one else beats me I'll look into converting
ppc32/64 this weekend.

2004-09-09 17:32:57

by Ingo Molnar

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs-2.6.9-rc1-mm4.patch


* Christoph Hellwig <[email protected]> wrote:

> > > latest patch attached - this should compile on every architecture.
> > > It's basically what Christoph suggested first time around :-)
> > > Compiles/boots on x86. Any other observations?
> >
> > i've attached generic-hardirqs-2.6.9-rc1-mm4.patch which is a merge
> > against -mm4. x86 and x64 compiles & boots fine. Since there are zero
> > changes to non-x86 architectures it should build fine on all platforms.
>
> Looks good to me. I no one else beats me I'll look into converting
> ppc32/64 this weekend.

you can find a pretty good approximation done by Scott Wood (and Andrey
Panin?) in the ppc/ppc64 portion of the VP patches:

http://redhat.com/~mingo/voluntary-preempt/voluntary-preempt-2.6.9-rc1-bk12-S0

basically you only have to zap some of the irq-threading changes such as
calls to redirect_hardirq(), do a s/generic_// and zap the PIC changes
(these are done for redirection too). Scott has tested those changes so
kernel/hardirq.c should work pretty well with ppc/ppc64.

Ingo

2004-09-09 18:00:24

by William Lee Irwin III

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs-2.6.9-rc1-mm4.patch

On Thu, Sep 09, 2004 at 10:53:14AM -0700, William Lee Irwin III wrote:
>> By any chance can the generic code be made not to be reliant on
>> irq_desc[] and/or irq_desc[] being an array?

On Thu, Sep 09, 2004 at 07:54:41PM +0200, Arjan van de Ven wrote:
> I would say one step at a time.
> First extract out the code that most arch's share already
> only THEN start working on seeing if the few remaining ones can be moved in,
> and if other cleanups are appropriate

I suppose that assumption can be swept for afterward.


-- wli

2004-09-09 18:00:27

by William Lee Irwin III

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs-2.6.9-rc1-mm4.patch

On Thu, Sep 09, 2004 at 07:24:01PM +0200, Ingo Molnar wrote:
> you can find a pretty good approximation done by Scott Wood (and Andrey
> Panin?) in the ppc/ppc64 portion of the VP patches:
> http://redhat.com/~mingo/voluntary-preempt/voluntary-preempt-2.6.9-rc1-bk12-S0
> basically you only have to zap some of the irq-threading changes such as
> calls to redirect_hardirq(), do a s/generic_// and zap the PIC changes
> (these are done for redirection too). Scott has tested those changes so
> kernel/hardirq.c should work pretty well with ppc/ppc64.

By any chance can the generic code be made not to be reliant on
irq_desc[] and/or irq_desc[] being an array?


-- wli

2004-09-09 18:00:25

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs-2.6.9-rc1-mm4.patch


On Thu, Sep 09, 2004 at 10:53:14AM -0700, William Lee Irwin III wrote:
> On Thu, Sep 09, 2004 at 07:24:01PM +0200, Ingo Molnar wrote:
> > you can find a pretty good approximation done by Scott Wood (and Andrey
> > Panin?) in the ppc/ppc64 portion of the VP patches:
> > http://redhat.com/~mingo/voluntary-preempt/voluntary-preempt-2.6.9-rc1-bk12-S0
> > basically you only have to zap some of the irq-threading changes such as
> > calls to redirect_hardirq(), do a s/generic_// and zap the PIC changes
> > (these are done for redirection too). Scott has tested those changes so
> > kernel/hardirq.c should work pretty well with ppc/ppc64.
>
> By any chance can the generic code be made not to be reliant on
> irq_desc[] and/or irq_desc[] being an array?

I would say one step at a time.
First extract out the code that most arch's share already
only THEN start working on seeing if the few remaining ones can be moved in,
and if other cleanups are appropriate


Attachments:
(No filename) (962.00 B)
(No filename) (189.00 B)
Download all attachments

2004-09-09 18:29:55

by Scott Wood

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs.patch, 2.6.9-rc1-bk14

On Wed, Sep 08, 2004 at 02:12:17PM +0100, Christoph Hellwig wrote:
> On Wed, Sep 08, 2004 at 03:05:52PM +0200, Ingo Molnar wrote:
> > is there any architecture that cannot make use of kernel/hardirq.c _at
> > all_?
>
> s390 doesn't need it at all because it doesn't have the concept of hardirqs.
>
> At least arm{,26}, m68k{,nommu} and parisc and sparc{,64} use extremly
> different models for irq handling

They're not irreparably different, though (at least, not all of
them); we had generic IRQ code (with threads) running in our 2.4
kernel on arm and sparc64.

-Scott

2004-09-09 20:18:14

by Russell King

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs-2.6.9-rc1-mm4.patch

On Thu, Sep 09, 2004 at 07:54:41PM +0200, Arjan van de Ven wrote:
>
> On Thu, Sep 09, 2004 at 10:53:14AM -0700, William Lee Irwin III wrote:
> > On Thu, Sep 09, 2004 at 07:24:01PM +0200, Ingo Molnar wrote:
> > > you can find a pretty good approximation done by Scott Wood (and Andrey
> > > Panin?) in the ppc/ppc64 portion of the VP patches:
> > > http://redhat.com/~mingo/voluntary-preempt/voluntary-preempt-2.6.9-rc1-bk12-S0
> > > basically you only have to zap some of the irq-threading changes such as
> > > calls to redirect_hardirq(), do a s/generic_// and zap the PIC changes
> > > (these are done for redirection too). Scott has tested those changes so
> > > kernel/hardirq.c should work pretty well with ppc/ppc64.
> >
> > By any chance can the generic code be made not to be reliant on
> > irq_desc[] and/or irq_desc[] being an array?
>
> I would say one step at a time.
> First extract out the code that most arch's share already
> only THEN start working on seeing if the few remaining ones can be moved in,
> and if other cleanups are appropriate

If it uses irq_desc then ARM won't use it. irq_desc is part of the
far-too-restrictive x86 IRQ handlign code which is unsuitable for
ARM platforms.

Sorry, try again.

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

2004-09-09 20:29:28

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs-2.6.9-rc1-mm4.patch

On Wednesday 08 of September 2004 23:14, Ingo Molnar wrote:
>
> * Ingo Molnar <[email protected]> wrote:
>
> > latest patch attached - this should compile on every architecture.
> > It's basically what Christoph suggested first time around :-)
> > Compiles/boots on x86. Any other observations?
>
> i've attached generic-hardirqs-2.6.9-rc1-mm4.patch which is a merge
> against -mm4. x86 and x64 compiles & boots fine. Since there are zero
> changes to non-x86 architectures it should build fine on all platforms.

I've got this trace (on x86-64):

general protection fault: 0000 [1] PREEMPT
CPU 0
Modules linked in: usbserial parport_pc lp parport joydev sg st sd_mod sr_mod
scsi_mod snd_seq_oss snd_seq_midi_evend
Pid: 694, comm: kjournald Not tainted 2.6.9-rc1-mm4
RIP: 0010:[<ffffffff802ac605>]
<ffffffff802ac605>{__journal_clean_checkpoint_list+389}
RSP: 0018:000001001f589b08 EFLAGS: 00010202
RAX: 0000000000000000 RBX: 6b6b6b6b6b6b6b6b RCX: 0000000000000000
RDX: 0000010001b2c4d8 RSI: 000001001fd2e150 RDI: 000001001fbab628
RBP: 00000100110c0708 R08: 000001001fd2e050 R09: 00000100110c0708
R10: 0000000000000000 R11: 0000000000000000 R12: 00000100110c0708
R13: 000001000b4345c8 R14: 000000000000007f R15: 0000010001ae8d70
FS: 0000002a9afefa80(0000) GS:ffffffff805f3580(0000) knlGS:0000000000000000
CS: 0010 DS: 0018 ES: 0018 CR0: 000000008005003b
CR2: 0000002a95654000 CR3: 0000000000101000 CR4: 00000000000006e0
Process kjournald (pid: 694, threadinfo 000001001f588000, task
0000010001ad9270)
Stack: 000001001f589b48 000001001fd2bc58 0000010001b2c4d8 0000010001b2c4d8
0000000000000001 0000000000000000 0000010001b2c4d8 000001000bc1f910
0000000000000000 ffffffff802a9039
Call Trace:<ffffffff802a9039>{journal_commit_transaction+2329}
<ffffffff8042f730>{thread_return+41} <ffffffff802b3465>{kjournald+725}
<ffffffff8015ece0>{autoremove_wake_function+0}
<ffffffff8015ece0>{autoremove_wake_function+0}
<ffffffff802b3a30>{commit_timeout+0} <ffffffff80111afb>{child_rip+8}
<ffffffff802b3190>{kjournald+0} <ffffffff80111af3>{child_rip+0}


Code: 48 8b 43 48 49 89 45 48 48 8b 54 24 10 4c 8b aa b8 00 00 00
RIP <ffffffff802ac605>{__journal_clean_checkpoint_list+389} RSP
<000001001f589b08>

with the patch applied. I don't know if the patch is the reason, but I
haven't got anything like that without it (please let me know if you
need .config etc.).

Greets,
RJW

--
- Would you tell me, please, which way I ought to go from here?
- That depends a good deal on where you want to get to.
-- Lewis Carroll "Alice's Adventures in Wonderland"

2004-09-09 20:43:37

by Andrew Morton

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs-2.6.9-rc1-mm4.patch

"Rafael J. Wysocki" <[email protected]> wrote:
>
> I've got this trace (on x86-64):
>
> general protection fault: 0000 [1] PREEMPT
> CPU 0
> Modules linked in: usbserial parport_pc lp parport joydev sg st sd_mod sr_mod
> scsi_mod snd_seq_oss snd_seq_midi_evend
> Pid: 694, comm: kjournald Not tainted 2.6.9-rc1-mm4
> RIP: 0010:[<ffffffff802ac605>]
> <ffffffff802ac605>{__journal_clean_checkpoint_list+389}

You should revert
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.9-rc1/2.6.9-rc1-mm4/broken-out/journal_clean_checkpoint_list-latency-fix.patch
- it seems to be sick.

2004-09-09 20:49:37

by Ingo Molnar

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs-2.6.9-rc1-mm4.patch


* Andrew Morton <[email protected]> wrote:

> "Rafael J. Wysocki" <[email protected]> wrote:
> >
> > I've got this trace (on x86-64):
> >
> > general protection fault: 0000 [1] PREEMPT
> > CPU 0
> > Modules linked in: usbserial parport_pc lp parport joydev sg st sd_mod sr_mod
> > scsi_mod snd_seq_oss snd_seq_midi_evend
> > Pid: 694, comm: kjournald Not tainted 2.6.9-rc1-mm4
> > RIP: 0010:[<ffffffff802ac605>]
> > <ffffffff802ac605>{__journal_clean_checkpoint_list+389}
>
> You should revert
> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.9-rc1/2.6.9-rc1-mm4/broken-out/journal_clean_checkpoint_list-latency-fix.patch
> - it seems to be sick.

the variant in the VP patch (for this latency) is pretty stable. Will
post splitups later.

Ingo

2004-09-09 20:57:28

by Scott Wood

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs-2.6.9-rc1-mm4.patch

On Thu, Sep 09, 2004 at 09:10:38PM +0100, Russell King wrote:
> If it uses irq_desc then ARM won't use it. irq_desc is part of the
> far-too-restrictive x86 IRQ handlign code which is unsuitable for
> ARM platforms.

What does ARM need that irq_desc doesn't provide, and which could not
be added? IMHO, it'd be better to fix the generic code than maintain
completely separate implementations.

-Scott

2004-09-09 21:04:09

by Russell King

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs-2.6.9-rc1-mm4.patch

On Thu, Sep 09, 2004 at 04:51:53PM -0400, Scott Wood wrote:
> On Thu, Sep 09, 2004 at 09:10:38PM +0100, Russell King wrote:
> > If it uses irq_desc then ARM won't use it. irq_desc is part of the
> > far-too-restrictive x86 IRQ handlign code which is unsuitable for
> > ARM platforms.
>
> What does ARM need that irq_desc doesn't provide, and which could not
> be added? IMHO, it'd be better to fix the generic code than maintain
> completely separate implementations.

Unfortunately, to deal with this issue would mean breaking off from
planned work, so I'm not going to be in a position to do an element
by element comparison of the structures for some time.

Maybe I can look at it after the weekend though.

However, I think if you just compare:

linux/include/linux/irq.h
linux/include/asm-arm/mach/irq.h

you'll realise how inadequate the "generic" IRQ code is for ARM.

Note that IRQ handling on ARM is a multi-level affair where we have
multiple levels of interrupt controllers which need to be traversed
in software.

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

2004-09-10 05:58:26

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [patch] generic-hardirqs-2.6.9-rc1-mm4.patch


On Thu, Sep 09, 2004 at 10:00:04PM +0100, Russell King wrote:
> you'll realise how inadequate the "generic" IRQ code is for ARM.
>
> Note that IRQ handling on ARM is a multi-level affair where we have
> multiple levels of interrupt controllers which need to be traversed
> in software.

I still propose the plan of action that is to consolidate first all
architectures that CAN share the x86 one, and THEN look at making that more
generic. The reason is simple; right now there are 20-ish copies; after the
initial consolidation there will be 4 or 5. Far easier to work on.


Attachments:
(No filename) (576.00 B)
(No filename) (189.00 B)
Download all attachments