2003-06-26 10:49:21

by Andrey Panin

[permalink] [raw]
Subject: [PATCH][RFC] irq handling code consolidation (common part)

Hi all,

the irq handling consolidation patch returns from the dead !
Now with runaway irq detection code included !

This patch (against 2.5.73) contains common part of it.

Please take a look.

Best regards.

--
Andrey Panin | Linux and UNIX system administrator
[email protected] | PGP key: wwwkeys.pgp.net


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

2003-06-26 10:59:10

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH][RFC] irq handling code consolidation (common part)

> +#ifndef HAVE_ARCH_IRQ_DESC
> +
> +/*
> + * Controller mappings for all interrupt sources:
> + */
> +irq_desc_t irq_desc[NR_IRQS] __cacheline_aligned = {
> + [0 ... NR_IRQS - 1] = {
> + .handler = &no_irq_type,
> + .lock = SPIN_LOCK_UNLOCKED,
> + }
> +};
> +
> +#endif

What about getting rid of that ifdef and having irq_desc always
in arch code? Seems a lot cleaner to me.

> +#if defined(CONFIG_SMP) && !defined(HAVE_ARCH_SYNCRONIZE_IRQ)
> +
> +inline void synchronize_irq(unsigned int irq)
> +{
> + irq_desc_t *desc = irq_desc(irq);
> +
> + /* is there anything to synchronize with? */
> + if (!desc->action)
> + return;
> +
> + while (desc->status & IRQ_INPROGRESS)
> + cpu_relax();
> +}
> +
> +#endif

Hmm, what arch can't use the generic version and why? I really
don't like the HAVE_ARCH_ macros if there's a way around it.

> +#ifndef HAVE_ARCH_IRQ_PROC
> +void register_irq_proc(unsigned int irq);
> +#endif

Again, what arch can't use the generic code?

> +#ifndef HAVE_ARCH_IRQ_PROBE
> +
> +/*
> + * IRQ autodetection code..
> + *
> + * This depends on the fact that any interrupt that
> + * comes in on to an unassigned handler will get stuck
> + * with "IRQ_WAITING" cleared and the interrupt
> + * disabled.
> + */

Which architecture uses it's own version here? Also we might
move this to a separate file as it doesn't make a lot of sense
without CONFIG_ISA

Otherwise it looks fine (of course)! Let's hope we'll get some variant
of it in before 2.6.

2003-06-26 17:45:26

by Anton Blanchard

[permalink] [raw]
Subject: Re: [PATCH][RFC] irq handling code consolidation (common part)


> the irq handling consolidation patch returns from the dead !
> Now with runaway irq detection code included !
>
> This patch (against 2.5.73) contains common part of it.

Great! Well it wasnt dead, I was also keeping it up to date and sending
it on to akpm :)

I have two suggestions that will help in my crusade to kill NR_IRQS.

1. define irq_desc, irq_valid, for_each_irq in include/linux/irq.h if
HAVE_ARCH_IRQ_DESC isnt defined (instead of in each architecture).
Basically I want to start using these macros in a few places and dont
want to break every architecture that hasnt converted to the new scheme.

On the other hand if we decide to move the irq descriptor definition
into each arch as hch suggested, this wont be necessary as all archs
will break anyway :)

2. define irq_atoi that converts an irq into a printable string. We have
a bunch of #ifdef CONFIG_SPARC stuff we can then get rid of, and other
archs can start using it if wanted (eg on ppc64 I can subtract our
software offset so the irqs printed match the hardware)

Anton

2003-06-27 04:19:28

by Andrey Panin

[permalink] [raw]
Subject: Re: [PATCH][RFC] irq handling code consolidation (common part)

On 177, 06 26, 2003 at 12:13:18PM +0100, Christoph Hellwig wrote:
> > +#ifndef HAVE_ARCH_IRQ_DESC
> > +
> > +/*
> > + * Controller mappings for all interrupt sources:
> > + */
> > +irq_desc_t irq_desc[NR_IRQS] __cacheline_aligned = {
> > + [0 ... NR_IRQS - 1] = {
> > + .handler = &no_irq_type,
> > + .lock = SPIN_LOCK_UNLOCKED,
> > + }
> > +};
> > +
> > +#endif
>
> What about getting rid of that ifdef and having irq_desc always
> in arch code? Seems a lot cleaner to me.

So it will be duplicated in allmost every architecture ?

> > +#if defined(CONFIG_SMP) && !defined(HAVE_ARCH_SYNCRONIZE_IRQ)
> > +
> > +inline void synchronize_irq(unsigned int irq)
> > +{
> > + irq_desc_t *desc = irq_desc(irq);
> > +
> > + /* is there anything to synchronize with? */
> > + if (!desc->action)
> > + return;
> > +
> > + while (desc->status & IRQ_INPROGRESS)
> > + cpu_relax();
> > +}
> > +
> > +#endif
>
> Hmm, what arch can't use the generic version and why? I really
> don't like the HAVE_ARCH_ macros if there's a way around it.

This function implemented differently in allmost every architecture.
I beleive that most of them can use generic version, but I'm still not sure.
v850 and mips define synchronize_irq() as barrier() for example.

> > +#ifndef HAVE_ARCH_IRQ_PROC
> > +void register_irq_proc(unsigned int irq);
> > +#endif
>
> Again, what arch can't use the generic code?

IIRC v850 architecture doesn't need it at all.

> > +#ifndef HAVE_ARCH_IRQ_PROBE
> > +
> > +/*
> > + * IRQ autodetection code..
> > + *
> > + * This depends on the fact that any interrupt that
> > + * comes in on to an unassigned handler will get stuck
> > + * with "IRQ_WAITING" cleared and the interrupt
> > + * disabled.
> > + */
>
> Which architecture uses it's own version here? Also we might
> move this to a separate file as it doesn't make a lot of sense
> without CONFIG_ISA

Some architectures provide empty stubs for these functions.
I'm not sure about CONFIG_ISA, IMHO any legacy device driver
can use irq autoprobing.

> Otherwise it looks fine (of course)! Let's hope we'll get some variant
> of it in before 2.6.

--
Andrey Panin | Linux and UNIX system administrator
[email protected] | PGP key: wwwkeys.pgp.net


Attachments:
(No filename) (2.18 kB)
(No filename) (189.00 B)
Download all attachments

2003-06-27 04:46:14

by Andrey Panin

[permalink] [raw]
Subject: Re: [PATCH][RFC] irq handling code consolidation (common part)

On 178, 06 27, 2003 at 03:55:54AM +1000, Anton Blanchard wrote:
>
> > the irq handling consolidation patch returns from the dead !
> > Now with runaway irq detection code included !
> >
> > This patch (against 2.5.73) contains common part of it.
>
> Great! Well it wasnt dead, I was also keeping it up to date and sending
> it on to akpm :)
>
> I have two suggestions that will help in my crusade to kill NR_IRQS.
>
> 1. define irq_desc, irq_valid, for_each_irq in include/linux/irq.h if
> HAVE_ARCH_IRQ_DESC isnt defined (instead of in each architecture).
> Basically I want to start using these macros in a few places and dont
> want to break every architecture that hasnt converted to the new scheme.

Why in include/linux/irq.h ? These macros are definetely arch specific.
Do you talk about generic array based implementation wrapped in
#ifdef ARCH_HAVE_FOO ?

> On the other hand if we decide to move the irq descriptor definition
> into each arch as hch suggested, this wont be necessary as all archs
> will break anyway :)

Why it will break ? Every arch defines irq descriptors itself now.
May be I'm missing some point here ?

> 2. define irq_atoi that converts an irq into a printable string. We have
> a bunch of #ifdef CONFIG_SPARC stuff we can then get rid of, and other
> archs can start using it if wanted (eg on ppc64 I can subtract our
> software offset so the irqs printed match the hardware)

I thinked about this already, but i wanted to finish cleanup work first.

BTW sparc implementation of irq_itoa() uses static buffer for the formatted
string, is it really irq/preempt safe ?

--
Andrey Panin | Linux and UNIX system administrator
[email protected] | PGP key: wwwkeys.pgp.net


Attachments:
(No filename) (1.67 kB)
(No filename) (189.00 B)
Download all attachments

2003-06-27 05:50:31

by Miles Bader

[permalink] [raw]
Subject: Re: [PATCH][RFC] irq handling code consolidation (common part)

Andrey Panin <[email protected]> writes:
> BTW sparc implementation of irq_itoa() uses static buffer for the formatted
> string, is it really irq/preempt safe ?

Passinf in a result buffer seems simplest (maybe have the arch define a
macro for the max-length).

[btw the name `irq_itoa' seems a bit odd; how about `irq_name' (or
`irq_rep' for clu-lovers)?]

-miLes
--
Somebody has to do something, and it's just incredibly pathetic that it
has to be us. -- Jerry Garcia