Skip to contentSkip to conversation

ppc RT: Realtime preempt support for PPC

PATCH In review Frank Rowand 3 participants 9 messages Mar 7, 2005 – Apr 5, 2005

lore
Share & tools
lore
#0 lore raw

cover ppc RT: Realtime preempt support for PPC

Ingo,

I have updated my patch of 16 Feb 2005 that adds realtime support for PowerPC (32 bit only). The patches will be sent in following emails.

The patch series applies over Ingo's real-time preempt patch:

http://people.redhat.com/mingo/realtime-preempt/realtime-preempt-2.6.11-rc4-V0.7.39-02

The patches are:

1/5 ppc_rt.patch - the core realtime functionality for PPC

2/5 ppc_cpu_khz.patch - This is separated out from the first patch to
reduce the noise in the first patch. It defines cpu_khz, which is needed by usecs_to_cyles() in kernel/latency.c.

3/5 ppc_mcount.patch - Implements mcount() and _mcount() for performance
instrumentation.

The following two patches should not be applied by anyone unless they want to test the other three patches on the IBM PPC 405GP eval board:

4/5 ibm_emac_core.patch - This patch has been separately submitted to PPC.

5/5 smp405.patch - a hack to allow compiling a PPC 405 kernel with
CONFIG_SMP=y

The changes from the previous patch are:

Added #ifdef CONFIG_PPC32 to raw_rwlock_t in include/linux/rt_lock.h. The ppc lock is a signed int instead of an unsigned long.

Replaced us_to_tb with cpu_khz to more consistent with i386 and to remove the #ifdef CONFIG_PPC in usecs_to_cycles().

Added mcount() and _mcount() that I previously said was a work in progress.

Added missing underscores in arch/ppc/lib/locks.c for __raw_read_trylock() and __raw_read_lock().

Changed tlb->mm to tlb_mm(tlb) in include/asm-ppc/tlb.h.

Reverted change of dma_spin_lock in include/asm-ppc/dma.h from raw_spinlock_t to spinlock_t to be consistent with kernel/dma.c.

This patch has been tested on the IBM PPC 405GP eval board only. You might notice that this board is not an SMP board. A few small changes were required to build an SMP kernel, even though only one cpu is actually active.

The testing has included a short (~ten minute) stress test for sixteen configuration permutations. The config files for the permutations are in the attached tar in the following directories:

instrumentation off instrumentation on
---------------------- ----------------------

SMP=n SMP=y SMP=n SMP=y
---------- ---------- ---------- ----------

 PREEMPT_NONE          pn          pn_smp          lat_pn      lat_pn_smp
 PREEMPT_VOLUNTARY     pv          pv_smp          lat_pv      lat_pv_smp
 PREEMPT_DESKTOP       pd          pd_smp          lat_pd      lat_pd_smp
 PREEMPT_RT            pr          pr_smp          lat_pr      lat_pr_smp

Any comments, suggestions, needed changes, etc are welcome.

Thanks,

-Frank

Signature · 2 lines
Frank Rowand <[email protected]>
MontaVista Software, Inc
#1 lore raw

hi Frank - sorry about the late reply, was busy with other things. Your ppc patches look mostly mergeable, with some small details still open:

Quoted 3 lines · Frank Rowand

The patches are:

1/5 ppc_rt.patch - the core realtime functionality for PPC

what is the rationale behind the rt_lock.h changes? The #ifdef CONFIG_PPC32 changes in generic code are not really acceptable, the -RT tree tries to keep a single spinlock definition and debugging primitives, across all architectures.

to drive things forward, i've applied the first 3 patches (except the rt_lock.h chunk from the first patch), and released it as part of the 40-03 patch:

http://redhat.com/~mingo/realtime-preempt/

so that you can send followup patches based on this. Patches #4 and #5 are routed via the upstream PPC tree, so -RT should not carry them, right?

Ingo

#2 lore raw
Quoted 16 lines · Ingo Molnar

hi Frank - sorry about the late reply, was busy with other things. Your ppc patches look mostly mergeable, with some small details still open:

Quoted 3 lines · Frank Rowand

The patches are:

1/5 ppc_rt.patch - the core realtime functionality for PPC

what is the rationale behind the rt_lock.h changes? The #ifdef CONFIG_PPC32 changes in generic code are not really acceptable, the -RT tree tries to keep a single spinlock definition and debugging primitives, across all architectures.

Quoted 6 lines

to drive things forward, i've applied the first 3 patches (except the rt_lock.h chunk from the first patch), and released it as part of the 40-03 patch:

http://redhat.com/~mingo/realtime-preempt/

Is no one else having trouble compiling this one? The attached one liner reverses a one line in the above patch.

kr

include/linux/jbd.h renamed from include/linux/jbd.h.orig +1 -0
@@ -65,6 +65,7 @@ extern int journal_enable_debug;
 		}							\
 	} while (0)
 #else
+#define jbd_debug(f, a...)   /**/
 #endif
 
 extern void * __jbd_kmalloc (const char *where, size_t size, int flags, int retry);
#3 lore raw
Quoted 9 lines · K.R. Foley
Quoted 6 lines

to drive things forward, i've applied the first 3 patches (except the rt_lock.h chunk from the first patch), and released it as part of the 40-03 patch:

http://redhat.com/~mingo/realtime-preempt/

Is no one else having trouble compiling this one? The attached one liner reverses a one line in the above patch.

yeah - a leftover of the ext3 patches. I've added your fix to 40-04.

Ingo

#4 lore raw
In reply to Ingo Molnar
Quoted 1 line · Ingo Molnar

hi Frank - sorry about the late reply, was busy with other things. Your

My turn to be late, but now I'm back from vacation :-).

Quoted 7 lines

ppc patches look mostly mergeable, with some small details still open:

Quoted 2 lines · Frank Rowand

The patches are:

Quoted 7 lines
Quoted 1 line

1/5 ppc_rt.patch - the core realtime functionality for PPC

what is the rationale behind the rt_lock.h changes? The #ifdef CONFIG_PPC32 changes in generic code are not really acceptable, the -RT tree tries to keep a single spinlock definition and debugging primitives, across all architectures.

The first "#ifdef CONFIG_PPC32" adds some debug fields to raw_spinlock_t. The debug fields are only used in the pre-existing arch/ppc/lib/locks.c, and are disabled unless CONFIG_DEBUG_SPINLOCK is defined. CONFIG_DEBUG_SPINLOCK is commented out in lib/Kconfig.debug as "# broken, disable for now". If CONFIG_DEBUG_SPINLOCK is ever enabled it will quickly become apparent that there are conflicts between arch/ppc/lib/locks.c and rt_lock.h (compile errors will occur) and the debug fields can easily be removed from locks.c.
Thus it is OK that you left this chunk out of your patch.

The second "#ifdef CONFIG_PPC32" is in raw_rwlock_t, making the lock field signed instead of unsigned. The PPC code uses the value of -1 to mean write locked, 0 to mean unlocked, and >0 to mean read locked. With one minor exception, all of the PPC raw_rwlock_t related code will work properly with an unsigned type because the code that checks the value of lock is assembly and treats lock as signed. The one non-assembly code that examines lock as a signed object is in arch/ppc/lib/locks.c and is disabled unless CONFIG_DEBUG_SPINLOCK is defined. If CONFIG_DEBUG_SPINLOCK is ever enabled this will be very evident as each call to __raw_write_unlock() will result in a printk() warning. The strongest reason I could advance for making lock signed for PPC32 is that any future C code that checks for a lock value less than zero will not function correctly and might not be very obvious.
Thus it is also OK that you left this chunk out of your patch.

Quoted 10 lines

to drive things forward, i've applied the first 3 patches (except the rt_lock.h chunk from the first patch), and released it as part of the 40-03 patch:

http://redhat.com/~mingo/realtime-preempt/

so that you can send followup patches based on this. Patches #4 and #5 are routed via the upstream PPC tree, so -RT should not carry them, right?

Yes, -RT should not carry them. Patch #4 went in via the PPC tree. Patch #5 is a temporary hack that is only for someone with an IBM 405gp reference platform who might want to test real-time with an SMP turned on.

-Frank

Signature · 2 lines
Frank Rowand <[email protected]>
MontaVista Software, Inc
#5 lore raw
Quoted 23 lines · Frank Rowand
Quoted 1 line · Ingo Molnar

hi Frank - sorry about the late reply, was busy with other things. Your

My turn to be late, but now I'm back from vacation :-).

Quoted 15 lines

ppc patches look mostly mergeable, with some small details still open:

Quoted 3 lines · Frank Rowand

The patches are:

1/5 ppc_rt.patch - the core realtime functionality for PPC

what is the rationale behind the rt_lock.h changes? The #ifdef CONFIG_PPC32 changes in generic code are not really acceptable, the -RT tree tries to keep a single spinlock definition and debugging primitives, across all architectures.

< stuff deleted >

Quoted 7 lines

The second "#ifdef CONFIG_PPC32" is in raw_rwlock_t, making the lock field signed instead of unsigned. The PPC code uses the value of -1 to mean write locked, 0 to mean unlocked, and >0 to mean read locked. With one minor exception, all of the PPC raw_rwlock_t related code will work properly with an unsigned type because the code that checks the value of lock is assembly and treats lock as signed. The one non-assembly code that examines lock as a signed object is in

Quoted 8 lines

arch/ppc/lib/locks.c and is disabled unless CONFIG_DEBUG_SPINLOCK is defined. If CONFIG_DEBUG_SPINLOCK is ever enabled this will be very evident as each call to __raw_write_unlock() will result in a printk() warning. The strongest reason I could advance for making lock signed for PPC32 is that any future C code that checks for a lock value less than zero will not function correctly and might not be very obvious.
Thus it is also OK that you left this chunk out of your patch.

< more stuff deleted >

I'm working on the architecture support for realtime on PPC64 now. If the lock field of struct raw_rwlock_t is a long instead of int then /proc/meminfo shows MemFree decreasing from 485608 kB to 485352 kB.

Do you have a preference for lock to be long instead of int?

Do you know if any of the other 64 bit architectures would have an issue with int?

-Frank

Signature · 2 lines
Frank Rowand <[email protected]>
MontaVista Software, Inc
#6 lore raw
Quoted 10 lines · Frank Rowand

< more stuff deleted >

I'm working on the architecture support for realtime on PPC64 now. If the lock field of struct raw_rwlock_t is a long instead of int then /proc/meminfo shows MemFree decreasing from 485608 kB to 485352 kB.

Do you have a preference for lock to be long instead of int?

Do you know if any of the other 64 bit architectures would have an issue with int?

that would be nice to know. I have no preference, other than if possible it should be unified, no #ifdefs or other conditionals.

Ingo

#7 lore raw
Quoted 7 lines · Ingo Molnar
Quoted 4 lines · Frank Rowand

< more stuff deleted >

I'm working on the architecture support for realtime on PPC64 now. If the lock field of struct raw_rwlock_t is a long instead of int then /proc/meminfo shows MemFree decreasing from 485608 kB to 485352 kB.

Quoted 8 lines
Quoted 3 lines

Do you have a preference for lock to be long instead of int?

Do you know if any of the other 64 bit architectures would have an issue with int?

that would be nice to know. I have no preference, other than if possible it should be unified, no #ifdefs or other conditionals.

Ingo

I looked at all the architectures and found that the disparity of the type of the "lock" field in struct rwlock_t is even larger than I had indicated in my earlier email. I am attaching a proof of concept patch to handle this. If this looks like a good method to you then I will create a real patch against your current patch, and include i386, mips, x86_64, and ppc.

include/linux/rt_lock.h +2 -1
Index: linux-2.6.10/include/linux/rt_lock.h
===================================================================
@@ -37,8 +37,9 @@ typedef struct {
  * but only one writer.
  */
 #ifdef CONFIG_SMP
+#include <asm/raw_spinlock.h>
 typedef struct {
-       volatile unsigned long lock;
+       ARCH_RAW_RWLOCK_LOCK
 # ifdef CONFIG_DEBUG_SPINLOCK
        unsigned magic;
 # endif
include/asm-ppc/raw_spinlock.h new file +6 -0
Index: linux-2.6.10/include/asm-ppc/raw_spinlock.h
===================================================================
@@ -0,0 +1,6 @@
+#ifndef __ASM_RAW_SPINLOCK_H
+#define __ASM_RAW_SPINLOCK_H
+
+#define ARCH_RAW_RWLOCK_LOCK volatile signed int lock;
+
+#endif /* __ASM_RAW_SPINLOCK_H */
include/asm-i386/raw_spinlock.h new file +6 -0
Index: linux-2.6.10/include/asm-i386/raw_spinlock.h
===================================================================
@@ -0,0 +1,6 @@
+#ifndef __ASM_RAW_SPINLOCK_H
+#define __ASM_RAW_SPINLOCK_H
+
+#define ARCH_RAW_RWLOCK_LOCK volatile unsigned int lock;
+
+#endif /* __ASM_RAW_SPINLOCK_H */

For the curious, the implementations of the "lock" field are listed below:

rwlock_t lock implemented as a spinlock field and a counter field

include/asm-parisc/spinlock.h
include/asm-sh/spinlock.h

rwlock_t lock implemented as a 31 bit counter field and a 1 bit write lock field
include/asm-ia64/spinlock.h

rwlock_t lock implemented as a 1 bit write lock field and a 31 bit counter field
include/asm-alpha/spinlock.h

rwlock_t lock implemented as an unsigned int field

include/asm-arm/spinlock.h include/asm-i386/spinlock.h include/asm-mips/spinlock.h include/asm-x86_64/spinlock.h include/asm-sparc/spinlock.h include/asm-sparc64/spinlock.h #ifdef CONFIG_DEBUG_SPINLOCK

rwlock_t lock implemented as a signed int field

include/asm-m32r/spinlock.h
include/asm-ppc64/spinlock.h
include/asm-ppc/spinlock.h

rwlock_t lock implemented as an unsigned long field

include/asm-s390/spinlock.h

rwlock_t lock implemented as an unsigned int (_not_ in a structure)

include/asm-sparc64/spinlock.h #ifdef CONFIG_DEBUG_SPINLOCK

SMP rwlock_t not supported (no SMP support)

include/asm-arm26/spinlock.h
include/asm-frv/spinlock.h
include/asm-h8300/spinlock.h
include/asm-m68knommu/spinlock.h
include/asm-m68k/spinlock.h
include/asm-sh64/spinlock.h

-Frank

Signature · 2 lines
Frank Rowand <[email protected]>
MontaVista Software, Inc
#8 lore raw

include/linux/rt_lock.h typedef struct { · line 42

Quoted 12 lines · Frank Rowand

I looked at all the architectures and found that the disparity of the type of the "lock" field in struct rwlock_t is even larger than I had indicated in my earlier email. I am attaching a proof of concept patch to handle this. If this looks like a good method to you then I will create a real patch against your current patch, and include i386, mips, x86_64, and ppc.

include/linux/rt_lock.h +2 -1
+#include <asm/raw_spinlock.h>
 
 typedef struct {
-       volatile unsigned long lock;
+       ARCH_RAW_RWLOCK_LOCK

maybe the simplest method would be to let architectures to define the raw spinlock type after all. I was hoping to standardize things across all architectures, but maybe it's not possible.

Ingo