change alloc function in pcpu_alloc_pages
in [Kernel]
|
Prev: linux-ide: hda: possibly failed opcode: 0x25 on Alpha with 2.6.34-rc3
Next: [PATCH]: x86: remove extra bootmem.h from arch/x86/mm/init_64.c
From: Christoph Lameter on 19 Apr 2010 13:50 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?
From: Tejun Heo on 19 Apr 2010 18:30 Hello, Christoph. On 04/20/2010 02:38 AM, Christoph Lameter wrote: > alloc_pages_exact_node results in more confusion because it does suggest > that fallback to other nodes is not allowed. I can't see why alloc_pages_exact_node() exists at all. It's in the mainline and if you look at the difference between alloc_pages_node() and alloc_pages_exact_node(), it's almost silly. :-( -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Tejun Heo on 22 Apr 2010 04:40 Hello, On 04/20/2010 05:05 PM, Mel Gorman wrote: > alloc_pages_exact_node() avoids a branch in a hot path that is checking for > something the caller already knows. That's the reason it exists. Yeah sure but Minchan is trying to tidy up the API by converting alloc_pages_node() users to use alloc_pages_exact_node(), at which point, the distinction becomes pretty useless. Wouldn't just making alloc_pages_node() do what alloc_pages_exact_node() does now and converting all its users be simpler? IIRC, the currently planned transformation looks like the following. alloc_pages() -> alloc_pages_any_node() alloc_pages_node() -> basically gonna be obsoleted by _exact_node alloc_pages_exact_node() -> gonna be used by most NUMA aware allocs So, let's just make sure no one calls alloc_pages_node() w/ -1 nid, kill alloc_pages_node() and rename alloc_pages_exact_node() to alloc_pages_node(). Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Minchan Kim on 22 Apr 2010 06:20 On Wed, Apr 21, 2010 at 7:48 PM, Tejun Heo <tj(a)kernel.org> wrote: > Hello, > > On 04/20/2010 05:05 PM, Mel Gorman wrote: >> alloc_pages_exact_node() avoids a branch in a hot path that is checking for >> something the caller already knows. That's the reason it exists. > > Yeah sure but Minchan is trying to tidy up the API by converting > alloc_pages_node() users to use alloc_pages_exact_node(), at which > point, the distinction becomes pretty useless. Wouldn't just making > alloc_pages_node() do what alloc_pages_exact_node() does now and > converting all its users be simpler? IIRC, the currently planned > transformation looks like the following. > > alloc_pages() -> alloc_pages_any_node() > alloc_pages_node() -> basically gonna be obsoleted by _exact_node > alloc_pages_exact_node() -> gonna be used by most NUMA aware allocs > > So, let's just make sure no one calls alloc_pages_node() w/ -1 nid, > kill alloc_pages_node() and rename alloc_pages_exact_node() to > alloc_pages_node(). Yes. It was a stupid idea. I hope Mel agree this suggestion. Thanks for careful review, Tejun. -- Kind regards, Minchan Kim -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Christoph Lameter on 21 Apr 2010 10:20
On Tue, 20 Apr 2010, Mel Gorman wrote: > alloc_pages_exact_node() avoids a branch in a hot path that is checking for > something the caller already knows. That's the reason it exists. We can avoid alloc_pages_exact_node() by making all callers of alloc_pages_node() never use -1. -1 is ambiguous and only rarely will a caller pass that to alloc_pages_node(). -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |