On Tue, Apr 13, 2010 at 4:09 PM, Bob Liu <lliubbo@xxxxxxxxx> wrote: > On 4/13/10, KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx> wrote: >> On Tue, 13 Apr 2010 13:34:52 +0900 >> >> Minchan Kim <minchan.kim@xxxxxxxxx> wrote: >> >> >> > On Tue, Apr 13, 2010 at 1:43 AM, Mel Gorman <mel@xxxxxxxxx> 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. >> > > Hm..It's a bit bigger. > Actually, what I want to do was in my original mail several days ago, > the title is "mempolicy:add GFP_THISNODE when allocing new page" Yes. At that time, I suggested new naming. I agreed this patch and Mel didn't opposed it if others think new naming is better. I still open my mind with new naming if you suggest better naming. How about 'allocate_pages_explicit_node?' -- Kind regards, Minchan Kim -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxxx For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href