Preempt on PowerPC/SMP appears to leak memory

Hi:

I recently applied the preempt-kernel-rml-2.4.21-pre1-1.patch from kernel.org to BenH's stable tree from rsync.penguinppc.org. With the following modification, I was able to get a preemptive kernel running on a dual 1Ghz PowerMac in SMP mode:

diff -urN linux-2.4.20-preempt/include/asm-ppc/spinlock.h linux-2.4.20-dev/include/asm-ppc/spinlock.h
@@ -49,7 +49,7 @@
 
 #else /* ! SPINLOCK_DEBUG */
 
-static inline void spin_lock(spinlock_t *lock)
+static inline void _raw_spin_lock(spinlock_t *lock)
 {
 	unsigned long tmp;
 

@@ -69,13 +69,13 @@
 	: "cr0", "memory");
 }
 
-static inline void spin_unlock(spinlock_t *lock)
+static inline void _raw_spin_unlock(spinlock_t *lock)
 {
 	__asm__ __volatile__("eieio		# spin_unlock": : :"memory");
 	lock->lock = 0;
 }
 
-#define spin_trylock(lock) (!test_and_set_bit(0,(lock)))
+#define _raw_spin_trylock(lock) (!test_and_set_bit(0,(lock)))
 
 #endif
 

@@ -119,7 +119,7 @@
 
 #else /* ! SPINLOCK_DEBUG */
 
-static __inline__ void read_lock(rwlock_t *rw)
+static __inline__ void _raw_read_lock(rwlock_t *rw)
 {
 	unsigned int tmp;
 

@@ -139,7 +139,7 @@
 	: "cr0", "memory");
 }
 
-static __inline__ void read_unlock(rwlock_t *rw)
+static __inline__ void _raw_read_unlock(rwlock_t *rw)
 {
 	unsigned int tmp;
 

@@ -154,7 +154,7 @@
 	: "cr0", "memory");
 }
 
-static __inline__ void write_lock(rwlock_t *rw)
+static __inline__ void _raw_write_lock(rwlock_t *rw)
 {
 	unsigned int tmp;
 

@@ -174,7 +174,7 @@
 	: "cr0", "memory");
 }
 
-static __inline__ void write_unlock(rwlock_t *rw)
+static __inline__ void _raw_write_unlock(rwlock_t *rw)
 {
 	__asm__ __volatile__("eieio		# write_unlock": : :"memory");
 	rw->lock = 0;

Unfortunately, something appears to be leaking memory across forks. Running while /bin/true; do /bin/echo -n; done manages to trigger the OOM killer within 2 minutes. Under normal load (make -j3 and gcc), the machine takes over an hour to leak away its 256MB of memory. In both cases, after the login shell is killed, a SysRq to 'dump memory' shows roughly 2MB free, with the rest allocated.

Before I debug/trace any further, I'm curious as to whether or not preempt is known to work on PowerPC in SMP mode. If it's supposed to, I'd be happy to capture any data that may help.

Anyway - any help or advice would be greatly appreciated.

Thanks,

- Dave
[email protected]

It should work OK. The memory leak is certainly unexpected.

Someone else has reported a similar leak on x86... so you are not alone. It might be a bug in the 2.4.20 kernel itself. Think you can track down what is leaking?

Robert Love

Oh, one other thing. An updated patch for 2.4.20 is up:

http://www.kernel.org/pub/linux/kernel/people/rml/preempt-kernel/v2.4/preempt-kernel-rml-2.4.20-2.patch

It has a couple fixes for proper protection of per-CPU data, including some PPC-specific ones.

Robert Love

On Sun, Apr 13, 2003 at 05:57:24PM -0400, Robert Love wrote:

| On Sat, 2003-04-12 at 11:29, David Brown wrote:
| 
| > I recently applied the preempt-kernel-rml-2.4.21-pre1-1.patch from
| > kernel.org to BenH's stable tree from rsync.penguinppc.org.
| 
| Oh, one other thing.  An updated patch for 2.4.20 is up:
| 
| http://www.kernel.org/pub/linux/kernel/people/rml/preempt-kernel/v2.4/preempt-kernel-rml-2.4.20-2.patch
| 
| It has a couple fixes for proper protection of per-CPU data, including
| some PPC-specific ones.
| 
| 	Robert Love

Hi Robert:

Thanks for the reply and the patch. I'll give the 2.4.20-2 patch a shot. If I'm still seeing the leak after applying it, I'll do my best to track it down (I already know it's somewhere in the fork() path, just based on the mean-time-before-death of a few test scripts I was playing with).

I probably won't be able to isolate it down to a single line; I should, however, be able to capture enough information so that someone with far more awareness of the kernel's guts than I can track it down. :)

Thanks again,

- Dave

On Sunday 13 April 2003 23:57, Robert Love wrote:

Hi Robert,

one thing in it seems bogus:

diff -Naurp preempt-1/kernel/exit.c preempt-2/kernel/exit.c
@@ -282,7 +282,9 @@ struct mm_struct * start_lazy_tlb(void)
        current->mm = NULL;
        /* active_mm is still 'mm' */
        atomic_inc(&mm->mm_count);
+       preempt_disable();
        enter_lazy_tlb(mm, current, smp_processor_id());
+       preempt_disable();
        return mm;
 }
 

This is an incremental diff snipplet. The second preempt_disable(); should be a preempt_enable(); no?

ciao, Marc

Yep. Thanks.

Robert Love

On Monday 14 April 2003 01:07, Robert Love wrote:

Hi Robert,

np. Two other small fixes you may want to include are attached.

1. fix "Exited with preempt count" if you exit kernel-nfsd
2. fix "Exited with preempt count" at machine_halt(); and machine_power_off();

Attached in this order.

ciao, Marc

@@ -250,6 +250,7 @@ nfsd(struct svc_rqst *rqstp)
 	svc_exit_thread(rqstp);
 
 	/* Release module */
+	unlock_kernel();
 	MOD_DEC_USE_COUNT;
 }
 

@@ -354,6 +354,7 @@ asmlinkage long sys_reboot(int magic1, i
 		notifier_call_chain(&reboot_notifier_list, SYS_HALT, NULL);
 		printk(KERN_EMERG "System halted.\n");
 		machine_halt();
+		unlock_kernel();
 		do_exit(0);
 		break;
 

@@ -361,6 +362,7 @@ asmlinkage long sys_reboot(int magic1, i
 		notifier_call_chain(&reboot_notifier_list, SYS_POWER_OFF, NULL);
 		printk(KERN_EMERG "Power down.\n");
 		machine_power_off();
+		unlock_kernel();
 		do_exit(0);
 		break;
 

On Sun, Apr 13, 2003 at 05:57:24PM -0400, Robert Love wrote:

| On Sat, 2003-04-12 at 11:29, David Brown wrote:
| 
| > I recently applied the preempt-kernel-rml-2.4.21-pre1-1.patch from
| > kernel.org to BenH's stable tree from rsync.penguinppc.org.
| 
| Oh, one other thing.  An updated patch for 2.4.20 is up:
| 
| http://www.kernel.org/pub/linux/kernel/people/rml/preempt-kernel/v2.4/preempt-kernel-rml-2.4.20-2.patch
| 
| It has a couple fixes for proper protection of per-CPU data, including
| some PPC-specific ones.

Hi Robert:

AFAICT, the leak is still there. I'll be looking at that in a bit. I used the following to get it to compile on the PPC box. I'm not sure if it's 100% correct, but it appears to work:

diff -ur linux-2.4.20-preempt/arch/ppc/kernel/mk_defs.c linux-2.4.20-ben/arch/ppc/kernel/mk_defs.c
@@ -44,6 +44,7 @@
 	DEFINE(MM, offsetof(struct task_struct, mm));
 #ifdef CONFIG_PREEMPT
 	DEFINE(PREEMPT_COUNT, offsetof(struct task_struct, preempt_count));
+	DEFINE(CPU, offsetof(struct task_struct, processor));
 #endif
 	DEFINE(ACTIVE_MM, offsetof(struct task_struct, active_mm));
 	DEFINE(TASK_STRUCT_SIZE, sizeof(struct task_struct));

It just adds a DEFINE() for the use of CPU() in arch/ppc/kernel/entry.S.

Thanks,

- Dave