LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* tick-common.c hack for s390 needed
@ 2008-03-18  9:31 Heiko Carstens
  2008-03-19  4:12 ` Arnd Bergmann
  2008-03-19  5:45 ` Christoph Hellwig
  0 siblings, 2 replies; 13+ messages in thread
From: Heiko Carstens @ 2008-03-18  9:31 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar; +Cc: linux-kernel, Martin Schwidefsky

Hi Thomas, Ingo,

I'm converting s390 from s390's NO_IDLE_HZ to GENERIC_CLOCKEVENTS and
therefore to the generic NO_HZ implementation.
One of the problems that need a patch for this is kernel/time/tick-common.c
which relies on the irq stuff present in include/linux/irq.h.
In particular s390 doesn't have something like irq_set_affinity which
causes this build error:

  CC      kernel/time/tick-common.o
kernel/time/tick-common.c: In function 'tick_periodic':
kernel/time/tick-common.c:70: error: implicit declaration of function 'get_irq_regs'
kernel/time/tick-common.c:70: error: invalid type argument of '->' (have 'int')
kernel/time/tick-common.c: In function 'tick_setup_device':
kernel/time/tick-common.c:171: error: implicit declaration of function 'irq_set_affinity'
kernel/time/tick-common.c: In function 'tick_check_new_device':
kernel/time/tick-common.c:216: error: implicit declaration of function 'irq_can_set_affinity'

Actually there's a huge #ifndef CONFIG_S390 in linux/irq.h ;)

To make the code work the patch below is necessary. It's ok since all
clock event devices on s390 are per cpu. However I think this patch is
ugly at best. Any ideas how to fix this in a better and more generic way?

---
 kernel/time/tick-common.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Index: linux-2.6/kernel/time/tick-common.c
===================================================================
--- linux-2.6.orig/kernel/time/tick-common.c
+++ linux-2.6/kernel/time/tick-common.c
@@ -19,6 +19,7 @@
 #include <linux/profile.h>
 #include <linux/sched.h>
 #include <linux/tick.h>
+#include <asm/irq_regs.h>
 
 #include "tick-internal.h"
 
@@ -167,9 +168,10 @@ static void tick_setup_device(struct tic
 	 * When the device is not per cpu, pin the interrupt to the
 	 * current cpu:
 	 */
+#ifndef CONFIG_S390
 	if (!cpus_equal(newdev->cpumask, cpumask))
 		irq_set_affinity(newdev->irq, cpumask);
-
+#endif
 	/*
 	 * When global broadcasting is active, check if the current
 	 * device is registered as a placeholder for broadcast mode.
@@ -213,9 +215,10 @@ static int tick_check_new_device(struct 
 		 * If the cpu affinity of the device interrupt can not
 		 * be set, ignore it.
 		 */
+#ifndef CONFIG_S390
 		if (!irq_can_set_affinity(newdev->irq))
 			goto out_bc;
-
+#endif
 		/*
 		 * If we have a cpu local device already, do not replace it
 		 * by a non cpu local device

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: tick-common.c hack for s390 needed
  2008-03-18  9:31 tick-common.c hack for s390 needed Heiko Carstens
@ 2008-03-19  4:12 ` Arnd Bergmann
  2008-03-19  5:45 ` Christoph Hellwig
  1 sibling, 0 replies; 13+ messages in thread
From: Arnd Bergmann @ 2008-03-19  4:12 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Thomas Gleixner, Ingo Molnar, linux-kernel, Martin Schwidefsky

On Tuesday 18 March 2008, Heiko Carstens wrote:
> Actually there's a huge #ifndef CONFIG_S390 in linux/irq.h ;)
> 
> To make the code work the patch below is necessary. It's ok since all
> clock event devices on s390 are per cpu. However I think this patch is
> ugly at best. Any ideas how to fix this in a better and more generic way?

You could have an #else path in linux/irq.h to just define these
functions as s390, maybe like

static inline int irq_set_affinity(unsigned int irq, cpumask_t cpumask)
{
	BUG();
	return 0;
}
static inline int irq_can_set_affinity(unsigned int irq)
{
	return 0;
}

	Arnd <><

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: tick-common.c hack for s390 needed
  2008-03-18  9:31 tick-common.c hack for s390 needed Heiko Carstens
  2008-03-19  4:12 ` Arnd Bergmann
@ 2008-03-19  5:45 ` Christoph Hellwig
  2008-03-21 10:15   ` Ingo Molnar
  1 sibling, 1 reply; 13+ messages in thread
From: Christoph Hellwig @ 2008-03-19  5:45 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Thomas Gleixner, Ingo Molnar, linux-kernel, Martin Schwidefsky

On Tue, Mar 18, 2008 at 10:31:19AM +0100, Heiko Carstens wrote:
> Hi Thomas, Ingo,
> 
> I'm converting s390 from s390's NO_IDLE_HZ to GENERIC_CLOCKEVENTS and
> therefore to the generic NO_HZ implementation.
> One of the problems that need a patch for this is kernel/time/tick-common.c
> which relies on the irq stuff present in include/linux/irq.h.
> In particular s390 doesn't have something like irq_set_affinity which
> causes this build error:

Generic code must never use linux/irq.h - it's more like asm-generic
file predating asm-generic.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: tick-common.c hack for s390 needed
  2008-03-19  5:45 ` Christoph Hellwig
@ 2008-03-21 10:15   ` Ingo Molnar
  2008-03-21 13:25     ` Heiko Carstens
  0 siblings, 1 reply; 13+ messages in thread
From: Ingo Molnar @ 2008-03-21 10:15 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Heiko Carstens, Thomas Gleixner, linux-kernel, Martin Schwidefsky


* Christoph Hellwig <hch@infradead.org> wrote:

> On Tue, Mar 18, 2008 at 10:31:19AM +0100, Heiko Carstens wrote:
> > Hi Thomas, Ingo,
> > 
> > I'm converting s390 from s390's NO_IDLE_HZ to GENERIC_CLOCKEVENTS and
> > therefore to the generic NO_HZ implementation.
> > One of the problems that need a patch for this is kernel/time/tick-common.c
> > which relies on the irq stuff present in include/linux/irq.h.
> > In particular s390 doesn't have something like irq_set_affinity which
> > causes this build error:
> 
> Generic code must never use linux/irq.h - it's more like asm-generic 
> file predating asm-generic.

agreed.

Heiko, if you remove all these:

 ./time/tick-broadcast.c:#include <linux/irq.h>
 ./time/tick-oneshot.c:#include <linux/irq.h>
 ./time/tick-common.c:#include <linux/irq.h>
 ./hrtimer.c:#include <linux/irq.h>

from kernel/*, do things improve on s390? If yes then please send a 
patch for it and i'll check whether there's any include file dependency 
fallout on x86.

	Ingo

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: tick-common.c hack for s390 needed
  2008-03-21 10:15   ` Ingo Molnar
@ 2008-03-21 13:25     ` Heiko Carstens
  2008-03-21 14:45       ` Thomas Gleixner
  0 siblings, 1 reply; 13+ messages in thread
From: Heiko Carstens @ 2008-03-21 13:25 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Christoph Hellwig, Thomas Gleixner, linux-kernel, Martin Schwidefsky

On Fri, Mar 21, 2008 at 11:15:04AM +0100, Ingo Molnar wrote:
> 
> * Christoph Hellwig <hch@infradead.org> wrote:
> 
> > On Tue, Mar 18, 2008 at 10:31:19AM +0100, Heiko Carstens wrote:
> > > Hi Thomas, Ingo,
> > > 
> > > I'm converting s390 from s390's NO_IDLE_HZ to GENERIC_CLOCKEVENTS and
> > > therefore to the generic NO_HZ implementation.
> > > One of the problems that need a patch for this is kernel/time/tick-common.c
> > > which relies on the irq stuff present in include/linux/irq.h.
> > > In particular s390 doesn't have something like irq_set_affinity which
> > > causes this build error:
> > 
> > Generic code must never use linux/irq.h - it's more like asm-generic 
> > file predating asm-generic.
> 
> agreed.
> 
> Heiko, if you remove all these:
> 
>  ./time/tick-broadcast.c:#include <linux/irq.h>
>  ./time/tick-oneshot.c:#include <linux/irq.h>
>  ./time/tick-common.c:#include <linux/irq.h>
>  ./hrtimer.c:#include <linux/irq.h>
> 
> from kernel/*, do things improve on s390? If yes then please send a 
> patch for it and i'll check whether there's any include file dependency 
> fallout on x86.

No, tick-common.c has calls to irq_set_affinity and irq_can_set_affinity, for
which the prototypes are in linux/irq.h.
Each and every architecture seems to include linux/irq.h from a different file
in asm-[your-favourite-arch]/*.h. I was able to fix the build breakage on x86
by adding an include <linux/interrupt.h>. But that won't work on other
architectures.
>From a logical point of view I would expect that asm/irq.h is supposed to
include linux/irq.h, no?

The patch below removes at least three of the includes for a beginning.
Still compiles on s390 and x86_64.

Subject: [PATCH] time: remove include/irq.h.

From: Heiko Carstens <heiko.carstens@de.ibm.com>

Generic code is not supposed to include linux/irq.h.

Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---

 kernel/hrtimer.c             |    1 -
 kernel/time/tick-broadcast.c |    1 -
 kernel/time/tick-oneshot.c   |    1 -
 3 files changed, 3 deletions(-)

Index: linux-2.6/kernel/hrtimer.c
===================================================================
--- linux-2.6.orig/kernel/hrtimer.c
+++ linux-2.6/kernel/hrtimer.c
@@ -32,7 +32,6 @@
  */
 
 #include <linux/cpu.h>
-#include <linux/irq.h>
 #include <linux/module.h>
 #include <linux/percpu.h>
 #include <linux/hrtimer.h>
Index: linux-2.6/kernel/time/tick-broadcast.c
===================================================================
--- linux-2.6.orig/kernel/time/tick-broadcast.c
+++ linux-2.6/kernel/time/tick-broadcast.c
@@ -14,7 +14,6 @@
 #include <linux/cpu.h>
 #include <linux/err.h>
 #include <linux/hrtimer.h>
-#include <linux/irq.h>
 #include <linux/percpu.h>
 #include <linux/profile.h>
 #include <linux/sched.h>
Index: linux-2.6/kernel/time/tick-oneshot.c
===================================================================
--- linux-2.6.orig/kernel/time/tick-oneshot.c
+++ linux-2.6/kernel/time/tick-oneshot.c
@@ -14,7 +14,6 @@
 #include <linux/cpu.h>
 #include <linux/err.h>
 #include <linux/hrtimer.h>
-#include <linux/irq.h>
 #include <linux/percpu.h>
 #include <linux/profile.h>
 #include <linux/sched.h>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: tick-common.c hack for s390 needed
  2008-03-21 13:25     ` Heiko Carstens
@ 2008-03-21 14:45       ` Thomas Gleixner
  2008-03-22 20:32         ` Heiko Carstens
  0 siblings, 1 reply; 13+ messages in thread
From: Thomas Gleixner @ 2008-03-21 14:45 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Ingo Molnar, Christoph Hellwig, linux-kernel, Martin Schwidefsky

On Fri, 21 Mar 2008, Heiko Carstens wrote:
> 
> The patch below removes at least three of the includes for a beginning.
> Still compiles on s390 and x86_64.
> 
> Subject: [PATCH] time: remove include/irq.h.
> 
> From: Heiko Carstens <heiko.carstens@de.ibm.com>
> 
> Generic code is not supposed to include linux/irq.h.
> 
> Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>

Applied, thanks,

	 tglx


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: tick-common.c hack for s390 needed
  2008-03-21 14:45       ` Thomas Gleixner
@ 2008-03-22 20:32         ` Heiko Carstens
  2008-03-23 22:34           ` Russell King
  0 siblings, 1 reply; 13+ messages in thread
From: Heiko Carstens @ 2008-03-22 20:32 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Ingo Molnar, Christoph Hellwig, linux-kernel, Martin Schwidefsky,
	paulus, ralf, rmk, davem, lethal, jdike

On Fri, Mar 21, 2008 at 03:45:52PM +0100, Thomas Gleixner wrote:
> On Fri, 21 Mar 2008, Heiko Carstens wrote:
> > 
> > The patch below removes at least three of the includes for a beginning.
> > Still compiles on s390 and x86_64.
> > 
> > Subject: [PATCH] time: remove include/irq.h.
> > 
> > From: Heiko Carstens <heiko.carstens@de.ibm.com>
> > 
> > Generic code is not supposed to include linux/irq.h.
> > 
> > Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
> 
> Applied, thanks,

And here comes the second part...

Subject: [PATCH] time: remove include/irq.h from tick-common.c

From: Heiko Carstens <heiko.carstens@de.ibm.com>

Generic code is not supposed to include irq.h. Replace this include
by linux/hardirq.h instead and add/replace an include of linux/irq.h
in asm header files where necessary.
This change should only matter for architectures that make use of
GENERIC_CLOCKEVENTS.
Architectures in question are mips, x86, arm, sh, powerpc, uml and sparc64.

I did some cross compile tests for mips, x86_64, arm, powerpc and sparc64.
This patch fixes also build breakages caused by the include replacement in
tick-common.h.

I didn't test uml and sh.

Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
 include/asm-arm/hardirq.h     |    2 +-
 include/asm-powerpc/hardirq.h |    2 +-
 include/asm-sparc64/hardirq.h |    1 +
 include/asm-sparc64/irq.h     |    1 -
 kernel/time/tick-common.c     |    2 +-
 5 files changed, 4 insertions(+), 4 deletions(-)

Index: linux-2.6/include/asm-arm/hardirq.h
===================================================================
--- linux-2.6.orig/include/asm-arm/hardirq.h
+++ linux-2.6/include/asm-arm/hardirq.h
@@ -3,7 +3,7 @@
 
 #include <linux/cache.h>
 #include <linux/threads.h>
-#include <asm/irq.h>
+#include <linux/irq.h>
 
 typedef struct {
 	unsigned int __softirq_pending;
Index: linux-2.6/include/asm-powerpc/hardirq.h
===================================================================
--- linux-2.6.orig/include/asm-powerpc/hardirq.h
+++ linux-2.6/include/asm-powerpc/hardirq.h
@@ -2,7 +2,7 @@
 #define _ASM_POWERPC_HARDIRQ_H
 #ifdef __KERNEL__
 
-#include <asm/irq.h>
+#include <linux/irq.h>
 #include <asm/bug.h>
 
 /* The __last_jiffy_stamp field is needed to ensure that no decrementer
Index: linux-2.6/include/asm-sparc64/hardirq.h
===================================================================
--- linux-2.6.orig/include/asm-sparc64/hardirq.h
+++ linux-2.6/include/asm-sparc64/hardirq.h
@@ -6,6 +6,7 @@
 #ifndef __SPARC64_HARDIRQ_H
 #define __SPARC64_HARDIRQ_H
 
+#include <linux/irq.h>
 #include <asm/cpudata.h>
 
 #define __ARCH_IRQ_STAT
Index: linux-2.6/kernel/time/tick-common.c
===================================================================
--- linux-2.6.orig/kernel/time/tick-common.c
+++ linux-2.6/kernel/time/tick-common.c
@@ -14,7 +14,7 @@
 #include <linux/cpu.h>
 #include <linux/err.h>
 #include <linux/hrtimer.h>
-#include <linux/irq.h>
+#include <linux/hardirq.h>
 #include <linux/percpu.h>
 #include <linux/profile.h>
 #include <linux/sched.h>
Index: linux-2.6/include/asm-sparc64/irq.h
===================================================================
--- linux-2.6.orig/include/asm-sparc64/irq.h
+++ linux-2.6/include/asm-sparc64/irq.h
@@ -10,7 +10,6 @@
 #include <linux/linkage.h>
 #include <linux/kernel.h>
 #include <linux/errno.h>
-#include <linux/interrupt.h>
 #include <asm/pil.h>
 #include <asm/ptrace.h>
 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: tick-common.c hack for s390 needed
  2008-03-22 20:32         ` Heiko Carstens
@ 2008-03-23 22:34           ` Russell King
  2008-03-24 10:14             ` Heiko Carstens
  2008-03-25 19:09             ` Thomas Gleixner
  0 siblings, 2 replies; 13+ messages in thread
From: Russell King @ 2008-03-23 22:34 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Thomas Gleixner, Ingo Molnar, Christoph Hellwig, linux-kernel,
	Martin Schwidefsky, paulus, ralf, davem, lethal, jdike

On Sat, Mar 22, 2008 at 09:32:23PM +0100, Heiko Carstens wrote:
> Generic code is not supposed to include irq.h. Replace this include
> by linux/hardirq.h instead and add/replace an include of linux/irq.h
> in asm header files where necessary.
> This change should only matter for architectures that make use of
> GENERIC_CLOCKEVENTS.
> Architectures in question are mips, x86, arm, sh, powerpc, uml and sparc64.
> 
> I did some cross compile tests for mips, x86_64, arm, powerpc and sparc64.
> This patch fixes also build breakages caused by the include replacement in
> tick-common.h.

I generally dislike adding optional linux/* includes in asm/* includes -
I'm nervous about this causing include loops.

However, there's a separate point to be discussed here.

That is, what interfaces are expected of every architecture in the kernel.
If generic code wants to be able to set the affinity of interrupts, then
that needs to become part of the interfaces listed in linux/interrupt.h
rather than linux/irq.h.

So what I suggest is this approach instead (against Linus' tree of a
couple of days ago) - we move irq_set_affinity() and irq_can_set_affinity()
to linux/interrupt.h, change the linux/irq.h includes to linux/interrupt.h
and include asm/irq_regs.h where needed (asm/irq_regs.h is supposed to be
rarely used include since not much touches the stacked parent context
registers.)

Build tested on ARM PXA family kernels and ARM's Realview platform
kernels which both use genirq.

diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index f8ab4ce..355e3b0 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -102,6 +102,25 @@ extern void disable_irq_nosync(unsigned int irq);
 extern void disable_irq(unsigned int irq);
 extern void enable_irq(unsigned int irq);
 
+#ifdef CONFIG_SMP
+
+extern int irq_set_affinity(unsigned int irq, cpumask_t cpumask);
+extern int irq_can_set_affinity(unsigned int irq);
+
+#else /* CONFIG_SMP */
+
+static inline int irq_set_affinity(unsigned int irq, cpumask_t cpumask)
+{
+	return -EINVAL;
+}
+
+static inline int irq_can_set_affinity(unsigned int irq)
+{
+	return 0;
+}
+
+#endif /* CONFIG_SMP */
+
 #ifdef CONFIG_GENERIC_HARDIRQS
 /*
  * Special lockdep variants of irq disabling/enabling.
diff --git a/include/linux/irq.h b/include/linux/irq.h
index 176e5e7..1883a85 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -228,21 +228,11 @@ static inline void set_pending_irq(unsigned int irq, cpumask_t mask)
 
 #endif /* CONFIG_GENERIC_PENDING_IRQ */
 
-extern int irq_set_affinity(unsigned int irq, cpumask_t cpumask);
-extern int irq_can_set_affinity(unsigned int irq);
-
 #else /* CONFIG_SMP */
 
 #define move_native_irq(x)
 #define move_masked_irq(x)
 
-static inline int irq_set_affinity(unsigned int irq, cpumask_t cpumask)
-{
-	return -EINVAL;
-}
-
-static inline int irq_can_set_affinity(unsigned int irq) { return 0; }
-
 #endif /* CONFIG_SMP */
 
 #ifdef CONFIG_IRQBALANCE
diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
index e1bd50c..fdfa0c7 100644
--- a/kernel/time/tick-broadcast.c
+++ b/kernel/time/tick-broadcast.c
@@ -14,7 +14,7 @@
 #include <linux/cpu.h>
 #include <linux/err.h>
 #include <linux/hrtimer.h>
-#include <linux/irq.h>
+#include <linux/interrupt.h>
 #include <linux/percpu.h>
 #include <linux/profile.h>
 #include <linux/sched.h>
diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
index 1bea399..4f38865 100644
--- a/kernel/time/tick-common.c
+++ b/kernel/time/tick-common.c
@@ -14,12 +14,14 @@
 #include <linux/cpu.h>
 #include <linux/err.h>
 #include <linux/hrtimer.h>
-#include <linux/irq.h>
+#include <linux/interrupt.h>
 #include <linux/percpu.h>
 #include <linux/profile.h>
 #include <linux/sched.h>
 #include <linux/tick.h>
 
+#include <asm/irq_regs.h>
+
 #include "tick-internal.h"
 
 /*
diff --git a/kernel/time/tick-oneshot.c b/kernel/time/tick-oneshot.c
index 0258d31..450c049 100644
--- a/kernel/time/tick-oneshot.c
+++ b/kernel/time/tick-oneshot.c
@@ -14,7 +14,7 @@
 #include <linux/cpu.h>
 #include <linux/err.h>
 #include <linux/hrtimer.h>
-#include <linux/irq.h>
+#include <linux/interrupt.h>
 #include <linux/percpu.h>
 #include <linux/profile.h>
 #include <linux/sched.h>


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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: tick-common.c hack for s390 needed
  2008-03-23 22:34           ` Russell King
@ 2008-03-24 10:14             ` Heiko Carstens
  2008-03-25 19:09             ` Thomas Gleixner
  1 sibling, 0 replies; 13+ messages in thread
From: Heiko Carstens @ 2008-03-24 10:14 UTC (permalink / raw)
  To: Russell King
  Cc: Thomas Gleixner, Ingo Molnar, Christoph Hellwig, linux-kernel,
	Martin Schwidefsky, paulus, ralf, davem, lethal, jdike

On Sun, Mar 23, 2008 at 10:34:55PM +0000, Russell King wrote:
> On Sat, Mar 22, 2008 at 09:32:23PM +0100, Heiko Carstens wrote:
> > Generic code is not supposed to include irq.h. Replace this include
> > by linux/hardirq.h instead and add/replace an include of linux/irq.h
> > in asm header files where necessary.
> > This change should only matter for architectures that make use of
> > GENERIC_CLOCKEVENTS.
> > Architectures in question are mips, x86, arm, sh, powerpc, uml and sparc64.
> > 
> > I did some cross compile tests for mips, x86_64, arm, powerpc and sparc64.
> > This patch fixes also build breakages caused by the include replacement in
> > tick-common.h.
> 
> I generally dislike adding optional linux/* includes in asm/* includes -
> I'm nervous about this causing include loops.
> 
> However, there's a separate point to be discussed here.
> 
> That is, what interfaces are expected of every architecture in the kernel.
> If generic code wants to be able to set the affinity of interrupts, then
> that needs to become part of the interfaces listed in linux/interrupt.h
> rather than linux/irq.h.

Every architecture besides one... as usual :/

> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> index f8ab4ce..355e3b0 100644
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
> @@ -102,6 +102,25 @@ extern void disable_irq_nosync(unsigned int irq);
>  extern void disable_irq(unsigned int irq);
>  extern void enable_irq(unsigned int irq);
> 
> +#ifdef CONFIG_SMP
> +
> +extern int irq_set_affinity(unsigned int irq, cpumask_t cpumask);
> +extern int irq_can_set_affinity(unsigned int irq);
> +
> +#else /* CONFIG_SMP */
> +
> +static inline int irq_set_affinity(unsigned int irq, cpumask_t cpumask)
> +{
> +	return -EINVAL;
> +}
> +
> +static inline int irq_can_set_affinity(unsigned int irq)
> +{
> +	return 0;
> +}
> +
> +#endif /* CONFIG_SMP */
> +

Could you add something like

#ifdef CONFIG_GENERIC_HARDIRQS

/* added stuff from above */

#else /* CONFIG_GENERIC_HARDIRQS */

static inline int irq_set_affinity(unsigned int irq, cpumask_t cpumask)
{
	WARN_ON(1);
	return -EINVAL;
}

static inline int irq_can_set_affinity(unsigned int irq)
{
	return 0;
}
#endif /* CONFIG_GENERIC_HARDIRQS */

Then it should do the right thing on s390 as well.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: tick-common.c hack for s390 needed
  2008-03-23 22:34           ` Russell King
  2008-03-24 10:14             ` Heiko Carstens
@ 2008-03-25 19:09             ` Thomas Gleixner
  2008-03-25 19:30               ` Russell King
  2008-04-01 11:02               ` Heiko Carstens
  1 sibling, 2 replies; 13+ messages in thread
From: Thomas Gleixner @ 2008-03-25 19:09 UTC (permalink / raw)
  To: Russell King
  Cc: Heiko Carstens, Ingo Molnar, Christoph Hellwig, linux-kernel,
	Martin Schwidefsky, paulus, ralf, davem, lethal, jdike

On Sun, 23 Mar 2008, Russell King wrote:

> On Sat, Mar 22, 2008 at 09:32:23PM +0100, Heiko Carstens wrote:
> > Generic code is not supposed to include irq.h. Replace this include
> > by linux/hardirq.h instead and add/replace an include of linux/irq.h
> > in asm header files where necessary.
> > This change should only matter for architectures that make use of
> > GENERIC_CLOCKEVENTS.
> > Architectures in question are mips, x86, arm, sh, powerpc, uml and sparc64.
> > 
> > I did some cross compile tests for mips, x86_64, arm, powerpc and sparc64.
> > This patch fixes also build breakages caused by the include replacement in
> > tick-common.h.
> 
> I generally dislike adding optional linux/* includes in asm/* includes -
> I'm nervous about this causing include loops.
> 
> However, there's a separate point to be discussed here.
> 
> That is, what interfaces are expected of every architecture in the kernel.
> If generic code wants to be able to set the affinity of interrupts, then
> that needs to become part of the interfaces listed in linux/interrupt.h
> rather than linux/irq.h.
> 
> So what I suggest is this approach instead (against Linus' tree of a
> couple of days ago) - we move irq_set_affinity() and irq_can_set_affinity()
> to linux/interrupt.h, change the linux/irq.h includes to linux/interrupt.h
> and include asm/irq_regs.h where needed (asm/irq_regs.h is supposed to be
> rarely used include since not much touches the stacked parent context
> registers.)

That patch makes a lot of sense and resolves all the issues. 

I push it through hrt.git and add the GENERIC_HARDIRQS dependency
which was requested by Heiko.

Thanks,

	tglx

> Build tested on ARM PXA family kernels and ARM's Realview platform
> kernels which both use genirq.
>
> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> index f8ab4ce..355e3b0 100644
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
> @@ -102,6 +102,25 @@ extern void disable_irq_nosync(unsigned int irq);
>  extern void disable_irq(unsigned int irq);
>  extern void enable_irq(unsigned int irq);
>  
> +#ifdef CONFIG_SMP
> +
> +extern int irq_set_affinity(unsigned int irq, cpumask_t cpumask);
> +extern int irq_can_set_affinity(unsigned int irq);
> +
> +#else /* CONFIG_SMP */
> +
> +static inline int irq_set_affinity(unsigned int irq, cpumask_t cpumask)
> +{
> +	return -EINVAL;
> +}
> +
> +static inline int irq_can_set_affinity(unsigned int irq)
> +{
> +	return 0;
> +}
> +
> +#endif /* CONFIG_SMP */
> +
>  #ifdef CONFIG_GENERIC_HARDIRQS
>  /*
>   * Special lockdep variants of irq disabling/enabling.
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index 176e5e7..1883a85 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -228,21 +228,11 @@ static inline void set_pending_irq(unsigned int irq, cpumask_t mask)
>  
>  #endif /* CONFIG_GENERIC_PENDING_IRQ */
>  
> -extern int irq_set_affinity(unsigned int irq, cpumask_t cpumask);
> -extern int irq_can_set_affinity(unsigned int irq);
> -
>  #else /* CONFIG_SMP */
>  
>  #define move_native_irq(x)
>  #define move_masked_irq(x)
>  
> -static inline int irq_set_affinity(unsigned int irq, cpumask_t cpumask)
> -{
> -	return -EINVAL;
> -}
> -
> -static inline int irq_can_set_affinity(unsigned int irq) { return 0; }
> -
>  #endif /* CONFIG_SMP */
>  
>  #ifdef CONFIG_IRQBALANCE
> diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
> index e1bd50c..fdfa0c7 100644
> --- a/kernel/time/tick-broadcast.c
> +++ b/kernel/time/tick-broadcast.c
> @@ -14,7 +14,7 @@
>  #include <linux/cpu.h>
>  #include <linux/err.h>
>  #include <linux/hrtimer.h>
> -#include <linux/irq.h>
> +#include <linux/interrupt.h>
>  #include <linux/percpu.h>
>  #include <linux/profile.h>
>  #include <linux/sched.h>
> diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
> index 1bea399..4f38865 100644
> --- a/kernel/time/tick-common.c
> +++ b/kernel/time/tick-common.c
> @@ -14,12 +14,14 @@
>  #include <linux/cpu.h>
>  #include <linux/err.h>
>  #include <linux/hrtimer.h>
> -#include <linux/irq.h>
> +#include <linux/interrupt.h>
>  #include <linux/percpu.h>
>  #include <linux/profile.h>
>  #include <linux/sched.h>
>  #include <linux/tick.h>
>  
> +#include <asm/irq_regs.h>
> +
>  #include "tick-internal.h"
>  
>  /*
> diff --git a/kernel/time/tick-oneshot.c b/kernel/time/tick-oneshot.c
> index 0258d31..450c049 100644
> --- a/kernel/time/tick-oneshot.c
> +++ b/kernel/time/tick-oneshot.c
> @@ -14,7 +14,7 @@
>  #include <linux/cpu.h>
>  #include <linux/err.h>
>  #include <linux/hrtimer.h>
> -#include <linux/irq.h>
> +#include <linux/interrupt.h>
>  #include <linux/percpu.h>
>  #include <linux/profile.h>
>  #include <linux/sched.h>
> 
> 
> -- 
> Russell King
>  Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
>  maintainer of:
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: tick-common.c hack for s390 needed
  2008-03-25 19:09             ` Thomas Gleixner
@ 2008-03-25 19:30               ` Russell King
  2008-04-01 11:02               ` Heiko Carstens
  1 sibling, 0 replies; 13+ messages in thread
From: Russell King @ 2008-03-25 19:30 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Heiko Carstens, Ingo Molnar, Christoph Hellwig, linux-kernel,
	Martin Schwidefsky, paulus, ralf, davem, lethal, jdike

On Tue, Mar 25, 2008 at 08:09:15PM +0100, Thomas Gleixner wrote:
> On Sun, 23 Mar 2008, Russell King wrote:
> 
> > On Sat, Mar 22, 2008 at 09:32:23PM +0100, Heiko Carstens wrote:
> > > Generic code is not supposed to include irq.h. Replace this include
> > > by linux/hardirq.h instead and add/replace an include of linux/irq.h
> > > in asm header files where necessary.
> > > This change should only matter for architectures that make use of
> > > GENERIC_CLOCKEVENTS.
> > > Architectures in question are mips, x86, arm, sh, powerpc, uml and sparc64.
> > > 
> > > I did some cross compile tests for mips, x86_64, arm, powerpc and sparc64.
> > > This patch fixes also build breakages caused by the include replacement in
> > > tick-common.h.
> > 
> > I generally dislike adding optional linux/* includes in asm/* includes -
> > I'm nervous about this causing include loops.
> > 
> > However, there's a separate point to be discussed here.
> > 
> > That is, what interfaces are expected of every architecture in the kernel.
> > If generic code wants to be able to set the affinity of interrupts, then
> > that needs to become part of the interfaces listed in linux/interrupt.h
> > rather than linux/irq.h.
> > 
> > So what I suggest is this approach instead (against Linus' tree of a
> > couple of days ago) - we move irq_set_affinity() and irq_can_set_affinity()
> > to linux/interrupt.h, change the linux/irq.h includes to linux/interrupt.h
> > and include asm/irq_regs.h where needed (asm/irq_regs.h is supposed to be
> > rarely used include since not much touches the stacked parent context
> > registers.)
> 
> That patch makes a lot of sense and resolves all the issues. 
> 
> I push it through hrt.git and add the GENERIC_HARDIRQS dependency
> which was requested by Heiko.

Okay.  Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: tick-common.c hack for s390 needed
  2008-03-25 19:09             ` Thomas Gleixner
  2008-03-25 19:30               ` Russell King
@ 2008-04-01 11:02               ` Heiko Carstens
  2008-04-01 11:24                 ` Thomas Gleixner
  1 sibling, 1 reply; 13+ messages in thread
From: Heiko Carstens @ 2008-04-01 11:02 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Russell King, Ingo Molnar, Christoph Hellwig, linux-kernel,
	Martin Schwidefsky, paulus, ralf, davem, lethal, jdike

On Tue, Mar 25, 2008 at 08:09:15PM +0100, Thomas Gleixner wrote:
> On Sun, 23 Mar 2008, Russell King wrote:
> 
> > On Sat, Mar 22, 2008 at 09:32:23PM +0100, Heiko Carstens wrote:
> > > Generic code is not supposed to include irq.h. Replace this include
> > > by linux/hardirq.h instead and add/replace an include of linux/irq.h
> > > in asm header files where necessary.
> > > This change should only matter for architectures that make use of
> > > GENERIC_CLOCKEVENTS.
> > > Architectures in question are mips, x86, arm, sh, powerpc, uml and sparc64.
> > > 
> > > I did some cross compile tests for mips, x86_64, arm, powerpc and sparc64.
> > > This patch fixes also build breakages caused by the include replacement in
> > > tick-common.h.
> > 
> > I generally dislike adding optional linux/* includes in asm/* includes -
> > I'm nervous about this causing include loops.
> > 
> > However, there's a separate point to be discussed here.
> > 
> > That is, what interfaces are expected of every architecture in the kernel.
> > If generic code wants to be able to set the affinity of interrupts, then
> > that needs to become part of the interfaces listed in linux/interrupt.h
> > rather than linux/irq.h.
> > 
> > So what I suggest is this approach instead (against Linus' tree of a
> > couple of days ago) - we move irq_set_affinity() and irq_can_set_affinity()
> > to linux/interrupt.h, change the linux/irq.h includes to linux/interrupt.h
> > and include asm/irq_regs.h where needed (asm/irq_regs.h is supposed to be
> > rarely used include since not much touches the stacked parent context
> > registers.)
> 
> That patch makes a lot of sense and resolves all the issues. 
> 
> I push it through hrt.git and add the GENERIC_HARDIRQS dependency
> which was requested by Heiko.

Thomas, would you mind if we push this patch via git-s390?
As far as I can see s390 is the only architecture which has a dependency on
this patch. This would make merging our NO_IDLE_HZ to GENERIC_CLOCKEVENTS
conversion easier.
I'd take the version you have in your mm branch in your hrt tree, if you're
ok with this.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: tick-common.c hack for s390 needed
  2008-04-01 11:02               ` Heiko Carstens
@ 2008-04-01 11:24                 ` Thomas Gleixner
  0 siblings, 0 replies; 13+ messages in thread
From: Thomas Gleixner @ 2008-04-01 11:24 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Russell King, Ingo Molnar, Christoph Hellwig, linux-kernel,
	Martin Schwidefsky, paulus, ralf, davem, lethal, jdike

On Tue, 1 Apr 2008, Heiko Carstens wrote:
> > That patch makes a lot of sense and resolves all the issues. 
> > 
> > I push it through hrt.git and add the GENERIC_HARDIRQS dependency
> > which was requested by Heiko.
> 
> Thomas, would you mind if we push this patch via git-s390?
> As far as I can see s390 is the only architecture which has a dependency on
> this patch. This would make merging our NO_IDLE_HZ to GENERIC_CLOCKEVENTS
> conversion easier.
> I'd take the version you have in your mm branch in your hrt tree, if you're
> ok with this.

Ok, I drop it.

Thanks,
	tglx
 

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2008-04-01 11:29 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-03-18  9:31 tick-common.c hack for s390 needed Heiko Carstens
2008-03-19  4:12 ` Arnd Bergmann
2008-03-19  5:45 ` Christoph Hellwig
2008-03-21 10:15   ` Ingo Molnar
2008-03-21 13:25     ` Heiko Carstens
2008-03-21 14:45       ` Thomas Gleixner
2008-03-22 20:32         ` Heiko Carstens
2008-03-23 22:34           ` Russell King
2008-03-24 10:14             ` Heiko Carstens
2008-03-25 19:09             ` Thomas Gleixner
2008-03-25 19:30               ` Russell King
2008-04-01 11:02               ` Heiko Carstens
2008-04-01 11:24                 ` Thomas Gleixner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).