Re: [Patch V3 2/9] kernel/profile.c: Replace cpu_to_mem() with cpu_to_node()
On 2015/8/20 8:00, David Rientjes wrote: > On Wed, 19 Aug 2015, Jiang Liu wrote: > >> On 2015/8/18 8:31, David Rientjes wrote: >>> On Mon, 17 Aug 2015, Jiang Liu wrote: >>> Function profile_cpu_callback() allocates memory without specifying __GFP_THISNODE flag, so replace cpu_to_mem() with cpu_to_node() because cpu_to_mem() may cause suboptimal memory allocation if there's no free memory on the node returned by cpu_to_mem(). >>> >>> Why is cpu_to_node() better with regard to free memory and NUMA locality? >> Hi David, >> Thanks for review. This is a special case pointed out by Tejun. >> For the imagined topology, A<->B<->X<->C<->D, where A, B, C, D has >> memory and X is memoryless. >> Possible fallback lists are: >> B: [ B, A, C, D] >> X: [ B, C, A, D] >> C: [ C, D, B, A] >> >> cpu_to_mem(X) will either return B or C. Let's assume it returns B. >> Then we will use "B: [ B, A, C, D]" to allocate memory for X, which >> is not the optimal fallback list for X. And cpu_to_node(X) returns >> X, and "X: [ B, C, A, D]" is the optimal fallback list for X. > > Ok, that makes sense, but I would prefer that this > alloc_pages_exact_node() change to alloc_pages_node() since, as you > mention in your commit message, __GFP_THISNODE is not set. Hi David, Sorry for slow response due to personal reasons! Function alloc_pages_exact_node() has been renamed as __alloc_pages_node() by commit 96db800f5d73, and __alloc_pages_node() is a slightly optimized version of alloc_pages_node() which doesn't fallback to current node for nid == NUMA_NO_NODE case. So it would be better to keep using __alloc_pages_node() because cpu_to_node() always returns valid node id. Thanks! Gerry > > In the longterm, if we setup both zonelists correctly (no __GFP_THISNODE > and with __GFP_THISNODE), then I'm not sure there's any reason to ever use > cpu_to_mem() for alloc_pages(). > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch V3 2/9] kernel/profile.c: Replace cpu_to_mem() with cpu_to_node()
On Wed, 19 Aug 2015, Jiang Liu wrote: > On 2015/8/18 8:31, David Rientjes wrote: > > On Mon, 17 Aug 2015, Jiang Liu wrote: > > > >> Function profile_cpu_callback() allocates memory without specifying > >> __GFP_THISNODE flag, so replace cpu_to_mem() with cpu_to_node() > >> because cpu_to_mem() may cause suboptimal memory allocation if > >> there's no free memory on the node returned by cpu_to_mem(). > >> > > > > Why is cpu_to_node() better with regard to free memory and NUMA locality? > Hi David, > Thanks for review. This is a special case pointed out by Tejun. > For the imagined topology, A<->B<->X<->C<->D, where A, B, C, D has > memory and X is memoryless. > Possible fallback lists are: > B: [ B, A, C, D] > X: [ B, C, A, D] > C: [ C, D, B, A] > > cpu_to_mem(X) will either return B or C. Let's assume it returns B. > Then we will use "B: [ B, A, C, D]" to allocate memory for X, which > is not the optimal fallback list for X. And cpu_to_node(X) returns > X, and "X: [ B, C, A, D]" is the optimal fallback list for X. Ok, that makes sense, but I would prefer that this alloc_pages_exact_node() change to alloc_pages_node() since, as you mention in your commit message, __GFP_THISNODE is not set. In the longterm, if we setup both zonelists correctly (no __GFP_THISNODE and with __GFP_THISNODE), then I'm not sure there's any reason to ever use cpu_to_mem() for alloc_pages(). -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch V3 2/9] kernel/profile.c: Replace cpu_to_mem() with cpu_to_node()
On 2015/8/18 8:31, David Rientjes wrote: > On Mon, 17 Aug 2015, Jiang Liu wrote: > >> Function profile_cpu_callback() allocates memory without specifying >> __GFP_THISNODE flag, so replace cpu_to_mem() with cpu_to_node() >> because cpu_to_mem() may cause suboptimal memory allocation if >> there's no free memory on the node returned by cpu_to_mem(). >> > > Why is cpu_to_node() better with regard to free memory and NUMA locality? Hi David, Thanks for review. This is a special case pointed out by Tejun. For the imagined topology, A<->B<->X<->C<->D, where A, B, C, D has memory and X is memoryless. Possible fallback lists are: B: [ B, A, C, D] X: [ B, C, A, D] C: [ C, D, B, A] cpu_to_mem(X) will either return B or C. Let's assume it returns B. Then we will use "B: [ B, A, C, D]" to allocate memory for X, which is not the optimal fallback list for X. And cpu_to_node(X) returns X, and "X: [ B, C, A, D]" is the optimal fallback list for X. Thanks! Gerry > >> It's safe to use cpu_to_mem() because build_all_zonelists() also >> builds suitable fallback zonelist for memoryless node. >> > > Why reference that cpu_to_mem() is safe if you're changing away from it? Sorry, it should be cpu_to_node() instead of cpu_to_mem(). > >> Signed-off-by: Jiang Liu >> --- >> kernel/profile.c |2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/kernel/profile.c b/kernel/profile.c >> index a7bcd28d6e9f..d14805bdcc4c 100644 >> --- a/kernel/profile.c >> +++ b/kernel/profile.c >> @@ -336,7 +336,7 @@ static int profile_cpu_callback(struct notifier_block >> *info, >> switch (action) { >> case CPU_UP_PREPARE: >> case CPU_UP_PREPARE_FROZEN: >> -node = cpu_to_mem(cpu); >> +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, -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch V3 2/9] kernel/profile.c: Replace cpu_to_mem() with cpu_to_node()
On Mon, 17 Aug 2015, Jiang Liu wrote: > Function profile_cpu_callback() allocates memory without specifying > __GFP_THISNODE flag, so replace cpu_to_mem() with cpu_to_node() > because cpu_to_mem() may cause suboptimal memory allocation if > there's no free memory on the node returned by cpu_to_mem(). > Why is cpu_to_node() better with regard to free memory and NUMA locality? > It's safe to use cpu_to_mem() because build_all_zonelists() also > builds suitable fallback zonelist for memoryless node. > Why reference that cpu_to_mem() is safe if you're changing away from it? > Signed-off-by: Jiang Liu > --- > kernel/profile.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/profile.c b/kernel/profile.c > index a7bcd28d6e9f..d14805bdcc4c 100644 > --- a/kernel/profile.c > +++ b/kernel/profile.c > @@ -336,7 +336,7 @@ static int profile_cpu_callback(struct notifier_block > *info, > switch (action) { > case CPU_UP_PREPARE: > case CPU_UP_PREPARE_FROZEN: > - node = cpu_to_mem(cpu); > + 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, -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/