Re: [PATCH] code clean rename alloc_pages_exact_node()

[KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>]

On Tue, 13 Apr 2010 13:34:52 +0900
Minchan Kim <minchan.kim@gmail.com> wrote:

> On Tue, Apr 13, 2010 at 1:43 AM, Mel Gorman <mel@csn.ul.ie> wrote:
> > On Sat, Apr 10, 2010 at 07:49:32PM +0800, Bob Liu wrote:
> >> Since alloc_pages_exact_node() is not for allocate page from
> >> exact node but just for removing check of node's valid,
> >> rename it to alloc_pages_from_valid_node(). Else will make
> >> people misunderstanding.
> >>
> >
> > I don't know about this change either but as I introduced the original
> > function name, I am biased. My reading of it is - allocate me pages and
> > I know exactly which node I need. I see how it it could be read as
> > "allocate me pages from exactly this node" but I don't feel the new
> > naming is that much clearer either.
> 
> Tend to agree.
> Then, don't change function name but add some comment?
> 
> /*
>  * allow pages from fallback if page allocator can't find free page in your nid.
>  * If you want to allocate page from exact node, please use
> __GFP_THISNODE flags with
>  * gfp_mask.
>  */
> static inline struct page *alloc_pages_exact_node(....
> 
I vote for this rather than renaming.

There are two functions
	allo_pages_node()
	alloc_pages_exact_node().

Sane progmrammers tend to see implementation details if there are 2
similar functions.

If I name the function,
	alloc_pages_node_verify_nid() ?

I think /* This doesn't support nid=-1, automatic behavior. */ is necessary
as comment.

OFF_TOPIC

If you want renaming,  I think we should define NID=-1 as

#define ARBITRARY_NID		(-1) or
#define CURRENT_NID		(-1) or
#define AUTO_NID		(-1)

or some. Then, we'll have concensus of NID=-1 support.
(Maybe some amount of programmers don't know what NID=-1 means.)

The function will be
	alloc_pages_node_no_auto_nid() /* AUTO_NID is not supported by this */
or
	alloc_pages_node_veryfy_nid()

Maybe patch will be bigger and may fail after discussion. But it seems
worth to try.

Thanks,
-Kame



--
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:"dont@kvack.org"> email@kvack.org </a>