[PATCH] lockdep: warn about lockdep disabling after kernel taint, fix

Previous thread: [RFC][PATCH] proc: export more page flags in /proc/kpageflags by Wu Fengguang on Monday, April 13, 2009 - 9:22 pm. (23 messages)

Next thread: [PATCH 1/3] v2 Make hierarchical RCU less IPI-happy by Paul E. McKenney on Monday, April 13, 2009 - 9:31 pm. (2 messages)
From: Stephen Rothwell
Date: Monday, April 13, 2009 - 9:43 pm

Hi all,

Today's linux-next build (sparc defconfig) failed like this:

In file included from kernel/panic.c:12:
include/linux/debug_locks.h: In function '__debug_locks_off':
include/linux/debug_locks.h:15: error: implicit declaration of function 'xchg'

Caused by commit 9eeba6138cefc0435695463ddadb0d95e0a6bcd2 ("lockdep: warn
about lockdep disabling after kernel taint").  xchg is defined in
asm/system.h on sparc. It looks like asm/atomic.h on 64bit sparc includes
asm/system.h, but not on 32bit.

Dave, arch/sparc/include/asm/atomic_32.h should really include
asm/system.h since xchg is used in there.

I have applied the following patch for today.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Tue, 14 Apr 2009 14:27:09 +1000
Subject: [PATCH] sparc: asm/atomic.h on 32bit should include asm/system.h for xchg

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 arch/sparc/include/asm/atomic_32.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/sparc/include/asm/atomic_32.h b/arch/sparc/include/asm/atomic_32.h
index ce46597..bb91b12 100644
--- a/arch/sparc/include/asm/atomic_32.h
+++ b/arch/sparc/include/asm/atomic_32.h
@@ -15,6 +15,8 @@
 
 #ifdef __KERNEL__
 
+#include <asm/system.h>
+
 #define ATOMIC_INIT(i)  { (i) }
 
 extern int __atomic_add_return(int, atomic_t *);
-- 
1.6.2.1

--

From: David Miller
Date: Tuesday, April 14, 2009 - 1:57 am

From: Stephen Rothwell <sfr@canb.auug.org.au>

That's true.

But atomic.h is not the proper place to obtain xchg() from, which is
asm/system.h, and that's what debug_locks.h needs to include if it
needs to use xchg().

This is why the s390 build broke in precisely the same way.
--

From: Heiko Carstens
Date: Tuesday, April 14, 2009 - 2:20 am

On Tue, 14 Apr 2009 01:57:27 -0700 (PDT)

Due to header include dependencies we can't include asm/system.h from
asm/atomic.h. So David's solution seems to be the right way.
How about this:

Subject: [PATCH] Fix xchg build breakage on s390.

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

In file included from kernel/panic.c:11:
include/linux/debug_locks.h: In function '__debug_locks_off':
include/linux/debug_locks.h:15: error: implicit declaration of function 'xchg'

Caused by 9eeba6138cefc0435695463ddadb0d95e0a6bcd2
"lockdep: warn about lockdep disabling after kernel taint"

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Reported-by: Sachin Sant <sachinp@in.ibm.com>
Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---

 include/linux/debug_locks.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6/include/linux/debug_locks.h
===================================================================
--- linux-2.6.orig/include/linux/debug_locks.h
+++ linux-2.6/include/linux/debug_locks.h
@@ -2,7 +2,7 @@
 #define __LINUX_DEBUG_LOCKING_H
 
 #include <linux/kernel.h>
-#include <asm/atomic.h>
+#include <asm/system.h>
 
 struct task_struct;
 
--

From: David Miller
Date: Tuesday, April 14, 2009 - 2:08 am

From: Stephen Rothwell <sfr@canb.auug.org.au>

Just in case it wasn't clear, I'm applying this :)
--

From: Stephen Rothwell
Date: Tuesday, April 14, 2009 - 3:26 am

Hi Dave,


Ah, ok, thanks :-)

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
From: Ingo Molnar
Date: Tuesday, April 14, 2009 - 2:15 am

I dont think Sparc is at fault here - system.h is where we 
historically provided xchg() from. Then as atomic.h was introduced 
and extended, some architectures (x86 amongst them) gradually 
migrated their definition of xchg() into atomic.h and related helper 
headers. (while still making it available for system.h)

This opened up the path for someone eventually only including 
asm/atomic.h from generic code and breaking the architectures that 
only provide it via system.h. So i've queued up the fix below - this 
should address the problem on Sparc and s390.

So moving it to atomic.h does make sense - but that's no excuse for 
breaking the build. Sorry about this!

	Ingo

------------->
From f99a7c31f427f8f783c7b1379755f91b59acaf72 Mon Sep 17 00:00:00 2001
From: Ingo Molnar <mingo@elte.hu>
Date: Tue, 14 Apr 2009 11:03:12 +0200
Subject: [PATCH] lockdep: warn about lockdep disabling after kernel taint, fix

Impact: build fix

Stephen Rothwell reported that the Sparc build broke:

 In file included from kernel/panic.c:12:
 include/linux/debug_locks.h: In function '__debug_locks_off':
 include/linux/debug_locks.h:15: error: implicit declaration of function 'xchg'

due to:

 9eeba61: lockdep: warn about lockdep disabling after kernel taint

There is some inconsistency between architectures about where exactly
xchg() is defined. Most have it in system.h but also in atomic.h (which
is arguably the more logical point for it). Some, such as Sparc only
have it in asm/system.h and not available via asm/atomic.h.

Use the widest set of headers in debug_locks.h and also include asm/system.h.

Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 include/linux/debug_locks.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/linux/debug_locks.h b/include/linux/debug_locks.h
index 493dedb..29b3ce3 100644
--- a/include/linux/debug_locks.h
+++ ...
From: Heiko Carstens
Date: Tuesday, April 14, 2009 - 2:23 am

On Tue, 14 Apr 2009 11:15:38 +0200

Ah, ok. Please ignore my other mail from a few seconds ago ;)
This fixes the s390 build break. Thanks!
--

From: Stephen Rothwell
Date: Tuesday, April 14, 2009 - 3:29 am

Hi Ingo,


Thanks.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
From: tip-bot for Ingo Molnar
Date: Tuesday, April 14, 2009 - 2:15 am

Commit-ID:  27b19565fe4ca5b0e9d2ae98ce4b81ca728bf445
Gitweb:     http://git.kernel.org/tip/27b19565fe4ca5b0e9d2ae98ce4b81ca728bf445
Author:     Ingo Molnar <mingo@elte.hu>
AuthorDate: Tue, 14 Apr 2009 11:03:12 +0200
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Tue, 14 Apr 2009 11:11:52 +0200

lockdep: warn about lockdep disabling after kernel taint, fix

Impact: build fix for Sparc and s390

Stephen Rothwell reported that the Sparc build broke:

 In file included from kernel/panic.c:12:
 include/linux/debug_locks.h: In function '__debug_locks_off':
 include/linux/debug_locks.h:15: error: implicit declaration of function 'xchg'

due to:

 9eeba61: lockdep: warn about lockdep disabling after kernel taint

There is some inconsistency between architectures about where exactly
xchg() is defined.

The traditional place is in system.h but the more logical point for it
is in atomic.h - where most architectures (especially new ones) have
it defined. These architecture also still offer it via system.h.

Some, such as Sparc or s390 only have it in asm/system.h and not available
via asm/atomic.h at all.

Use the widest set of headers in debug_locks.h and also include asm/system.h.

Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
LKML-Reference: <20090414144317.026498df.sfr@canb.auug.org.au>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 include/linux/debug_locks.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/linux/debug_locks.h b/include/linux/debug_locks.h
index 493dedb..29b3ce3 100644
--- a/include/linux/debug_locks.h
+++ b/include/linux/debug_locks.h
@@ -3,6 +3,7 @@
 
 #include <linux/kernel.h>
 #include <asm/atomic.h>
+#include <asm/system.h>
 
 struct task_struct;
 
--

From: Frederic Weisbecker
Date: Tuesday, April 14, 2009 - 4:45 am

I'm sorry about this build breakage...

I've grep'ed the definitions of xchg() for this pach and indeed remember
that some architectures defined it in asm/atomic.h and
others in asm/system.h

And I've also seen some of them including system.h from
atomic.h so I thought it was safe but.. heh...

--

Previous thread: [RFC][PATCH] proc: export more page flags in /proc/kpageflags by Wu Fengguang on Monday, April 13, 2009 - 9:22 pm. (23 messages)

Next thread: [PATCH 1/3] v2 Make hierarchical RCU less IPI-happy by Paul E. McKenney on Monday, April 13, 2009 - 9:31 pm. (2 messages)