Fix memory leak in swsusp

This fixes 2 memory leaks in swsusp: during relocating pagedir, eaten pages were not properly freed in error path and even regular freeing path was freeing one page too little. Please apply,

Review line 809

Quoted 7 lines

(quoted diff) +1 -0

@@ -803,32 +804,31 @@
 		return 0;
 	}
 
+	err = -ENOMEM;
 	while ((m = (void *) __get_free_pages(GFP_ATOMIC, pagedir_order))) {
 		memset(m, 0, PAGE_SIZE);

BTW, what does this memset do?

Review line 809

Quoted 7 lines

(quoted diff) +1 -0

@@ -803,32 +804,31 @@
 		return 0;
 	}
 
+	err = -ENOMEM;
 	while ((m = (void *) __get_free_pages(GFP_ATOMIC, pagedir_order))) {
 		memset(m, 0, PAGE_SIZE);

BTW, what does this memset do?

Quoted 9 lines

kernel/power/swsusp.c line 809

Quoted 7 lines

kernel/power/swsusp.c renamed from linux/kernel/power/swsusp.c +1 -0

@@ -803,32 +804,31 @@
 		return 0;
 	}
 
+	err = -ENOMEM;
 	while ((m = (void *) __get_free_pages(GFP_ATOMIC, pagedir_order))) {
 		memset(m, 0, PAGE_SIZE);

BTW, what does this memset do?

Here's incremental fix, it compiles but not tested.

BTW I have problems getting mail to you:

Quoted 10 lines

Quoted 9 lines

kernel/power/swsusp.c line 809

Quoted 7 lines

kernel/power/swsusp.c renamed from linux/kernel/power/swsusp.c +1 -0

@@ -803,32 +804,31 @@
 		return 0;
 	}
 
+	err = -ENOMEM;
 	while ((m = (void *) __get_free_pages(GFP_ATOMIC, pagedir_order))) {
 		memset(m, 0, PAGE_SIZE);

BTW, what does this memset do?

Quoted 1 line

Here's incremental fix, it compiles but not tested.

Thanks, but my question remains: why do we need the memset? The area allocated is either thrown away because it collides or is overwritten with the pagedir stuff straight away.

Quoted 4 lines

Thanks, but my question remains: why do we need the memset? The area allocated is either thrown away because it collides or is overwritten with the pagedir stuff straight away.

You are right, it should not be needed.

Quoted 5 lines

Quoted 3 lines

Thanks, but my question remains: why do we need the memset? The area allocated is either thrown away because it collides or is overwritten with the pagedir stuff straight away.

You are right, it should not be needed.

Great. Here's the patch that removes the memset calls from both pmdisk and swsusp. It depends on the previous patches in this thread.

If you want more cleanups, copy_pagedir() should be probably replaced by simple memset...

kernel/power/pmdisk.c +3 -0

+			pm_pagedir_nosave =
+				memcpy(m, old_pagedir,
+				       PAGE_SIZE << pagedir_order);

We were avoiding the use of memcpy because it messes up the preempt count with 3DNow, and potentially as other unseen side effects. The preempt could possibly simply be reset at resume time, but the point remains.

Quoted 4 lines · Nigel Cunningham

We were avoiding the use of memcpy because it messes up the preempt count with 3DNow, and potentially as other unseen side effects. The preempt could possibly simply be reset at resume time, but the point remains.

eh? memcpy just copies memory. Maybe your meant copy_*_user()?

Quoted 5 lines

Quoted 3 lines

We were avoiding the use of memcpy because it messes up the preempt count with 3DNow, and potentially as other unseen side effects. The preempt could possibly simply be reset at resume time, but the point remains.

eh? memcpy just copies memory. Maybe your meant copy_*_user()?

See arch/i386/lib/memcpy.c The 3dnow routine does kernel_fpu_begin()/..end() which futzes with preempt count. What I'm missing though is that the count afterwards should be the same as it was before the memcpy. Why is this a problem for the suspend folks?

Quoted 1 line · Nigel Cunningham

We were avoiding the use of memcpy because it messes up the preempt count with 3DNow, and potentially as other unseen side effects. The preempt could possibly simply be reset at resume time, but the point remains.

eh? memcpy just copies memory. Maybe your meant copy_*_user()?

Hi.
At some stage, you copy the page that contains the preempt count for the process that is doing the suspending. If you use memcpy on a 3Dnow machine, the preempt count is incremented prior to doing the copy of the page. Then, at resume time, it is one too high.

Quoted 4 lines

At some stage, you copy the page that contains the preempt count for the process that is doing the suspending. If you use memcpy on a 3Dnow machine, the preempt count is incremented prior to doing the copy of the page. Then, at resume time, it is one too high.

Well, if we know that it is one too high, why not decrement right after the resume?

At some stage, you copy the page that contains the preempt count for the process that is doing the suspending. If you use memcpy on a 3Dnow machine, the preempt count is incremented prior to doing the copy of the page. Then, at resume time, it is one too high.

We were avoiding the use of memcpy because it messes up the preempt count with 3DNow, and potentially as other unseen side effects. The preempt could possibly simply be reset at resume time, but the point remains.