[PATCH] code clean rename alloc_pages_exact_node()

[PATCH] code clean rename alloc_pages_exact_node()

[Bob Liu]

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.

Signed-off-by: Bob Liu <lliubbo@gmail.com>
---
 arch/ia64/hp/common/sba_iommu.c   |    2 +-
 arch/ia64/kernel/uncached.c       |    2 +-
 arch/ia64/sn/pci/pci_dma.c        |    2 +-
 arch/powerpc/platforms/cell/ras.c |    4 ++--
 arch/x86/kvm/vmx.c                |    3 ++-
 drivers/misc/sgi-xp/xpc_uv.c      |    5 +++--
 include/linux/gfp.h               |    2 +-
 kernel/profile.c                  |    8 ++++----
 mm/filemap.c                      |    2 +-
 mm/hugetlb.c                      |    2 +-
 mm/memory-failure.c               |    2 +-
 mm/mempolicy.c                    |    2 +-
 mm/migrate.c                      |    2 +-
 mm/slab.c                         |    3 ++-
 mm/slob.c                         |    4 ++--
 15 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/arch/ia64/hp/common/sba_iommu.c b/arch/ia64/hp/common/sba_iommu.c
index e14c492..e578f08 100644
--- a/arch/ia64/hp/common/sba_iommu.c
+++ b/arch/ia64/hp/common/sba_iommu.c
@@ -1140,7 +1140,7 @@ sba_alloc_coherent (struct device *dev, size_t size, dma_addr_t *dma_handle, gfp
 #ifdef CONFIG_NUMA
 	{
 		struct page *page;
-		page = alloc_pages_exact_node(ioc->node == MAX_NUMNODES ?
+		page = alloc_pages_from_valid_node(ioc->node == MAX_NUMNODES ?
 		                        numa_node_id() : ioc->node, flags,
 		                        get_order(size));
 
diff --git a/arch/ia64/kernel/uncached.c b/arch/ia64/kernel/uncached.c
index c4696d2..ab8c085 100644
--- a/arch/ia64/kernel/uncached.c
+++ b/arch/ia64/kernel/uncached.c
@@ -98,7 +98,7 @@ static int uncached_add_chunk(struct uncached_pool *uc_pool, int nid)
 
 	/* attempt to allocate a granule's worth of cached memory pages */
 
-	page = alloc_pages_exact_node(nid,
+	page = alloc_pages_from_valid_node(nid,
 				GFP_KERNEL | __GFP_ZERO | GFP_THISNODE,
 				IA64_GRANULE_SHIFT-PAGE_SHIFT);
 	if (!page) {
diff --git a/arch/ia64/sn/pci/pci_dma.c b/arch/ia64/sn/pci/pci_dma.c
index a9d310d..e09ccf7 100644
--- a/arch/ia64/sn/pci/pci_dma.c
+++ b/arch/ia64/sn/pci/pci_dma.c
@@ -91,7 +91,7 @@ static void *sn_dma_alloc_coherent(struct device *dev, size_t size,
 	 */
 	node = pcibus_to_node(pdev->bus);
 	if (likely(node >=0)) {
-		struct page *p = alloc_pages_exact_node(node,
+		struct page *p = alloc_pages_from_valid_node(node,
 						flags, get_order(size));
 
 		if (likely(p))
diff --git a/arch/powerpc/platforms/cell/ras.c b/arch/powerpc/platforms/cell/ras.c
index 1d3c4ef..6d32594 100644
--- a/arch/powerpc/platforms/cell/ras.c
+++ b/arch/powerpc/platforms/cell/ras.c
@@ -123,8 +123,8 @@ static int __init cbe_ptcal_enable_on_node(int nid, int order)
 
 	area->nid = nid;
 	area->order = order;
-	area->pages = alloc_pages_exact_node(area->nid, GFP_KERNEL|GFP_THISNODE,
-						area->order);
+	area->pages = alloc_pages_from_valid_node(area->nid,
+			GFP_KERNEL | GFP_THISNODE, area->order);
 
 	if (!area->pages) {
 		printk(KERN_WARNING "%s: no page on node %d\n",
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 753ffc2..f554b8c 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -1405,7 +1405,8 @@ static struct vmcs *alloc_vmcs_cpu(int cpu)
 	struct page *pages;
 	struct vmcs *vmcs;
 
-	pages = alloc_pages_exact_node(node, GFP_KERNEL, vmcs_config.order);
+	pages = alloc_pages_from_valid_node(node, GFP_KERNEL,
+			vmcs_config.order);
 	if (!pages)
 		return NULL;
 	vmcs = page_address(pages);
diff --git a/drivers/misc/sgi-xp/xpc_uv.c b/drivers/misc/sgi-xp/xpc_uv.c
index 1f59ee2..ba3544e 100644
--- a/drivers/misc/sgi-xp/xpc_uv.c
+++ b/drivers/misc/sgi-xp/xpc_uv.c
@@ -238,8 +238,9 @@ xpc_create_gru_mq_uv(unsigned int mq_size, int cpu, char *irq_name,
 	mq->mmr_blade = uv_cpu_to_blade_id(cpu);
 
 	nid = cpu_to_node(cpu);
-	page = alloc_pages_exact_node(nid, GFP_KERNEL | __GFP_ZERO | GFP_THISNODE,
-				pg_order);
+	page = alloc_pages_from_valid_node(nid,
+			GFP_KERNEL | __GFP_ZERO | GFP_THISNODE, pg_order);
+
 	if (page == NULL) {
 		dev_err(xpc_part, "xpc_create_gru_mq_uv() failed to alloc %d "
 			"bytes of memory on nid=%d for GRU mq\n", mq_size, nid);
diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index 4c6d413..c94f2ed 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -288,7 +288,7 @@ static inline struct page *alloc_pages_node(int nid, gfp_t gfp_mask,
 	return __alloc_pages(gfp_mask, order, node_zonelist(nid, gfp_mask));
 }
 
-static inline struct page *alloc_pages_exact_node(int nid, gfp_t gfp_mask,
+static inline struct page *alloc_pages_from_valid_node(int nid, gfp_t gfp_mask,
 						unsigned int order)
 {
 	VM_BUG_ON(nid < 0 || nid >= MAX_NUMNODES);
diff --git a/kernel/profile.c b/kernel/profile.c
index a55d3a3..4bf82da 100644
--- a/kernel/profile.c
+++ b/kernel/profile.c
@@ -366,7 +366,7 @@ static int __cpuinit profile_cpu_callback(struct notifier_block *info,
 		node = cpu_to_node(cpu);
 		per_cpu(cpu_profile_flip, cpu) = 0;
 		if (!per_cpu(cpu_profile_hits, cpu)[1]) {
-			page = alloc_pages_exact_node(node,
+			page = alloc_pages_from_valid_node(node,
 					GFP_KERNEL | __GFP_ZERO,
 					0);
 			if (!page)
@@ -374,7 +374,7 @@ static int __cpuinit profile_cpu_callback(struct notifier_block *info,
 			per_cpu(cpu_profile_hits, cpu)[1] = page_address(page);
 		}
 		if (!per_cpu(cpu_profile_hits, cpu)[0]) {
-			page = alloc_pages_exact_node(node,
+			page = alloc_pages_from_valid_node(node,
 					GFP_KERNEL | __GFP_ZERO,
 					0);
 			if (!page)
@@ -568,14 +568,14 @@ static int create_hash_tables(void)
 		int node = cpu_to_node(cpu);
 		struct page *page;
 
-		page = alloc_pages_exact_node(node,
+		page = alloc_pages_from_valid_node(node,
 				GFP_KERNEL | __GFP_ZERO | GFP_THISNODE,
 				0);
 		if (!page)
 			goto out_cleanup;
 		per_cpu(cpu_profile_hits, cpu)[1]
 				= (struct profile_hit *)page_address(page);
-		page = alloc_pages_exact_node(node,
+		page = alloc_pages_from_valid_node(node,
 				GFP_KERNEL | __GFP_ZERO | GFP_THISNODE,
 				0);
 		if (!page)
diff --git a/mm/filemap.c b/mm/filemap.c
index 140ebda..0424c3b 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -463,7 +463,7 @@ struct page *__page_cache_alloc(gfp_t gfp)
 {
 	if (cpuset_do_page_mem_spread()) {
 		int n = cpuset_mem_spread_node();
-		return alloc_pages_exact_node(n, gfp, 0);
+		return alloc_pages_from_valid_node(n, gfp, 0);
 	}
 	return alloc_pages(gfp, 0);
 }
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 6034dc9..01dd9c6 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -607,7 +607,7 @@ static struct page *alloc_fresh_huge_page_node(struct hstate *h, int nid)
 	if (h->order >= MAX_ORDER)
 		return NULL;
 
-	page = alloc_pages_exact_node(nid,
+	page = alloc_pages_from_valid_node(nid,
 		htlb_alloc_mask|__GFP_COMP|__GFP_THISNODE|
 						__GFP_REPEAT|__GFP_NOWARN,
 		huge_page_order(h));
diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 620b0b4..43abda1 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -1126,7 +1126,7 @@ EXPORT_SYMBOL(unpoison_memory);
 static struct page *new_page(struct page *p, unsigned long private, int **x)
 {
 	int nid = page_to_nid(p);
-	return alloc_pages_exact_node(nid, GFP_HIGHUSER_MOVABLE, 0);
+	return alloc_pages_from_valid_node(nid, GFP_HIGHUSER_MOVABLE, 0);
 }
 
 /*
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 08f40a2..6838cd8 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -842,7 +842,7 @@ 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_from_valid_node(node, GFP_HIGHUSER_MOVABLE, 0);
 }
 
 /*
diff --git a/mm/migrate.c b/mm/migrate.c
index d3f3f7f..a057a1a 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -770,7 +770,7 @@ static struct page *new_page_node(struct page *p, unsigned long private,
 
 	*result = &pm->status;
 
-	return alloc_pages_exact_node(pm->node,
+	return alloc_pages_from_valid_node(pm->node,
 				GFP_HIGHUSER_MOVABLE | GFP_THISNODE, 0);
 }
 
diff --git a/mm/slab.c b/mm/slab.c
index 730d45b..4f71736 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -1717,7 +1717,8 @@ static void *kmem_getpages(struct kmem_cache *cachep, gfp_t flags, int nodeid)
 	if (cachep->flags & SLAB_RECLAIM_ACCOUNT)
 		flags |= __GFP_RECLAIMABLE;
 
-	page = alloc_pages_exact_node(nodeid, flags | __GFP_NOTRACK, cachep->gfporder);
+	page = alloc_pages_from_valid_node(nodeid, flags | __GFP_NOTRACK,
+			cachep->gfporder);
 	if (!page)
 		return NULL;
 
diff --git a/mm/slob.c b/mm/slob.c
index 837ebd6..9e3f95b 100644
--- a/mm/slob.c
+++ b/mm/slob.c
@@ -46,7 +46,7 @@
  * NUMA support in SLOB is fairly simplistic, pushing most of the real
  * logic down to the page allocator, and simply doing the node accounting
  * on the upper levels. In the event that a node id is explicitly
- * provided, alloc_pages_exact_node() with the specified node id is used
+ * provided, alloc_pages_from_valid_node() with the specified node id is used
  * instead. The common case (or when the node id isn't explicitly provided)
  * will default to the current node, as per numa_node_id().
  *
@@ -244,7 +244,7 @@ static void *slob_new_pages(gfp_t gfp, int order, int node)
 
 #ifdef CONFIG_NUMA
 	if (node != -1)
-		page = alloc_pages_exact_node(node, gfp, order);
+		page = alloc_pages_from_valid_node(node, gfp, order);
 	else
 #endif
 		page = alloc_pages(gfp, order);
-- 
1.5.6.3

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

[PATCH] add alloc_pages_exact_node()

[Bob Liu]

Add alloc_pages_exact_node() to allocate pages from exact
node.

Signed-off-by: Bob Liu <lliubbo@gmail.com>
---
 arch/powerpc/platforms/cell/ras.c |    4 ++--
 include/linux/gfp.h               |    7 +++++++
 mm/mempolicy.c                    |    2 +-
 mm/migrate.c                      |    3 +--
 4 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/platforms/cell/ras.c b/arch/powerpc/platforms/cell/ras.c
index 6d32594..93a5afd 100644
--- a/arch/powerpc/platforms/cell/ras.c
+++ b/arch/powerpc/platforms/cell/ras.c
@@ -123,8 +123,8 @@ static int __init cbe_ptcal_enable_on_node(int nid, int order)
 
 	area->nid = nid;
 	area->order = order;
-	area->pages = alloc_pages_from_valid_node(area->nid,
-			GFP_KERNEL | GFP_THISNODE, area->order);
+	area->pages = alloc_pages_exact_node(area->nid, GFP_KERNEL,
+				area->order);
 
 	if (!area->pages) {
 		printk(KERN_WARNING "%s: no page on node %d\n",
diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index c94f2ed..70cf2ae 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -296,6 +296,13 @@ static inline struct page *alloc_pages_from_valid_node(int nid, gfp_t gfp_mask,
 	return __alloc_pages(gfp_mask, order, node_zonelist(nid, gfp_mask));
 }
 
+static inline struct page *alloc_pages_exact_node(int nid, gfp_t gfp_mask,
+						unsigned int order)
+{
+	return alloc_pages_from_valid_node(nid, gfp_mask | GFP_THISNODE,
+			order);
+}
+
 #ifdef CONFIG_NUMA
 extern struct page *alloc_pages_current(gfp_t gfp_mask, unsigned order);
 
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 6838cd8..08f40a2 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -842,7 +842,7 @@ 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_from_valid_node(node, GFP_HIGHUSER_MOVABLE, 0);
+	return alloc_pages_exact_node(node, GFP_HIGHUSER_MOVABLE, 0);
 }
 
 /*
diff --git a/mm/migrate.c b/mm/migrate.c
index a057a1a..17330c5 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -770,8 +770,7 @@ static struct page *new_page_node(struct page *p, unsigned long private,
 
 	*result = &pm->status;
 
-	return alloc_pages_from_valid_node(pm->node,
-				GFP_HIGHUSER_MOVABLE | GFP_THISNODE, 0);
+	return alloc_pages_exact_node(pm->node, GFP_HIGHUSER_MOVABLE, 0);
 }
 
 /*
-- 
1.5.6.3

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

Re: [PATCH] code clean rename alloc_pages_exact_node()

[Minchan Kim]

On Sat, Apr 10, 2010 at 8:49 PM, Bob Liu <lliubbo@gmail.com> 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.
>
> Signed-off-by: Bob Liu <lliubbo@gmail.com>
Reviewed-by: Minchan Kim <minchan.kim@gmail.com>

I like this naming.
-- 
Kind regards,
Minchan Kim

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

Re: [PATCH] add alloc_pages_exact_node()

[Minchan Kim]

Hi, Bob.

On Sat, Apr 10, 2010 at 8:49 PM, Bob Liu <lliubbo@gmail.com> wrote:
> Add alloc_pages_exact_node() to allocate pages from exact
> node.
>
> Signed-off-by: Bob Liu <lliubbo@gmail.com>
> ---
>  arch/powerpc/platforms/cell/ras.c |    4 ++--
>  include/linux/gfp.h               |    7 +++++++
>  mm/mempolicy.c                    |    2 +-
>  mm/migrate.c                      |    3 +--
>  4 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/platforms/cell/ras.c b/arch/powerpc/platforms/cell/ras.c
> index 6d32594..93a5afd 100644
> --- a/arch/powerpc/platforms/cell/ras.c
> +++ b/arch/powerpc/platforms/cell/ras.c
> @@ -123,8 +123,8 @@ static int __init cbe_ptcal_enable_on_node(int nid, int order)
>
>        area->nid = nid;
>        area->order = order;
> -       area->pages = alloc_pages_from_valid_node(area->nid,
> -                       GFP_KERNEL | GFP_THISNODE, area->order);
> +       area->pages = alloc_pages_exact_node(area->nid, GFP_KERNEL,
> +                               area->order);
>
>        if (!area->pages) {
>                printk(KERN_WARNING "%s: no page on node %d\n",
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index c94f2ed..70cf2ae 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -296,6 +296,13 @@ static inline struct page *alloc_pages_from_valid_node(int nid, gfp_t gfp_mask,
>        return __alloc_pages(gfp_mask, order, node_zonelist(nid, gfp_mask));
>  }
>
> +static inline struct page *alloc_pages_exact_node(int nid, gfp_t gfp_mask,
> +                                               unsigned int order)
> +{
> +       return alloc_pages_from_valid_node(nid, gfp_mask | GFP_THISNODE,
> +                       order);
> +}
> +
>  #ifdef CONFIG_NUMA
>  extern struct page *alloc_pages_current(gfp_t gfp_mask, unsigned order);
>
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index 6838cd8..08f40a2 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -842,7 +842,7 @@ 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_from_valid_node(node, GFP_HIGHUSER_MOVABLE, 0);
> +       return alloc_pages_exact_node(node, GFP_HIGHUSER_MOVABLE, 0);
>  }

It's behavior change. Please, write down why you want to change
behavior in log.
Although I knew it, you need to explain it for others and git log,

-- 
Kind regards,
Minchan Kim

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

Re: [PATCH] code clean rename alloc_pages_exact_node()

[Bob Liu]

On Mon, Apr 12, 2010 at 11:34 AM, Minchan Kim <minchan.kim@gmail.com> wrote:
> On Sat, Apr 10, 2010 at 8:49 PM, Bob Liu <lliubbo@gmail.com> 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.
>>
>> Signed-off-by: Bob Liu <lliubbo@gmail.com>
> Reviewed-by: Minchan Kim <minchan.kim@gmail.com>
>

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>

Re: [PATCH] add alloc_pages_exact_node()

[Bob Liu]

On Mon, Apr 12, 2010 at 11:38 AM, Minchan Kim <minchan.kim@gmail.com> wrote:
> Hi, Bob.
>
> On Sat, Apr 10, 2010 at 8:49 PM, Bob Liu <lliubbo@gmail.com> wrote:
>> Add alloc_pages_exact_node() to allocate pages from exact
>> node.
>>
>> Signed-off-by: Bob Liu <lliubbo@gmail.com>
>> ---
>>  arch/powerpc/platforms/cell/ras.c |    4 ++--
>>  include/linux/gfp.h               |    7 +++++++
>>  mm/mempolicy.c                    |    2 +-
>>  mm/migrate.c                      |    3 +--
>>  4 files changed, 11 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/cell/ras.c b/arch/powerpc/platforms/cell/ras.c
>> index 6d32594..93a5afd 100644
>> --- a/arch/powerpc/platforms/cell/ras.c
>> +++ b/arch/powerpc/platforms/cell/ras.c
>> @@ -123,8 +123,8 @@ static int __init cbe_ptcal_enable_on_node(int nid, int order)
>>
>>        area->nid = nid;
>>        area->order = order;
>> -       area->pages = alloc_pages_from_valid_node(area->nid,
>> -                       GFP_KERNEL | GFP_THISNODE, area->order);
>> +       area->pages = alloc_pages_exact_node(area->nid, GFP_KERNEL,
>> +                               area->order);
>>
>>        if (!area->pages) {
>>                printk(KERN_WARNING "%s: no page on node %d\n",
>> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
>> index c94f2ed..70cf2ae 100644
>> --- a/include/linux/gfp.h
>> +++ b/include/linux/gfp.h
>> @@ -296,6 +296,13 @@ static inline struct page *alloc_pages_from_valid_node(int nid, gfp_t gfp_mask,
>>        return __alloc_pages(gfp_mask, order, node_zonelist(nid, gfp_mask));
>>  }
>>
>> +static inline struct page *alloc_pages_exact_node(int nid, gfp_t gfp_mask,
>> +                                               unsigned int order)
>> +{
>> +       return alloc_pages_from_valid_node(nid, gfp_mask | GFP_THISNODE,
>> +                       order);
>> +}
>> +
>>  #ifdef CONFIG_NUMA
>>  extern struct page *alloc_pages_current(gfp_t gfp_mask, unsigned order);
>>
>> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
>> index 6838cd8..08f40a2 100644
>> --- a/mm/mempolicy.c
>> +++ b/mm/mempolicy.c
>> @@ -842,7 +842,7 @@ 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_from_valid_node(node, GFP_HIGHUSER_MOVABLE, 0);
>> +       return alloc_pages_exact_node(node, GFP_HIGHUSER_MOVABLE, 0);
>>  }
>
> It's behavior change. Please, write down why you want to change
> behavior in log.
> Although I knew it, you need to explain it for others and git log,
>
Hm, ok.
I will add the log later. Thank you for your suggestion.

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

Re: [PATCH] add alloc_pages_exact_node()

[Mel Gorman]

On Sat, Apr 10, 2010 at 07:49:33PM +0800, Bob Liu wrote:
> Add alloc_pages_exact_node() to allocate pages from exact
> node.
> 

It feels a bit overkill to add a new API that adds one self-explanatory
flag. 

> Signed-off-by: Bob Liu <lliubbo@gmail.com>
> ---
>  arch/powerpc/platforms/cell/ras.c |    4 ++--
>  include/linux/gfp.h               |    7 +++++++
>  mm/mempolicy.c                    |    2 +-
>  mm/migrate.c                      |    3 +--
>  4 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/cell/ras.c b/arch/powerpc/platforms/cell/ras.c
> index 6d32594..93a5afd 100644
> --- a/arch/powerpc/platforms/cell/ras.c
> +++ b/arch/powerpc/platforms/cell/ras.c
> @@ -123,8 +123,8 @@ static int __init cbe_ptcal_enable_on_node(int nid, int order)
>  
>  	area->nid = nid;
>  	area->order = order;
> -	area->pages = alloc_pages_from_valid_node(area->nid,
> -			GFP_KERNEL | GFP_THISNODE, area->order);
> +	area->pages = alloc_pages_exact_node(area->nid, GFP_KERNEL,
> +				area->order);
>  

This behaves differently now. If this is a bug fix, it needs to be a
standalone patch. It's the same for other call sites. You are actually
changing behaviour and offhand, I don't know why.

>  	if (!area->pages) {
>  		printk(KERN_WARNING "%s: no page on node %d\n",
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index c94f2ed..70cf2ae 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -296,6 +296,13 @@ static inline struct page *alloc_pages_from_valid_node(int nid, gfp_t gfp_mask,
>  	return __alloc_pages(gfp_mask, order, node_zonelist(nid, gfp_mask));
>  }
>  
> +static inline struct page *alloc_pages_exact_node(int nid, gfp_t gfp_mask,
> +						unsigned int order)
> +{
> +	return alloc_pages_from_valid_node(nid, gfp_mask | GFP_THISNODE,
> +			order);
> +}
> +
>  #ifdef CONFIG_NUMA
>  extern struct page *alloc_pages_current(gfp_t gfp_mask, unsigned order);
>  
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index 6838cd8..08f40a2 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -842,7 +842,7 @@ 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_from_valid_node(node, GFP_HIGHUSER_MOVABLE, 0);
> +	return alloc_pages_exact_node(node, GFP_HIGHUSER_MOVABLE, 0);
>  }
>  
>  /*
> diff --git a/mm/migrate.c b/mm/migrate.c
> index a057a1a..17330c5 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -770,8 +770,7 @@ static struct page *new_page_node(struct page *p, unsigned long private,
>  
>  	*result = &pm->status;
>  
> -	return alloc_pages_from_valid_node(pm->node,
> -				GFP_HIGHUSER_MOVABLE | GFP_THISNODE, 0);
> +	return alloc_pages_exact_node(pm->node, GFP_HIGHUSER_MOVABLE, 0);
>  }
>  
>  /*
> -- 
> 1.5.6.3
> 

-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

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

Re: [PATCH] code clean rename alloc_pages_exact_node()

[Mel Gorman]

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.

I recognise I'm not the best at naming though so I don't object to a
namechange if people really feel the new name is better.

> Signed-off-by: Bob Liu <lliubbo@gmail.com>
> ---
>  arch/ia64/hp/common/sba_iommu.c   |    2 +-
>  arch/ia64/kernel/uncached.c       |    2 +-
>  arch/ia64/sn/pci/pci_dma.c        |    2 +-
>  arch/powerpc/platforms/cell/ras.c |    4 ++--
>  arch/x86/kvm/vmx.c                |    3 ++-
>  drivers/misc/sgi-xp/xpc_uv.c      |    5 +++--
>  include/linux/gfp.h               |    2 +-
>  kernel/profile.c                  |    8 ++++----
>  mm/filemap.c                      |    2 +-
>  mm/hugetlb.c                      |    2 +-
>  mm/memory-failure.c               |    2 +-
>  mm/mempolicy.c                    |    2 +-
>  mm/migrate.c                      |    2 +-
>  mm/slab.c                         |    3 ++-
>  mm/slob.c                         |    4 ++--
>  15 files changed, 24 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/ia64/hp/common/sba_iommu.c b/arch/ia64/hp/common/sba_iommu.c
> index e14c492..e578f08 100644
> --- a/arch/ia64/hp/common/sba_iommu.c
> +++ b/arch/ia64/hp/common/sba_iommu.c
> @@ -1140,7 +1140,7 @@ sba_alloc_coherent (struct device *dev, size_t size, dma_addr_t *dma_handle, gfp
>  #ifdef CONFIG_NUMA
>  	{
>  		struct page *page;
> -		page = alloc_pages_exact_node(ioc->node == MAX_NUMNODES ?
> +		page = alloc_pages_from_valid_node(ioc->node == MAX_NUMNODES ?
>  		                        numa_node_id() : ioc->node, flags,
>  		                        get_order(size));
>  
> diff --git a/arch/ia64/kernel/uncached.c b/arch/ia64/kernel/uncached.c
> index c4696d2..ab8c085 100644
> --- a/arch/ia64/kernel/uncached.c
> +++ b/arch/ia64/kernel/uncached.c
> @@ -98,7 +98,7 @@ static int uncached_add_chunk(struct uncached_pool *uc_pool, int nid)
>  
>  	/* attempt to allocate a granule's worth of cached memory pages */
>  
> -	page = alloc_pages_exact_node(nid,
> +	page = alloc_pages_from_valid_node(nid,
>  				GFP_KERNEL | __GFP_ZERO | GFP_THISNODE,
>  				IA64_GRANULE_SHIFT-PAGE_SHIFT);
>  	if (!page) {
> diff --git a/arch/ia64/sn/pci/pci_dma.c b/arch/ia64/sn/pci/pci_dma.c
> index a9d310d..e09ccf7 100644
> --- a/arch/ia64/sn/pci/pci_dma.c
> +++ b/arch/ia64/sn/pci/pci_dma.c
> @@ -91,7 +91,7 @@ static void *sn_dma_alloc_coherent(struct device *dev, size_t size,
>  	 */
>  	node = pcibus_to_node(pdev->bus);
>  	if (likely(node >=0)) {
> -		struct page *p = alloc_pages_exact_node(node,
> +		struct page *p = alloc_pages_from_valid_node(node,
>  						flags, get_order(size));
>  
>  		if (likely(p))
> diff --git a/arch/powerpc/platforms/cell/ras.c b/arch/powerpc/platforms/cell/ras.c
> index 1d3c4ef..6d32594 100644
> --- a/arch/powerpc/platforms/cell/ras.c
> +++ b/arch/powerpc/platforms/cell/ras.c
> @@ -123,8 +123,8 @@ static int __init cbe_ptcal_enable_on_node(int nid, int order)
>  
>  	area->nid = nid;
>  	area->order = order;
> -	area->pages = alloc_pages_exact_node(area->nid, GFP_KERNEL|GFP_THISNODE,
> -						area->order);
> +	area->pages = alloc_pages_from_valid_node(area->nid,
> +			GFP_KERNEL | GFP_THISNODE, area->order);
>  
>  	if (!area->pages) {
>  		printk(KERN_WARNING "%s: no page on node %d\n",
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 753ffc2..f554b8c 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -1405,7 +1405,8 @@ static struct vmcs *alloc_vmcs_cpu(int cpu)
>  	struct page *pages;
>  	struct vmcs *vmcs;
>  
> -	pages = alloc_pages_exact_node(node, GFP_KERNEL, vmcs_config.order);
> +	pages = alloc_pages_from_valid_node(node, GFP_KERNEL,
> +			vmcs_config.order);
>  	if (!pages)
>  		return NULL;
>  	vmcs = page_address(pages);
> diff --git a/drivers/misc/sgi-xp/xpc_uv.c b/drivers/misc/sgi-xp/xpc_uv.c
> index 1f59ee2..ba3544e 100644
> --- a/drivers/misc/sgi-xp/xpc_uv.c
> +++ b/drivers/misc/sgi-xp/xpc_uv.c
> @@ -238,8 +238,9 @@ xpc_create_gru_mq_uv(unsigned int mq_size, int cpu, char *irq_name,
>  	mq->mmr_blade = uv_cpu_to_blade_id(cpu);
>  
>  	nid = cpu_to_node(cpu);
> -	page = alloc_pages_exact_node(nid, GFP_KERNEL | __GFP_ZERO | GFP_THISNODE,
> -				pg_order);
> +	page = alloc_pages_from_valid_node(nid,
> +			GFP_KERNEL | __GFP_ZERO | GFP_THISNODE, pg_order);
> +
>  	if (page == NULL) {
>  		dev_err(xpc_part, "xpc_create_gru_mq_uv() failed to alloc %d "
>  			"bytes of memory on nid=%d for GRU mq\n", mq_size, nid);
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index 4c6d413..c94f2ed 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -288,7 +288,7 @@ static inline struct page *alloc_pages_node(int nid, gfp_t gfp_mask,
>  	return __alloc_pages(gfp_mask, order, node_zonelist(nid, gfp_mask));
>  }
>  
> -static inline struct page *alloc_pages_exact_node(int nid, gfp_t gfp_mask,
> +static inline struct page *alloc_pages_from_valid_node(int nid, gfp_t gfp_mask,
>  						unsigned int order)
>  {
>  	VM_BUG_ON(nid < 0 || nid >= MAX_NUMNODES);
> diff --git a/kernel/profile.c b/kernel/profile.c
> index a55d3a3..4bf82da 100644
> --- a/kernel/profile.c
> +++ b/kernel/profile.c
> @@ -366,7 +366,7 @@ static int __cpuinit profile_cpu_callback(struct notifier_block *info,
>  		node = cpu_to_node(cpu);
>  		per_cpu(cpu_profile_flip, cpu) = 0;
>  		if (!per_cpu(cpu_profile_hits, cpu)[1]) {
> -			page = alloc_pages_exact_node(node,
> +			page = alloc_pages_from_valid_node(node,
>  					GFP_KERNEL | __GFP_ZERO,
>  					0);
>  			if (!page)
> @@ -374,7 +374,7 @@ static int __cpuinit profile_cpu_callback(struct notifier_block *info,
>  			per_cpu(cpu_profile_hits, cpu)[1] = page_address(page);
>  		}
>  		if (!per_cpu(cpu_profile_hits, cpu)[0]) {
> -			page = alloc_pages_exact_node(node,
> +			page = alloc_pages_from_valid_node(node,
>  					GFP_KERNEL | __GFP_ZERO,
>  					0);
>  			if (!page)
> @@ -568,14 +568,14 @@ static int create_hash_tables(void)
>  		int node = cpu_to_node(cpu);
>  		struct page *page;
>  
> -		page = alloc_pages_exact_node(node,
> +		page = alloc_pages_from_valid_node(node,
>  				GFP_KERNEL | __GFP_ZERO | GFP_THISNODE,
>  				0);
>  		if (!page)
>  			goto out_cleanup;
>  		per_cpu(cpu_profile_hits, cpu)[1]
>  				= (struct profile_hit *)page_address(page);
> -		page = alloc_pages_exact_node(node,
> +		page = alloc_pages_from_valid_node(node,
>  				GFP_KERNEL | __GFP_ZERO | GFP_THISNODE,
>  				0);
>  		if (!page)
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 140ebda..0424c3b 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -463,7 +463,7 @@ struct page *__page_cache_alloc(gfp_t gfp)
>  {
>  	if (cpuset_do_page_mem_spread()) {
>  		int n = cpuset_mem_spread_node();
> -		return alloc_pages_exact_node(n, gfp, 0);
> +		return alloc_pages_from_valid_node(n, gfp, 0);
>  	}
>  	return alloc_pages(gfp, 0);
>  }
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 6034dc9..01dd9c6 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -607,7 +607,7 @@ static struct page *alloc_fresh_huge_page_node(struct hstate *h, int nid)
>  	if (h->order >= MAX_ORDER)
>  		return NULL;
>  
> -	page = alloc_pages_exact_node(nid,
> +	page = alloc_pages_from_valid_node(nid,
>  		htlb_alloc_mask|__GFP_COMP|__GFP_THISNODE|
>  						__GFP_REPEAT|__GFP_NOWARN,
>  		huge_page_order(h));
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index 620b0b4..43abda1 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -1126,7 +1126,7 @@ EXPORT_SYMBOL(unpoison_memory);
>  static struct page *new_page(struct page *p, unsigned long private, int **x)
>  {
>  	int nid = page_to_nid(p);
> -	return alloc_pages_exact_node(nid, GFP_HIGHUSER_MOVABLE, 0);
> +	return alloc_pages_from_valid_node(nid, GFP_HIGHUSER_MOVABLE, 0);
>  }
>  
>  /*
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index 08f40a2..6838cd8 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -842,7 +842,7 @@ 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_from_valid_node(node, GFP_HIGHUSER_MOVABLE, 0);
>  }
>  
>  /*
> diff --git a/mm/migrate.c b/mm/migrate.c
> index d3f3f7f..a057a1a 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -770,7 +770,7 @@ static struct page *new_page_node(struct page *p, unsigned long private,
>  
>  	*result = &pm->status;
>  
> -	return alloc_pages_exact_node(pm->node,
> +	return alloc_pages_from_valid_node(pm->node,
>  				GFP_HIGHUSER_MOVABLE | GFP_THISNODE, 0);
>  }
>  
> diff --git a/mm/slab.c b/mm/slab.c
> index 730d45b..4f71736 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -1717,7 +1717,8 @@ static void *kmem_getpages(struct kmem_cache *cachep, gfp_t flags, int nodeid)
>  	if (cachep->flags & SLAB_RECLAIM_ACCOUNT)
>  		flags |= __GFP_RECLAIMABLE;
>  
> -	page = alloc_pages_exact_node(nodeid, flags | __GFP_NOTRACK, cachep->gfporder);
> +	page = alloc_pages_from_valid_node(nodeid, flags | __GFP_NOTRACK,
> +			cachep->gfporder);
>  	if (!page)
>  		return NULL;
>  
> diff --git a/mm/slob.c b/mm/slob.c
> index 837ebd6..9e3f95b 100644
> --- a/mm/slob.c
> +++ b/mm/slob.c
> @@ -46,7 +46,7 @@
>   * NUMA support in SLOB is fairly simplistic, pushing most of the real
>   * logic down to the page allocator, and simply doing the node accounting
>   * on the upper levels. In the event that a node id is explicitly
> - * provided, alloc_pages_exact_node() with the specified node id is used
> + * provided, alloc_pages_from_valid_node() with the specified node id is used
>   * instead. The common case (or when the node id isn't explicitly provided)
>   * will default to the current node, as per numa_node_id().
>   *
> @@ -244,7 +244,7 @@ static void *slob_new_pages(gfp_t gfp, int order, int node)
>  
>  #ifdef CONFIG_NUMA
>  	if (node != -1)
> -		page = alloc_pages_exact_node(node, gfp, order);
> +		page = alloc_pages_from_valid_node(node, gfp, order);
>  	else
>  #endif
>  		page = alloc_pages(gfp, order);
> -- 
> 1.5.6.3
> 

-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

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

Re: [PATCH] code clean rename alloc_pages_exact_node()

[Minchan Kim]

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

-- 
Kind regards,
Minchan Kim

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

Re: [PATCH] code clean rename alloc_pages_exact_node()

[KAMEZAWA Hiroyuki]

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>

Re: [PATCH] code clean rename alloc_pages_exact_node()

[Bob Liu]

On 4/13/10, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> 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.
>

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@gmail.com>
---
 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@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>

Re: [PATCH] code clean rename alloc_pages_exact_node()

[KAMEZAWA Hiroyuki]

On Tue, 13 Apr 2010 15:09:42 +0800
Bob Liu <lliubbo@gmail.com> wrote:

> On 4/13/10, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> > 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.
> >
> 
> 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().
> 

Hmm. your patch for mempolicy seems good and it's BUGFIX.
So, this patch should go as it is.

If you want to add comments to alloc_pages_exact_node(), please do.

But I think it's better to divide BUGFIX and CLEANUP patches.

I'll ack your patch for mempolicy.
Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

Naming issue never needs quick fix. How about repositing as it is ?
Minchan, how do you think ?

Thanks,
-Kame

> 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@gmail.com>
> ---
>  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@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>

Re: [PATCH] code clean rename alloc_pages_exact_node()

[Minchan Kim]

On Tue, Apr 13, 2010 at 4:09 PM, Bob Liu <lliubbo@gmail.com> wrote:
> On 4/13/10, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
>> 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.
>>
>
> 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@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>

Re: [PATCH] code clean rename alloc_pages_exact_node()

[Minchan Kim]

On Tue, Apr 13, 2010 at 4:09 PM, Bob Liu <lliubbo@gmail.com> wrote:
> On 4/13/10, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
>> 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.
>>
>
> 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@gmail.com>
Reviewed-by: Minchan Kim <minchan.kim@gmail.com>

Long time ago, I already reviewed this patch with Bob. :)

-- 
Kind regards,
Minchan Kim

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

Re: [PATCH] code clean rename alloc_pages_exact_node()

[Minchan Kim]

On Tue, Apr 13, 2010 at 4:32 PM, KAMEZAWA Hiroyuki
<kamezawa.hiroyu@jp.fujitsu.com> wrote:
> On Tue, 13 Apr 2010 15:09:42 +0800
> Bob Liu <lliubbo@gmail.com> wrote:
>
>> On 4/13/10, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
>> > 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.
>> >
>>
>> 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().
>>
>
> Hmm. your patch for mempolicy seems good and it's BUGFIX.
> So, this patch should go as it is.
>
> If you want to add comments to alloc_pages_exact_node(), please do.
>
> But I think it's better to divide BUGFIX and CLEANUP patches.
>
> I'll ack your patch for mempolicy.
> Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>
> Naming issue never needs quick fix. How about repositing as it is ?
> Minchan, how do you think ?


I feel it would be better to weed out function usage when I saw
alloc_slab_page at least.

/*
 * Slab allocation and freeing
 */
static inline struct page *alloc_slab_page(gfp_t flags, int node,
                                        struct kmem_cache_order_objects oo)
{
       ...
       if (node == -1)
                return alloc_pages(flags, order);
        else
                return alloc_pages_node(node, flags, order);
}


-- 
Kind regards,
Minchan Kim

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

Re: [PATCH] code clean rename alloc_pages_exact_node()

[Mel Gorman]

On Tue, Apr 13, 2010 at 03:09:42PM +0800, Bob Liu wrote:
> On 4/13/10, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> > 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.
> >
> 
> 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"
> 

Sorry Bob, I still haven't actually read that thread. There has been a lot
going on :(

> 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@gmail.com>

This appears to be a valid bug fix.  I agree that the way things are structured
that __GFP_THISNODE should be used in new_node_page(). But maybe a follow-on
patch is also required. The behaviour is now;

o new_node_page will not return NULL if the target node is empty (fine).
o migrate_pages will translate this into -ENOMEM (fine)
o do_migrate_pages breaks early if it gets -ENOMEM ?

It's the last part I'd like you to double check. migrate_pages() takes a
nodemask of allowed nodes to migrate to. Rather than sending this down
to the allocator, it iterates over the nodes allowed in the mask. If one
of those nodes is full, it returns -ENOMEM.

If -ENOMEM is returned from migrate_pages, should it not move to the next node?

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

-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

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

Re: [PATCH] code clean rename alloc_pages_exact_node()

[Christoph Lameter]

On Tue, 13 Apr 2010, Bob Liu wrote:

> What I concern is *just* we shouldn't fallback to other nodes if the
> dest node haven't enough free pages during migrate_pages().

Why not?

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

Re: [PATCH] code clean rename alloc_pages_exact_node()

[Christoph Lameter]

On Tue, 13 Apr 2010, Mel Gorman wrote:

> This appears to be a valid bug fix.  I agree that the way things are structured
> that __GFP_THISNODE should be used in new_node_page(). But maybe a follow-on
> patch is also required. The behaviour is now;

What bug is being fixed? migrate_pages() is a best effort approach.
__GFP_THISNODE is used when allocation on a specific node is absolutely
required for correctness (as in SLAB).

> If -ENOMEM is returned from migrate_pages, should it not move to the next node?

Which it does now if you dont apply the "bugfix".

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