Re: [PATCH]: Cleanup of __alloc_pages

Paul Jackson
2005-11-13T07:47:22Z

> Yes, the fact that GFP_ATOMIC also implies "use the emergency pool" is
> unfortunate, and perhaps the two should always have been separated out, at
> least to make the programmer think about whether the code really needs
> access to the emergency pools. Usually it does.

Ah - now it makes more sense.

The key invisible fact in the gfp.h line:

#define GFP_ATOMIC (__GFP_VALID | __GFP_HIGH)

is that __GFP_WAIT is *not* set (making it mean don't sleep).
All the other commonly used GFP_* flags do have __GFP_WAIT.

I have no issue with ATOMIC also meaning "use emergency pool".
That's an appropriate simplication, that fits the usage well.

I just had a mental block on the invisible unset __GFP_WAIT bit.

Would you look kindly on a patch that did:

--- 2.6.14-mm2.orig/include/linux/gfp.h 2005-11-12 23:36:57.258103418 -0800
+++ 2.6.14-mm2/include/linux/gfp.h 2005-11-12 23:42:35.287219455 -0800
@@ -58,6 +58,7 @@ struct vm_area_struct;
__GFP_NOFAIL|__GFP_NORETRY|__GFP_NO_GROW|__GFP_COMP| \
__GFP_NOMEMALLOC|__GFP_HARDWALL)

+/* GFP_ATOMIC means both !wait (__GFP_WAIT not set) and use emergency pool */
#define GFP_ATOMIC (__GFP_VALID | __GFP_HIGH)
#define GFP_NOIO (__GFP_VALID | __GFP_WAIT)
#define GFP_NOFS (__GFP_VALID | __GFP_WAIT | __GFP_IO)

--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <pj@sgi.com> 1.925.600.0401