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: Mel Gorman on 13 Apr 2010 11:50 On Wed, Apr 14, 2010 at 12:24:59AM +0900, Minchan Kim wrote: > alloc_pages_node is called with cpu_to_node(cpu). > I think cpu_to_node(cpu) never returns -1. > (But I am not sure we need double check.) > > So we can use alloc_pages_exact_node instead of alloc_pages_node. > It could avoid comparison and branch as 6484eb3e2a81807722 tried. > Well, numa_node_id() is implemented as #ifndef numa_node_id #define numa_node_id() (cpu_to_node(raw_smp_processor_id())) #endif and the mapping table on x86 at least is based on possible CPUs in init_cpu_to_node() leaves the mapping as 0 if the APIC is bad or the numa node is reported in apicid_to_node as -1. It would appear on power that the node will be 0 for possible CPUs as well. Hence, I believe this to be safe but a confirmation from Tejun would be nice. I would continue digging but this looks like an initialisation path so I'll move on to the next patch rather than spending more time. > Cc: Tejun Heo <tj(a)kernel.org> > Cc: Christoph Lameter <cl(a)linux-foundation.org> > Signed-off-by: Minchan Kim <minchan.kim(a)gmail.com> > --- > mm/percpu.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/mm/percpu.c b/mm/percpu.c > index 768419d..ec3e671 100644 > --- a/mm/percpu.c > +++ b/mm/percpu.c > @@ -720,7 +720,7 @@ static int pcpu_alloc_pages(struct pcpu_chunk *chunk, > for (i = page_start; i < page_end; i++) { > struct page **pagep = &pages[pcpu_page_idx(cpu, i)]; > > - *pagep = alloc_pages_node(cpu_to_node(cpu), gfp, 0); > + *pagep = alloc_pages_exact_node(cpu_to_node(cpu), gfp, 0); > if (!*pagep) { > pcpu_free_pages(chunk, pages, populated, > page_start, page_end); > -- > 1.7.0.5 > -- Mel Gorman Part-time Phd Student Linux Technology Center University of Limerick IBM Dublin Software Lab -- 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 14 Apr 2010 19:40 Hello, On 04/14/2010 12:48 AM, Mel Gorman wrote: > and the mapping table on x86 at least is based on possible CPUs in > init_cpu_to_node() leaves the mapping as 0 if the APIC is bad or the numa > node is reported in apicid_to_node as -1. It would appear on power that > the node will be 0 for possible CPUs as well. > > Hence, I believe this to be safe but a confirmation from Tejun would be > nice. I would continue digging but this looks like an initialisation path > so I'll move on to the next patch rather than spending more time. This being a pretty cold path, I don't really see much benefit in converting it to alloc_pages_node_exact(). It ain't gonna make any difference. I'd rather stay with the safer / boring one unless there's a pressing reason to convert. 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 14 Apr 2010 21:40 Hi, Tejun. On Thu, Apr 15, 2010 at 8:39 AM, Tejun Heo <tj(a)kernel.org> wrote: > Hello, > > On 04/14/2010 12:48 AM, Mel Gorman wrote: >> and the mapping table on x86 at least is based on possible CPUs in >> init_cpu_to_node() leaves the mapping as 0 if the APIC is bad or the numa >> node is reported in apicid_to_node as -1. It would appear on power that >> the node will be 0 for possible CPUs as well. >> >> Hence, I believe this to be safe but a confirmation from Tejun would be >> nice. I would continue digging but this looks like an initialisation path >> so I'll move on to the next patch rather than spending more time. > > This being a pretty cold path, I don't really see much benefit in > converting it to alloc_pages_node_exact(). It ain't gonna make any > difference. I'd rather stay with the safer / boring one unless > there's a pressing reason to convert. Actually, It's to weed out not-good API usage as well as some performance gain. But I don't think to need it strongly. Okay. Please keep in mind about this and correct it if you confirms it in future. :) > > Thanks. > > -- > 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: Tejun Heo on 15 Apr 2010 03:20 Hello, On 04/15/2010 10:31 AM, Minchan Kim wrote: > Hi, Tejun. >> This being a pretty cold path, I don't really see much benefit in >> converting it to alloc_pages_node_exact(). It ain't gonna make any >> difference. I'd rather stay with the safer / boring one unless >> there's a pressing reason to convert. > > Actually, It's to weed out not-good API usage as well as some > performance gain. But I don't think to need it strongly. > Okay. Please keep in mind about this and correct it if you confirms > it in future. :) Hmm... if most users are converting over to alloc_pages_node_exact(), I think it would be better to convert percpu too. I thought it was a performance optimization (of rather silly kind too). So, this is to weed out -1 node id usage? Wouldn't it be better to update alloc_pages_node() such that it whines once per each caller if it's called with -1 node id and after updating most users convert the warning into WARN_ON_ONCE()? Having two variants for this seems rather extreme to me. 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 15 Apr 2010 04:10
On Thu, Apr 15, 2010 at 4:21 PM, Tejun Heo <tj(a)kernel.org> wrote: > Hello, > > On 04/15/2010 10:31 AM, Minchan Kim wrote: >> Hi, Tejun. >>> This being a pretty cold path, I don't really see much benefit in >>> converting it to alloc_pages_node_exact(). It ain't gonna make any >>> difference. I'd rather stay with the safer / boring one unless >>> there's a pressing reason to convert. >> >> Actually, It's to weed out not-good API usage as well as some >> performance gain. But I don't think to need it strongly. >> Okay. Please keep in mind about this and correct it if you confirms >> it in future. :) > > Hmm... if most users are converting over to alloc_pages_node_exact(), > I think it would be better to convert percpu too. I thought it was a > performance optimization (of rather silly kind too). So, this is to > weed out -1 node id usage? Wouldn't it be better to update > alloc_pages_node() such that it whines once per each caller if it's > called with -1 node id and after updating most users convert the > warning into WARN_ON_ONCE()? Having two variants for this seems > rather extreme to me. Yes. I don't like it. With it, someone who does care about API usage uses alloc_pages_exact_node but someone who don't have a time or careless uses alloc_pages_node. It would make API fragmentation and not good. Maybe we can weed out -1 and make new API which is more clear. * struct page *alloc_pages_any_node(gfp_t gfp_mask, unsigned int order); * struct page *alloc_pages_exact_node(int nid, gfp_mask, unsigned int order); So firstly we have to make sure users who use alloc_pages_node can change alloc_pages_node with alloc_pages_exact_node. After all of it was weed out, I will change alloc_pages_node with alloc_pages_any_node. -- 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/ |