mm: Fix overcommit overflow

PATCH In review Alan Cox 2 participants 3 messages Apr 30, 2008

Subsystems (2)

Memory management Memory mapping

lore

Share & tools

lore

Alan Cox

author

2008-04-30 20:51 UTC #0 lore raw

PATCH 1/2 mm: Fix overcommit overflow

Sami Farin reported an overflow in the overcommit handling on 64bit boxes.

We use atomic_t for page counting which is fine on 32bit but on 64bit overflows. We can use atomic64_t but there is a problem - most heavy users of zero overcommit are embedded people whose 64bit atomics are really slow and expensive operations. Thus we use a few defines to flip 32 or 64bit according to the size of a long

(Split into two diffs to keep ownership correct)

Signed-off-by Alan Cox <[email protected]>

include/linux/mman.h +17 -2

diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.25-mm1/include/linux/mman.h linux-2.6.25-mm1/include/linux/mman.h
@@ -15,16 +15,31 @@
 
 #include <asm/atomic.h>
 
+/* 32bit platforms have a virtual address space in pages we
+   can fit into an atomic_t. We want to avoid atomic64_t on
+   such boxes as it is often expensive and most strict overcommit
+   users turn out to be embedded low power processors */
+
+#if (BITS_PER_LONG == 32)
+#define vm_atomic_t atomic_t
+#define vm_atomic_read atomic_read
+#define vm_atomic_add atomic_add
+#else
+#define vm_atomic_t atomic64_t
+#define vm_atomic_read atomic64_read
+#define vm_atomic_add atomic64_add
+#endif
+
 extern int sysctl_overcommit_memory;
 extern int sysctl_overcommit_ratio;
-extern atomic_t vm_committed_space;
+extern vm_atomic_t vm_committed_space;
 
 #ifdef CONFIG_SMP
 extern void vm_acct_memory(long pages);
 #else
 static inline void vm_acct_memory(long pages)
 {
-	atomic_add(pages, &vm_committed_space);
+	vm_atomic_add(pages, &vm_committed_space);
 }
 #endif

mm/mmap.c +2 -2

diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.25-mm1/mm/mmap.c linux-2.6.25-mm1/mm/mmap.c
@@ -80,7 +80,7 @@
 int sysctl_overcommit_memory = OVERCOMMIT_GUESS;  /* heuristic overcommit */
 int sysctl_overcommit_ratio = 50;	/* default is 50% */
 int sysctl_max_map_count __read_mostly = DEFAULT_MAX_MAP_COUNT;
-atomic_t vm_committed_space = ATOMIC_INIT(0);
+vm_atomic_t vm_committed_space = ATOMIC_INIT(0);
 
 /*
  * Check that a process has enough memory to allocate a new virtual

@@ -177,7 +177,7 @@
 	 * cast `allowed' as a signed long because vm_committed_space
 	 * sometimes has a negative value
 	 */
-	if (atomic_read(&vm_committed_space) < (long)allowed)
+	if (vm_atomic_read(&vm_committed_space) < (long)allowed)
 		return 0;
 error:
 	vm_unacct_memory(pages);

mm/swap.c +1 -1

diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.25-mm1/mm/swap.c linux-2.6.25-mm1/mm/swap.c
@@ -503,7 +503,7 @@
 	local = &__get_cpu_var(committed_space);
 	*local += pages;
 	if (*local > ACCT_THRESHOLD || *local < -ACCT_THRESHOLD) {
-		atomic_add(*local, &vm_committed_space);
+		vm_atomic_add(*local, &vm_committed_space);
 		*local = 0;
 	}
 	preempt_enable();

Andrew Morton

maintainer

2008-04-30 21:10 UTC #1 lore raw

Quoted 23 lines · On Wed, 30 Apr 2008 21:42:41 +0100 Alan Cox <[email protected]> wrote:

Sami Farin reported an overflow in the overcommit handling on 64bit boxes.
We use atomic_t for page counting which is fine on 32bit but on 64bit overflows. We can use atomic64_t but there is a problem - most heavy users of zero overcommit are embedded people whose 64bit atomics are really slow and expensive operations. Thus we use a few defines to flip 32 or 64bit according to the size of a long
(Split into two diffs to keep ownership correct)
Signed-off-by Alan Cox <[email protected]>
diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.25-mm1/include/linux/mman.h linux-2.6.25-mm1/include/linux/mman.h
include/linux/mman.h +4 -0
@@ -15,16 +15,31 @@
 
 #include <asm/atomic.h>
 
+/* 32bit platforms have a virtual address space in pages we
+   can fit into an atomic_t. We want to avoid atomic64_t on
+   such boxes as it is often expensive and most strict overcommit
+   users turn out to be embedded low power processors */

include/linux/mman.h line 31

Quoted 10 lines

include/linux/mman.h +10 -0

+
+#if (BITS_PER_LONG == 32)
+#define vm_atomic_t atomic_t
+#define vm_atomic_read atomic_read
+#define vm_atomic_add atomic_add
+#else
+#define vm_atomic_t atomic64_t
+#define vm_atomic_read atomic64_read
+#define vm_atomic_add atomic64_add
+#endif

ew.

Quoted 19 lines

include/linux/mman.h +2 -2

 extern int sysctl_overcommit_memory;
 extern int sysctl_overcommit_ratio;
-extern atomic_t vm_committed_space;
+extern vm_atomic_t vm_committed_space;
 
 #ifdef CONFIG_SMP
 extern void vm_acct_memory(long pages);
 #else
 static inline void vm_acct_memory(long pages)
 {
-	atomic_add(pages, &vm_committed_space);
+	vm_atomic_add(pages, &vm_committed_space);
 }
 #endif

diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.25-mm1/mm/mmap.c linux-2.6.25-mm1/mm/mmap.c

mm/mmap.c line 83

Quoted 5 lines

mm/mmap.c +1 -1

@@ -80,7 +80,7 @@
 int sysctl_overcommit_memory = OVERCOMMIT_GUESS;  /* heuristic overcommit */
 int sysctl_overcommit_ratio = 50;	/* default is 50% */
 int sysctl_max_map_count __read_mostly = DEFAULT_MAX_MAP_COUNT;
-atomic_t vm_committed_space = ATOMIC_INIT(0);
+vm_atomic_t vm_committed_space = ATOMIC_INIT(0);

That'll need to be ATOMIC64_INIT on 64-bit.

Quoted 16 lines

mm/mmap.c +1 -1

 /*
  * Check that a process has enough memory to allocate a new virtual

@@ -177,7 +177,7 @@
 	 * cast `allowed' as a signed long because vm_committed_space
 	 * sometimes has a negative value
 	 */
-	if (atomic_read(&vm_committed_space) < (long)allowed)
+	if (vm_atomic_read(&vm_committed_space) < (long)allowed)
 		return 0;
 error:
 	vm_unacct_memory(pages);

diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.25-mm1/mm/swap.c linux-2.6.25-mm1/mm/swap.c

mm/swap.c line 509

Quoted 8 lines

mm/swap.c +1 -1

@@ -503,7 +503,7 @@
 	local = &__get_cpu_var(committed_space);
 	*local += pages;
 	if (*local > ACCT_THRESHOLD || *local < -ACCT_THRESHOLD) {
-		atomic_add(*local, &vm_committed_space);
+		vm_atomic_add(*local, &vm_committed_space);
 		*local = 0;
 	}
 	preempt_enable();

But afacit the existing atomic_long_t does exactly what you want?

Alan Cox

author

2008-04-30 21:22 UTC #2 lore raw

Quoted 3 lines

Quoted 1 line
preempt_enable();
But afacit the existing atomic_long_t does exactly what you want?

We have a portable atomic_long_t .. oooh scratch that patch then...

Alan