numa api comments

numa api comments

[Christoph Hellwig]

(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 majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>

Re: numa api comments

[Martin J. Bligh]

>  - 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.

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.

>  - 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)) {

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 majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>

Re: numa api comments

[Paul Jackson]

Martin groaned:
> 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?

-- 
                          I won't rest till it's the best ...
                          Programmer, Linux Scalability
                          Paul Jackson <pj@sgi.com> 1.650.933.1373
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>