Preempt on PowerPC/SMP appears to leak memory

Preempt on PowerPC/SMP appears to leak memory

[David Brown]

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
--- linux-2.4.20-preempt/include/asm-ppc/spinlock.h	2003-04-06 22:35:45.000000000 -0400
+++ linux-2.4.20-dev/include/asm-ppc/spinlock.h	2003-04-06 01:30:48.000000000 -0500
@@ -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
  dave@codewhore.org

Re: Preempt on PowerPC/SMP appears to leak memory

[Robert Love]

On Sat, 2003-04-12 at 11:29, David Brown wrote:

> 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.

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

Re: Preempt on PowerPC/SMP appears to leak memory

[Robert Love]

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

Re: Preempt on PowerPC/SMP appears to leak memory

[David Brown]

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

Re: Preempt on PowerPC/SMP appears to leak memory

[Marc-Christian Petersen]

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

Hi Robert,

> http://www.kernel.org/pub/linux/kernel/people/rml/preempt-kernel/v2.4/preem
>pt-kernel-rml-2.4.20-2.patch
> It has a couple fixes for proper protection of per-CPU data, including
> some PPC-specific ones.
one thing in it seems bogus:

diff -Naurp preempt-1/kernel/exit.c preempt-2/kernel/exit.c
--- preempt-1/kernel/exit.c        2003-04-14 00:31:39 +0200
+++ preempt-2/kernel/exit.c        2003-04-14 00:31:48 +0200
@@ -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

Re: Preempt on PowerPC/SMP appears to leak memory

[Robert Love]

On Sun, 2003-04-13 at 19:03, Marc-Christian Petersen wrote:

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

Yep.  Thanks.

	Robert Love

Re: Preempt on PowerPC/SMP appears to leak memory

[Marc-Christian Petersen]

[-- Attachment #1: Type: text/plain, Size: 490 bytes --]

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

Hi Robert,

> On Sun, 2003-04-13 at 19:03, Marc-Christian Petersen wrote:
> > This is an incremental diff snipplet. The second preempt_disable();
> > should be a preempt_enable(); no?
> Yep.  Thanks.
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

[-- Attachment #2: 00_preempt-kernel-rml-2.4.20-nfsd-fix.patch --]
[-- Type: text/x-diff, Size: 282 bytes --]

--- linux-old/fs/nfsd/nfssvc.c	2002-12-18 01:03:59.000000000 +0100
+++ linux-wolk4/fs/nfsd//nfssvc.c	2003-01-01 06:53:44.000000000 +0100
@@ -250,6 +250,7 @@ nfsd(struct svc_rqst *rqstp)
 	svc_exit_thread(rqstp);
 
 	/* Release module */
+	unlock_kernel();
 	MOD_DEC_USE_COUNT;
 }
 

[-- Attachment #3: 01_preempt-kernel-rml-2.4.20-halt-reboot-fix.patch --]
[-- Type: text/x-diff, Size: 599 bytes --]

--- linux-old/kernel/sys.c	2003-01-06 17:57:12.000000000 +0100
+++ linux-wolk4/kernel/sys.c	2003-01-06 18:34:59.000000000 +0100
@@ -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;
 

Re: Preempt on PowerPC/SMP appears to leak memory

[David Brown]

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
--- linux-2.4.20-preempt/arch/ppc/kernel/mk_defs.c	2003-04-12 02:57:31.000000000 -0400
+++ linux-2.4.20-ben/arch/ppc/kernel/mk_defs.c	2003-04-13 13:57:00.000000000 -0400
@@ -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