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"
What I concern is *just* we shouldn't fallback to other nodes if the
dest node haven't enough free pages during migrate_pages().
The detail is below:
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>
---
mm/mempolicy.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 08f40a2..fc5ddf5 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -842,7 +842,8 @@ static void migrate_page_add(struct page *page,
struct list_head *pagelist,
static struct page *new_node_page(struct page *page, unsigned long
node, int **x)
{
- return alloc_pages_exact_node(node, GFP_HIGHUSER_MOVABLE, 0);
+ return alloc_pages_exact_node(node,
+ GFP_HIGHUSER_MOVABLE | GFP_THISNODE, 0);
}
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>