numa api comments
(based on the code in 2.6.6-rc1-mm1)
- the
if (unlikely(order >= MAX_ORDER)) return NULL;
in alloc_pages_node and your new alloc_pages should probably move into __alloc_pages, thus making alloc_pages_current as an entinity of it's own superflous. It's naming is rather strange anyway.
- you add an extern for __alloc_page_vma but it doesn't seem to be implemented at all
- alloc_page_vma arguments seems backwards. We usually have gfp_flags arguments last which is kinda natural. Can we change the prototype to
alloc_page_vma(struct vm_area_struct *vma, unsigned long addr, unsigned gfp_mask);
? Dito for sched.h, that one is used even more..
- could you please move the struct vm_area_struct forward delcaration somewhere near the top of gfp.h (I usually prefer just below the includes) ? In the middle of the prototypes it looks rather distracting.
- does mm.h as a widely-used header really need to include mempolicy.h? AFAICS a forward-declaration of struct mempolicy would do it.
- can we please have a for_each_node() instead of mess like
for (nd = find_first_bit(nodes, MAX_NUMNODES);
nd < MAX_NUMNODES;
nd = find_next_bit(nodes, MAX_NUMNODES, 1+nd)) {?
- swapin_readahead() seems to be used only in mm/memory.c, what about making it static?
- alloc_page_interleave should probably reuse te existing alloc_pages_node, ala:
static struct page *alloc_page_interleave(unsigned gfp, unsigned nid)
{
struct page *page = alloc_pages_node(nid, gfp_mask, 0);
if (page && page_zone(page) == zl->zones[0]) {
zl->zones[0]->pageset[get_cpu()].interleave_hit++;
put_cpu();
}
return page;
} - the addition of mpol_set_vma_default() to gazillions of vma initializations looking almost the same says we really want some helper for it finally..
--
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>
Quoted 8 lines
- the
if (unlikely(order >= MAX_ORDER)) return NULL;in
alloc_pages_nodeand your newalloc_pagesshould probably move into__alloc_pages, thus makingalloc_pages_currentas an entinity of it's own superflous. It's naming is rather strange anyway.
This comes up again and again, it probably needs a big fat comment to explain itself (I know I've asked the same before at least once ;-)). The alloc_pages wrapper bit is inlined, as order is normally a constant, and thus that check will compile away 99% of the time. If we move it into the main __alloc_pages function, it'll be another check in the fastpath that we don't need most of the time.
Quoted 5 lines
- can we please have a
for_each_node()instead of mess likefor (nd = find_first_bit(nodes, MAX_NUMNODES); nd < MAX_NUMNODES; nd = find_next_bit(nodes, MAX_NUMNODES, 1+nd)) {
I'd swear we had one of those already to iterate over 1 .. numnodes, but I can't find it. Grrr.
M.
--
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>
Martin groaned:
Quoted 2 lines
I'd swear we had one of those already to iterate over 1 .. numnodes, but I can't find it. Grrr.
Are you thinking of Matthew Dobson's nodemask patch, which has for_each_node() and a couple related loops, in include/linux/nodemask.h?
Signature · 8 lines
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.650.933.1373
--
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>