Re: [PATCH] mempolicy:add GFP_THISNODE when allocing new page

[Bob Liu <lliubbo@gmail.com>]

On 4/6/10, Minchan Kim <minchan.kim@gmail.com> wrote:
> On Tue, Apr 6, 2010 at 11:59 AM, Bob Liu <lliubbo@gmail.com> wrote:
>> In funtion migrate_pages(), if the dest node have no
>> enough free pages,it will fallback to other nodes.
>> Add GFP_THISNODE to avoid this, the same as what
>> funtion new_page_node() do in migrate.c.
>>
>> Signed-off-by: Bob Liu <lliubbo@gmail.com>
>
> Yes. It can be fixed. but I have a different concern.
>
> I looked at 6484eb3e2a81807722c5f28ef.
> "   page allocator: do not check NUMA node ID when the caller knows
> the node is valid
>
>    Callers of alloc_pages_node() can optionally specify -1 as a node to mean
>    "allocate from the current node".  However, a number of the callers in
>    fast paths know for a fact their node is valid.  To avoid a comparison
> and
>    branch, this patch adds alloc_pages_exact_node() that only checks the nid
>    with VM_BUG_ON().  Callers that know their node is valid are then
>    converted."
>
> alloc_pages_exact_node's naming would be not good.
> It is not for allocate page from exact node but just for
> removing check of node's valid.
> Some people like me who is poor english could misunderstood it.
>
> How about changing name with following?
> /* This function can allocate page to fallback list of node*/
> alloc_pages_by_nodeid(...)
>
> And instead of it, let's change alloc_pages_exact_node with following.
> static inline struct page *alloc_pages_exact_node(...)
> {
>  VM_BUG_ON ..
>  return __alloc_pages(gfp_mask|__GFP_THISNODE...);
> }
>
> I think it's more clear than old.
> What do you think about it?
>
Hm.. I agree with you, I was also misunderstanding by the name.
But let's still waiting for some other reply.

By the way, what about your opinion using GFP_THISNODE or
__GFP_THISNODE in __alloc_pages().
I think GFP_THISNODE is ok.

Thanks.
-- 
Regards,
--Bob

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