Fixes for NUMA allocations on memoryless nodes
Subsystems (6)
cover Fixes for NUMA allocations on memoryless nodes
This patchset fixes the dysfunctional behavior of for GFP_THISNODE and MPOL_INTERLEAVE on systems with memoryless nodes. We introduce a new "node_memory_map" to be able to determine efficiently if a node has memory.
Tested on IA64 NUMA and compile tested on i386 SMP (to verify that the new fallbacks work right)
Signature · 5 lines
-- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 1/3 NUMA: introduce node_memory_map
It is necessary to know if nodes have memory since we have recently begun to add support for memoryless nodes. For that purpose we introduce a new bitmap called
node_memory_map
A node has its bit in node_memory_map set if it has memory. If a node has memory then it has at least one zone defined in its pgdat structure that is located in the pgdat itself.
The node_memory_map can then be used in various places to insure that we do the right thing when we encounter a memoryless node.
include/linux/nodemask.h +11 -0
@@ -64,12 +64,16 @@* * int node_online(node) Is some node online? * int node_possible(node) Is some node possible? +* int node_memory(node) Does a node have memory? * * int any_online_node(mask) First online node in mask * * node_set_online(node) set bit 'node' in node_online_map * node_set_offline(node) clear bit 'node' in node_online_map * +* node_set_memory(node) set bit 'node' in node_memory_map +* node_clear_memoryd(node) clear bit 'node' in node_memory_map +* * for_each_node(node) for-loop node over node_possible_map * for_each_online_node(node) for-loop node over node_online_map *
@@ -344,12 +348,14 @@ static inline void __nodes_remap(nodemasextern nodemask_t node_online_map;extern nodemask_t node_possible_map; +extern nodemask_t node_memory_map;#if MAX_NUMNODES >1 #define num_online_nodes () nodes_weight(node_online_map)#define num_possible_nodes () nodes_weight(node_possible_map)#define node_online (node) node_isset((node), node_online_map)#define node_possible (node) node_isset((node), node_possible_map) +#define node_memory (node) node_isset((node), node_memory_map)#define first_online_node first_node(node_online_map)#define next_online_node (nid) next_node((nid), node_online_map)extern int nr_node_ids;
@@ -358,6 +364,8 @@ extern int nr_node_ids;#define num_possible_nodes () 1#define node_online (node) ((node) == 0)#define node_possible (node) ((node) == 0) +#define node_memory (node) ((node) == 0) +#define node_populated (node) ((node) == 0)#define first_online_node 0#define next_online_node (nid) (MAX_NUMNODES)#define nr_node_ids 1
@@ -375,6 +383,9 @@ extern int nr_node_ids;#define node_set_online (node) set_bit((node), node_online_map.bits)#define node_set_offline (node) clear_bit((node), node_online_map.bits) +#define node_set_memory (node) set_bit((node), node_memory_map.bits) +#define node_clear_memory (node) clear_bit((node), node_memory_map.bits) +#define for_each_node (node) for_each_node_mask((node), node_possible_map)#define for_each_online_node (node) for_each_node_mask((node), node_online_map)
mm/page_alloc.c +6 -0
@@ -54,6 +54,9 @@ nodemask_t node_online_map __read_mostlyEXPORT_SYMBOL (node_online_map);nodemask_t node_possible_map__read_mostly =NODE_MASK_ALL ;EXPORT_SYMBOL (node_possible_map); +nodemask_t node_memory_map__read_mostly =NODE_MASK_NONE ; +EXPORT_SYMBOL (node_memory_map); +unsigned long totalram_pages__read_mostly ;unsigned long totalreserve_pages__read_mostly ;long nr_swap_pages;
@@ -2299,6 +2302,9 @@ static void build_zonelists(pg_data_t *p/* calculate node order -- i.e., DMA last! */ build_zonelists_in_zone_order (pgdat, j); } + +if (pgdat->node_present_pages) +node_set_memory (local_node); }/* Construct the zonelist performance cache - see further mmzone.h */
Signature · 5 lines
-- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
David Rientjes
reviewerRe: [patch 1/3] NUMA: introduce node_memory_map
include/linux/nodemask.h line 68
Quoted 10 lines · On Tue, 12 Jun 2007, [email protected] wrote:
Index:
linux-2.6.22-rc4-mm2/include/linux/nodemask.h
===================================================================include/linux/nodemask.h +1
-0@@ -64,12 +64,16 @@* * int node_online(node) Is some node online? * int node_possible(node) Is some node possible? +* int node_memory(node) Does a node have memory? *
This name doesn't make sense; wouldn't node_has_memory() be better?
include/linux/nodemask.h line 76
Quoted 8 lines
include/linux/nodemask.h +3
-0* int any_online_node(mask) First online node in mask * * node_set_online(node) set bit 'node' in node_online_map * node_set_offline(node) clear bit 'node' in node_online_map * +* node_set_memory(node) set bit 'node' in node_memory_map +* node_clear_memoryd(node) clear bit 'node' in node_memory_map +*
Extra 'd' there in node_clear_memory().
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [patch 1/3] NUMA: introduce node_memory_map
Quoted 6 lines · David Rientjes
Quoted 4 lines
* int
node_online(node)Is some node online?
* intnode_possible(node)Is some node possible?
+ * intnode_memory(node)Does a node have memory?
*This name doesn't make sense; wouldn't
node_has_memory()be better?
node_set_has_memory and node_clear_has_memory sounds a bit strange.
Quoted 11 lines
Quoted 9 lines
* int
any_online_node(mask)First online node in mask
*
*node_set_online(node)set bit 'node' innode_online_map
*node_set_offline(node)clear bit 'node' innode_online_map
*
+ *node_set_memory(node)set bit 'node' innode_memory_map
+ *node_clear_memoryd(node)clear bit 'node' innode_memory_map
+ *Extra 'd' there in
node_clear_memory().
Tss.... It survived my editing.
Fix typo
include/linux/nodemask.h +1 -1
@@ -72,7 +72,7 @@* node_set_offline(node) clear bit 'node' in node_online_map * * node_set_memory(node) set bit 'node' in node_memory_map -* node_clear_memoryd(node) clear bit 'node' in node_memory_map +* node_clear_memory(node) clear bit 'node' in node_memory_map * * for_each_node(node) for-loop node over node_possible_map * for_each_online_node(node) for-loop node over node_online_map
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
David Rientjes
reviewerRe: [patch 1/3] NUMA: introduce node_memory_map
Quoted 11 lines · Christoph Lameter
Quoted 6 lines · David Rientjes
Quoted 4 lines
* int
node_online(node)Is some node online?
* intnode_possible(node)Is some node possible?
+ * intnode_memory(node)Does a node have memory?
*This name doesn't make sense; wouldn't
node_has_memory()be better?
node_set_has_memoryandnode_clear_has_memorysounds a bit strange.
This will probably be one of those things that people see in the source and have to look up everytime. node_has_memory() is straight-forward and to the point.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [patch 1/3] NUMA: introduce node_memory_map
Quoted 9 lines · David Rientjes
Quoted 7 lines · Christoph Lameter
Quoted 5 lines · David Rientjes
Quoted 4 lines
* int
node_online(node)Is some node online?
* intnode_possible(node)Is some node possible?
+ * intnode_memory(node)Does a node have memory?
*
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
This name doesn't make sense; wouldn't
node_has_memory()be better?
node_set_has_memoryandnode_clear_has_memorysounds a bit strange.This will probably be one of those things that people see in the source and have to look up everytime.
node_has_memory()is straight-forward and to the point.
But node_possible is similar to node_memory.
Would you also prefer node_is_possible over node_possible?
node_is_online?
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
David Rientjes
reviewerRe: [patch 1/3] NUMA: introduce node_memory_map
Quoted 16 lines · Christoph Lameter
Quoted 7 lines
Quoted 7 lines · David Rientjes
Quoted 6 lines
Quoted 5 lines
* int
node_online(node)Is some node online?
* intnode_possible(node)Is some node possible?
+ * intnode_memory(node)Does a node have memory?
*Quoted 8 lines
Quoted 4 lines
Quoted 1 line
This name doesn't make sense; wouldn't
node_has_memory()be better?
node_set_has_memoryandnode_clear_has_memorysounds a bit strange.This will probably be one of those things that people see in the source and have to look up everytime.
node_has_memory()is straight-forward and to the point.
Quoted 6 lines
But
node_possibleis similar tonode_memory.Would you also prefer
node_is_possibleovernode_possible?
node_is_online?
I think the problem is that online and possible are adverbs and adjectives, respectively, and memory is a noun. That's why when it appears in source code, it doesn't make a lot of sense for node_memory to return a boolean value. I suspect it would return the memory, whatever that is.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [patch 1/3] NUMA: introduce node_memory_map
Quoted 5 lines · David Rientjes
I think the problem is that online and possible are adverbs and adjectives, respectively, and memory is a noun. That's why when it appears in source code, it doesn't make a lot of sense for
node_memoryto return a boolean value. I suspect it would return the memory, whatever that is.
A sentence such as "This is a memory node" in opposition to "This is a memoryless nide" would put memory to use as an adjective.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
David Rientjes
reviewerRe: [patch 1/3] NUMA: introduce node_memory_map
Quoted 3 lines · Christoph Lameter
A sentence such as "This is a memory node" in opposition to "This is a memoryless nide" would put memory to use as an adjective.
Ok, is_memory_node() is great too ;)
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [patch 1/3] NUMA: introduce node_memory_map
In reply to David RientjesQuoted 9 lines · David Rientjes
Quoted 7 lines · Christoph Lameter
Quoted 5 lines · David Rientjes
Quoted 4 lines
* int
node_online(node)Is some node online?
* intnode_possible(node)Is some node possible?
+ * intnode_memory(node)Does a node have memory?
*
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
This name doesn't make sense; wouldn't
node_has_memory()be better?
node_set_has_memoryandnode_clear_has_memorysounds a bit strange.This will probably be one of those things that people see in the source and have to look up everytime.
node_has_memory()is straight-forward and to the point.
Indeed, I did and (I like to think) I helped write the patches :)
Why not just make the boolean sensible?
We can keep
node_set_memory()node_clear_memory()
but change node_memory() to node_has_memory() ?
Thanks,
Nish
Signature · 8 lines
Nishanth Aravamudan <[email protected]> IBM Linux Technology Center -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [patch 1/3] NUMA: introduce node_memory_map
Quoted 1 line
Indeed, I did and (I like to think) I helped write the patches :)
The patches contain your signoff because of your authorship...
Quoted 6 lines
We can keep
node_set_memory()node_clear_memory()but change
node_memory()tonode_has_memory()?
Hmmm.... That deviates from how the other node_xxx() things are so it disturbed my sense of order. We have no three word node_is/has_xxx yet.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [patch 1/3] NUMA: introduce node_memory_map
Quoted 7 lines · Christoph Lameter
Quoted 2 lines
Indeed, I did and (I like to think) I helped write the patches :)
The patches contain your signoff because of your authorship...
Quoted 2 lines
We can keep
Quoted 8 lines
Quoted 4 lines
node_set_memory()node_clear_memory()but change
node_memory()tonode_has_memory()?Hmmm.... That deviates from how the other
node_xxx()things are so it disturbed my sense of order. We have no three word node_is/has_xxx yet.
Yeah, I realize that -- but I also agree with David that
node_memory()
is not intuitive at all. And we've already admitted that a few of the macros in there are inconsistent already :)
Thanks,
Nish
Signature · 8 lines
Nishanth Aravamudan <[email protected]> IBM Linux Technology Center -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
David Rientjes
reviewerRe: [patch 1/3] NUMA: introduce node_memory_map
Quoted 7 lines · Nishanth Aravamudan
Yeah, I realize that -- but I also agree with David that
node_memory()is not intuitive at all. And we've already admitted that a few of the macros in there are inconsistent already :)
Creating new macros that conform to the others in terms of node_<single-word-here>() is great, but useless if it isn't readable in soruce code.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [patch 1/3] NUMA: introduce node_memory_map
In reply to Nishanth AravamudanQuoted 2 lines · Nishanth Aravamudan
is not intuitive at all. And we've already admitted that a few of the macros in there are inconsistent already :)
I do not want to add another inconsistency.
if (node_memory(node))
is pretty clear as far as I can tell.
Some of the macros in include/linux/nodemask.h are inconsistent. How can we make those consistent. Could you come up with a consistent naming scheme? Add the explanation for that scheme.
But that should be a separate patch. And the patch would have to change all uses of those macros in the kernel source tree.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [patch 1/3] NUMA: introduce node_memory_map
Quoted 10 lines · Christoph Lameter
Quoted 2 lines · Nishanth Aravamudan
is not intuitive at all. And we've already admitted that a few of the macros in there are inconsistent already :)
I do not want to add another inconsistency.
if (
node_memory(node))is pretty clear as far as I can tell.
Fair enough.
Quoted 3 lines
Some of the macros in
include/linux/nodemask.hare inconsistent. How can we make those consistent. Could you come up with a consistent naming scheme? Add the explanation for that scheme.
I'll think it over :)
Quoted 2 lines
But that should be a separate patch. And the patch would have to change all uses of those macros in the kernel source tree.
Agreed, will be a down-the-road thing. In the meantime, I'm rebasing the hugetlb pool patch and sysfs interface patch on top of your stack and will repost.
Thanks,
Nish
Signature · 8 lines
Nishanth Aravamudan <[email protected]> IBM Linux Technology Center -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [patch 1/3] NUMA: introduce node_memory_map
In reply to Nishanth AravamudanQuoted 16 lines · Nishanth Aravamudan
Quoted 8 lines · David Rientjes
Quoted 6 lines · Christoph Lameter
Quoted 4 lines · David Rientjes
Quoted 4 lines
* int
node_online(node)Is some node online?
* intnode_possible(node)Is some node possible?
+ * intnode_memory(node)Does a node have memory?
*Quoted 6 lines
Quoted 3 lines
Quoted 1 line
This name doesn't make sense; wouldn't
node_has_memory()be better?
node_set_has_memoryandnode_clear_has_memorysounds a bit strange.This will probably be one of those things that people see in the source and have to look up everytime.
node_has_memory()is straight-forward and to the point.
Quoted 8 lines
Indeed, I did and (I like to think) I helped write the patches :)
Why not just make the boolean sensible?
We can keep
node_set_memory()node_clear_memory()
node_clear_memory() sounds like something to memset all of that node's memory or something.
Quoted 1 line
but change
node_memory()tonode_has_memory()?
-apw
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 2/3 Fix GFP_THISNODE behavior for memoryless nodes
In reply to [email protected]GFP_THISNODE checks that the zone selected is within the pgdat (node) of the first zone of a nodelist. That only works if the node has memory. A memoryless node will have its first zone on another pgdat (node).
Thus GFP_THISNODE may be returning memory on other nodes. GFP_THISNODE should fail if there is no local memory on a node.
So we add a check to verify that the node specified has memory in alloc_pages_node(). If the node has no memory then return NULL.
The case of alloc_pages(GFP_THISNODE) is not changed. alloc_pages() (with no memory policies in effect) is understood to prefer the current node. If a process is running on a node with no memory then its default allocations come from the next neighboring node. GFP_THISNODE will then force the memory to come from that node.
include/linux/gfp.h +7 -0
@@ -175,6 +175,13 @@ static inline struct page *alloc_pages_nif (nid <0 ) nid =numa_node_id (); +/* +* Check for the special case that GFP_THISNODE is used on a +* memoryless node +*/ +if ((gfp_mask & __GFP_THISNODE) && !node_memory (nid)) +return NULL ; +return __alloc_pages (gfp_mask, order,NODE_DATA (nid)->node_zonelists +gfp_zone (gfp_mask)); }
Signature · 5 lines
-- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
David Rientjes
reviewerRe: [patch 2/3] Fix GFP_THISNODE behavior for memoryless nodes
Quoted 5 lines · On Tue, 12 Jun 2007, [email protected] wrote:
Index:
linux-2.6.22-rc4-mm2/include/linux/gfp.h
===================================================================
include/linux/gfp.h static inline struct page *alloc_pages_n · line 184
Quoted 10 lines
include/linux/gfp.h +7
-0@@ -175,6 +175,13 @@ static inline struct page *alloc_pages_nif (nid <0 ) nid =numa_node_id (); +/* +* Check for the special case that GFP_THISNODE is used on a +* memoryless node +*/ +if ((gfp_mask & __GFP_THISNODE) && !node_memory (nid)) +return NULL ; +
unlikely()?
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [patch 2/3] Fix GFP_THISNODE behavior for memoryless nodes
Quoted 7 lines · David Rientjes
Quoted 7 lines
===================================================================
include/linux/gfp.h +0
-0@@ -175,6 +175,13 @@ static inline struct page *alloc_pages_nif (nid <0 ) nid =numa_node_id ();
Quoted 9 lines
include/linux/gfp.h static inline struct page *alloc_pages_n · line 184
Quoted 7 lines
include/linux/gfp.h +7
-0+/* +* Check for the special case that GFP_THISNODE is used on a +* memoryless node +*/ +if ((gfp_mask & __GFP_THISNODE) && !node_memory (nid)) +return NULL ; +
unlikely()?
The gfp_mask is typically constant. So the whole expression is folded by the compiler into either
if (!node_memory(nid))
return NULL
or
nothing.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
David Rientjes
reviewerRe: [patch 2/3] Fix GFP_THISNODE behavior for memoryless nodes
Quoted 22 lines · Christoph Lameter
Quoted 7 lines · David Rientjes
Quoted 7 lines
===================================================================
include/linux/gfp.h +0
-0@@ -175,6 +175,13 @@ static inline struct page *alloc_pages_nif (nid <0 ) nid =numa_node_id ();Quoted 9 lines
include/linux/gfp.h static inline struct page *alloc_pages_n · line 184
Quoted 7 lines
include/linux/gfp.h +7
-0+/* +* Check for the special case that GFP_THISNODE is used on a +* memoryless node +*/ +if ((gfp_mask & __GFP_THISNODE) && !node_memory (nid)) +return NULL ; +
unlikely()?The
gfp_maskis typically constant. So the whole expression is folded by the compiler into either
Quoted 7 lines
if (!node_memory(nid))
return NULLor
nothing.
That's the point. Isn't !node_memory(nid) unlikely?
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [patch 2/3] Fix GFP_THISNODE behavior for memoryless nodes
Quoted 1 line · David Rientjes
That's the point. Isn't !node_memory(nid) unlikely?
Correct.
Use unlikely
Signed-off-cy: Christoph Lameter <[email protected]>
include/linux/gfp.h +1 -1
@@ -179,7 +179,7 @@ static inline struct page *alloc_pages_n* Check for the special case that GFP_THISNODE is used on a * memoryless node */ -if ((gfp_mask & __GFP_THISNODE) && !node_memory (nid)) +if (unlikely ((gfp_mask & __GFP_THISNODE) && !node_memory (nid)))return NULL ; return __alloc_pages(gfp_mask, order,
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [patch 2/3] Fix GFP_THISNODE behavior for memoryless nodes
Quoted 9 lines · Christoph Lameter
Quoted 1 line · David Rientjes
That's the point. Isn't !node_memory(nid) unlikely?
Correct.
Use unlikely
Signed-off-cy: Christoph Lameter <[email protected]>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Nishanth Aravamudan
Acked-byRe: [patch 2/3] Fix GFP_THISNODE behavior for memoryless nodes
Quoted 6 lines · David Rientjes
Quoted 4 lines · Christoph Lameter
Quoted 1 line · David Rientjes
That's the point. Isn't !node_memory(nid) unlikely?
Quoted 7 lines
Quoted 5 lines
Correct.
Use unlikely
Signed-off-cy: Christoph Lameter <[email protected]>
Acked-by David Rientjes <[email protected]>
Yep, definitely makes sense.
Signature · 8 lines
Nishanth Aravamudan <[email protected]> IBM Linux Technology Center -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [patch 2/3] Fix GFP_THISNODE behavior for memoryless nodes
In reply to [email protected]Quoted 28 lines · On Tue, 2007-06-12 at 13:48 -0700, [email protected] wrote:
GFP_THISNODEchecks that the zone selected is within the pgdat (node) of the first zone of a nodelist. That only works if the node has memory. A memoryless node will have its first zone on another pgdat (node).Thus
GFP_THISNODEmay be returning memory on other nodes.GFP_THISNODEshould fail if there is no local memory on a node.So we add a check to verify that the node specified has memory in
alloc_pages_node(). If the node has no memory then return NULL.The case of
alloc_pages(GFP_THISNODE)is not changed.alloc_pages()(with no memory policies in effect) is understood to prefer the current node. If a process is running on a node with no memory then its default allocations come from the next neighboring node.GFP_THISNODEwill then force the memory to come from that node.Signed-off-by Christoph Lameter <[email protected]>Signed-off-by Nishanth Aravamudan <[email protected]>Index:
linux-2.6.22-rc4-mm2/include/linux/gfp.h
===================================================================include/linux/gfp.h +0
-0@@ -175,6 +175,13 @@ static inline struct page *alloc_pages_nif (nid <0 ) nid =numa_node_id ();
include/linux/gfp.h static inline struct page *alloc_pages_n · line 187
Quoted 10 lines
include/linux/gfp.h +7
-0+/* +* Check for the special case that GFP_THISNODE is used on a +* memoryless node +*/ +if ((gfp_mask & __GFP_THISNODE) && !node_memory (nid)) +return NULL ; +return __alloc_pages (gfp_mask, order,NODE_DATA (nid)->node_zonelists +gfp_zone (gfp_mask)); }
Attached patch fixes alloc_pages_node() so that it never returns an off-node page when GFP_THISNODE is specified by. This requires a fix to SLUB early allocation, included in the patch. Works on HP ia64 platform with small DMA only node and "zone order" zonelists. Will test on x86_64 real soon now...
---
PATCH fix GFP_THISNODE for DMA only nodes and zone-order zonelists
The map of nodes with memory may include nodes with just DMA/DMA32 memory. Using this map/mask together with GFP_THISNODE will not guarantee on-node allocations at higher zones. Modify checks in alloc_pages_node() to ensure that the first zone in the selected zonelist is "on-node".
This change will result in alloc_pages_node() returning NULL when GFP_THISNODE is specified and the first zone in the zonelist selected by (nid, gfp_zone(gfp_mask) is not on node 'nid'. This, in turn, BUGs out in slub.c:early_kmem_cache_node_alloc() which apparently can't handle a NULL page from new_slab(). Fix SLUB to handle NULL page in early allocation.
include/linux/gfp.h | 11 ++++++++--- mm/slub.c | 22 ++++++++++++---------- 2 files changed, 20 insertions(+), 13 deletions(-)
include/linux/gfp.h +8 -3
@@ -168,6 +168,9 @@ FASTCALL(__alloc_pages(gfp_t, unsigned istatic inline struct page *alloc_pages_node (int nid,gfp_t gfp_mask,unsigned int order) { +pg_data_t *pgdat; +struct zonelist *zonelist; +if (unlikely (order >=MAX_ORDER ))return NULL ;
@@ -179,11 +182,13 @@ static inline struct page *alloc_pages_n* Check for the special case that GFP_THISNODE is used on a * memoryless node */ -if ((gfp_mask & __GFP_THISNODE) && !node_memory (nid)) + pgdat =NODE_DATA (nid); + zonelist = pgdat->node_zonelists +gfp_zone (gfp_mask); +if ((gfp_mask & __GFP_THISNODE) && + pgdat != zonelist->zones[0 ]->zone_pgdat)return NULL ; -return __alloc_pages (gfp_mask, order, -NODE_DATA (nid)->node_zonelists +gfp_zone (gfp_mask)); +return __alloc_pages (gfp_mask, order, zonelist); }#ifdef CONFIG_NUMA
mm/slub.c +12 -10
@@ -1870,16 +1870,18 @@ static struct kmem_cache_node * __init e/* new_slab() disables interupts */ local_irq_enable (); -BUG_ON (!page); - n = page->freelist; -BUG_ON (!n); - page->freelist =get_freepointer (kmalloc_caches, n); - page->inuse++; - kmalloc_caches->node[node] = n; -setup_object_debug (kmalloc_caches, page, n); -init_kmem_cache_node (n); -atomic_long_inc (&n->nr_slabs); -add_partial (n, page); +if (page) { + n = page->freelist; +BUG_ON (!n); + page->freelist =get_freepointer (kmalloc_caches, n); + page->inuse++; + kmalloc_caches->node[node] = n; +setup_object_debug (kmalloc_caches, page, n); +init_kmem_cache_node (n); +atomic_long_inc (&n->nr_slabs); +add_partial (n, page); + }else + kmalloc_caches->node[node] =NULL ;return n; }
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [patch 2/3] Fix GFP_THISNODE behavior for memoryless nodes
Quoted 84 lines · Lee Schermerhorn
Quoted 28 lines · On Tue, 2007-06-12 at 13:48 -0700, [email protected] wrote:
GFP_THISNODEchecks that the zone selected is within the pgdat (node) of the first zone of a nodelist. That only works if the node has memory. A memoryless node will have its first zone on another pgdat (node).Thus
GFP_THISNODEmay be returning memory on other nodes.GFP_THISNODEshould fail if there is no local memory on a node.So we add a check to verify that the node specified has memory in
alloc_pages_node(). If the node has no memory then return NULL.The case of
alloc_pages(GFP_THISNODE)is not changed.alloc_pages()(with no memory policies in effect) is understood to prefer the current node. If a process is running on a node with no memory then its default allocations come from the next neighboring node.GFP_THISNODEwill then force the memory to come from that node.Signed-off-by Christoph Lameter <[email protected]>Signed-off-by Nishanth Aravamudan <[email protected]>Index:
linux-2.6.22-rc4-mm2/include/linux/gfp.h
===================================================================include/linux/gfp.h +0
-0@@ -175,6 +175,13 @@ static inline struct page *alloc_pages_nif (nid <0 ) nid =numa_node_id ();include/linux/gfp.h static inline struct page *alloc_pages_n · line 187
Quoted 10 lines
include/linux/gfp.h +7
-0+/* +* Check for the special case that GFP_THISNODE is used on a +* memoryless node +*/ +if ((gfp_mask & __GFP_THISNODE) && !node_memory (nid)) +return NULL ; +return __alloc_pages (gfp_mask, order,NODE_DATA (nid)->node_zonelists +gfp_zone (gfp_mask)); }Attached patch fixes
alloc_pages_node()so that it never returns an off-node page whenGFP_THISNODEis specified by. This requires a fix to SLUB early allocation, included in the patch. Works on HP ia64 platform with small DMA only node and "zone order" zonelists. Will test onx86_64real soon now...---
PATCH fix
GFP_THISNODEfor DMA only nodes and zone-order zonelistsThe map of nodes with memory may include nodes with just DMA/DMA32 memory. Using this map/mask together with
GFP_THISNODEwill not guarantee on-node allocations at higher zones. Modify checks inalloc_pages_node()to ensure that the first zone in the selected zonelist is "on-node".This change will result in
alloc_pages_node()returning NULL whenGFP_THISNODEis specified and the first zone in the zonelist selected by (nid,gfp_zone(gfp_mask)is not on node 'nid'. This, in turn, BUGs out in slub.c:early_kmem_cache_node_alloc() which apparently can't handle a NULL page fromnew_slab(). Fix SLUB to handle NULL page in early allocation.Signed-off-by Lee Schermerhorn <[email protected]>include/linux/gfp.h | 11 ++++++++--- mm/slub.c | 22 ++++++++++++---------- 2 files changed, 20 insertions(+), 13 deletions(-)Index:
Linux/include/linux/gfp.h
===================================================================include/linux/gfp.h +3
-0@@ -168,6 +168,9 @@ FASTCALL(__alloc_pages(gfp_t, unsigned istatic inline struct page *alloc_pages_node (int nid,gfp_t gfp_mask,unsigned int order) { +pg_data_t *pgdat; +struct zonelist *zonelist; +if (unlikely (order >=MAX_ORDER ))return NULL ;
include/linux/gfp.h static inline struct page *alloc_pages_n · line 188
Quoted 8 lines
include/linux/gfp.h +4
-1@@ -179,11 +182,13 @@ static inline struct page *alloc_pages_n* Check for the special case that GFP_THISNODE is used on a * memoryless node */ -if ((gfp_mask & __GFP_THISNODE) && !node_memory (nid)) + pgdat =NODE_DATA (nid); + zonelist = pgdat->node_zonelists +gfp_zone (gfp_mask); +if ((gfp_mask & __GFP_THISNODE) && + pgdat != zonelist->zones[0 ]->zone_pgdat)
This hunk won't apply on top of the unlikely() change... Rediffed below, adding into the set to test now...
From: Lee Schermerhorn <[email protected]>
PATCH fix GFP_THISNODE for DMA only nodes and zone-order zonelists
The map of nodes with memory may include nodes with just DMA/DMA32 memory. Using this map/mask together with GFP_THISNODE will not guarantee on-node allocations at higher zones. Modify checks in alloc_pages_node() to ensure that the first zone in the selected zonelist is "on-node".
This change will result in alloc_pages_node() returning NULL when GFP_THISNODE is specified and the first zone in the zonelist selected by (nid, gfp_zone(gfp_mask) is not on node 'nid'. This, in turn, BUGs out in slub.c:early_kmem_cache_node_alloc() which apparently can't handle a NULL page from new_slab(). Fix SLUB to handle NULL page in early allocation.
include/linux/gfp.h +8 -3
@@ -168,6 +168,9 @@ FASTCALL(__alloc_pages(gfp_t, unsigned int, struct zonelist *));static inline struct page *alloc_pages_node (int nid,gfp_t gfp_mask,unsigned int order) { +pg_data_t *pgdat; +struct zonelist *zonelist; +if (unlikely (order >=MAX_ORDER ))return NULL ;
@@ -175,15 +178,17 @@ static inline struct page *alloc_pages_node(int nid, gfp_t gfp_mask,if (nid <0 ) nid =numa_node_id (); + pgdat =NODE_DATA (nid); + zonelist = pgdat->node_zonelists +gfp_zone (gfp_mask);/* * Check for the special case that GFP_THISNODE is used on a * memoryless node */ -if (unlikely ((gfp_mask & __GFP_THISNODE) && !node_memory (nid))) +if (unlikely ((gfp_mask & __GFP_THISNODE) && + pgdat != zonelist->zones[0 ]->zone_pgdat))return NULL ; -return __alloc_pages (gfp_mask, order, -NODE_DATA (nid)->node_zonelists +gfp_zone (gfp_mask)); +return __alloc_pages (gfp_mask, order, zonelist); }#ifdef CONFIG_NUMA
mm/slub.c +12 -10
@@ -1870,16 +1870,18 @@ static struct kmem_cache_node * __init early_kmem_cache_node_alloc(gfp_t gfpflag/* new_slab() disables interupts */ local_irq_enable (); -BUG_ON (!page); - n = page->freelist; -BUG_ON (!n); - page->freelist =get_freepointer (kmalloc_caches, n); - page->inuse++; - kmalloc_caches->node[node] = n; -setup_object_debug (kmalloc_caches, page, n); -init_kmem_cache_node (n); -atomic_long_inc (&n->nr_slabs); -add_partial (n, page); +if (page) { + n = page->freelist; +BUG_ON (!n); + page->freelist =get_freepointer (kmalloc_caches, n); + page->inuse++; + kmalloc_caches->node[node] = n; +setup_object_debug (kmalloc_caches, page, n); +init_kmem_cache_node (n); +atomic_long_inc (&n->nr_slabs); +add_partial (n, page); + }else + kmalloc_caches->node[node] =NULL ;return n; }
Signature · 8 lines
Nishanth Aravamudan <[email protected]> IBM Linux Technology Center -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [patch 2/3] Fix GFP_THISNODE behavior for memoryless nodes
In reply to Lee SchermerhornQuoted 3 lines · Lee Schermerhorn
SLUB early allocation, included in the patch. Works on HP ia64 platform with small DMA only node and "zone order" zonelists. Will test on
x86_64real soon now...
I do not see the difference?? How does this work? node_memory(x) fails there?
Quoted 5 lines
The map of nodes with memory may include nodes with just DMA/DMA32 memory. Using this map/mask together with
GFP_THISNODEwill not guarantee on-node allocations at higher zones. Modify checks inalloc_pages_node()to ensure that the first zone in the selected zonelist is "on-node".
That check is already done by __alloc_pages.
Quoted 6 lines
This change will result in
alloc_pages_node()returning NULL whenGFP_THISNODEis specified and the first zone in the zonelist selected by (nid,gfp_zone(gfp_mask)is not on node 'nid'. This, in turn, BUGs out in slub.c:early_kmem_cache_node_alloc() which apparently can't handle a NULL page fromnew_slab(). Fix SLUB to handle NULL page in early allocation.
Ummm... Slub would need to consult node_memory_map instead I guess.
Quoted 22 lines
Index:
Linux/mm/slub.c
===================================================================mm/slub.c +4
-10@@ -1870,16 +1870,18 @@ static struct kmem_cache_node * __init e/* new_slab() disables interupts */ local_irq_enable (); -BUG_ON (!page); - n = page->freelist; -BUG_ON (!n); - page->freelist =get_freepointer (kmalloc_caches, n); - page->inuse++; - kmalloc_caches->node[node] = n; -setup_object_debug (kmalloc_caches, page, n); -init_kmem_cache_node (n); -atomic_long_inc (&n->nr_slabs); -add_partial (n, page); +if (page) { + n = page->freelist; +BUG_ON (!n); + page->freelist =get_freepointer (kmalloc_caches, n);
mm/slub.c static struct kmem_cache_node * __init e · line 1886
Quoted 10 lines
mm/slub.c +8
-0+ page->inuse++; + kmalloc_caches->node[node] = n; +setup_object_debug (kmalloc_caches, page, n); +init_kmem_cache_node (n); +atomic_long_inc (&n->nr_slabs); +add_partial (n, page); + }else + kmalloc_caches->node[node] =NULL ;return n; }
It would be easier to modify SLUB to loop over node_memory_map instead of node_online_map? Potentially we have to change all loops over online node in the slab allocators.
--- include/linux/nodemask.h | 1 + mm/slub.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-)
include/linux/nodemask.h +1 -0
@@ -377,5 +377,6 @@ extern int nr_node_ids;#define for_each_node (node) for_each_node_mask((node), node_possible_map)#define for_each_online_node (node) for_each_node_mask((node), node_online_map) +#define for_each_memory_node (node) for_each_node_mask((node), node_memory_map)#endif /* __LINUX_NODEMASK_H */
mm/slub.c +1 -1
@@ -1836,7 +1836,7 @@ static int init_kmem_cache_nodes(structelse local_node =0 ; -for_each_online_node (node) { +for_each_memory_node (node) {struct kmem_cache_node *n;if (local_node == node)
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [patch 2/3] Fix GFP_THISNODE behavior for memoryless nodes
Quoted 54 lines · Christoph Lameter
Quoted 3 lines · Lee Schermerhorn
SLUB early allocation, included in the patch. Works on HP ia64 platform with small DMA only node and "zone order" zonelists. Will test on
x86_64real soon now...I do not see the difference?? How does this work?
node_memory(x)fails there?Quoted 5 lines
The map of nodes with memory may include nodes with just DMA/DMA32 memory. Using this map/mask together with
GFP_THISNODEwill not guarantee on-node allocations at higher zones. Modify checks inalloc_pages_node()to ensure that the first zone in the selected zonelist is "on-node".That check is already done by
__alloc_pages.Quoted 6 lines
This change will result in
alloc_pages_node()returning NULL whenGFP_THISNODEis specified and the first zone in the zonelist selected by (nid,gfp_zone(gfp_mask)is not on node 'nid'. This, in turn, BUGs out in slub.c:early_kmem_cache_node_alloc() which apparently can't handle a NULL page fromnew_slab(). Fix SLUB to handle NULL page in early allocation.Ummm... Slub would need to consult
node_memory_mapinstead I guess.Quoted 28 lines
Index:
Linux/mm/slub.c
===================================================================mm/slub.c +10
-10@@ -1870,16 +1870,18 @@ static struct kmem_cache_node * __init e/* new_slab() disables interupts */ local_irq_enable (); -BUG_ON (!page); - n = page->freelist; -BUG_ON (!n); - page->freelist =get_freepointer (kmalloc_caches, n); - page->inuse++; - kmalloc_caches->node[node] = n; -setup_object_debug (kmalloc_caches, page, n); -init_kmem_cache_node (n); -atomic_long_inc (&n->nr_slabs); -add_partial (n, page); +if (page) { + n = page->freelist; +BUG_ON (!n); + page->freelist =get_freepointer (kmalloc_caches, n); + page->inuse++; + kmalloc_caches->node[node] = n; +setup_object_debug (kmalloc_caches, page, n); +init_kmem_cache_node (n); +atomic_long_inc (&n->nr_slabs); +add_partial (n, page);
Quoted 8 lines
mm/slub.c static struct kmem_cache_node * __init e · line 1886
Quoted 4 lines
mm/slub.c +2
-0+ }else + kmalloc_caches->node[node] =NULL ;return n; }It would be easier to modify SLUB to loop over
node_memory_mapinstead ofnode_online_map? Potentially we have to change all loops over online node in the slab allocators.
So, I think we are really close to closing the gaps here. Just need to figure out how to fix Lee's platform so it does what he wants, I think. I've tested the current set (which is going to change again, once we figure out how to deal with SLUB (I'm guessing we'll go with your patch Christoph, but it didn't exist when I was testing earlier :) and Lee's platform properly). But everything, including the sysfs allocator, works with a 4-node x86_64, with all nodes populated and a 4-node ppc64, with only 2 nodes populated.
I would like to roll up the patches and small fixes into a set of 4 or 5 patches that Andrew can pick up, so once this is all stable, I'll post a fresh series. Sound good, Andrew?
Thanks,
Nish
Signature · 8 lines
Nishanth Aravamudan <[email protected]> IBM Linux Technology Center -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [patch 2/3] Fix GFP_THISNODE behavior for memoryless nodes
Quoted 3 lines · Nishanth Aravamudan
I would like to roll up the patches and small fixes into a set of 4 or 5 patches that Andrew can pick up, so once this is all stable, I'll post a fresh series. Sound good, Andrew?
NACK. This patchset is not ready for any inclusion and nothing like that should go into 2.6.22. We first need to assess the breakage that results if GFP_THISNODE now returns NULL for memoryless nodes. So far GFP_THISNODE returns memory on the nearest node and that seems to make lots of things keep working.
Plus the implementation of GFP_THISNODE that we have so far is a bit complex. It would be good if that could be simpler.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [patch 2/3] Fix GFP_THISNODE behavior for memoryless nodes
Quoted 11 lines · Christoph Lameter
Quoted 3 lines · Nishanth Aravamudan
I would like to roll up the patches and small fixes into a set of 4 or 5 patches that Andrew can pick up, so once this is all stable, I'll post a fresh series. Sound good, Andrew?
NACK. This patchset is not ready for any inclusion and nothing like that should go into 2.6.22. We first need to assess the breakage that results if
GFP_THISNODEnow returns NULL for memoryless nodes. So farGFP_THISNODEreturns memory on the nearest node and that seems to make lots of things keep working.
Who the heck said anything about mainline?
See my other reply for discussing GFP_THISNODE.
Thanks,
Nish
Signature · 8 lines
Nishanth Aravamudan <[email protected]> IBM Linux Technology Center -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [patch 2/3] Fix GFP_THISNODE behavior for memoryless nodes
Quoted 1 line · Nishanth Aravamudan
Who the heck said anything about mainline?
Well we do have discovered issues that are bugs. Handling of memoryless nodes is rather strange.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [patch 2/3] Fix GFP_THISNODE behavior for memoryless nodes
Quoted 6 lines · Christoph Lameter
Quoted 1 line · Nishanth Aravamudan
Who the heck said anything about mainline?
Well we do have discovered issues that are bugs. Handling of memoryless nodes is rather strange.
Without a doubt :)
Sorry, the real reason for wrapping the patches and reposting, for me, is that we've had a lot of versions flying around, with small fixlets here and there. I wanted to start a new thread for the 3 or so patches I see that implement the core of dealing with memoryless nodes, and then keep the discussion going there, but that was purely for my own sanity.
Sorry if I overreacted.
Thanks,
Nish
Signature · 8 lines
Nishanth Aravamudan <[email protected]> IBM Linux Technology Center -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [patch 2/3] Fix GFP_THISNODE behavior for memoryless nodes
Quoted 5 lines · Nishanth Aravamudan
Sorry, the real reason for wrapping the patches and reposting, for me, is that we've had a lot of versions flying around, with small fixlets here and there. I wanted to start a new thread for the 3 or so patches I see that implement the core of dealing with memoryless nodes, and then keep the discussion going there, but that was purely for my own sanity.
Ok. I will try to put all the fixes together. Basically that will be the three patches plus Lee's new suggestion for alloc_pages_node. Plus fixes to slab / slub / the uncached allocator etc where I see that the online map is used but what was really intended was the node_memory_map. There is also stuff in the oom killer, vmscan.c etc that seems to make that assumption. Sigh.
That still leaves the issue of the name.
node_memoryset_node_has_memoryset_node_no_memory
Ok?
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [patch 2/3] Fix GFP_THISNODE behavior for memoryless nodes
Quoted 9 lines · Christoph Lameter
Quoted 6 lines · Nishanth Aravamudan
Sorry, the real reason for wrapping the patches and reposting, for me, is that we've had a lot of versions flying around, with small fixlets here and there. I wanted to start a new thread for the 3 or so patches I see that implement the core of dealing with memoryless nodes, and then keep the discussion going there, but that was purely for my own sanity.
Quoted 6 lines
Ok. I will try to put all the fixes together. Basically that will be the three patches plus Lee's new suggestion for
alloc_pages_node. Plus fixes to slab / slub / the uncached allocator etc where I see that the online map is used but what was really intended was thenode_memory_map. There is also stuff in the oom killer,vmscan.cetc that seems to make that assumption. Sigh.
Yeah, it's a big problem to solve, from an audit perspective.
Quoted 3 lines
That still leaves the issue of the name.
node_memory
node_has_memory?
Reading code, if I see:
if (node_has_memory(nid))
it's almost reading an English sentence! :)
Quoted 2 lines
set_node_has_memoryset_node_no_memory
node_set_has_memorynode_set_no_memory?
Just to be close to (but not quite the same as) online/offline.
Thanks,
Nish
Signature · 8 lines
Nishanth Aravamudan <[email protected]> IBM Linux Technology Center -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [patch 2/3] Fix GFP_THISNODE behavior for memoryless nodes
In reply to Christoph LameterQuoted 8 lines · Christoph Lameter
Quoted 3 lines · Lee Schermerhorn
SLUB early allocation, included in the patch. Works on HP ia64 platform with small DMA only node and "zone order" zonelists. Will test on
x86_64real soon now...I do not see the difference?? How does this work?
node_memory(x)fails there?
On my system, pseudo-node 4 contains ~512MB of DMA zone. With the new zoneorder auto-config patch [which this platform also needs], the zonelist for gfp_zone == ZONE_NORMAL [hugetlb attempts to allocate from this zone--as it should] contains the following zones:
zone0-normal, zone1-normal, zone2-normal, zone3-normal, zone4-dma
node_memory(4) returns "true" -- node does have memory, in the dma zone, but it's last in the list, as required. alloc_pages_node() would call __alloc_pages() with this zonelist. If get_page_from_freelist() finds the requested page in zone0-normal, the check that the zone's pgdat == the pgdat of zonelist->zone[0] will succeed and we'll return an off-node page.
Quoted 8 lines
Quoted 6 lines
The map of nodes with memory may include nodes with just DMA/DMA32 memory. Using this map/mask together with
GFP_THISNODEwill not guarantee on-node allocations at higher zones. Modify checks inalloc_pages_node()to ensure that the first zone in the selected zonelist is "on-node".That check is already done by
__alloc_pages.
You mean in get_page_from_freelist()? No, it only checks that the zone under consideration is on the same node as the zone at the start of the list. This can be off-node if the node is populated only at lower zones; and the zonelists are in zone-order.
Quoted 9 lines
Quoted 7 lines
This change will result in
alloc_pages_node()returning NULL whenGFP_THISNODEis specified and the first zone in the zonelist selected by (nid,gfp_zone(gfp_mask)is not on node 'nid'. This, in turn, BUGs out in slub.c:early_kmem_cache_node_alloc() which apparently can't handle a NULL page fromnew_slab(). Fix SLUB to handle NULL page in early allocation.Ummm... Slub would need to consult
node_memory_mapinstead I guess.
Probably should check the node_memory_map to avoid attempting allocations from completely memoryless nodes. However, it should still be able to handle nulls from alloc_pages_nodes() because of the scenarios discussed above.
Lee
Quoted 29 lines
Quoted 29 lines
Index:
Linux/mm/slub.c
===================================================================mm/slub.c +10
-10@@ -1870,16 +1870,18 @@ static struct kmem_cache_node * __init e/* new_slab() disables interupts */ local_irq_enable (); -BUG_ON (!page); - n = page->freelist; -BUG_ON (!n); - page->freelist =get_freepointer (kmalloc_caches, n); - page->inuse++; - kmalloc_caches->node[node] = n; -setup_object_debug (kmalloc_caches, page, n); -init_kmem_cache_node (n); -atomic_long_inc (&n->nr_slabs); -add_partial (n, page); +if (page) { + n = page->freelist; +BUG_ON (!n); + page->freelist =get_freepointer (kmalloc_caches, n); + page->inuse++; + kmalloc_caches->node[node] = n; +setup_object_debug (kmalloc_caches, page, n); +init_kmem_cache_node (n); +atomic_long_inc (&n->nr_slabs); +add_partial (n, page);
Quoted 8 lines
mm/slub.c static struct kmem_cache_node * __init e · line 1886
Quoted 4 lines
mm/slub.c +2
-0+ }else + kmalloc_caches->node[node] =NULL ;return n; }It would be easier to modify SLUB to loop over
node_memory_mapinstead ofnode_online_map? Potentially we have to change all loops over online node in the slab allocators.
Again, node_memory_map can't detect the "first zone in zonelist off-node" situation. That's the one that alloc_pages_node() must guard
against. So, it can/should/must return NULL when attempting to
allocate from a higher zone that is off-node.
Quoted 26 lines
--- include/linux/nodemask.h | 1 + mm/slub.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-)Index:
linux-2.6/include/linux/nodemask.h
===================================================================include/linux/nodemask.h +1
-0@@ -377,5 +377,6 @@ extern int nr_node_ids;#define for_each_node (node) for_each_node_mask((node), node_possible_map)#define for_each_online_node (node) for_each_node_mask((node), node_online_map) +#define for_each_memory_node (node) for_each_node_mask((node), node_memory_map)#endif /* __LINUX_NODEMASK_H */ Index:
linux-2.6/mm/slub.c
===================================================================
mm/slub.c static int init_kmem_cache_nodes(struct · line 1842
Quoted 8 lines
mm/slub.c +1
-1@@ -1836,7 +1836,7 @@ static int init_kmem_cache_nodes(structelse local_node =0 ; -for_each_online_node (node) { +for_each_memory_node (node) {struct kmem_cache_node *n;if (local_node == node)
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [patch 2/3] Fix GFP_THISNODE behavior for memoryless nodes
Quoted 6 lines · Lee Schermerhorn
Quoted 1 line
That check is already done by
__alloc_pages.You mean in
get_page_from_freelist()? No, it only checks that the zone under consideration is on the same node as the zone at the start of the list. This can be off-node if the node is populated only at lower zones; and the zonelists are in zone-order.
See the later discussion. I did not see the use the nodes pgdat here that you only have in alloc_pages_node().
Quoted 6 lines
Quoted 1 line
Ummm... Slub would need to consult
node_memory_mapinstead I guess.Probably should check the
node_memory_mapto avoid attempting allocations from completely memoryless nodes. However, it should still be able to handle nulls fromalloc_pages_nodes()because of the scenarios discussed above.
It is able to handle NULLs during usual operations but not during bootstrap.
Quoted 4 lines
Again,
node_memory_mapcan't detect the "first zone in zonelist off-node" situation. That's the one thatalloc_pages_node()must guard
against. So, it can/should/must return NULL when attempting to
allocate from a higher zone that is off-node.
I think GFP_THISNODE should not fail in that case.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [patch 2/3] Fix GFP_THISNODE behavior for memoryless nodes
Quoted 12 lines · Christoph Lameter
Quoted 6 lines · Lee Schermerhorn
Quoted 1 line
That check is already done by
__alloc_pages.You mean in
get_page_from_freelist()? No, it only checks that the zone under consideration is on the same node as the zone at the start of the list. This can be off-node if the node is populated only at lower zones; and the zonelists are in zone-order.See the later discussion. I did not see the use the nodes pgdat here that you only have in
alloc_pages_node().
Quoted 8 lines
Quoted 6 lines
Quoted 1 line
Ummm... Slub would need to consult
node_memory_mapinstead I guess.Probably should check the
node_memory_mapto avoid attempting allocations from completely memoryless nodes. However, it should still be able to handle nulls fromalloc_pages_nodes()because of the scenarios discussed above.It is able to handle NULLs during usual operations but not during bootstrap.
??? it has to handle memoryless nodes, right? if so, why can't it handle alloc_pages_node('THISNODE,...) returning NULL? Too late in the process?
Quoted 7 lines
Quoted 5 lines
Again,
node_memory_mapcan't detect the "first zone in zonelist off-node" situation. That's the one thatalloc_pages_node()must guard
against. So, it can/should/must return NULL when attempting to
allocate from a higher zone that is off-node.I think
GFP_THISNODEshould not fail in that case.
You think it should return off-node memory? If it can, we have to check the returned page's nid--e.g., in alloc_fresh_huge_page()--where we really don't want an off-node page. Could do that, I suppose.
Or do you mean that GPF_THISNODE should return memory from the lower zones, if it satisfies the order requirement? Your custom 'thisnode' zonelist will enable this, right? That would work for my particular config of my platform, but on a larger platform, I'd have a larger DMA zone and wouldn't want hugetlb pages coming from there.
I guess that in the long run, I need to hope that Nish's per node huge page attribute goes in. Then, we can provide a fancy script to configure huge pages on a specific set of nodes, rather than relying on the kernel to distribute them evenly across the set of nodes that "make sense" for the given platform.
Later,
Lee
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [patch 2/3] Fix GFP_THISNODE behavior for memoryless nodes
Quoted 5 lines · Lee Schermerhorn
Quoted 1 line
It is able to handle NULLs during usual operations but not during bootstrap.
??? it has to handle memoryless nodes, right? if so, why can't it handle
alloc_pages_node('THISNODE,...)returning NULL? Too late in the process?
It should not call alloc_pages_node for a node that has no memory during bootstrap.
Quoted 5 lines
Or do you mean that
GPF_THISNODEshould return memory from the lower zones, if it satisfies the order requirement? Your custom 'thisnode' zonelist will enable this, right? That would work for my particular config of my platform, but on a larger platform, I'd have a larger DMA zone and wouldn't want hugetlb pages coming from there.
Right. The fixes here are for the general vm and not for hugetlb. They are definitely not HP platform specific.
Quoted 5 lines
I guess that in the long run, I need to hope that Nish's per node huge page attribute goes in. Then, we can provide a fancy script to configure huge pages on a specific set of nodes, rather than relying on the kernel to distribute them evenly across the set of nodes that "make sense" for the given platform.
Right.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [patch 2/3] Fix GFP_THISNODE behavior for memoryless nodes
In reply to Lee SchermerhornQuoted 17 lines · Lee Schermerhorn
include/linux/gfp.h +3
-1@@ -168,6 +168,9 @@ FASTCALL(__alloc_pages(gfp_t, unsigned istatic inline struct page *alloc_pages_node (int nid,gfp_t gfp_mask,unsigned int order) { +pg_data_t *pgdat; +struct zonelist *zonelist; +if (unlikely (order >=MAX_ORDER ))return NULL ;@@ -179,11 +182,13 @@ static inline struct page *alloc_pages_n* Check for the special case that GFP_THISNODE is used on a * memoryless node */ -if ((gfp_mask & __GFP_THISNODE) && !node_memory (nid))
include/linux/gfp.h static inline struct page *alloc_pages_n · line 192
Quoted 10 lines
include/linux/gfp.h +5
-2+ pgdat =NODE_DATA (nid); + zonelist = pgdat->node_zonelists +gfp_zone (gfp_mask); +if ((gfp_mask & __GFP_THISNODE) && + pgdat != zonelist->zones[0 ]->zone_pgdat)return NULL ; -return __alloc_pages (gfp_mask, order, -NODE_DATA (nid)->node_zonelists +gfp_zone (gfp_mask)); +return __alloc_pages (gfp_mask, order, zonelist); }
Good idea but I think this does not address the case where the DMA zone of a node was moved to the end of the zonelist. In that case the first zone is not on the first pgdat but the node has memory. The memory of the node is listed elsewhere in the nodelist. I can probably modify __alloc_pages to make GFP_THISNODE to check all zones but we do not have the pgdat reference there. Sigh.
How about generating a special THISNODE zonelist in build_zonelist that only contains the zones of a single node. Then just use that one if GFP_THISNODE is set. Then we get rid of all the GFP_THISNODE crap that I added to __alloc_pages?
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [patch 2/3] Fix GFP_THISNODE behavior for memoryless nodes
Quoted 37 lines · Christoph Lameter
Quoted 17 lines · Lee Schermerhorn
include/linux/gfp.h +3
-1@@ -168,6 +168,9 @@ FASTCALL(__alloc_pages(gfp_t, unsigned istatic inline struct page *alloc_pages_node (int nid,gfp_t gfp_mask,unsigned int order) { +pg_data_t *pgdat; +struct zonelist *zonelist; +if (unlikely (order >=MAX_ORDER ))return NULL ;@@ -179,11 +182,13 @@ static inline struct page *alloc_pages_n* Check for the special case that GFP_THISNODE is used on a * memoryless node */ -if ((gfp_mask & __GFP_THISNODE) && !node_memory (nid))include/linux/gfp.h static inline struct page *alloc_pages_n · line 192
Quoted 10 lines
include/linux/gfp.h +5
-2+ pgdat =NODE_DATA (nid); + zonelist = pgdat->node_zonelists +gfp_zone (gfp_mask); +if ((gfp_mask & __GFP_THISNODE) && + pgdat != zonelist->zones[0 ]->zone_pgdat)return NULL ; -return __alloc_pages (gfp_mask, order, -NODE_DATA (nid)->node_zonelists +gfp_zone (gfp_mask)); +return __alloc_pages (gfp_mask, order, zonelist); }Good idea but I think this does not address the case where the DMA zone of a node was moved to the end of the zonelist. In that case the first zone is not on the first pgdat but the node has memory. The memory of the node is listed elsewhere in the nodelist. I can probably modify
__alloc_pagesto makeGFP_THISNODEto check all zones but we do not have the pgdat reference there. Sigh.
Quoted 4 lines
How about generating a special THISNODE zonelist in
build_zonelistthat only contains the zones of a single node. Then just use that one ifGFP_THISNODEis set. Then we get rid of all theGFP_THISNODEcrap that I added to__alloc_pages?
Makes sense to me.
Thanks,
NIsh
Signature · 8 lines
Nishanth Aravamudan <[email protected]> IBM Linux Technology Center -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 3/3 Fix MPOL_INTERLEAVE behavior for memoryless nodes
In reply to [email protected]MPOL_INTERLEAVE currently simply loops over all nodes. Allocations on memoryless nodes will be redirected to nodes with memory. This results in an imbalance because the neighboring nodes to memoryless nodes will get significantly more interleave hits that the rest of the nodes.
We can avoid this imbalance by clearing the nodes in the interleave node set that have no memory.
mm/mempolicy.c +2 -1
@@ -185,7 +185,8 @@ static struct mempolicy *mpol_new(int moswitch (mode) {case MPOL_INTERLEAVE : policy->v.nodes = *nodes; -if (nodes_weight (*nodes) ==0 ) { +nodes_and (policy->v.nodes, policy->v.nodes, node_memory_map); +if (nodes_weight (policy->v.nodes) ==0 ) {kmem_cache_free (policy_cache, policy);return ERR_PTR (-EINVAL ); }
Signature · 5 lines
-- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>