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)