Allocate sparse vmemmap block above 4G
Subsystems (4)
PATCH 1/1 Allocate sparse vmemmap block above 4G
Resend the patch for more people to review
On some single node x64 system with huge amount of physical memory e.g > 64G. the memmap size maybe very big.
If the memmap is allocated from low pages, it may occupies too much memory below 4G.
then swiotlb could fail to reserve bounce buffer under 4G which will lead to boot failure.
This patch will first try to allocate memmap memory above 4G in sparse vmemmap code.
If it failed, it will allocate memmap above MAX_DMA_ADDRESS. This patch is against 2.6.24-rc1-git14
arch/x86/mm/init_64.c +7 -0
@@ -448,6 +448,13 @@ void online_page(struct page *page) num_physpages++; } +void *__meminit alloc_bootmem_high_node (pg_data_t *pgdat,unsigned long size, +unsigned long align) +{ +return __alloc_bootmem_core (pgdat->bdata, size, + align, (4UL *1024 *1024 *1024 ),0 ,1 ); +} +#ifdef CONFIG_MEMORY_HOTPLUG /* * Memory is added always to NORMAL zone. This means you will never get
include/linux/bootmem.h +4 -0
@@ -61,6 +61,10 @@ extern void *__alloc_bootmem_core(structunsigned long limit, int strict_goal); +extern void *alloc_bootmem_high_node (pg_data_t *pgdat, +unsigned long size, +unsigned long align); +#ifndef CONFIG_HAVE_ARCH_BOOTMEM_NODE extern void reserve_bootmem (unsigned long addr,unsigned long size);#define alloc_bootmem (x) \
mm/bootmem.c +8 -0
@@ -492,3 +492,11 @@ void * __init __alloc_bootmem_low_node(preturn __alloc_bootmem_core (pgdat->bdata, size, align, goal,ARCH_LOW_ADDRESS_LIMIT ,0 ); } + +__attribute__((weak))__meminit +void *alloc_bootmem_high_node (pg_data_t *pgdat,unsigned long size, +unsigned long align) +{ +return NULL ; +} +
mm/sparse-vmemmap.c +5 -1
@@ -43,9 +43,13 @@ void * __meminit vmemmap_alloc_block(unsif (page)return page_address (page);return NULL ; - }else + }else { +void *p =alloc_bootmem_high_node (NODE_DATA (node), size, size); +if (p) +return p;return __alloc_bootmem_node (NODE_DATA (node), size, size,__pa (MAX_DMA_ADDRESS )); + } } void __meminit vmemmap_verify(pte_t *pte, int node,
On (08/11/07 08:52), Zou Nan hai didst pronounce:
Quoted 10 lines
Resend the patch for more people to review
On some single node x64 system with huge amount of physical memory e.g > 64G. the memmap size maybe very big.
If the memmap is allocated from low pages, it may occupies too much memory below 4G.
then swiotlb could fail to reserve bounce buffer under 4G which will lead to boot failure.
Quoted 5 lines
This patch will first try to allocate memmap memory above 4G in sparse vmemmap code.
If it failed, it will allocate memmap aboveMAX_DMA_ADDRESS. This patch is against 2.6.24-rc1-git14
You never state that you depend on the strict_goal patch either here or in a leader mail. The usual way of getting peoples attention is to have three mails. In your case it would be
[PATCH 0/2] Allocate vmemmap from highmem where possible [PATCH 1/2] Strictly place bootmem allocations when requested [PATCH 2/2] Allocate sparse vmemmap block above 4G on x86_64
Maybe you have gone through all this already and I'm coming so late that I missed it all. If that is the case, sorry for the noise.
arch/x86/mm/init_64.c void online_page(struct page *page) · line 453
Quoted 13 lines
Signed-off-by Zou Nan hai <[email protected]>Signed-off-by Suresh Siddha <[email protected]>diff -Nraup
a/arch/x86/mm/init_64.cb/arch/x86/mm/init_64.carch/x86/mm/init_64.c +3
-0@@ -448,6 +448,13 @@ void online_page(struct page *page) num_physpages++; } +void *__meminit alloc_bootmem_high_node (pg_data_t *pgdat,unsigned long size, +unsigned long align) +{
Wrong markup there I believe. The __meminit markup is for functions that are needed at runtime when memory is hot-added or hot-removed. Bootmem functions do not qualify. __init is sufficient.
The name is confusing as well. I don't know what a high node is, but I think you mean alloc_bootmem_highmem or alloc_bootmem_highmem_zone. That in *itself* is confusing on x86_64 because the memory above 4GiB is ZONE_NORMAL, not ZONE_HIGHMEM. Calling it alloc_bootmem_nondma() makes it worse.
I think you need to rework this to have an arch-specific function like arch_alloc_vmemmap_block() that by default allocates with ____alloc_bootmem_node() and otherwise uses an arch-specific function. In this case, it would know to call __alloc_bootmem_core() with the proper addressing limits.
arch/x86/mm/init_64.c void online_page(struct page *page) · line 456
Quoted 3 lines
arch/x86/mm/init_64.c +3
-0+return __alloc_bootmem_core (pgdat->bdata, size, + align, (4UL *1024 *1024 *1024 ),0 ,1 ); +}
More magic values, both the 4GiB address here and the magic "1" at the end are problems.
Quoted 8 lines
arch/x86/mm/init_64.c +1
-0+#ifdef CONFIG_MEMORY_HOTPLUG /* * Memory is added always to NORMAL zone. This means you will never get diff -Nraup
a/include/linux/bootmem.hb/include/linux/bootmem.h
include/linux/bootmem.h extern void *__alloc_bootmem_core(struct · line 67
Quoted 7 lines
include/linux/bootmem.h +4
-0@@ -61,6 +61,10 @@ extern void *__alloc_bootmem_core(structunsigned long limit, int strict_goal); +extern void *alloc_bootmem_high_node (pg_data_t *pgdat, +unsigned long size, +unsigned long align); +
Confusing. Your declaration here makes it look like a bootmem API function but it is an arch-specific function that only exists for x86-64. I know it gets overridden later when a weak symbol but the more common approach is to have Kconfig define something like ARCH_HIGHMEM_VMEMMAP and
#ifdef CONFIG_ARCH_HIGHMEM_VMEMMAP
extern void *alloc_bootmem_high_node(pg_data_t *pgdat,
unsigned long size,
unsigned long align);
#else
static inline void *alloc_bootmem_high_node(pg_data_t *pgdat,
unsigned long size,
unsigned long align) {
return NULL;
}#endif /* CONFIG_ARCH_HIGHMEM_VMEMMAP
It's be similar if you have an arch-specific callback for vmalloc block allocations instead of extending the bootmem API.
mm/bootmem.c void * __init __alloc_bootmem_low_node(p · line 496
Quoted 13 lines
include/linux/bootmem.h +0
-0#ifndef CONFIG_HAVE_ARCH_BOOTMEM_NODE extern void reserve_bootmem (unsigned long addr,unsigned long size);#define alloc_bootmem (x) \diff -Nraup
a/mm/bootmem.cb/mm/bootmem.cmm/bootmem.c +2
-0@@ -492,3 +492,11 @@ void * __init __alloc_bootmem_low_node(preturn __alloc_bootmem_core (pgdat->bdata, size, align, goal,ARCH_LOW_ADDRESS_LIMIT ,0 ); } + +__attribute__((weak)) __meminit
__meminit vs _init again
Quoted 10 lines
mm/bootmem.c +6
-0+void *alloc_bootmem_high_node (pg_data_t *pgdat,unsigned long size, +unsigned long align) +{ +return NULL ; +} +diff -Nraup
a/mm/sparse-vmemmap.cb/mm/sparse-vmemmap.c
mm/sparse-vmemmap.c void * __meminit vmemmap_alloc_block(uns · line 51
Quoted 10 lines
mm/sparse-vmemmap.c +4
-1@@ -43,9 +43,13 @@ void * __meminit vmemmap_alloc_block(unsif (page)return page_address (page);return NULL ; - }else + }else { +void *p =alloc_bootmem_high_node (NODE_DATA (node), size, size); +if (p) +return p;return __alloc_bootmem_node (NODE_DATA (node), size, size,__pa (MAX_DMA_ADDRESS ));
If you had an arch-specific function, I guess this would turn into
} else {
return arch_vmemmap_alloc_block(...);
}Quoted 8 lines
mm/sparse-vmemmap.c +1
-0+ } } void __meminit vmemmap_verify(pte_t *pte, int node,
Signature · 4 lines
-- Mel Gorman Part-time Phd Student Linux Technology Center University of Limerick IBM Dublin Software Lab
Quoted 30 lines
-----Original Message-----
From: Mel Gorman [mailto:[email protected]]
Sent: 2007年11月8日 22:07
To: Zou, Nanhai
Cc: LKML; Linus Torvalds; Greg KH; Dave Jones; Martin Ebourne; Siddha, Suresh B; Andi Kleen; Andrew Morton; Christoph Lameter; Andy Whitcroft
Subject: Re: [Patch] Allocate sparse vmemmap block above 4GOn (08/11/07 08:52), Zou Nan hai didst pronounce:
Quoted 10 lines
Resend the patch for more people to review
On some single node x64 system with huge amount of physical memory e.g > 64G. the memmap size maybe very big.
If the memmap is allocated from low pages, it may occupies too much memory below 4G.
then swiotlb could fail to reserve bounce buffer under 4G which will lead to boot failure.Quoted 5 lines
This patch will first try to allocate memmap memory above 4G in sparse vmemmap code.
If it failed, it will allocate memmap aboveMAX_DMA_ADDRESS. This patch is against 2.6.24-rc1-git14You never state that you depend on the
strict_goalpatch either here or in a leader mail. The usual way of getting peoples attention is to have three mails. In your case it would be
Quoted 6 lines
[PATCH 0/2] Allocate vmemmap from highmem where possible [PATCH 1/2] Strictly place bootmem allocations when requested [PATCH 2/2] Allocate sparse vmemmap block above 4G on
x86_64Maybe you have gone through all this already and I'm coming so late that I missed it all. If that is the case, sorry for the noise.
Hi Mel,
Thanks for reviewing.
Quoted 11 lines
Quoted 11 lines
Signed-off-by Zou Nan hai <[email protected]>Signed-off-by Suresh Siddha <[email protected]>diff -Nraup
a/arch/x86/mm/init_64.cb/arch/x86/mm/init_64.carch/x86/mm/init_64.c +0
-0@@ -448,6 +448,13 @@ void online_page(struct page *page) num_physpages++; }
Quoted 9 lines
arch/x86/mm/init_64.c void online_page(struct page *page) · line 451
Quoted 1 line
arch/x86/mm/init_64.c +1
-0+void * __meminit alloc_bootmem_high_node(pg_data_t *pgdat, unsigned longsize,
arch/x86/mm/init_64.c void online_page(struct page *page) · line 453
Quoted 2 lines
arch/x86/mm/init_64.c +2
-0+ unsigned long align) +{Wrong markup there I believe. The
__meminitmarkup is for functions that are needed at runtime when memory is hot-added or hot-removed. Bootmem functions do not qualify.__initis sufficient.
__meminit here is to avoid section link warning here.
this function is called by vmemmap_alloc_block which is a __meminit function, so if I mark it as __meminit, there were be warning about section mismatch, although this function will not be called during memory hotplug time.
Quoted 12 lines
The name is confusing as well. I don't know what a high node is, but I think you mean
alloc_bootmem_highmemoralloc_bootmem_highmem_zone. That in *itself* is confusing onx86_64because the memory above 4GiB isZONE_NORMAL, notZONE_HIGHMEM. Calling italloc_bootmem_nondma()makes it worse.I think you need to rework this to have an arch-specific function like
arch_alloc_vmemmap_block()that by default allocates with____alloc_bootmem_node()and otherwise uses an arch-specific function. In this case, it would know to call__alloc_bootmem_core()with the proper addressing limits.
Quoted 7 lines
arch/x86/mm/init_64.c void online_page(struct page *page) · line 456
Quoted 3 lines
arch/x86/mm/init_64.c +3
-0+return __alloc_bootmem_core (pgdat->bdata, size, + align, (4UL *1024 *1024 *1024 ),0 ,1 ); +}More magic values, both the 4GiB address here and the magic "1" at the end are problems.
Yes, the 4UL*1024*1024*1024 could be a define here.
However I think define constant for a boolean parameter is overkilling. Look at kernel code, e.g.
the force parameter in get_user_pages
and
the sync parameter in try_to_wake_up
we don't do things like
#define TRY_TO_WAKEUP_SYNC 1
#define TRY_TO_WAKEUP_NOSYNC 0
There are other examples in kernel code. I believe it is a common practice not to define constant for a Boolean parameter. How do you think?
Quoted 22 lines
Quoted 8 lines
arch/x86/mm/init_64.c +1
-0+#ifdef CONFIG_MEMORY_HOTPLUG /* * Memory is added always to NORMAL zone. This means you will never get diff -Nraup
a/include/linux/bootmem.hb/include/linux/bootmem.hinclude/linux/bootmem.h extern void *__alloc_bootmem_core(struct · line 67
Quoted 7 lines
include/linux/bootmem.h +4
-0@@ -61,6 +61,10 @@ extern void *__alloc_bootmem_core(structunsigned long limit, int strict_goal); +extern void *alloc_bootmem_high_node (pg_data_t *pgdat, +unsigned long size, +unsigned long align); +Confusing. Your declaration here makes it look like a bootmem API function but it is an arch-specific function that only exists for x86-64. I know it gets overridden later when a weak symbol but the more common approach is to have
Kconfigdefine something likeARCH_HIGHMEM_VMEMMAPand
Quoted 8 lines
#ifdef
CONFIG_ARCH_HIGHMEM_VMEMMAP
extern void *alloc_bootmem_high_node(pg_data_t *pgdat,
unsigned long size,
unsigned long align);
#else
static inline void *alloc_bootmem_high_node(pg_data_t *pgdat,unsigned long size, unsigned long align) {
I was told by Andrew that
Quoted 7 lines
return NULL; }#endif /*
CONFIG_ARCH_HIGHMEM_VMEMMAPIt's be similar if you have an arch-specific callback for vmalloc block allocations instead of extending the bootmem API.
I was told by Andrew that ARCH_HAS_XXX is not preferred half a year before in
http://lkml.org/lkml/2007/5/17/287
So I reworked that patch to the preferred __attribute__ ((weak)))
.....
Quoted 57 lines
mm/bootmem.c void * __init __alloc_bootmem_low_node(p · line 496
Quoted 13 lines
include/linux/bootmem.h +0
-0#ifndef CONFIG_HAVE_ARCH_BOOTMEM_NODE extern void reserve_bootmem (unsigned long addr,unsigned long size);#define alloc_bootmem (x) \diff -Nraup
a/mm/bootmem.cb/mm/bootmem.cmm/bootmem.c +2
-0@@ -492,3 +492,11 @@ void * __init __alloc_bootmem_low_node(preturn __alloc_bootmem_core (pgdat->bdata, size, align, goal,ARCH_LOW_ADDRESS_LIMIT ,0 ); } + +__attribute__((weak)) __meminit
__meminitvs_initagainQuoted 10 lines
mm/bootmem.c +6
-0+void *alloc_bootmem_high_node (pg_data_t *pgdat,unsigned long size, +unsigned long align) +{ +return NULL ; +} +diff -Nraup
a/mm/sparse-vmemmap.cb/mm/sparse-vmemmap.cmm/sparse-vmemmap.c void * __meminit vmemmap_alloc_block(uns · line 51
Quoted 10 lines
mm/sparse-vmemmap.c +4
-1@@ -43,9 +43,13 @@ void * __meminit vmemmap_alloc_block(unsif (page)return page_address (page);return NULL ; - }else + }else { +void *p =alloc_bootmem_high_node (NODE_DATA (node), size, size); +if (p) +return p;return __alloc_bootmem_node (NODE_DATA (node), size, size,__pa (MAX_DMA_ADDRESS ));If you had an arch-specific function, I guess this would turn into
} else { return arch_vmemmap_alloc_block(...); }mm/sparse-vmemmap.c void * __meminit vmemmap_alloc_block(uns · line 55
Quoted 8 lines
mm/sparse-vmemmap.c +1
-0+ } } void __meminit vmemmap_verify(pte_t *pte, int node,--
--
Mel Gorman
Part-time Phd Student Linux Technology Center
University of Limerick IBM Dublin Software Lab
Quoted 4 lines · Nanhai
Quoted 3 lines
More magic values, both the 4GiB address here and the magic "1" at the end are problems.
Yes, the 4UL*1024*1024*1024 could be a define here.
The 4GB boundary here is MAX_DMA32_ADDRESS I guess? We are only having this problem because of the two DMA zones on x86_64. I thought Andi was getting rid of the first one at 16MB. If he would do so then ZONE_DMA could be used instead of DMA32 and everything will be fine.
For now you may want to put
#ifdef CONFIG_ZONE_DMA32
around this code since it depends on DMA32.
On (09/11/07 09:28), Zou, Nanhai didst pronounce:
Quoted 9 lines
Quoted 4 lines
Wrong markup there I believe. The
__meminitmarkup is for functions that are needed at runtime when memory is hot-added or hot-removed. Bootmem functions do not qualify.__initis sufficient.
__meminithere is to avoid section link warning here.
this function is called byvmemmap_alloc_blockwhich is a__meminitfunction, so if I mark it as__meminit, there were be warning about section mismatch, although this function will not be called during memory hotplug time.
Ok, that is fair enough and good enough reason to mark it __meminit. It's a pity in one sense because a dead function remains in the loaded kernel image but I can't think of a nice way around it right now.
Quoted 30 lines
Quoted 12 lines
The name is confusing as well. I don't know what a high node is, but I think you mean
alloc_bootmem_highmemoralloc_bootmem_highmem_zone. That in *itself* is confusing onx86_64because the memory above 4GiB isZONE_NORMAL, notZONE_HIGHMEM. Calling italloc_bootmem_nondma()makes it worse.I think you need to rework this to have an arch-specific function like
arch_alloc_vmemmap_block()that by default allocates with____alloc_bootmem_node()and otherwise uses an arch-specific function. In this case, it would know to call__alloc_bootmem_core()with the proper addressing limits.Quoted 7 lines
arch/x86/mm/init_64.c void online_page(struct page *page) · line 456
Quoted 3 lines
arch/x86/mm/init_64.c +3
-0+return __alloc_bootmem_core (pgdat->bdata, size, + align, (4UL *1024 *1024 *1024 ),0 ,1 ); +}More magic values, both the 4GiB address here and the magic "1" at the end are problems.
Yes, the 4UL*1024*1024*1024 could be a define here.
However I think define constant for a boolean parameter is overkilling. Look at kernel code, e.g.
the force parameter inget_user_pages
and
the sync parameter intry_to_wake_up
we don't do things like
#defineTRY_TO_WAKEUP_SYNC1
#defineTRY_TO_WAKEUP_NOSYNC0
Quoted 3 lines
There are other examples in kernel code. I believe it is a common practice not to define constant for a Boolean parameter. How do you think?
I guess it's a question of personal taste. I am not a fan of magic numbers because it's not clear from the call-site what "1" means. It could be a value of 1 or a boolean true requiring that I double check the function being called. I would have used either a #define or defined
__something() - internal to a C filesomething() - non-strict versionsomething_strict() - the strict version
so it's obvious from the call-site what the intention is. However, I imagine there are people puking at that notion too.
Fix up the 4GB magic number at least so it reflects that it is DMA32 you are talking about.
Quoted 35 lines
Quoted 22 lines
Quoted 8 lines
+
#ifdefCONFIG_MEMORY_HOTPLUG
/*
* Memory is added always to NORMAL zone. This means you will never get
diff -Nraupa/include/linux/bootmem.hb/include/linux/bootmem.hinclude/linux/bootmem.h extern void *__alloc_bootmem_core(struct · line 67
Quoted 7 lines
include/linux/bootmem.h +4
-0@@ -61,6 +61,10 @@ extern void *__alloc_bootmem_core(structunsigned long limit, int strict_goal); +extern void *alloc_bootmem_high_node (pg_data_t *pgdat, +unsigned long size, +unsigned long align); +Confusing. Your declaration here makes it look like a bootmem API function but it is an arch-specific function that only exists for x86-64. I know it gets overridden later when a weak symbol but the more common approach is to have
Kconfigdefine something likeARCH_HIGHMEM_VMEMMAPandQuoted 8 lines
#ifdef
CONFIG_ARCH_HIGHMEM_VMEMMAP
extern void *alloc_bootmem_high_node(pg_data_t *pgdat,
unsigned long size,
unsigned long align);
#else
static inline void *alloc_bootmem_high_node(pg_data_t *pgdat,unsigned long size, unsigned long align) {I was told by Andrew that
Quoted 4 lines
return NULL; }#endif /*
CONFIG_ARCH_HIGHMEM_VMEMMAP
Quoted 8 lines
Quoted 3 lines
It's be similar if you have an arch-specific callback for vmalloc block allocations instead of extending the bootmem API.
I was told by Andrew that
ARCH_HAS_XXXis not preferred half a year before in
http://lkml.org/lkml/2007/5/17/287
So I reworked that patch to the preferred__attribute__((weak)))
.....
Ok, my bad. I wasn't aware of that.
Quoted 59 lines
Quoted 58 lines
Quoted 9 lines
include/linux/bootmem.h +0
-0#ifndef CONFIG_HAVE_ARCH_BOOTMEM_NODE extern void reserve_bootmem (unsigned long addr,unsigned long size);#define alloc_bootmem (x) \diff -Nraup
a/mm/bootmem.cb/mm/bootmem.cmm/bootmem.c void * __init __alloc_bootmem_low_node(p · line 496
Quoted 5 lines
mm/bootmem.c +2
-0@@ -492,3 +492,11 @@ void * __init __alloc_bootmem_low_node(preturn __alloc_bootmem_core (pgdat->bdata, size, align, goal,ARCH_LOW_ADDRESS_LIMIT ,0 ); } + +__attribute__((weak)) __meminit
__meminitvs_initagainQuoted 10 lines
mm/bootmem.c +6
-0+void *alloc_bootmem_high_node (pg_data_t *pgdat,unsigned long size, +unsigned long align) +{ +return NULL ; +} +diff -Nraup
a/mm/sparse-vmemmap.cb/mm/sparse-vmemmap.cmm/sparse-vmemmap.c void * __meminit vmemmap_alloc_block(uns · line 51
Quoted 10 lines
mm/sparse-vmemmap.c +4
-1@@ -43,9 +43,13 @@ void * __meminit vmemmap_alloc_block(unsif (page)return page_address (page);return NULL ; - }else + }else { +void *p =alloc_bootmem_high_node (NODE_DATA (node), size, size); +if (p) +return p;return __alloc_bootmem_node (NODE_DATA (node), size, size,__pa (MAX_DMA_ADDRESS ));If you had an arch-specific function, I guess this would turn into
} else { return arch_vmemmap_alloc_block(...); }mm/sparse-vmemmap.c void * __meminit vmemmap_alloc_block(uns · line 55
Quoted 8 lines
mm/sparse-vmemmap.c +1
-0+ } } void __meminit vmemmap_verify(pte_t *pte, int node,--
--
Mel Gorman
Part-time Phd Student Linux Technology Center
University of Limerick IBM Dublin Software Lab
Signature · 4 lines
-- Mel Gorman Part-time Phd Student Linux Technology Center University of Limerick IBM Dublin Software Lab