On 4/6/10, Minchan Kim <minchan.kim@xxxxxxxxx> wrote:
> On Tue, Apr 6, 2010 at 11:59 AM, Bob Liu <lliubbo@xxxxxxxxx> 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@xxxxxxxxx>
>
> 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@xxxxxxxxxx For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>