Skip to contentSkip to conversation

2.5: kill PF_MEMDIE

PATCH In review Robert Love 3 participants 3 messages Nov 25, 2002

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 2.5: kill PF_MEMDIE

PF_MEMDIE seems to serve no needed purpose in 2.5. In fact, it seems it has no point in 2.4, either.

Attached patch, against 2.5.49-mm1, makes PF_MEMDIE die.

Robert Love

 include/linux/sched.h |    1 -
 mm/oom_kill.c         |    2 +-
 mm/page_alloc.c       |    2 +-
 3 files changed, 2 insertions(+), 3 deletions(-)
include/linux/sched.h +0 -1
diff -urN linux-2.5.49-mm1/include/linux/sched.h linux/include/linux/sched.h
@@ -421,7 +421,6 @@
 #define PF_DUMPCORE	0x00000200	/* dumped core */
 #define PF_SIGNALED	0x00000400	/* killed by a signal */
 #define PF_MEMALLOC	0x00000800	/* Allocating memory */
-#define PF_MEMDIE	0x00001000	/* Killed for out-of-memory */
 #define PF_FLUSHER	0x00002000	/* responsible for disk writeback */
 #define PF_NOWARN	0x00004000	/* debug: don't warn if alloc fails */
 
mm/oom_kill.c +1 -1
diff -urN linux-2.5.49-mm1/mm/oom_kill.c linux/mm/oom_kill.c
@@ -146,7 +146,7 @@
 	 * exit() and clear out its resources quickly...
 	 */
 	p->time_slice = HZ;
-	p->flags |= PF_MEMALLOC | PF_MEMDIE;
+	p->flags |= PF_MEMALLOC;
 
 	/* This process has hardware access, be more careful. */
 	if (cap_t(p->cap_effective) & CAP_TO_MASK(CAP_SYS_RAWIO)) {
mm/page_alloc.c +1 -1
diff -urN linux-2.5.49-mm1/mm/page_alloc.c linux/mm/page_alloc.c
@@ -613,7 +613,7 @@
 	/* here we're in the low on memory slow path */
 
 rebalance:
-	if ((current->flags & (PF_MEMALLOC | PF_MEMDIE)) && !in_interrupt()) {
+	if ((current->flags & PF_MEMALLOC) && !in_interrupt()) {
 		/* go through the zonelist yet again, ignoring mins */
 		for (i = 0; zones[i] != NULL; i++) {
 			struct zone *z = zones[i];
#1 lore raw
Quoted 4 lines · Robert Love

PF_MEMDIE seems to serve no needed purpose in 2.5. In fact, it seems it has no point in 2.4, either.

Attached patch, against 2.5.49-mm1, makes PF_MEMDIE die.

What makes you say no needed purpose? I agree it's screwed in 2.5, but its purpose is to make more memory available to the OOMkilled task, so it's not held up on its way out to freeing memory: seems reasonable.

I'm no great lover of PF_MEMDIE and PF_ flag manipulation, but let it live until we have an alternative? I think the better patch is not to lose PF_MEMDIE (and conceivably others) once it has been set.

(Shall we agree to turn a blind eye to the risk of oom_kill_task manipulating p->flags while it may be another CPU's current->flags?)

Hugh

mm/page_alloc.c +2 -1
@@ -432,7 +432,8 @@
 
 	current->flags |= PF_MEMALLOC;
 	ret = try_to_free_pages(classzone, gfp_flags, order);
-	current->flags = cflags;
+	if (!(cflags & PF_MEMALLOC))
+		current->flags &= ~PF_MEMALLOC;
 	return ret;
 }
 
mm/swap_state.c +9 -4
@@ -120,6 +120,7 @@
 {
 	swp_entry_t entry;
 	int pf_flags;
+	int err;
 
 	if (!PageLocked(page))
 		BUG();
@@ -149,9 +150,15 @@
 		/*
 		 * Add it to the swap cache and mark it dirty
 		 */
-		switch (add_to_page_cache(page, &swapper_space, entry.val)) {
+		err = add_to_page_cache(page, &swapper_space, entry.val);
+
+		if (!(pf_flags & PF_NOWARN))
+			current->flags &= ~PF_NOWARN;
+		if (pf_flags & PF_MEMALLOC)
+			current->flags |= PF_MEMALLOC;
+
+		switch (err) {
 		case 0:				/* Success */
-			current->flags = pf_flags;
 			SetPageUptodate(page);
 			ClearPageDirty(page);
 			set_page_dirty(page);
@@ -159,13 +166,11 @@
 			return 1;
 		case -EEXIST:
 			/* Raced with "speculative" read_swap_cache_async */
-			current->flags = pf_flags;
 			INC_CACHE_INFO(exist_race);
 			swap_free(entry);
 			continue;
 		default:
 			/* -ENOMEM radix-tree allocation failure */
-			current->flags = pf_flags;
 			swap_free(entry);
 			return 0;
 		}
#2 lore raw
In reply to Robert Love
Quoted 2 lines · Robert Love

PF_MEMDIE seems to serve no needed purpose in 2.5. In fact, it seems it has no point in 2.4, either.

Actually it does. If the task that's being OOM killed is in try_to_free_pages() it'll clear PF_MEMALLOC before going back into __alloc_pages() and it might not die.

Rik

Signature · 3 lines
Bravely reimplemented by the knights who say "NIH".
http://www.surriel.com/		http://guru.conectiva.com/
Current spamtrap:  <a href=mailto:"[email protected]">[email protected]</a>