lkml.org 
[lkml]   [2010]   [Apr]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 2/6] change alloc function in pcpu_alloc_pages
On Mon, 19 Apr 2010, Minchan Kim wrote:

> Let's tidy my table.
>
> I made quick patch to show the concept with one example of pci-dma.
> (Sorry but I attach patch since web gmail's mangling.)
>
> On UMA, we can change alloc_pages with
> alloc_pages_exact_node(numa_node_id(),....)
> (Actually, the patch is already merged mmotm)

UMA does not have the concept of nodes. Whatever node you specify is
irrelevant. Please remove the patch from mmotm.

> on NUMA, alloc_pages is some different meaning, so I don't want to change it.

No it has the same meaning. It means allocate a page.

> on NUMA, alloc_pages_node means _ANY_NODE_.

It means allocate on the indicated node if possible. Memory could come
from any node due to fallback (in order of node preference).

> So let's remove nid argument and change naming with alloc_pages_any_node.

??? What in the world are you doing?

> Then, whole users of alloc_pages_node can be changed between
> alloc_pages_exact_node and alloc_pages_any_node.
>
> It was my intention. What's your concern?

I dont see the point.

> again:
> - page = alloc_pages_node(dev_to_node(dev), flag, get_order(size));
> + nid = dev_to_node(dev);
> + /*
> + * If pci-dma maintainer makes sure nid never has NUMA_NO_NODE
> + * we can remove this ugly checking.
> + */
> + if (nid == NUMA_NO_NODE)
> + page = alloc_pages_any_node(flag, get_order(size));

s/alloc_pages_any_node/alloc_pages/

> + else
> + page = alloc_pages_exact_node(nid, flag, get_order(size));

s/alloc_pages_exact_node/alloc_pages_node/

> -static inline struct page *alloc_pages_node(int nid, gfp_t gfp_mask,
> +static inline struct page *alloc_pagse_any_node(gfp_t gfp_mask,
> unsigned int order)
> {
> - /* Unknown node is current node */
> - if (nid < 0)
> - nid = numa_node_id();
> -
> + int nid = numa_node_id();
> return __alloc_pages(gfp_mask, order, node_zonelist(nid, gfp_mask));
> }
>

This is very confusing. Because it is

alloc_pages_numa_node_id()


alloca_pages_any_node suggests that the kernel randomly picks a node?
diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
index a4ac764..dc511cb 100644
--- a/arch/x86/kernel/pci-dma.c
+++ b/arch/x86/kernel/pci-dma.c
@@ -152,12 +152,21 @@ void *dma_generic_alloc_coherent(struct device *dev, size_t size,
unsigned long dma_mask;
struct page *page;
dma_addr_t addr;
+ int nid;

dma_mask = dma_alloc_coherent_mask(dev, flag);

flag |= __GFP_ZERO;
again:
- page = alloc_pages_node(dev_to_node(dev), flag, get_order(size));
+ nid = dev_to_node(dev);
+ /*
+ * If pci-dma maintainer makes sure nid never has NUMA_NO_NODE
+ * we can remove this ugly checking.
+ */
+ if (nid == NUMA_NO_NODE)
+ page = alloc_pages_any_node(flag, get_order(size));
+ else
+ page = alloc_pages_exact_node(nid, flag, get_order(size));
if (!page)
return NULL;

diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index 4c6d413..47fba21 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -278,13 +278,10 @@ __alloc_pages(gfp_t gfp_mask, unsigned int order,
return __alloc_pages_nodemask(gfp_mask, order, zonelist, NULL);
}

-static inline struct page *alloc_pages_node(int nid, gfp_t gfp_mask,
+static inline struct page *alloc_pagse_any_node(gfp_t gfp_mask,
unsigned int order)
{
- /* Unknown node is current node */
- if (nid < 0)
- nid = numa_node_id();
-
+ int nid = numa_node_id();
return __alloc_pages(gfp_mask, order, node_zonelist(nid, gfp_mask));
}

@@ -308,7 +305,7 @@ extern struct page *alloc_page_vma(gfp_t gfp_mask,
struct vm_area_struct *vma, unsigned long addr);
#else
#define alloc_pages(gfp_mask, order) \
- alloc_pages_node(numa_node_id(), gfp_mask, order)
+ alloc_pages_exact_node(numa_node_id(), gfp_mask, order)
#define alloc_page_vma(gfp_mask, vma, addr) alloc_pages(gfp_mask, 0)
#endif
#define alloc_page(gfp_mask) alloc_pages(gfp_mask, 0)
\
 
 \ /
  Last update: 2010-04-19 19:49    [W:0.133 / U:0.152 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site