Hello Steven,
A while ago I already mentioned that 2.6.25 did not compile on ARM.
(e.g. AT91SAM9261-EK)
Today I looked at it again with the latest 2.6.25 kernel with latest RT-patch.
Several errors are gone, but I still I get these warnings on every
file that is being compiled:
-----------------------------------------------------------------
CC arch/arm/kernel/asm-offsets.s
In file included from include/linux/rt_lock.h:14,
from include/linux/spinlock.h:117,
from include/linux/seqlock.h:29,
from include/linux/time.h:8,
from include/linux/timex.h:57,
from include/linux/sched.h:54,
from arch/arm/kernel/asm-offsets.c:13:
include/asm/atomic.h:241:1: warning: "cmpxchg" redefined
In file included from include/asm/system.h:378,
from include/asm/bitops.h:27,
from include/linux/bitops.h:17,
from include/linux/kernel.h:15,
from include/linux/sched.h:52,
from arch/arm/kernel/asm-offsets.c:13:
include/asm-generic/cmpxchg.h:19:1: warning: this is the location of
the previous definition
...
and many more...
...
-----------------------------------------------------------------
I looked at the broken-out series which patches are causing these
warnings and these are the offending patches:
* arm-fix-atomic-cmpxchg.patch
* arm-cmpxchg.patch
When these patches are removed from the Preempt-RT patchset everything
compiles again, without warnings.
But these patches are probably there for a reason, and leaving them
out could break some things. (Although I believe that these patches
are just there because they apply on 2.6.25-RT)
But, Here I need some (RT-mutex-expert) review help.
Because this ARM architecture (ARM <= V5) does not have a real
cmpxchg() assembler
instruction, and the implementation is almost the same as the generic
definition, the generic definition can be used instead.
However, The generic implementation does NOT define __HAVE_ARCH_CMPXCHG.
My questions are:
* What are the results of NOT defining __HAVE_ARCH_CMPXCHG in
kernel/rtmutex.c ?
* Does the RT-mutex still behave correctly?
* Why is this check in rtmutex.c ? Is it really needed, because there
is always a generic implementation of cmpxchg() these days?
FURTHER: The patch arm-fix-atomic-cmpxchg.patch made the
local_irq_save() a raw_local_irq_save().
Looking at the generic implementation I believe that this code should
also use the raw_* types. I attached a new patch for this to this
mail.
So, attached 3 patches:
* revert-arm-fix-atomic-cmpxchg.patch (reverts the same named patch in
preempt-RT set)
* revert-arm-cmpxchg.patch (reverts the same named patch in preempt-RT set)
* make-generic-cmpxchg-use-raw-intlock-types.patch
Can you clear some things up here? And also repair the RT-patch for
ARM and 2.6.25 ;-) (and newer)
Kind regards,
Remy
Make generic cmpxchg() use raw_local_irq_save()
The generic routines for the cmpxchg() code path should use the raw_*
types on preempt-RT.
Signed-off-by: Remy Bohmer <linux@xxxxxxxxxx>
---
include/asm-generic/cmpxchg-local.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
Index: linux-2.6.25.11/include/asm-generic/cmpxchg-local.h
===================================================================
--- linux-2.6.25.11.orig/include/asm-generic/cmpxchg-local.h 2008-07-13 20:00:25.000000000 +0200
+++ linux-2.6.25.11/include/asm-generic/cmpxchg-local.h 2008-07-16 19:38:34.000000000 +0200
@@ -20,7 +20,7 @@ static inline unsigned long __cmpxchg_lo
if (size == 8 && sizeof(unsigned long) != 8)
wrong_size_cmpxchg(ptr);
- local_irq_save(flags);
+ raw_local_irq_save(flags);
switch (size) {
case 1: prev = *(u8 *)ptr;
if (prev == old)
@@ -41,7 +41,7 @@ static inline unsigned long __cmpxchg_lo
default:
wrong_size_cmpxchg(ptr);
}
- local_irq_restore(flags);
+ raw_local_irq_restore(flags);
return prev;
}
@@ -54,11 +54,11 @@ static inline u64 __cmpxchg64_local_gene
u64 prev;
unsigned long flags;
- local_irq_save(flags);
+ raw_local_irq_save(flags);
prev = *(u64 *)ptr;
if (prev == old)
*(u64 *)ptr = new;
- local_irq_restore(flags);
+ raw_local_irq_restore(flags);
return prev;
}
Revert the patch named arm-cmpxchg.patch in the preempt-rt patchset
Currently the patch arm-cmpxchg.patch is part of the preempt-rt
patchset. Although it applies correctly, it introduces now a 2nd
implementation of the cmpxchg() macro, and thus a compile error.
Because this architecture does not have a real cmpxchg() assembler
instruction, and the implementation is almost the same as the generic
definition, the generic definition is now used instead.
BUT: the __HAVE_ARCH_CMPXCHG is not set any longer, because it is not
set in the generic implementation. Some review is required
for the kernel/rtmutex.c code to make sure the code still
behaves well without this flag set.
I have only compile tested this patch!
I have not confronted any hardware with this patch yet.
**** REVIEW REQUIRED on rtmutex code! ****
NOTE: Instead of applying this patch, it is even better to remove
the arm-cmpxchg.patch from the preempt-rt set because this patch
only reverts that patch.
Signed-off-by: Remy Bohmer <linux@xxxxxxxxxx>
---
include/asm-arm/atomic.h | 35 -----------------------------------
1 file changed, 35 deletions(-)
Index: linux-2.6.25.11/include/asm-arm/atomic.h
===================================================================
--- linux-2.6.25.11.orig/include/asm-arm/atomic.h 2008-07-16 17:14:01.000000000 +0200
+++ linux-2.6.25.11/include/asm-arm/atomic.h 2008-07-16 17:15:15.000000000 +0200
@@ -213,41 +213,6 @@ static inline void atomic_clear_mask(uns
raw_local_irq_restore(flags);
}
-#ifndef CONFIG_SMP
-/*
- * Atomic compare and exchange.
- */
-#define __HAVE_ARCH_CMPXCHG 1
-
-extern unsigned long wrong_size_cmpxchg(volatile void *ptr);
-
-static inline unsigned long __cmpxchg(volatile void *ptr,
- unsigned long old,
- unsigned long new, int size)
-{
- unsigned long flags, prev;
- volatile unsigned long *p = ptr;
-
- if (size == 4) {
- local_irq_save(flags);
- if ((prev = *p) == old)
- *p = new;
- local_irq_restore(flags);
- return(prev);
- } else
- return wrong_size_cmpxchg(ptr);
-}
-
-#define cmpxchg(ptr,o,n) \
-({ \
- __typeof__(*(ptr)) _o_ = (o); \
- __typeof__(*(ptr)) _n_ = (n); \
- (__typeof__(*(ptr))) __cmpxchg((ptr), (unsigned long)_o_, \
- (unsigned long)_n_, sizeof(*(ptr))); \
-})
-
-#endif
-
#endif /* __LINUX_ARM_ARCH__ */
#define atomic_xchg(v, new) (xchg(&((v)->counter), new))
Revert patch arm-fix-atomic-cmpxchg.patch in preempt-rt patchset
To be able to revert the patch arm-cmpxchg.patch this patch has to be
reverted also.
(Actually this change has been moved to the include/asm-generic/cmpxchg-local.h
through another patch)
Signed-off-by: Remy Bohmer <linux@xxxxxxxxxx>
---
include/asm-arm/atomic.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Index: linux-2.6.25.11/include/asm-arm/atomic.h
===================================================================
--- linux-2.6.25.11.orig/include/asm-arm/atomic.h 2008-07-16 17:08:42.000000000 +0200
+++ linux-2.6.25.11/include/asm-arm/atomic.h 2008-07-16 17:14:01.000000000 +0200
@@ -229,10 +229,10 @@ static inline unsigned long __cmpxchg(vo
volatile unsigned long *p = ptr;
if (size == 4) {
- raw_local_irq_save(flags);
+ local_irq_save(flags);
if ((prev = *p) == old)
*p = new;
- raw_local_irq_restore(flags);
+ local_irq_restore(flags);
return(prev);
} else
return wrong_size_cmpxchg(ptr);
