Build breakage ...

Build breakage ...

[Ralf Baechle]

ee3ce191e8eaa4cc15c51a28b34143b36404c4f5 breaks MIPS completely:

In file included from include/linux/bitops.h:9,
                 from include/linux/thread_info.h:20,
                 from include/linux/preempt.h:9,
                 from include/linux/spinlock.h:49,
                 from include/linux/capability.h:45,
                 from include/linux/sched.h:46,
                 from arch/mips/kernel/asm-offsets.c:13:
include/asm/bitops.h: In function ‘set_bit’:
include/asm/bitops.h:100: warning: implicit declaration of function ‘BUILD_BUG_ON’
include/asm/bitops.h:100: warning: implicit declaration of function ‘typecheck’
include/asm/bitops.h:100: error: expected expression before ‘unsigned’
include/asm/bitops.h:102: error: expected expression before ‘unsigned’
include/asm/bitops.h: In function ‘clear_bit’:
include/asm/bitops.h:148: error: expected expression before ‘unsigned’
include/asm/bitops.h:150: error: expected expression before ‘unsigned’
include/asm/bitops.h: In function ‘change_bit’:
include/asm/bitops.h:198: error: expected expression before ‘unsigned’
include/asm/bitops.h:200: error: expected expression before ‘unsigned’

That sort of patches really should go to /dev/null so short before a release.

  Ralf

Re: Build breakage ...

[Russell King]

On Sun, Nov 26, 2006 at 10:49:28PM +0000, Ralf Baechle wrote:
> ee3ce191e8eaa4cc15c51a28b34143b36404c4f5 breaks MIPS completely:
> 
> In file included from include/linux/bitops.h:9,
>                  from include/linux/thread_info.h:20,
>                  from include/linux/preempt.h:9,
>                  from include/linux/spinlock.h:49,
>                  from include/linux/capability.h:45,
>                  from include/linux/sched.h:46,
>                  from arch/mips/kernel/asm-offsets.c:13:
> include/asm/bitops.h: In function ???set_bit???:
> include/asm/bitops.h:100: warning: implicit declaration of function ???BUILD_BUG_ON???
> include/asm/bitops.h:100: warning: implicit declaration of function ???typecheck???
> include/asm/bitops.h:100: error: expected expression before ???unsigned???
> include/asm/bitops.h:102: error: expected expression before ???unsigned???
> include/asm/bitops.h: In function ???clear_bit???:
> include/asm/bitops.h:148: error: expected expression before ???unsigned???
> include/asm/bitops.h:150: error: expected expression before ???unsigned???
> include/asm/bitops.h: In function ???change_bit???:
> include/asm/bitops.h:198: error: expected expression before ???unsigned???
> include/asm/bitops.h:200: error: expected expression before ???unsigned???
> 
> That sort of patches really should go to /dev/null so short before a release.

Ditto on ARM.  This level of breakage is simply not acceptable soo
close to a release, and needs the change reverting.

Note that on ARM, "allmodconfig" is really meaningless since it only
tests one configuration.  Ditto for the other all*config options.
kautobuild (or building a range of defconfigs) is the only real way
to check for breakage on ARM.

arch/arm/mach-sa1100/leds-assabet.c:34: warning: implicit declaration of function 'BUILD_BUG_ON'
arch/arm/mach-sa1100/leds-assabet.c:34: warning: implicit declaration of function 'typecheck'
arch/arm/mach-sa1100/leds-assabet.c:34: error: syntax error before 'unsigned'
arch/arm/mach-sa1100/leds-assabet.c:113: error: syntax error before 'unsigned'
arch/arm/mach-sa1100/leds-cerf.c:31: warning: implicit declaration of function 'BUILD_BUG_ON'
arch/arm/mach-sa1100/leds-cerf.c:31: warning: implicit declaration of function 'typecheck'
arch/arm/mach-sa1100/leds-cerf.c:31: error: syntax error before 'unsigned'
arch/arm/mach-sa1100/leds-cerf.c:109: error: syntax error before 'unsigned'
arch/arm/mach-sa1100/leds-hackkit.c:35: warning: implicit declaration of function 'BUILD_BUG_ON'
arch/arm/mach-sa1100/leds-hackkit.c:35: warning: implicit declaration of function 'typecheck'
arch/arm/mach-sa1100/leds-hackkit.c:35: error: syntax error before 'unsigned'
arch/arm/mach-sa1100/leds-hackkit.c:111: error: syntax error before 'unsigned'
arch/arm/mach-sa1100/leds-lart.c:34: warning: implicit declaration of function 'BUILD_BUG_ON'
arch/arm/mach-sa1100/leds-lart.c:34: warning: implicit declaration of function 'typecheck'
arch/arm/mach-sa1100/leds-lart.c:34: error: syntax error before 'unsigned'
arch/arm/mach-sa1100/leds-lart.c:100: error: syntax error before 'unsigned'
arch/arm/mach-pxa/leds-idp.c:36: warning: implicit declaration of function 'BUILD_BUG_ON'
arch/arm/mach-pxa/leds-idp.c:36: warning: implicit declaration of function 'typecheck'
arch/arm/mach-pxa/leds-idp.c:36: error: syntax error before 'unsigned'
arch/arm/mach-pxa/leds-idp.c:115: error: syntax error before 'unsigned'
arch/arm/mach-pxa/leds-lubbock.c:50: warning: implicit declaration of function 'BUILD_BUG_ON'
arch/arm/mach-pxa/leds-lubbock.c:50: warning: implicit declaration of function 'typecheck'
arch/arm/mach-pxa/leds-lubbock.c:50: error: syntax error before 'unsigned'
arch/arm/mach-pxa/leds-lubbock.c:124: error: syntax error before 'unsigned'
arch/arm/mach-pxa/leds-mainstone.c:45: warning: implicit declaration of function 'BUILD_BUG_ON'
arch/arm/mach-pxa/leds-mainstone.c:45: warning: implicit declaration of function 'typecheck'
arch/arm/mach-pxa/leds-mainstone.c:45: error: syntax error before 'unsigned'
arch/arm/mach-pxa/leds-mainstone.c:119: error: syntax error before 'unsigned'
arch/arm/mach-pxa/leds-trizeps4.c:39: warning: implicit declaration of function 'BUILD_BUG_ON'
arch/arm/mach-pxa/leds-trizeps4.c:39: warning: implicit declaration of function 'typecheck'
arch/arm/mach-pxa/leds-trizeps4.c:39: error: syntax error before 'unsigned'
arch/arm/mach-pxa/leds-trizeps4.c:132: error: syntax error before 'unsigned'


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

Re: Build breakage ...

[Linus Torvalds]

On Sun, 26 Nov 2006, Ralf Baechle wrote:
> 
> That sort of patches really should go to /dev/null so short before a release.

Yeah, I don't think it was worth it.

That said, Alexey did check it more than most patches like this get 
checked (ie checking allmodconfig on i386, x86_64, alpha, arm), so it's a 
bit unlucky that MIPS got bitten by this - it was not a badly tested 
patch per se.

Does the obvious fix (to include <linux/kernel.h> in irqflags.h) fix it 
for you?

		Linus

Re: Build breakage ...

[Russell King]

On Sun, Nov 26, 2006 at 11:05:16PM +0000, Russell King wrote:
> Ditto on ARM.  This level of breakage is simply not acceptable soo
> close to a release, and needs the change reverting.
> 
> Note that on ARM, "allmodconfig" is really meaningless since it only
> tests one configuration.  Ditto for the other all*config options.
> kautobuild (or building a range of defconfigs) is the only real way
> to check for breakage on ARM.

BTW, it should be pointed out that ARM has for the last I don't know
how many months been checking the type of "flags" passed into
local_irq_save() and friends...  If a generic solution is adopted
(during the merge phase _only_ please) then the ARM specific version
should probably be removed.

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

Re: Build breakage ...

[Linus Torvalds]

On Sun, 26 Nov 2006, Linus Torvalds wrote:
> 
> Does the obvious fix (to include <linux/kernel.h> in irqflags.h) fix it 
> for you?

Btw, Alexey, why did you do _both a BUILD_BUG_ON and a "typecheck()"?

If there are any broken users, we shouldn't break the build, but a 
_warning_ is certainly appropriate.

I think I'll just commit this..

Ralf, Russell, does this work for you guys?

		Linus
---
diff --git a/include/linux/irqflags.h b/include/linux/irqflags.h
index 4fe740b..8c5d9d1 100644
--- a/include/linux/irqflags.h
+++ b/include/linux/irqflags.h
@@ -11,11 +11,10 @@
 #ifndef _LINUX_TRACE_IRQFLAGS_H
 #define _LINUX_TRACE_IRQFLAGS_H
 
-#define BUILD_CHECK_IRQ_FLAGS(flags)					\
-	do {								\
-		BUILD_BUG_ON(sizeof(flags) != sizeof(unsigned long));	\
-		typecheck(unsigned long, flags);			\
-	} while (0)
+#include <linux/kernel.h>
+
+#define BUILD_CHECK_IRQ_FLAGS(flags) \
+	typecheck(unsigned long, flags)
 
 #ifdef CONFIG_TRACE_IRQFLAGS
   extern void trace_hardirqs_on(void);

Re: Build breakage ...

[Russell King]

On Sun, Nov 26, 2006 at 03:12:54PM -0800, Linus Torvalds wrote:
> 
> 
> On Sun, 26 Nov 2006, Linus Torvalds wrote:
> > 
> > Does the obvious fix (to include <linux/kernel.h> in irqflags.h) fix it 
> > for you?
> 
> Btw, Alexey, why did you do _both a BUILD_BUG_ON and a "typecheck()"?
> 
> If there are any broken users, we shouldn't break the build, but a 
> _warning_ is certainly appropriate.
> 
> I think I'll just commit this..
> 
> Ralf, Russell, does this work for you guys?

Not at all.  It creates even more problems for me, with this circular
dependency:

  linux/bitops.h -> asm-arm/bitops.h -> asm-arm/system.h
   -> linux/irqflags.h -> linux/kernel.h -> linux/bitops.h

We really need, as a priority, to sort out these include files ASAP,
and stop bunging random stuff into unrelated random headers.  Shouldn't
stuff like roundup_pow_of_two() and long_log2() live somewhere else
other than such a generic run-of-the-mill include such as linux/kernel.h ?

  CHK     include/linux/version.h
make[2]: `include/asm-arm/mach-types.h' is up to date.
  Using /home/rmk/git/linux-2.6-rmk as source for kernel
  GEN     /home/rmk/git/build/assabet/Makefile
  CHK     include/linux/utsrelease.h
  CC      arch/arm/kernel/asm-offsets.s
In file included from /home/rmk/git/linux-2.6-rmk/include/linux/irqflags.h:14,
                 from include2/asm/system.h:214,
                 from include2/asm/bitops.h:23,
                 from /home/rmk/git/linux-2.6-rmk/include/linux/bitops.h:9,
                 from /home/rmk/git/linux-2.6-rmk/include/linux/thread_info.h:20,
                 from /home/rmk/git/linux-2.6-rmk/include/linux/preempt.h:9,
                 from /home/rmk/git/linux-2.6-rmk/include/linux/spinlock.h:49,
                 from /home/rmk/git/linux-2.6-rmk/include/linux/capability.h:45,
                 from /home/rmk/git/linux-2.6-rmk/include/linux/sched.h:46,
                 from /home/rmk/git/linux-2.6-rmk/arch/arm/kernel/asm-offsets.c:13:
/home/rmk/git/linux-2.6-rmk/include/linux/kernel.h: In function `roundup_pow_of_two':
/home/rmk/git/linux-2.6-rmk/include/linux/kernel.h:169: warning: implicit declaration of function `fls_long'
In file included from /home/rmk/git/linux-2.6-rmk/include/linux/thread_info.h:20,
                 from /home/rmk/git/linux-2.6-rmk/include/linux/preempt.h:9,
                 from /home/rmk/git/linux-2.6-rmk/include/linux/spinlock.h:49,
                 from /home/rmk/git/linux-2.6-rmk/include/linux/capability.h:45,
                 from /home/rmk/git/linux-2.6-rmk/include/linux/sched.h:46,
                 from /home/rmk/git/linux-2.6-rmk/arch/arm/kernel/asm-offsets.c:13:
/home/rmk/git/linux-2.6-rmk/include/linux/bitops.h: At top level:
/home/rmk/git/linux-2.6-rmk/include/linux/bitops.h:57: error: conflicting types for 'fls_long'
/home/rmk/git/linux-2.6-rmk/include/linux/kernel.h:169: error: previous implicit declaration of 'fls_long' was here
make[2]: *** [arch/arm/kernel/asm-offsets.s] Error 1
make[1]: *** [prepare0] Error 2
make: *** [_all] Error 2

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

Re: Build breakage ...

[Kyle McMartin]

On Sun, Nov 26, 2006 at 10:49:28PM +0000, Ralf Baechle wrote:
> ee3ce191e8eaa4cc15c51a28b34143b36404c4f5 breaks MIPS completely:
> 

Ack on parisc.

[PARISC] Fix incorrent type of flags in <asm/semaphore.h>

[Kyle McMartin]

I still think using BUILD_BUG_ON() is unacceptable, especially given how
vague the error message was.

Signed-off-by: Kyle McMartin <kyle@parisc-linux.org>
---
diff --git a/include/asm-parisc/semaphore.h b/include/asm-parisc/semaphore.h
index c9ee41c..d45827a 100644
--- a/include/asm-parisc/semaphore.h
+++ b/include/asm-parisc/semaphore.h
@@ -115,7 +115,8 @@ extern __inline__ int down_interruptible
  */
 extern __inline__ int down_trylock(struct semaphore * sem)
 {
-	int flags, count;
+	unsigned long flags;
+	int count;
 
 	spin_lock_irqsave(&sem->sentry, flags);
 	count = sem->count - 1;
@@ -131,7 +132,8 @@ extern __inline__ int down_trylock(struc
  */
 extern __inline__ void up(struct semaphore * sem)
 {
-	int flags;
+	unsigned long flags;
+
 	spin_lock_irqsave(&sem->sentry, flags);
 	if (sem->count < 0) {
 		__up(sem);

Re: Build breakage ...

[Ralf Baechle]

On Sun, Nov 26, 2006 at 03:06:10PM -0800, Linus Torvalds wrote:

> That said, Alexey did check it more than most patches like this get 
> checked (ie checking allmodconfig on i386, x86_64, alpha, arm), so it's a 
> bit unlucky that MIPS got bitten by this - it was not a badly tested 
> patch per se.
> 
> Does the obvious fix (to include <linux/kernel.h> in irqflags.h) fix it 
> for you?

It changes the sympthoms:

  CC      arch/mips/kernel/asm-offsets.s
In file included from include/linux/irqflags.h:14,
                 from include/asm/bitops.h:34,
                 from include/linux/bitops.h:9,
                 from include/linux/thread_info.h:20,
                 from include/linux/preempt.h:9,
                 from include/linux/spinlock.h:49,
                 from include/linux/capability.h:45,
                 from include/linux/sched.h:46,
                 from arch/mips/kernel/asm-offsets.c:13:
include/linux/kernel.h: In function ‘roundup_pow_of_two’:
include/linux/kernel.h:169: warning: implicit declaration of function ‘fls_long’
In file included from include/linux/thread_info.h:20,
                 from include/linux/preempt.h:9,
                 from include/linux/spinlock.h:49,
                 from include/linux/capability.h:45,
                 from include/linux/sched.h:46,
                 from arch/mips/kernel/asm-offsets.c:13:
include/linux/bitops.h: At top level:
include/linux/bitops.h:57: error: conflicting types for ‘fls_long’
include/linux/kernel.h:169: error: previous implicit declaration of ‘fls_long’ was here

So the new problem is circular includes:

   ... <linux/bitops.h> -> <asm/bitops.h> -> <linux/irqflags.h> ->
  <linux/kernel.h> -> <linux/bitops.h> ...

include/asm-mips/bitops.h needs to include irqflags because some older
MIPS variants do not have any atomic instructions.  So the fix needs to
to break that loop.

  Ralf

Re: Build breakage ...

[Linus Torvalds]

On Sun, 26 Nov 2006, Russell King wrote:
> > 
> > Ralf, Russell, does this work for you guys?
> 
> Not at all.  It creates even more problems for me, with this circular
> dependency:

Ok. I just reverted it then. 

Pls verify that this is all good, and I didn't mess anything up due to the 
manual conflict resolution.

		Linus

---
commit b8e6ec865fd1d8838b6ce9516977b65e9f08f876
Author: Linus Torvalds <torvalds@woody.osdl.org>
Date:   Sun Nov 26 16:27:17 2006 -0800

    Revert "[PATCH] Enforce "unsigned long flags;" when spinlocking"
    
    This reverts commit ee3ce191e8eaa4cc15c51a28b34143b36404c4f5, since it
    broke on at least ARM, MIPS and PA-RISC due to complicated header file
    dependencies.
    
    Conflicts in include/linux/spinlock.h (due to the "nested" variety
    fixes) fixed up by hand.
    
    Cc: Alexey Dobriyan <adobriyan@gmail.com>
    Cc: Ralf Baechle <ralf@linux-mips.org>
    Cc: Kyle McMartin <kyle@parisc-linux.org>
    Cc: Russell King <rmk+lkml@arm.linux.org.uk>
    Signed-off-by: Linus Torvalds <torvalds@osdl.org>

diff --git a/include/linux/irqflags.h b/include/linux/irqflags.h
index 4fe740b..412e025 100644
--- a/include/linux/irqflags.h
+++ b/include/linux/irqflags.h
@@ -11,12 +11,6 @@
 #ifndef _LINUX_TRACE_IRQFLAGS_H
 #define _LINUX_TRACE_IRQFLAGS_H
 
-#define BUILD_CHECK_IRQ_FLAGS(flags)					\
-	do {								\
-		BUILD_BUG_ON(sizeof(flags) != sizeof(unsigned long));	\
-		typecheck(unsigned long, flags);			\
-	} while (0)
-
 #ifdef CONFIG_TRACE_IRQFLAGS
   extern void trace_hardirqs_on(void);
   extern void trace_hardirqs_off(void);
@@ -56,15 +50,10 @@
 #define local_irq_disable() \
 	do { raw_local_irq_disable(); trace_hardirqs_off(); } while (0)
 #define local_irq_save(flags) \
-	do {					\
-		BUILD_CHECK_IRQ_FLAGS(flags);	\
-		raw_local_irq_save(flags);	\
-		trace_hardirqs_off();		\
-	} while (0)
+	do { raw_local_irq_save(flags); trace_hardirqs_off(); } while (0)
 
 #define local_irq_restore(flags)				\
 	do {							\
-		BUILD_CHECK_IRQ_FLAGS(flags);			\
 		if (raw_irqs_disabled_flags(flags)) {		\
 			raw_local_irq_restore(flags);		\
 			trace_hardirqs_off();			\
@@ -80,16 +69,8 @@
  */
 # define raw_local_irq_disable()	local_irq_disable()
 # define raw_local_irq_enable()		local_irq_enable()
-# define raw_local_irq_save(flags)		\
-	do {					\
-		BUILD_CHECK_IRQ_FLAGS(flags);	\
-		local_irq_save(flags);		\
-	} while (0)
-# define raw_local_irq_restore(flags)		\
-	do {					\
-		BUILD_CHECK_IRQ_FLAGS(flags);	\
-		local_irq_restore(flags);	\
-	} while (0)
+# define raw_local_irq_save(flags)	local_irq_save(flags)
+# define raw_local_irq_restore(flags)	local_irq_restore(flags)
 #endif /* CONFIG_TRACE_IRQFLAGS_SUPPORT */
 
 #ifdef CONFIG_TRACE_IRQFLAGS_SUPPORT
@@ -99,11 +80,7 @@
 		raw_safe_halt();				\
 	} while (0)
 
-#define local_save_flags(flags)			\
-	do {					\
-		BUILD_CHECK_IRQ_FLAGS(flags);	\
-		raw_local_save_flags(flags);	\
-	} while (0)
+#define local_save_flags(flags)		raw_local_save_flags(flags)
 
 #define irqs_disabled()						\
 ({								\
@@ -113,11 +90,7 @@
 	raw_irqs_disabled_flags(flags);				\
 })
 
-#define irqs_disabled_flags(flags)	\
-({					\
-	BUILD_CHECK_IRQ_FLAGS(flags);	\
-	raw_irqs_disabled_flags(flags);	\
-})
+#define irqs_disabled_flags(flags)	raw_irqs_disabled_flags(flags)
 #endif		/* CONFIG_X86 */
 
 #endif
diff --git a/include/linux/spinlock.h b/include/linux/spinlock.h
index 57f670d..8451052 100644
--- a/include/linux/spinlock.h
+++ b/include/linux/spinlock.h
@@ -52,7 +52,6 @@
 #include <linux/thread_info.h>
 #include <linux/kernel.h>
 #include <linux/stringify.h>
-#include <linux/irqflags.h>
 
 #include <asm/system.h>
 
@@ -184,52 +183,24 @@ do {								\
 #define read_lock(lock)			_read_lock(lock)
 
 #if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
-#define spin_lock_irqsave(lock, flags)			\
-	do {						\
-		BUILD_CHECK_IRQ_FLAGS(flags);		\
-		flags = _spin_lock_irqsave(lock);	\
-	} while (0)
-#define read_lock_irqsave(lock, flags)			\
-	do {						\
-		BUILD_CHECK_IRQ_FLAGS(flags);		\
-		flags = _read_lock_irqsave(lock);	\
-	} while (0)
-#define write_lock_irqsave(lock, flags)			\
-	do {						\
-		BUILD_CHECK_IRQ_FLAGS(flags);		\
-		flags = _write_lock_irqsave(lock);	\
-	} while (0)
+
+#define spin_lock_irqsave(lock, flags)	flags = _spin_lock_irqsave(lock)
+#define read_lock_irqsave(lock, flags)	flags = _read_lock_irqsave(lock)
+#define write_lock_irqsave(lock, flags)	flags = _write_lock_irqsave(lock)
 
 #ifdef CONFIG_DEBUG_LOCK_ALLOC
-#define spin_lock_irqsave_nested(lock, flags, subclass)			\
-	do {								\
-		BUILD_CHECK_IRQ_FLAGS(flags);				\
-		flags = _spin_lock_irqsave_nested(lock, subclass);	\
-	} while (0)
+#define spin_lock_irqsave_nested(lock, flags, subclass) \
+	flags = _spin_lock_irqsave_nested(lock, subclass)
 #else
-#define spin_lock_irqsave_nested(lock, flags, subclass)			\
-	do {								\
-		BUILD_CHECK_IRQ_FLAGS(flags);				\
-		flags = _spin_lock_irqsave(lock);			\
-	} while (0)
+#define spin_lock_irqsave_nested(lock, flags, subclass) \
+	flags = _spin_lock_irqsave(lock)
 #endif
 
 #else
-#define spin_lock_irqsave(lock, flags)			\
-	do {						\
-		BUILD_CHECK_IRQ_FLAGS(flags);		\
-		_spin_lock_irqsave(lock, flags);	\
-	} while (0)
-#define read_lock_irqsave(lock, flags)			\
-	do {						\
-		BUILD_CHECK_IRQ_FLAGS(flags);		\
-		_read_lock_irqsave(lock, flags);	\
-	} while (0)
-#define write_lock_irqsave(lock, flags)			\
-	do {						\
-		BUILD_CHECK_IRQ_FLAGS(flags);		\
-		_write_lock_irqsave(lock, flags);	\
-	} while (0)
+
+#define spin_lock_irqsave(lock, flags)	_spin_lock_irqsave(lock, flags)
+#define read_lock_irqsave(lock, flags)	_read_lock_irqsave(lock, flags)
+#define write_lock_irqsave(lock, flags)	_write_lock_irqsave(lock, flags)
 #define spin_lock_irqsave_nested(lock, flags, subclass)	\
 	spin_lock_irqsave(lock, flags)
 
@@ -268,24 +239,15 @@ do {								\
 #endif
 
 #define spin_unlock_irqrestore(lock, flags) \
-	do {						\
-		BUILD_CHECK_IRQ_FLAGS(flags);		\
-		_spin_unlock_irqrestore(lock, flags);	\
-	} while (0)
+					_spin_unlock_irqrestore(lock, flags)
 #define spin_unlock_bh(lock)		_spin_unlock_bh(lock)
 
 #define read_unlock_irqrestore(lock, flags) \
-	do {						\
-		BUILD_CHECK_IRQ_FLAGS(flags);		\
-		_read_unlock_irqrestore(lock, flags);	\
-	} while (0)
+					_read_unlock_irqrestore(lock, flags)
 #define read_unlock_bh(lock)		_read_unlock_bh(lock)
 
 #define write_unlock_irqrestore(lock, flags) \
-	do {						\
-		BUILD_CHECK_IRQ_FLAGS(flags);		\
-		_write_unlock_irqrestore(lock, flags);	\
-	} while (0)
+					_write_unlock_irqrestore(lock, flags)
 #define write_unlock_bh(lock)		_write_unlock_bh(lock)
 
 #define spin_trylock_bh(lock)	__cond_lock(lock, _spin_trylock_bh(lock))
@@ -299,7 +261,6 @@ do {								\
 
 #define spin_trylock_irqsave(lock, flags) \
 ({ \
-	BUILD_CHECK_IRQ_FLAGS(flags); \
 	local_irq_save(flags); \
 	spin_trylock(lock) ? \
 	1 : ({ local_irq_restore(flags); 0; }); \

Re: Build breakage ...

[Russell King]

On Sun, Nov 26, 2006 at 04:29:00PM -0800, Linus Torvalds wrote:
> On Sun, 26 Nov 2006, Russell King wrote:
> > > 
> > > Ralf, Russell, does this work for you guys?
> > 
> > Not at all.  It creates even more problems for me, with this circular
> > dependency:
> 
> Ok. I just reverted it then. 
> 
> Pls verify that this is all good, and I didn't mess anything up due to the 
> manual conflict resolution.

That fixes the ARM assabet build, which should mean the other ARM builds
are also fixed.  Thanks.

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

Re: Build breakage ...

[Ralf Baechle]

On Sun, Nov 26, 2006 at 04:29:00PM -0800, Linus Torvalds wrote:

> > > Ralf, Russell, does this work for you guys?
> > 
> > Not at all.  It creates even more problems for me, with this circular
> > dependency:
> 
> Ok. I just reverted it then. 
> 
> Pls verify that this is all good, and I didn't mess anything up due to the 
> manual conflict resolution.

Thanks, 2ea5814472c3c910aed5c5b60f1f3b1000e353f1 builds again for MIPS.

If you deciede to put the patch back in after 2.6.19 I'm considering to
replace the local_irq_{save,restore} calls in the various atomic operations
in <asm/{atomic,bitops,system}.h with their raw_* equivalents.

What I dislike about Alexey's patch is that is finally tries to cast
unsigned long as the data type for the flags into stone.  The natural
data type to use on MIPS and several other architectures is a 32-bit
integer - or just a single bit on a stingy day ;-).  Time for flags_t
maybe?

  Ralf

Re: Build breakage ...

[Alexey Dobriyan]

On Mon, Nov 27, 2006 at 04:43:32PM +0000, Ralf Baechle wrote:
> On Sun, Nov 26, 2006 at 04:29:00PM -0800, Linus Torvalds wrote:
> > > > Ralf, Russell, does this work for you guys?
> > >
> > > Not at all.  It creates even more problems for me, with this circular
> > > dependency:
> >
> > Ok. I just reverted it then.
> >
> > Pls verify that this is all good, and I didn't mess anything up due to the
> > manual conflict resolution.
>
> Thanks, 2ea5814472c3c910aed5c5b60f1f3b1000e353f1 builds again for MIPS.
>
> If you deciede to put the patch back in after 2.6.19 I'm considering to
> replace the local_irq_{save,restore} calls in the various atomic operations
> in <asm/{atomic,bitops,system}.h with their raw_* equivalents.
>
> What I dislike about Alexey's patch is that is finally tries to cast
> unsigned long as the data type for the flags into stone.  The natural
> data type to use on MIPS and several other architectures is a 32-bit
> integer - or just a single bit on a stingy day ;-).  Time for flags_t
> maybe?

Hey, I've even posted RFC about that! IMO, flags_t is way too generic.

I can do

1) typedef unsigned long __bitwise__ irq_flags_t;
2) very core locking functions switched to irq_flags_t +
   additional small patch to keep level of compiler warnings the same
2) conversion to irq_flags_t: can be done slowly, only sparse sees new
   warnigns
3) irq_flags_t forked and became arch specific type
4) arch maintatiners choose better than "unsigned long" type if they want