Thread (2 messages) 2 messages, 2 authors, 2008-08-12

Re: [PATCH 02/30] mm: gfp_to_alloc_flags()

From: Neil Brown <hidden>
Date: 2008-08-12 05:02:10
Also in: linux-mm, lkml

On Thursday July 24, a.p.zijlstra@chello.nl wrote:
Factor out the gfp to alloc_flags mapping so it can be used in other places.

Signed-off-by: Peter Zijlstra <redacted>
---
 mm/internal.h   |   10 +++++
 mm/page_alloc.c |   95 +++++++++++++++++++++++++++++++-------------------------
 2 files changed, 64 insertions(+), 41 deletions(-)
This patch all looks "obviously correct" and a nice factorisation of
code, except the last little bit:
quoted hunk
@@ -1618,6 +1627,10 @@ nofail_alloc:
 	if (!wait)
 		goto nopage;
 
+	/* Avoid recursion of direct reclaim */
+	if (p->flags & PF_MEMALLOC)
+		goto nopage;
+
 	cond_resched();
 
 	/* We now go into synchronous reclaim */
-- 
I don't remember seeing it before (though my memory is imperfect) and
it doesn't seem to fit with the rest of the patch (except spatially).

There is a test above for PF_MEMALLOC which will result in a "goto"
somewhere else unless "in_interrupt()".
There is immediately above a test for "!wait".
So the only way this test can fire is when in_interrupt and wait.
But if that happens, then the
	might_sleep_if(wait)
at the top should have thrown a warning...  It really shouldn't happen.

So it looks like it is useless code:  there is already protection
against recursion in this case.

Did I miss something?
If I did, maybe more text in the changelog entry (or the comment)
would help.

Thanks,
NeilBrown
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help