* [PATCH] CHECK_IRQ_PER_CPU() to avoid dead code in __do_IRQ()
@ 2005-08-08 10:50 Karsten Wiese
2005-08-08 11:19 ` Alexander Nyberg
0 siblings, 1 reply; 5+ messages in thread
From: Karsten Wiese @ 2005-08-08 10:50 UTC (permalink / raw)
To: Ingo Molnar; +Cc: linux-kernel
Hi Ingo,
This version of the patch has better coding style
thanks to comments from Ingo Oeser.
Please comment or pass it on as appropriate.
Patch is for mainline 2.6.13-rc6.
Thanks,
Karsten
------
From: Karsten Wiese <annabellesgarden@yahoo.de>
IRQ_PER_CPU is not used by all architectures.
This patch introduces the macros
ARCH_HAS_IRQ_PER_CPU and CHECK_IRQ_PER_CPU() to avoid the generation of
dead code in __do_IRQ().
ARCH_HAS_IRQ_PER_CPU is defined by architectures using
IRQ_PER_CPU in their
include/asm_ARCH/irq.h
file.
Through grepping the tree I found the following
architectures currently use IRQ_PER_CPU:
cris, ia64, ppc, ppc64 and parisc.
Signed-off-by: Karsten Wiese <annabellesgarden@yahoo.de>
diff -upr linux-2.6.13-rc6/include/asm-cris/irq.h linux-2.6.13/include/asm-cris/irq.h
--- linux-2.6.13-rc6/include/asm-cris/irq.h 2005-08-08 11:46:10.000000000 +0200
+++ linux-2.6.13/include/asm-cris/irq.h 2005-08-08 11:41:12.000000000 +0200
@@ -1,6 +1,11 @@
#ifndef _ASM_IRQ_H
#define _ASM_IRQ_H
+/*
+ * IRQ line status macro IRQ_PER_CPU is used
+ */
+#define ARCH_HAS_IRQ_PER_CPU
+
#include <asm/arch/irq.h>
extern __inline__ int irq_canonicalize(int irq)
diff -upr linux-2.6.13-rc6/include/asm-ia64/irq.h linux-2.6.13/include/asm-ia64/irq.h
--- linux-2.6.13-rc6/include/asm-ia64/irq.h 2005-03-02 08:38:33.000000000 +0100
+++ linux-2.6.13/include/asm-ia64/irq.h 2005-08-06 18:06:53.000000000 +0200
@@ -14,6 +14,11 @@
#define NR_IRQS 256
#define NR_IRQ_VECTORS NR_IRQS
+/*
+ * IRQ line status macro IRQ_PER_CPU is used
+ */
+#define ARCH_HAS_IRQ_PER_CPU
+
static __inline__ int
irq_canonicalize (int irq)
{
diff -upr linux-2.6.13-rc6/include/asm-parisc/irq.h linux-2.6.13/include/asm-parisc/irq.h
--- linux-2.6.13-rc6/include/asm-parisc/irq.h 2005-08-08 11:45:26.000000000 +0200
+++ linux-2.6.13/include/asm-parisc/irq.h 2005-08-06 18:05:22.000000000 +0200
@@ -26,6 +26,11 @@
#define NR_IRQS (CPU_IRQ_MAX + 1)
+/*
+ * IRQ line status macro IRQ_PER_CPU is used
+ */
+#define ARCH_HAS_IRQ_PER_CPU
+
static __inline__ int irq_canonicalize(int irq)
{
return (irq == 2) ? 9 : irq;
diff -upr linux-2.6.13-rc6/include/asm-ppc/irq.h linux-2.6.13/include/asm-ppc/irq.h
--- linux-2.6.13-rc6/include/asm-ppc/irq.h 2005-08-08 11:46:10.000000000 +0200
+++ linux-2.6.13/include/asm-ppc/irq.h 2005-08-08 11:41:14.000000000 +0200
@@ -19,6 +19,11 @@
#define IRQ_POLARITY_POSITIVE 0x2 /* high level or low->high edge */
#define IRQ_POLARITY_NEGATIVE 0x0 /* low level or high->low edge */
+/*
+ * IRQ line status macro IRQ_PER_CPU is used
+ */
+#define ARCH_HAS_IRQ_PER_CPU
+
#if defined(CONFIG_40x)
#include <asm/ibm4xx.h>
diff -upr linux-2.6.13-rc6/include/asm-ppc64/irq.h linux-2.6.13/include/asm-ppc64/irq.h
--- linux-2.6.13-rc6/include/asm-ppc64/irq.h 2005-03-02 08:38:33.000000000 +0100
+++ linux-2.6.13/include/asm-ppc64/irq.h 2005-08-06 18:06:58.000000000 +0200
@@ -33,6 +33,11 @@
#define IRQ_POLARITY_POSITIVE 0x2 /* high level or low->high edge */
#define IRQ_POLARITY_NEGATIVE 0x0 /* low level or high->low edge */
+/*
+ * IRQ line status macro IRQ_PER_CPU is used
+ */
+#define ARCH_HAS_IRQ_PER_CPU
+
#define get_irq_desc(irq) (&irq_desc[(irq)])
/* Define a way to iterate across irqs. */
diff -upr linux-2.6.13-rc6/include/linux/irq.h linux-2.6.13/include/linux/irq.h
--- linux-2.6.13-rc6/include/linux/irq.h 2005-08-08 11:46:10.000000000 +0200
+++ linux-2.6.13/include/linux/irq.h 2005-08-08 11:55:11.000000000 +0200
@@ -32,7 +32,12 @@
#define IRQ_WAITING 32 /* IRQ not yet seen - for autodetection */
#define IRQ_LEVEL 64 /* IRQ level triggered */
#define IRQ_MASKED 128 /* IRQ masked - shouldn't be seen again */
-#define IRQ_PER_CPU 256 /* IRQ is per CPU */
+#if defined(ARCH_HAS_IRQ_PER_CPU)
+# define IRQ_PER_CPU 256 /* IRQ is per CPU */
+# define CHECK_IRQ_PER_CPU(var) ((var) & IRQ_PER_CPU)
+#else
+# define CHECK_IRQ_PER_CPU(var) 0
+#endif
/*
* Interrupt controller descriptor. This is all we need
diff -upr linux-2.6.13-rc6/kernel/irq/handle.c linux-2.6.13/kernel/irq/handle.c
--- linux-2.6.13-rc6/kernel/irq/handle.c 2005-08-08 11:46:11.000000000 +0200
+++ linux-2.6.13/kernel/irq/handle.c 2005-08-08 11:53:00.000000000 +0200
@@ -111,7 +111,7 @@ fastcall unsigned int __do_IRQ(unsigned
unsigned int status;
kstat_this_cpu.irqs[irq]++;
- if (desc->status & IRQ_PER_CPU) {
+ if (CHECK_IRQ_PER_CPU(desc->status)) {
irqreturn_t action_ret;
/*
___________________________________________________________
Gesendet von Yahoo! Mail - Jetzt mit 1GB Speicher kostenlos - Hier anmelden: http://mail.yahoo.de
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] CHECK_IRQ_PER_CPU() to avoid dead code in __do_IRQ()
2005-08-08 10:50 [PATCH] CHECK_IRQ_PER_CPU() to avoid dead code in __do_IRQ() Karsten Wiese
@ 2005-08-08 11:19 ` Alexander Nyberg
2005-08-08 15:36 ` Karsten Wiese
2005-08-09 14:10 ` Zwane Mwaikambo
0 siblings, 2 replies; 5+ messages in thread
From: Alexander Nyberg @ 2005-08-08 11:19 UTC (permalink / raw)
To: Karsten Wiese; +Cc: Ingo Molnar, linux-kernel
>
> IRQ_PER_CPU is not used by all architectures.
> This patch introduces the macros
> ARCH_HAS_IRQ_PER_CPU and CHECK_IRQ_PER_CPU() to avoid the generation of
> dead code in __do_IRQ().
>
> ARCH_HAS_IRQ_PER_CPU is defined by architectures using
> IRQ_PER_CPU in their
> include/asm_ARCH/irq.h
> file.
>
> Through grepping the tree I found the following
> architectures currently use IRQ_PER_CPU:
>
> cris, ia64, ppc, ppc64 and parisc.
>
There are many places where one could replace run-time tests with
#ifdef's but it makes reading more difficult (and in longer terms
maintainence). Have you benchmarked any workload that benefits
from this?
>
> diff -upr linux-2.6.13-rc6/include/asm-cris/irq.h linux-2.6.13/include/asm-cris/irq.h
> --- linux-2.6.13-rc6/include/asm-cris/irq.h 2005-08-08 11:46:10.000000000 +0200
> +++ linux-2.6.13/include/asm-cris/irq.h 2005-08-08 11:41:12.000000000 +0200
> @@ -1,6 +1,11 @@
> #ifndef _ASM_IRQ_H
> #define _ASM_IRQ_H
>
> +/*
> + * IRQ line status macro IRQ_PER_CPU is used
> + */
> +#define ARCH_HAS_IRQ_PER_CPU
> +
> #include <asm/arch/irq.h>
>
> extern __inline__ int irq_canonicalize(int irq)
> diff -upr linux-2.6.13-rc6/include/asm-ia64/irq.h linux-2.6.13/include/asm-ia64/irq.h
> --- linux-2.6.13-rc6/include/asm-ia64/irq.h 2005-03-02 08:38:33.000000000 +0100
> +++ linux-2.6.13/include/asm-ia64/irq.h 2005-08-06 18:06:53.000000000 +0200
> @@ -14,6 +14,11 @@
> #define NR_IRQS 256
> #define NR_IRQ_VECTORS NR_IRQS
>
> +/*
> + * IRQ line status macro IRQ_PER_CPU is used
> + */
> +#define ARCH_HAS_IRQ_PER_CPU
> +
> static __inline__ int
> irq_canonicalize (int irq)
> {
> diff -upr linux-2.6.13-rc6/include/asm-parisc/irq.h linux-2.6.13/include/asm-parisc/irq.h
> --- linux-2.6.13-rc6/include/asm-parisc/irq.h 2005-08-08 11:45:26.000000000 +0200
> +++ linux-2.6.13/include/asm-parisc/irq.h 2005-08-06 18:05:22.000000000 +0200
> @@ -26,6 +26,11 @@
>
> #define NR_IRQS (CPU_IRQ_MAX + 1)
>
> +/*
> + * IRQ line status macro IRQ_PER_CPU is used
> + */
> +#define ARCH_HAS_IRQ_PER_CPU
> +
> static __inline__ int irq_canonicalize(int irq)
> {
> return (irq == 2) ? 9 : irq;
> diff -upr linux-2.6.13-rc6/include/asm-ppc/irq.h linux-2.6.13/include/asm-ppc/irq.h
> --- linux-2.6.13-rc6/include/asm-ppc/irq.h 2005-08-08 11:46:10.000000000 +0200
> +++ linux-2.6.13/include/asm-ppc/irq.h 2005-08-08 11:41:14.000000000 +0200
> @@ -19,6 +19,11 @@
> #define IRQ_POLARITY_POSITIVE 0x2 /* high level or low->high edge */
> #define IRQ_POLARITY_NEGATIVE 0x0 /* low level or high->low edge */
>
> +/*
> + * IRQ line status macro IRQ_PER_CPU is used
> + */
> +#define ARCH_HAS_IRQ_PER_CPU
> +
> #if defined(CONFIG_40x)
> #include <asm/ibm4xx.h>
>
> diff -upr linux-2.6.13-rc6/include/asm-ppc64/irq.h linux-2.6.13/include/asm-ppc64/irq.h
> --- linux-2.6.13-rc6/include/asm-ppc64/irq.h 2005-03-02 08:38:33.000000000 +0100
> +++ linux-2.6.13/include/asm-ppc64/irq.h 2005-08-06 18:06:58.000000000 +0200
> @@ -33,6 +33,11 @@
> #define IRQ_POLARITY_POSITIVE 0x2 /* high level or low->high edge */
> #define IRQ_POLARITY_NEGATIVE 0x0 /* low level or high->low edge */
>
> +/*
> + * IRQ line status macro IRQ_PER_CPU is used
> + */
> +#define ARCH_HAS_IRQ_PER_CPU
> +
> #define get_irq_desc(irq) (&irq_desc[(irq)])
>
> /* Define a way to iterate across irqs. */
> diff -upr linux-2.6.13-rc6/include/linux/irq.h linux-2.6.13/include/linux/irq.h
> --- linux-2.6.13-rc6/include/linux/irq.h 2005-08-08 11:46:10.000000000 +0200
> +++ linux-2.6.13/include/linux/irq.h 2005-08-08 11:55:11.000000000 +0200
> @@ -32,7 +32,12 @@
> #define IRQ_WAITING 32 /* IRQ not yet seen - for autodetection */
> #define IRQ_LEVEL 64 /* IRQ level triggered */
> #define IRQ_MASKED 128 /* IRQ masked - shouldn't be seen again */
> -#define IRQ_PER_CPU 256 /* IRQ is per CPU */
> +#if defined(ARCH_HAS_IRQ_PER_CPU)
> +# define IRQ_PER_CPU 256 /* IRQ is per CPU */
> +# define CHECK_IRQ_PER_CPU(var) ((var) & IRQ_PER_CPU)
> +#else
> +# define CHECK_IRQ_PER_CPU(var) 0
> +#endif
>
> /*
> * Interrupt controller descriptor. This is all we need
> diff -upr linux-2.6.13-rc6/kernel/irq/handle.c linux-2.6.13/kernel/irq/handle.c
> --- linux-2.6.13-rc6/kernel/irq/handle.c 2005-08-08 11:46:11.000000000 +0200
> +++ linux-2.6.13/kernel/irq/handle.c 2005-08-08 11:53:00.000000000 +0200
> @@ -111,7 +111,7 @@ fastcall unsigned int __do_IRQ(unsigned
> unsigned int status;
>
> kstat_this_cpu.irqs[irq]++;
> - if (desc->status & IRQ_PER_CPU) {
> + if (CHECK_IRQ_PER_CPU(desc->status)) {
> irqreturn_t action_ret;
>
> /*
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] CHECK_IRQ_PER_CPU() to avoid dead code in __do_IRQ()
2005-08-08 11:19 ` Alexander Nyberg
@ 2005-08-08 15:36 ` Karsten Wiese
2005-08-08 15:51 ` Zachary Amsden
2005-08-09 14:10 ` Zwane Mwaikambo
1 sibling, 1 reply; 5+ messages in thread
From: Karsten Wiese @ 2005-08-08 15:36 UTC (permalink / raw)
To: Alexander Nyberg; +Cc: Ingo Molnar, linux-kernel
Am Montag, 8. August 2005 13:19 schrieb Alexander Nyberg:
>
> There are many places where one could replace run-time tests with
> #ifdef's but it makes reading more difficult (and in longer terms
> maintainence). Have you benchmarked any workload that benefits
> from this?
Performance gain is small, but measurable: 0,02%
Tested on an Atlon64 running at 1000MHz.
I took this value from 9 runs (each with/without the patch) of
$ time lame x.wav
where each took about 1 minute.
3000 Interrupts/s were generated at the time by running
$ jackd -R -dalsa -p64 -n2
0,02% really isn't that much....but Athlon64 is better than P4
with branch predictions, I think.
Erm... ok, I won't insist on having this patch applied ;-)
Karsten
___________________________________________________________
Gesendet von Yahoo! Mail - Jetzt mit 1GB Speicher kostenlos - Hier anmelden: http://mail.yahoo.de
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] CHECK_IRQ_PER_CPU() to avoid dead code in __do_IRQ()
2005-08-08 15:36 ` Karsten Wiese
@ 2005-08-08 15:51 ` Zachary Amsden
0 siblings, 0 replies; 5+ messages in thread
From: Zachary Amsden @ 2005-08-08 15:51 UTC (permalink / raw)
To: Karsten Wiese; +Cc: Alexander Nyberg, Ingo Molnar, linux-kernel
Karsten Wiese wrote:
>Am Montag, 8. August 2005 13:19 schrieb Alexander Nyberg:
>
>
>>There are many places where one could replace run-time tests with
>>#ifdef's but it makes reading more difficult (and in longer terms
>>maintainence). Have you benchmarked any workload that benefits
>>from this?
>>
>>
>
>Performance gain is small, but measurable: 0,02%
>Tested on an Atlon64 running at 1000MHz.
>I took this value from 9 runs (each with/without the patch) of
> $ time lame x.wav
>where each took about 1 minute.
>3000 Interrupts/s were generated at the time by running
> $ jackd -R -dalsa -p64 -n2
>
>0,02% really isn't that much....but Athlon64 is better than P4
>with branch predictions, I think.
>
>Erm... ok, I won't insist on having this patch applied ;-)
>
> Karsten
>
>
Removing dead code is always good - 0.02% is small, but if 100 kernel
developers all did the same, that adds up to 2% rather quickly, and that
is nothing to sneeze at. I like your patch, but you should add some
comments for maintainability about what CHECK_IRQ_PER_CPU does - see
include/asm-generic/pgtable.h for similar styling. If also probably
doesn't hurt to leave IRQ_PER_CPU defined even when
ARCH_HAS_CHECK_IRQ_PER_CPU is not, since it looks cleaner and prevents
future collisions with bits defined inside of an #ifdef.
Zach
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] CHECK_IRQ_PER_CPU() to avoid dead code in __do_IRQ()
2005-08-08 11:19 ` Alexander Nyberg
2005-08-08 15:36 ` Karsten Wiese
@ 2005-08-09 14:10 ` Zwane Mwaikambo
1 sibling, 0 replies; 5+ messages in thread
From: Zwane Mwaikambo @ 2005-08-09 14:10 UTC (permalink / raw)
To: Alexander Nyberg; +Cc: Karsten Wiese, Ingo Molnar, linux-kernel
On Mon, 8 Aug 2005, Alexander Nyberg wrote:
> > IRQ_PER_CPU is not used by all architectures.
> > This patch introduces the macros
> > ARCH_HAS_IRQ_PER_CPU and CHECK_IRQ_PER_CPU() to avoid the generation of
> > dead code in __do_IRQ().
> >
> > ARCH_HAS_IRQ_PER_CPU is defined by architectures using
> > IRQ_PER_CPU in their
> > include/asm_ARCH/irq.h
> > file.
> >
> > Through grepping the tree I found the following
> > architectures currently use IRQ_PER_CPU:
> >
> > cris, ia64, ppc, ppc64 and parisc.
> >
>
> There are many places where one could replace run-time tests with
> #ifdef's but it makes reading more difficult (and in longer terms
> maintainence). Have you benchmarked any workload that benefits
> from this?
I doubt you'd be able to collect convincing benchmark data, but skipping a
branch (possibly mispredicted) is worth it IMO.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2005-08-09 14:04 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-08-08 10:50 [PATCH] CHECK_IRQ_PER_CPU() to avoid dead code in __do_IRQ() Karsten Wiese
2005-08-08 11:19 ` Alexander Nyberg
2005-08-08 15:36 ` Karsten Wiese
2005-08-08 15:51 ` Zachary Amsden
2005-08-09 14:10 ` Zwane Mwaikambo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox