questions about init_memory_mapping_high()
Hello, guys.
I've been looking at init_memory_mapping_high() added by commit 1411e0ec31 (x86-64, numa: Put pgtable to local node memory) and I got curious about several things.
1. The only rationale given in the commit description is that a RED-PEN is killed, which was the following.
/*
* RED-PEN putting page tables only on node 0 could
* cause a hotspot and fill up ZONE_DMA. The page tables
* need roughly 0.5KB per GB.
*/
This already wasn't true with top-down memblock allocation.
The 0.5KB per GiB comment is for 32bit w/ 3 level mapping. On 64bit, it's ~4KiB per GiB when using 2MiB mappings and, well, very small per GiB if 1GiB mapping is used. Even with 2MiB mapping, 1TiB mapping would only be 4MiB. Under ZONE_DMA, this could be problematic but with top-down this can't be a problem in any realistic way in foreseeable future.
2. In most cases, the kernel mapping ends up using 1GiB mappings and when using 1GiB mappings, a single second level table would cover 512GiB of memory. IOW, little, if any, is gained by trying to allocate the page table on node local memory when 1GiB mappings are used, they end up sharing the same page somewhere anyway.
I guess this was the reason why the commit message showed usage of 2MiB mappings so that each node would end up with their own third level page tables. Is this something we need to optimize for? I don't recall seeing recent machines which don't use 1GiB pages for the linear mapping. Are there NUMA machines which can't use 1GiB mappings?
Or was this for the future where we would be using a lot more than 512GiB of memory? If so, wouldn't that be a bit over-reaching? Wouldn't we be likely to have 512GiB mappings if we get to a point where NUMA locality of such mappings actually become a problem?
3. The new code creates linear mapping only for memory regions where e820 actually says there is memory as opposed to mapping from base to top. Again, I'm not sure what the intention of this change was. Having larger mappings over holes is much cheaper than having to break down the mappings into smaller sized mappings around the holes both in terms of memory and run time overhead. Why would we want to match the linear address mapping to the e820 map exactly?
Also, Yinghai, can you please try to write commit descriptions with more details? It really sucks for other people when they have to guess what the actual changes and underlying intentions are. The commit adding init_memory_mapping_high() is very anemic on details about how the behavior changes and the only intention given there is RED-PEN removal even which is largely a miss.
Thank you.
Signature · 1 line
tejun
Quoted 15 lines · Tejun Heo
Hello, guys.
I've been looking at
init_memory_mapping_high()added by commit1411e0ec31(x86-64, numa: Put pgtable to local node memory) and I got curious about several things.1. The only rationale given in the commit description is that a RED-PEN is killed, which was the following.
/*
* RED-PEN putting page tables only on node 0 could
* cause a hotspot and fill upZONE_DMA. The page tables
* need roughly 0.5KB per GB.
*/
Quoted 8 lines
This already wasn't true with top-down memblock allocation.
The 0.5KB per GiB comment is for 32bit w/ 3 level mapping. On 64bit, it's ~4KiB per GiB when using 2MiB mappings and, well, very small per GiB if 1GiB mapping is used. Even with 2MiB mapping, 1TiB mapping would only be 4MiB. Under
ZONE_DMA, this could be problematic but with top-down this can't be a problem in any realistic way in foreseeable future.
before that patch set:
page table for [0, 4g) is just under and near 512M.
page table for [4g, 128) is just under and near 2g ( assume 0-2g is ram under 4g)
first patch in the patch set will
page table for [0, 4g) is just under and near 2g.( assume 0-2g is ram under 4g) page table for [4g, 128) is just under and near 128g
so top down could put most page table on last node.
for debug purpose case, 2M and 1G page could be disabled.
code excerpt from init_memory_mapping()
printk(KERN_INFO "init_memory_mapping: %016lx-%016lx\n", start, end);
#if defined(CONFIG_DEBUG_PAGEALLOC) || defined(CONFIG_KMEMCHECK)
/*
* For CONFIG_DEBUG_PAGEALLOC, identity mapping will use small pages.
* This will simplify cpa(), which otherwise needs to support splitting
* large pages into small in interrupt context, etc.
*/
use_pse = use_gbpages = 0;
#else
use_pse = cpu_has_pse;
use_gbpages = direct_gbpages;#endif
Quoted 14 lines
2. In most cases, the kernel mapping ends up using 1GiB mappings and when using 1GiB mappings, a single second level table would cover 512GiB of memory. IOW, little, if any, is gained by trying to allocate the page table on node local memory when 1GiB mappings are used, they end up sharing the same page somewhere anyway.
I guess this was the reason why the commit message showed usage of 2MiB mappings so that each node would end up with their own third level page tables. Is this something we need to optimize for? I don't recall seeing recent machines which don't use 1GiB pages for the linear mapping. Are there NUMA machines which can't use 1GiB mappings?
Quoted 4 lines
Or was this for the future where we would be using a lot more than 512GiB of memory? If so, wouldn't that be a bit over-reaching? Wouldn't we be likely to have 512GiB mappings if we get to a point where NUMA locality of such mappings actually become a problem?
till now:
amd 64 cpu does support 1gb page.
Intel CPU Nehalem-EX does not. and several vendors do provide 8 sockets NUMA system with 1024g and 2048g RAM
cpu after Nehalem-EX looks support 1gb page.
Quoted 8 lines
3. The new code creates linear mapping only for memory regions where e820 actually says there is memory as opposed to mapping from base to top. Again, I'm not sure what the intention of this change was. Having larger mappings over holes is much cheaper than having to break down the mappings into smaller sized mappings around the holes both in terms of memory and run time overhead. Why would we want to match the linear address mapping to the e820 map exactly?
we don't need to map those holes if there is any.
for hotplug case, they should map new added memory later.
Quoted 7 lines
Also, Yinghai, can you please try to write commit descriptions with more details? It really sucks for other people when they have to guess what the actual changes and underlying intentions are. The commit adding
init_memory_mapping_high()is very anemic on details about how the behavior changes and the only intention given there is RED-PEN removal even which is largely a miss.
i don't know what you are talking about. that changelog is clear enough.
Yinghai
Quoted 7 lines · Yinghai Lu
Quoted 6 lines
I guess this was the reason why the commit message showed usage of 2MiB mappings so that each node would end up with their own third level page tables. Is this something we need to optimize for? I don't recall seeing recent machines which don't use 1GiB pages for the linear mapping. Are there NUMA machines which can't use 1GiB mappings?
Quoted 5 lines
till now:
amd 64 cpu does support 1gb page.Intel CPU Nehalem-EX does not. and several vendors do provide 8 sockets NUMA system with 1024g and 2048g RAM
That's interesting. Didn't expect that. So, this one is an actually valid reason for implementing per node mapping. Is this Nehalem-EX only thing? Or is it applicable to all xeons upto now?
Quoted 9 lines
Quoted 7 lines
3. The new code creates linear mapping only for memory regions where e820 actually says there is memory as opposed to mapping from base to top. Again, I'm not sure what the intention of this change was. Having larger mappings over holes is much cheaper than having to break down the mappings into smaller sized mappings around the holes both in terms of memory and run time overhead. Why would we want to match the linear address mapping to the e820 map exactly?
we don't need to map those holes if there is any.
Yeah, sure, my point was that not mapping those holes is likely to be worse. Wouldn't it be better to get low and high ends of the occupied area and expand those to larger mapping size? It's worse to match the memory map exactly. You unnecessarily end up with smaller mappings.
Quoted 1 line
for hotplug case, they should map new added memory later.
Sure.
Quoted 8 lines
Quoted 6 lines
Also, Yinghai, can you please try to write commit descriptions with more details? It really sucks for other people when they have to guess what the actual changes and underlying intentions are. The commit adding
init_memory_mapping_high()is very anemic on details about how the behavior changes and the only intention given there is RED-PEN removal even which is largely a miss.i don't know what you are talking about. that changelog is clear enough.
Ah well, if you still think the changelog is clear enough, I give up. I guess I'll just keep rewriting your changelogs.
Thanks.
--
tejun
Quoted 11 lines · Tejun Heo
Quoted 10 lines · Yinghai Lu
Quoted 6 lines
I guess this was the reason why the commit message showed usage of 2MiB mappings so that each node would end up with their own third level page tables. Is this something we need to optimize for? I don't recall seeing recent machines which don't use 1GiB pages for the linear mapping. Are there NUMA machines which can't use 1GiB mappings?
till now:
amd 64 cpu does support 1gb page.
Quoted 6 lines
Quoted 2 lines
Intel CPU Nehalem-EX does not. and several vendors do provide 8 sockets NUMA system with 1024g and 2048g RAM
That's interesting. Didn't expect that. So, this one is an actually valid reason for implementing per node mapping. Is this Nehalem-EX only thing? Or is it applicable to all xeons upto now?
only have access for Nehalem-EX and Westmere-EX till now.
Quoted 9 lines
Quoted 9 lines
Quoted 8 lines
3. The new code creates linear mapping only for memory regions where e820 actually says there is memory as opposed to mapping from base to top. Again, I'm not sure what the intention of this change was. Having larger mappings over holes is much cheaper than having to break down the mappings into smaller sized mappings around the holes both in terms of memory and run time overhead. Why would we want to match the linear address mapping to the e820 map exactly?
Quoted 6 lines
Quoted 1 line
we don't need to map those holes if there is any.
Yeah, sure, my point was that not mapping those holes is likely to be worse. Wouldn't it be better to get low and high ends of the occupied area and expand those to larger mapping size? It's worse to match the memory map exactly. You unnecessarily end up with smaller mappings.
it will reuse previous not used entries in the init_memory_mapping().
Quoted 12 lines
Quoted 2 lines
for hotplug case, they should map new added memory later.
Sure.
Quoted 7 lines
Quoted 6 lines
Also, Yinghai, can you please try to write commit descriptions with more details? It really sucks for other people when they have to guess what the actual changes and underlying intentions are. The commit adding
init_memory_mapping_high()is very anemic on details about how the behavior changes and the only intention given there is RED-PEN removal even which is largely a miss.
Quoted 4 lines
Quoted 1 line
i don't know what you are talking about. that changelog is clear enough.
Ah well, if you still think the changelog is clear enough, I give up. I guess I'll just keep rewriting your changelogs.
Thank you very much.
Yinghai
Hey,
Quoted 9 lines · Yinghai Lu
Quoted 6 lines · Tejun Heo
Quoted 2 lines
Intel CPU Nehalem-EX does not. and several vendors do provide 8 sockets NUMA system with 1024g and 2048g RAM
That's interesting. Didn't expect that. So, this one is an actually valid reason for implementing per node mapping. Is this Nehalem-EX only thing? Or is it applicable to all xeons upto now?
only have access for Nehalem-EX and Westmere-EX till now.
I see. I was wondering whether it was a worthwhile optimization if it was an one-off thing for Nehalem-EX.
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 7 lines
3. The new code creates linear mapping only for memory regions where e820 actually says there is memory as opposed to mapping from base to top. Again, I'm not sure what the intention of this change was. Having larger mappings over holes is much cheaper than having to break down the mappings into smaller sized mappings around the holes both in terms of memory and run time overhead. Why would we want to match the linear address mapping to the e820 map exactly?
Quoted 8 lines
Quoted 6 lines
Quoted 1 line
we don't need to map those holes if there is any.
Yeah, sure, my point was that not mapping those holes is likely to be worse. Wouldn't it be better to get low and high ends of the occupied area and expand those to larger mapping size? It's worse to match the memory map exactly. You unnecessarily end up with smaller mappings.
it will reuse previous not used entries in the
init_memory_mapping().
Hmmm... I'm not really following. Can you elaborate? The reason why smaller mapping is bad is because of increased TLB pressure. What does using the existing entries have to do with it?
Thanks.
--
tejun
Quoted 10 lines · Tejun Heo
Quoted 6 lines
Quoted 4 lines
Yeah, sure, my point was that not mapping those holes is likely to be worse. Wouldn't it be better to get low and high ends of the occupied area and expand those to larger mapping size? It's worse to match the memory map exactly. You unnecessarily end up with smaller mappings.
it will reuse previous not used entries in the
init_memory_mapping().Hmmm... I'm not really following. Can you elaborate? The reason why smaller mapping is bad is because of increased TLB pressure. What does using the existing entries have to do with it?
assume 1g page is used. first node will actually mapped 512G already. so if the system only have 1024g. then first 512g page table will on node0 ram. second 512g page table will be on node4.
when only 2M are used, it is 1G boundary. for 1024g system. page table (about 512k) for mem 0-128g is on node0.
page table (about 512k) for mem 128g-256g is on node1.
...
Do you mean we need to put those all 512k together to reduce TLB presure?
Yinghai
Hey, again.
Quoted 8 lines · Yinghai Lu
Quoted 3 lines
Hmmm... I'm not really following. Can you elaborate? The reason why smaller mapping is bad is because of increased TLB pressure. What does using the existing entries have to do with it?
assume 1g page is used. first node will actually mapped 512G already. so if the system only have 1024g. then first 512g page table will on node0 ram. second 512g page table will be on node4.
Quoted 5 lines
when only 2M are used, it is 1G boundary. for 1024g system. page table (about 512k) for mem 0-128g is on node0.
page table (about 512k) for mem 128g-256g is on node1.
...
Do you mean we need to put those all 512k together to reduce TLB presure?
Nope, let's say the machine supports 1GiB mapping, has 8GiB of memory where [0,4)GiB is node 0 and [4,8)GiB node1, and there's a hole of 128MiB right on top of 4GiB. Before the change, the page mapping code wouldn't care about the whole and just map the whole [0,8)GiB area with eight 1GiB mapping. Now with your change, [4, 5)GiB will be mapped using 2MiB mappings to avoid mapping the 128MiB hole.
We end up unnecessarily using smaller size mappings (512 2MiB mappings instead of 1 1GiB mapping) thus increasing TLB pressure. There is no reason to match the linear address mapping exactly to the physical memory map. It is no accident that the original code didn't consider memory holes. Using larger mappings over them is more beneficial to trying to punch holes with smaller mappings.
This rather important change was made without any description or explanation, which I find somewhat disturbing. Anyways, what we can do is just taking bottom and top addresses of occupied NUMA regions and round them down and up, respectively, to the largest page mapping size supported as long as the top address doesn't go over max_pfn instead of mapping exactly according to the memblocks.
Thanks.
Signature · 1 line
tejun
Quoted 31 lines · Tejun Heo
Hey, again.
Quoted 8 lines · Yinghai Lu
Quoted 3 lines
Hmmm... I'm not really following. Can you elaborate? The reason why smaller mapping is bad is because of increased TLB pressure. What does using the existing entries have to do with it?
assume 1g page is used. first node will actually mapped 512G already. so if the system only have 1024g. then first 512g page table will on node0 ram. second 512g page table will be on node4.
Quoted 5 lines
when only 2M are used, it is 1G boundary. for 1024g system. page table (about 512k) for mem 0-128g is on node0.
page table (about 512k) for mem 128g-256g is on node1.
...
Do you mean we need to put those all 512k together to reduce TLB presure?Nope, let's say the machine supports 1GiB mapping, has 8GiB of memory where [0,4)GiB is node 0 and [4,8)GiB node1, and there's a hole of 128MiB right on top of 4GiB. Before the change, the page mapping code wouldn't care about the whole and just map the whole [0,8)GiB area with eight 1GiB mapping. Now with your change, [4, 5)GiB will be mapped using 2MiB mappings to avoid mapping the 128MiB hole.
We end up unnecessarily using smaller size mappings (512 2MiB mappings instead of 1 1GiB mapping) thus increasing TLB pressure. There is no reason to match the linear address mapping exactly to the physical memory map. It is no accident that the original code didn't consider memory holes. Using larger mappings over them is more beneficial to trying to punch holes with smaller mappings.
Quoted 7 lines
This rather important change was made without any description or explanation, which I find somewhat disturbing. Anyways, what we can do is just taking bottom and top addresses of occupied NUMA regions and round them down and up, respectively, to the largest page mapping size supported as long as the top address doesn't go over
max_pfninstead of mapping exactly according to the memblocks.
ok, please check two patches that fix the problem.
thanks
Yinghai
PATCH 1/2 x86,mm: Introduce init_memory_mapping_ext()
In reply to Tejun HeoAdd extra input tbl_end. it could be smaller than end.
Prepare for init_memory_mapping_high() to align boundary to 1G. aka end could round up to 1g bound, and will be bigger then original node end.
init_memory_mapping will call init_memory_mapping_ext with tbl=end.
--- arch/x86/include/asm/page_types.h | 7 +++++-- arch/x86/mm/init.c | 22 +++++++++++++++------- 2 files changed, 20 insertions(+), 9 deletions(-)
arch/x86/include/asm/page_types.h +5 -2
@@ -51,8 +51,11 @@ static inline phys_addr_t get_max_mappedreturn (phys_addr_t )max_pfn_mapped <<PAGE_SHIFT ; } -extern unsigned long init_memory_mapping (unsigned long start, -unsigned long end); +unsigned long init_memory_mapping_ext (unsigned long start, +unsigned long end, +unsigned long tbl_end); + +unsigned long init_memory_mapping (unsigned long start,unsigned long end);void init_memory_mapping_high (void );
arch/x86/mm/init.c +15 -7
@@ -30,10 +30,12 @@ int direct_gbpages#endif ; -static void __init find_early_table_space (unsigned long end,int use_pse, +static void __init find_early_table_space (unsigned long end, +unsigned long tbl_end, +int use_pse,int use_gbpages) { -unsigned long puds, pmds, ptes, tables, start =0 , good_end = end; +unsigned long puds, pmds, ptes, tables, start =0 ;phys_addr_t base; puds = (end +PUD_SIZE -1 ) >>PUD_SHIFT ;
@@ -66,10 +68,10 @@ static void __init find_early_table_spac/* for fixmap */ tables +=roundup (__end_of_fixed_addresses *sizeof (pte_t),PAGE_SIZE ); - good_end = max_pfn_mapped <<PAGE_SHIFT ; + tbl_end = max_pfn_mapped <<PAGE_SHIFT ;#endif - base =memblock_find_in_range (start, good_end, tables,PAGE_SIZE ); + base =memblock_find_in_range (start, tbl_end, tables,PAGE_SIZE );if (base ==MEMBLOCK_ERROR )panic ("Cannot find space for the kernel page tables" );
@@ -114,8 +116,9 @@ static int __meminit save_mr(struct map_* This runs before bootmem is initialized and gets pages directly from * the physical memory. To access them they are temporarily mapped. */ -unsigned long __init_refok init_memory_mapping (unsigned long start, -unsigned long end) +unsigned long __init_refok init_memory_mapping_ext (unsigned long start, +unsigned long end, +unsigned long tbl_end) {unsigned long page_size_mask =0 ;unsigned long start_pfn, end_pfn;
@@ -258,7 +261,7 @@ unsigned long __init_refok init_memory_m* nodes are discovered. */ if (!after_bootmem) -find_early_table_space (end, use_pse, use_gbpages); +find_early_table_space (end, tbl_end, use_pse, use_gbpages);for (i =0 ; i < nr_range; i++) ret = kernel_physical_mapping_init(mr[i].start, mr[i].end,
@@ -282,6 +285,11 @@ unsigned long __init_refok init_memory_mreturn ret >>PAGE_SHIFT ; } +unsigned long __init_refok init_memory_mapping (unsigned long start, +unsigned long end) +{ +return init_memory_mapping_ext (start, end, end); +}/* * devmem_is_allowed() checks to see if /dev/mem access to a certain address
PATCH 2/2 x86,mm,64bit: Round up memory boundary for init_memory_mapping_high()
In reply to Tejun Heotj pointed out:
when node does not have 1G aligned boundary, like 128M.init_memory_mapping_high() could render smaller mapping by 128M on one node, and 896M on next node with 2M pages instead of 1g page. that could increase TLB presure.
So if gb page is used, try to align the boundary to 1G before calling init_memory_mapping_ext(), to make sure only use one 1g entry for that cross node 1G.
Need to init_meory_mapping_ext() to table tbl_end, to make sure pgtable is on previous node instead of next node.
on one AMD 512g system with not aligned boundary (extra 768M) before the patch
[ 0.000000] init_memory_mapping: [0x00000000000000-0x000000d7f9ffff] [ 0.000000] 0000000000 - 00c0000000 page 1G [ 0.000000] 00c0000000 - 00d7e00000 page 2M [ 0.000000] 00d7e00000 - 00d7fa0000 page 4k [ 0.000000] kernel direct mapping tables up to d7fa0000 @ [0xd7f9d000-0xd7f9ffff] pre-allocated [ 0.000000] kernel direct mapping tables up to d7fa0000 @ [0xd7f9d000-0xd7f9efff] final [ 0.000000] memblock_x86_reserve_range: [0xd7f9d000-0xd7f9efff] PGTABLE ... [ 0.000000] Adding active range (0, 0x10, 0x98) 0 entries of 3200 used [ 0.000000] Adding active range (0, 0x100, 0xd7fa0) 1 entries of 3200 used [ 0.000000] Adding active range (0, 0x100000, 0x1028000) 2 entries of 3200 used [ 0.000000] Adding active range (1, 0x1028000, 0x2028000) 3 entries of 3200 used [ 0.000000] Adding active range (2, 0x2028000, 0x3028000) 4 entries of 3200 used [ 0.000000] Adding active range (3, 0x3028000, 0x4028000) 5 entries of 3200 used [ 0.000000] Adding active range (4, 0x4028000, 0x5028000) 6 entries of 3200 used [ 0.000000] Adding active range (5, 0x5028000, 0x6028000) 7 entries of 3200 used [ 0.000000] Adding active range (6, 0x6028000, 0x7028000) 8 entries of 3200 used [ 0.000000] Adding active range (7, 0x7028000, 0x8028000) 9 entries of 3200 used [ 0.000000] init_memory_mapping: [0x00000100000000-0x00001027ffffff] [ 0.000000] 0100000000 - 1000000000 page 1G [ 0.000000] 1000000000 - 1028000000 page 2M [ 0.000000] kernel direct mapping tables up to 1028000000 @ [0x1027ffe000-0x1027ffffff] pre-allocated [ 0.000000] kernel direct mapping tables up to 1028000000 @ [0x1027ffe000-0x1027ffefff] final [ 0.000000] memblock_x86_reserve_range: [0x1027ffe000-0x1027ffefff] PGTABLE [ 0.000000] init_memory_mapping: [0x00001028000000-0x00002027ffffff] [ 0.000000] 1028000000 - 1040000000 page 2M [ 0.000000] 1040000000 - 2000000000 page 1G [ 0.000000] 2000000000 - 2028000000 page 2M [ 0.000000] kernel direct mapping tables up to 2028000000 @ [0x2027ffe000-0x2027ffffff] pre-allocated [ 0.000000] kernel direct mapping tables up to 2028000000 @ [0x2027ffe000-0x2027ffefff] final [ 0.000000] memblock_x86_reserve_range: [0x2027ffe000-0x2027ffefff] PGTABLE [ 0.000000] init_memory_mapping: [0x00002028000000-0x00003027ffffff] [ 0.000000] 2028000000 - 2040000000 page 2M [ 0.000000] 2040000000 - 3000000000 page 1G [ 0.000000] 3000000000 - 3028000000 page 2M [ 0.000000] kernel direct mapping tables up to 3028000000 @ [0x3027ffe000-0x3027ffffff] pre-allocated [ 0.000000] kernel direct mapping tables up to 3028000000 @ [0x3027ffe000-0x3027ffefff] final [ 0.000000] memblock_x86_reserve_range: [0x3027ffe000-0x3027ffefff] PGTABLE [ 0.000000] init_memory_mapping: [0x00003028000000-0x00004027ffffff] [ 0.000000] 3028000000 - 3040000000 page 2M [ 0.000000] 3040000000 - 4000000000 page 1G [ 0.000000] 4000000000 - 4028000000 page 2M [ 0.000000] kernel direct mapping tables up to 4028000000 @ [0x4027ffe000-0x4027ffffff] pre-allocated [ 0.000000] kernel direct mapping tables up to 4028000000 @ [0x4027ffe000-0x4027ffefff] final [ 0.000000] memblock_x86_reserve_range: [0x4027ffe000-0x4027ffefff] PGTABLE [ 0.000000] init_memory_mapping: [0x00004028000000-0x00005027ffffff] [ 0.000000] 4028000000 - 4040000000 page 2M [ 0.000000] 4040000000 - 5000000000 page 1G [ 0.000000] 5000000000 - 5028000000 page 2M [ 0.000000] kernel direct mapping tables up to 5028000000 @ [0x5027ffe000-0x5027ffffff] pre-allocated [ 0.000000] kernel direct mapping tables up to 5028000000 @ [0x5027ffe000-0x5027ffefff] final [ 0.000000] memblock_x86_reserve_range: [0x5027ffe000-0x5027ffefff] PGTABLE [ 0.000000] init_memory_mapping: [0x00005028000000-0x00006027ffffff] [ 0.000000] 5028000000 - 5040000000 page 2M [ 0.000000] 5040000000 - 6000000000 page 1G [ 0.000000] 6000000000 - 6028000000 page 2M [ 0.000000] kernel direct mapping tables up to 6028000000 @ [0x6027ffe000-0x6027ffffff] pre-allocated [ 0.000000] kernel direct mapping tables up to 6028000000 @ [0x6027ffe000-0x6027ffefff] final [ 0.000000] memblock_x86_reserve_range: [0x6027ffe000-0x6027ffefff] PGTABLE [ 0.000000] init_memory_mapping: [0x00006028000000-0x00007027ffffff] [ 0.000000] 6028000000 - 6040000000 page 2M [ 0.000000] 6040000000 - 7000000000 page 1G [ 0.000000] 7000000000 - 7028000000 page 2M [ 0.000000] kernel direct mapping tables up to 7028000000 @ [0x7027ffe000-0x7027ffffff] pre-allocated [ 0.000000] kernel direct mapping tables up to 7028000000 @ [0x7027ffe000-0x7027ffefff] final [ 0.000000] memblock_x86_reserve_range: [0x7027ffe000-0x7027ffefff] PGTABLE [ 0.000000] init_memory_mapping: [0x00007028000000-0x00008027ffffff] [ 0.000000] 7028000000 - 7040000000 page 2M [ 0.000000] 7040000000 - 8000000000 page 1G [ 0.000000] 8000000000 - 8028000000 page 2M [ 0.000000] kernel direct mapping tables up to 8028000000 @ [0x8027ffd000-0x8027ffffff] pre-allocated [ 0.000000] kernel direct mapping tables up to 8028000000 @ [0x8027ffd000-0x8027ffefff] final [ 0.000000] memblock_x86_reserve_range: [0x8027ffd000-0x8027ffefff] PGTABLE
after patch
[ 0.000000] init_memory_mapping: [0x00000000000000-0x000000d7f9ffff] [ 0.000000] 0000000000 - 00c0000000 page 1G [ 0.000000] 00c0000000 - 00d7e00000 page 2M [ 0.000000] 00d7e00000 - 00d7fa0000 page 4k [ 0.000000] kernel direct mapping tables up to d7fa0000 @ [0xd7f9d000-0xd7f9ffff] pre-allocated [ 0.000000] kernel direct mapping tables up to d7fa0000 @ [0xd7f9d000-0xd7f9efff] final [ 0.000000] memblock_x86_reserve_range: [0xd7f9d000-0xd7f9efff] PGTABLE ... [ 0.000000] Adding active range (0, 0x10, 0x98) 0 entries of 3200 used [ 0.000000] Adding active range (0, 0x100, 0xd7fa0) 1 entries of 3200 used [ 0.000000] Adding active range (0, 0x100000, 0x1028000) 2 entries of 3200 used [ 0.000000] Adding active range (1, 0x1028000, 0x2028000) 3 entries of 3200 used [ 0.000000] Adding active range (2, 0x2028000, 0x3028000) 4 entries of 3200 used [ 0.000000] Adding active range (3, 0x3028000, 0x4028000) 5 entries of 3200 used [ 0.000000] Adding active range (4, 0x4028000, 0x5028000) 6 entries of 3200 used [ 0.000000] Adding active range (5, 0x5028000, 0x6028000) 7 entries of 3200 used [ 0.000000] Adding active range (6, 0x6028000, 0x7028000) 8 entries of 3200 used [ 0.000000] Adding active range (7, 0x7028000, 0x8028000) 9 entries of 3200 used [ 0.000000] init_memory_mapping: [0x00000100000000-0x0000103fffffff] [ 0.000000] 0100000000 - 1040000000 page 1G [ 0.000000] kernel direct mapping tables up to 1040000000 @ [0x1027fff000-0x1027ffffff] pre-allocated [ 0.000000] init_memory_mapping: [0x00001040000000-0x0000203fffffff] [ 0.000000] 1040000000 - 2040000000 page 1G [ 0.000000] kernel direct mapping tables up to 2040000000 @ [0x2027fff000-0x2027ffffff] pre-allocated [ 0.000000] init_memory_mapping: [0x00002040000000-0x0000303fffffff] [ 0.000000] 2040000000 - 3040000000 page 1G [ 0.000000] kernel direct mapping tables up to 3040000000 @ [0x3027fff000-0x3027ffffff] pre-allocated [ 0.000000] init_memory_mapping: [0x00003040000000-0x0000403fffffff] [ 0.000000] 3040000000 - 4040000000 page 1G [ 0.000000] kernel direct mapping tables up to 4040000000 @ [0x4027fff000-0x4027ffffff] pre-allocated [ 0.000000] init_memory_mapping: [0x00004040000000-0x0000503fffffff] [ 0.000000] 4040000000 - 5040000000 page 1G [ 0.000000] kernel direct mapping tables up to 5040000000 @ [0x5027fff000-0x5027ffffff] pre-allocated [ 0.000000] init_memory_mapping: [0x00005040000000-0x0000603fffffff] [ 0.000000] 5040000000 - 6040000000 page 1G [ 0.000000] kernel direct mapping tables up to 6040000000 @ [0x6027fff000-0x6027ffffff] pre-allocated [ 0.000000] init_memory_mapping: [0x00006040000000-0x0000703fffffff] [ 0.000000] 6040000000 - 7040000000 page 1G [ 0.000000] kernel direct mapping tables up to 7040000000 @ [0x7027fff000-0x7027ffffff] pre-allocated [ 0.000000] init_memory_mapping: [0x00007040000000-0x00008027ffffff] [ 0.000000] 7040000000 - 8000000000 page 1G [ 0.000000] 8000000000 - 8028000000 page 2M [ 0.000000] kernel direct mapping tables up to 8028000000 @ [0x8027ffd000-0x8027ffffff] pre-allocated [ 0.000000] kernel direct mapping tables up to 8028000000 @ [0x8027ffd000-0x8027ffefff] final [ 0.000000] memblock_x86_reserve_range: [0x8027ffd000-0x8027ffefff] PGTABLE
So it fixs the extra mapping problem.
--- arch/x86/mm/init_64.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-)
arch/x86/mm/init_64.c +19 -2
@@ -614,6 +614,7 @@ struct mapping_work_data {unsigned long start;unsigned long end;unsigned long pfn_mapped; +unsigned long align; };static int __init_refok
@@ -621,7 +622,14 @@ mapping_work_fn(unsigned long start_pfn, {struct mapping_work_data *data = datax;unsigned long pfn_mapped; -unsigned long final_start, final_end; +unsigned long final_start, final_end, tbl_end; + + tbl_end = end_pfn <<PAGE_SHIFT ; +/* need to align them to 1G or 2M boundary to avoid smaller mapping */ + start_pfn =round_down (start_pfn, data->align>>PAGE_SHIFT ); +if (start_pfn < data->pfn_mapped) + start_pfn = data->pfn_mapped; + end_pfn =round_up (end_pfn, data->align>>PAGE_SHIFT ); final_start = max_t(unsigned long, start_pfn<<PAGE_SHIFT, data->start); final_end = min_t(unsigned long, end_pfn<<PAGE_SHIFT, data->end);
@@ -629,7 +637,7 @@ mapping_work_fn(unsigned long start_pfn,if (final_end <= final_start)return 0 ; - pfn_mapped =init_memory_mapping (final_start, final_end); + pfn_mapped =init_memory_mapping_ext (final_start, final_end, tbl_end);if (pfn_mapped > data->pfn_mapped) data->pfn_mapped = pfn_mapped;
@@ -641,6 +649,15 @@ static unsigned long __init_refok init_memory_mapping_active_regions(unsigned long start, unsigned long end) {struct mapping_work_data data; +int use_gbpages; + +/* see init_memory_mapping() for the setting */ +#if defined(CONFIG_DEBUG_PAGEALLOC ) || defined(CONFIG_KMEMCHECK ) + use_gbpages =0 ; +#else + use_gbpages = direct_gbpages; +#endif + data.align = use_gbpages ?1UL <<30 :1UL <<21 ; data.start = start; data.end = end;
Re: [PATCH 2/2] x86,mm,64bit: Round up memory boundary for init_memory_mapping_high()
arch/x86/mm/init_64.c static unsigned long __init_refok · line 659
Quoted 11 lines · Yinghai Lu
init_memory_mapping_active_regions(unsigned long start, unsigned long end){arch/x86/mm/init_64.c +8
-0struct mapping_work_data data; +int use_gbpages; + +/* see init_memory_mapping() for the setting */ +#if defined(CONFIG_DEBUG_PAGEALLOC ) || defined(CONFIG_KMEMCHECK ) + use_gbpages =0 ; +#else + use_gbpages = direct_gbpages; +#endif
Sigh. You should *never* ever even think about writing such code. It only results in crap, and in crap duplicated elsewhere as well:
if defined(CONFIG_DEBUG_PAGEALLOC) || defined(CONFIG_KMEMCHECK)
/*
* For CONFIG_DEBUG_PAGEALLOC, identity mapping will use small pages.
* This will simplify cpa(), which otherwise needs to support splitting
* large pages into small in interrupt context, etc.
*/
use_pse = use_gbpages = 0;
#else
use_pse = cpu_has_pse;
use_gbpages = direct_gbpages;#endif
Thanks,
Ingo
Re: [PATCH 2/2] x86,mm,64bit: Round up memory boundary for init_memory_mapping_high()
Quoted 25 lines · Ingo Molnar
arch/x86/mm/init_64.c static unsigned long __init_refok · line 659
Quoted 11 lines · Yinghai Lu
init_memory_mapping_active_regions(unsigned long start, unsigned long end){arch/x86/mm/init_64.c +8
-0struct mapping_work_data data; +int use_gbpages; + +/* see init_memory_mapping() for the setting */ +#if defined(CONFIG_DEBUG_PAGEALLOC ) || defined(CONFIG_KMEMCHECK ) + use_gbpages =0 ; +#else + use_gbpages = direct_gbpages; +#endif Sigh. You should *never* ever even think about writing such code. It only results in crap, and in crap duplicated elsewhere as well:
if
defined(CONFIG_DEBUG_PAGEALLOC)||defined(CONFIG_KMEMCHECK)
/*
* ForCONFIG_DEBUG_PAGEALLOC, identity mapping will use small pages.
* This will simplifycpa(), which otherwise needs to support splitting
* large pages into small in interrupt context, etc.
*/
use_pse=use_gbpages= 0;
Quoted 4 lines
#else use_pse = cpu_has_pse; use_gbpages = direct_gbpages;#endif
sorry, actually i copied from there.
or i could add max_map_unit_size variable?
Thanks
Yinghai
PATCH 1/3 x86, mm: Introduce global page_size_mask
In reply to Ingo MolnarAdd probe_page_size_mask() to detect if need to use 1G or 2M. and store them in page_size_mask.
Only probe them at first init_memory_mapping calling.
second and later init_memory_mapping() calling does not need probe again. also we don't need to pass use_gbpages around.
--- arch/x86/include/asm/pgtable.h | 1 arch/x86/mm/init.c | 70 +++++++++++++++++++++-------------------- 2 files changed, 37 insertions(+), 34 deletions(-)
arch/x86/include/asm/pgtable.h +1 -0
@@ -597,6 +597,7 @@ static inline int pgd_none(pgd_t pgd)#ifndef __ASSEMBLY__extern int direct_gbpages; +extern int page_size_mask;/* local pte updates need not use xchg for locking */ static inline pte_t native_local_ptep_get_and_clear (pte_t *ptep)
arch/x86/mm/init.c +36 -34
114 lines not on this page · raw message
PATCH 2/3 x86,mm: Introduce init_memory_mapping_ext()
In reply to Ingo MolnarAdd extra input tbl_end. it could be smaller than end.
Prepare for init_memory_mapping_high() to align boundary to 1G. aka end could round up to 1g bound, and will be bigger then original node end.
init_memory_mapping will call init_memory_mapping_ext with tbl=end.
-v2: updated after page_size_mask change
--- arch/x86/include/asm/page_types.h | 7 +++++-- arch/x86/mm/init.c | 21 ++++++++++++++------- 2 files changed, 19 insertions(+), 9 deletions(-)
arch/x86/include/asm/page_types.h +5 -2
@@ -51,8 +51,11 @@ static inline phys_addr_t get_max_mappedreturn (phys_addr_t )max_pfn_mapped <<PAGE_SHIFT ; } -extern unsigned long init_memory_mapping (unsigned long start, -unsigned long end); +unsigned long init_memory_mapping_ext (unsigned long start, +unsigned long end, +unsigned long tbl_end); + +unsigned long init_memory_mapping (unsigned long start,unsigned long end);void init_memory_mapping_high (void );
arch/x86/mm/init.c +14 -7
@@ -32,9 +32,10 @@ int direct_gbpagesint page_size_mask =-1 ; -static void __init find_early_table_space (unsigned long end) +static void __init find_early_table_space (unsigned long end, +unsigned long tbl_end) { -unsigned long puds, pmds, ptes, tables, start =0 , good_end = end; +unsigned long puds, pmds, ptes, tables, start =0 ;phys_addr_t base; puds = (end +PUD_SIZE -1 ) >>PUD_SHIFT ;
@@ -67,10 +68,10 @@ static void __init find_early_table_spac/* for fixmap */ tables +=roundup (__end_of_fixed_addresses *sizeof (pte_t),PAGE_SIZE ); - good_end = max_pfn_mapped <<PAGE_SHIFT ; + tbl_end = max_pfn_mapped <<PAGE_SHIFT ;#endif - base =memblock_find_in_range (start, good_end, tables,PAGE_SIZE ); + base =memblock_find_in_range (start, tbl_end, tables,PAGE_SIZE );if (base ==MEMBLOCK_ERROR )panic ("Cannot find space for the kernel page tables" );
@@ -144,8 +145,9 @@ static int __meminit save_mr(struct map_* This runs before bootmem is initialized and gets pages directly from * the physical memory. To access them they are temporarily mapped. */ -unsigned long __init_refok init_memory_mapping (unsigned long start, -unsigned long end) +unsigned long __init_refok init_memory_mapping_ext (unsigned long start, +unsigned long end, +unsigned long tbl_end) {unsigned long start_pfn, end_pfn;unsigned long ret =0 ;
@@ -260,7 +262,7 @@ unsigned long __init_refok init_memory_m* nodes are discovered. */ if (!after_bootmem) -find_early_table_space (end); +find_early_table_space (end, tbl_end);for (i =0 ; i < nr_range; i++) ret = kernel_physical_mapping_init(mr[i].start, mr[i].end,
@@ -284,6 +286,11 @@ unsigned long __init_refok init_memory_mreturn ret >>PAGE_SHIFT ; } +unsigned long __init_refok init_memory_mapping (unsigned long start, +unsigned long end) +{ +return init_memory_mapping_ext (start, end, end); +}/* * devmem_is_allowed() checks to see if /dev/mem access to a certain address
PATCH 3/3 x86,mm,64bit: Round up memory boundary for init_memory_mapping_high()
In reply to Ingo Molnartj pointed out:
when node does not have 1G aligned boundary, like 128M.init_memory_mapping_high() could render smaller mapping by 128M on one node, and 896M on next node with 2M pages instead of 1g page. that could increase TLB presure.
So if gb page is used, try to align the boundary to 1G before calling init_memory_mapping_ext(), to make sure only use one 1g entry for that cross node 1G.
Need to init_meory_mapping_ext() to table tbl_end, to make sure pgtable is on previous node instead of next node.
on one AMD 512g system with not aligned boundary (extra 768M) before the patch
[ 0.000000] init_memory_mapping: [0x00000000000000-0x000000d7f9ffff] [ 0.000000] 0000000000 - 00c0000000 page 1G [ 0.000000] 00c0000000 - 00d7e00000 page 2M [ 0.000000] 00d7e00000 - 00d7fa0000 page 4k [ 0.000000] kernel direct mapping tables up to d7fa0000 @ [0xd7f9d000-0xd7f9ffff] pre-allocated [ 0.000000] kernel direct mapping tables up to d7fa0000 @ [0xd7f9d000-0xd7f9efff] final [ 0.000000] memblock_x86_reserve_range: [0xd7f9d000-0xd7f9efff] PGTABLE ... [ 0.000000] Adding active range (0, 0x10, 0x98) 0 entries of 3200 used [ 0.000000] Adding active range (0, 0x100, 0xd7fa0) 1 entries of 3200 used [ 0.000000] Adding active range (0, 0x100000, 0x1028000) 2 entries of 3200 used [ 0.000000] Adding active range (1, 0x1028000, 0x2028000) 3 entries of 3200 used [ 0.000000] Adding active range (2, 0x2028000, 0x3028000) 4 entries of 3200 used [ 0.000000] Adding active range (3, 0x3028000, 0x4028000) 5 entries of 3200 used [ 0.000000] Adding active range (4, 0x4028000, 0x5028000) 6 entries of 3200 used [ 0.000000] Adding active range (5, 0x5028000, 0x6028000) 7 entries of 3200 used [ 0.000000] Adding active range (6, 0x6028000, 0x7028000) 8 entries of 3200 used [ 0.000000] Adding active range (7, 0x7028000, 0x8028000) 9 entries of 3200 used [ 0.000000] init_memory_mapping: [0x00000100000000-0x00001027ffffff] [ 0.000000] 0100000000 - 1000000000 page 1G [ 0.000000] 1000000000 - 1028000000 page 2M [ 0.000000] kernel direct mapping tables up to 1028000000 @ [0x1027ffe000-0x1027ffffff] pre-allocated [ 0.000000] kernel direct mapping tables up to 1028000000 @ [0x1027ffe000-0x1027ffefff] final [ 0.000000] memblock_x86_reserve_range: [0x1027ffe000-0x1027ffefff] PGTABLE [ 0.000000] init_memory_mapping: [0x00001028000000-0x00002027ffffff] [ 0.000000] 1028000000 - 1040000000 page 2M [ 0.000000] 1040000000 - 2000000000 page 1G [ 0.000000] 2000000000 - 2028000000 page 2M [ 0.000000] kernel direct mapping tables up to 2028000000 @ [0x2027ffe000-0x2027ffffff] pre-allocated [ 0.000000] kernel direct mapping tables up to 2028000000 @ [0x2027ffe000-0x2027ffefff] final [ 0.000000] memblock_x86_reserve_range: [0x2027ffe000-0x2027ffefff] PGTABLE [ 0.000000] init_memory_mapping: [0x00002028000000-0x00003027ffffff] [ 0.000000] 2028000000 - 2040000000 page 2M [ 0.000000] 2040000000 - 3000000000 page 1G [ 0.000000] 3000000000 - 3028000000 page 2M [ 0.000000] kernel direct mapping tables up to 3028000000 @ [0x3027ffe000-0x3027ffffff] pre-allocated [ 0.000000] kernel direct mapping tables up to 3028000000 @ [0x3027ffe000-0x3027ffefff] final [ 0.000000] memblock_x86_reserve_range: [0x3027ffe000-0x3027ffefff] PGTABLE [ 0.000000] init_memory_mapping: [0x00003028000000-0x00004027ffffff] [ 0.000000] 3028000000 - 3040000000 page 2M [ 0.000000] 3040000000 - 4000000000 page 1G [ 0.000000] 4000000000 - 4028000000 page 2M [ 0.000000] kernel direct mapping tables up to 4028000000 @ [0x4027ffe000-0x4027ffffff] pre-allocated [ 0.000000] kernel direct mapping tables up to 4028000000 @ [0x4027ffe000-0x4027ffefff] final [ 0.000000] memblock_x86_reserve_range: [0x4027ffe000-0x4027ffefff] PGTABLE [ 0.000000] init_memory_mapping: [0x00004028000000-0x00005027ffffff] [ 0.000000] 4028000000 - 4040000000 page 2M [ 0.000000] 4040000000 - 5000000000 page 1G [ 0.000000] 5000000000 - 5028000000 page 2M [ 0.000000] kernel direct mapping tables up to 5028000000 @ [0x5027ffe000-0x5027ffffff] pre-allocated [ 0.000000] kernel direct mapping tables up to 5028000000 @ [0x5027ffe000-0x5027ffefff] final [ 0.000000] memblock_x86_reserve_range: [0x5027ffe000-0x5027ffefff] PGTABLE [ 0.000000] init_memory_mapping: [0x00005028000000-0x00006027ffffff] [ 0.000000] 5028000000 - 5040000000 page 2M [ 0.000000] 5040000000 - 6000000000 page 1G [ 0.000000] 6000000000 - 6028000000 page 2M [ 0.000000] kernel direct mapping tables up to 6028000000 @ [0x6027ffe000-0x6027ffffff] pre-allocated [ 0.000000] kernel direct mapping tables up to 6028000000 @ [0x6027ffe000-0x6027ffefff] final [ 0.000000] memblock_x86_reserve_range: [0x6027ffe000-0x6027ffefff] PGTABLE [ 0.000000] init_memory_mapping: [0x00006028000000-0x00007027ffffff] [ 0.000000] 6028000000 - 6040000000 page 2M [ 0.000000] 6040000000 - 7000000000 page 1G [ 0.000000] 7000000000 - 7028000000 page 2M [ 0.000000] kernel direct mapping tables up to 7028000000 @ [0x7027ffe000-0x7027ffffff] pre-allocated [ 0.000000] kernel direct mapping tables up to 7028000000 @ [0x7027ffe000-0x7027ffefff] final [ 0.000000] memblock_x86_reserve_range: [0x7027ffe000-0x7027ffefff] PGTABLE [ 0.000000] init_memory_mapping: [0x00007028000000-0x00008027ffffff] [ 0.000000] 7028000000 - 7040000000 page 2M [ 0.000000] 7040000000 - 8000000000 page 1G [ 0.000000] 8000000000 - 8028000000 page 2M [ 0.000000] kernel direct mapping tables up to 8028000000 @ [0x8027ffd000-0x8027ffffff] pre-allocated [ 0.000000] kernel direct mapping tables up to 8028000000 @ [0x8027ffd000-0x8027ffefff] final [ 0.000000] memblock_x86_reserve_range: [0x8027ffd000-0x8027ffefff] PGTABLE
after patch
...
[ 0.000000] init_memory_mapping: [0x00000100000000-0x0000103fffffff] [ 0.000000] 0100000000 - 1040000000 page 1G [ 0.000000] kernel direct mapping tables up to 1040000000 @ [0x1027fff000-0x1027ffffff] pre-allocated [ 0.000000] init_memory_mapping: [0x00001040000000-0x0000203fffffff] [ 0.000000] 1040000000 - 2040000000 page 1G [ 0.000000] kernel direct mapping tables up to 2040000000 @ [0x2027fff000-0x2027ffffff] pre-allocated [ 0.000000] init_memory_mapping: [0x00002040000000-0x0000303fffffff] [ 0.000000] 2040000000 - 3040000000 page 1G [ 0.000000] kernel direct mapping tables up to 3040000000 @ [0x3027fff000-0x3027ffffff] pre-allocated [ 0.000000] init_memory_mapping: [0x00003040000000-0x0000403fffffff] [ 0.000000] 3040000000 - 4040000000 page 1G [ 0.000000] kernel direct mapping tables up to 4040000000 @ [0x4027fff000-0x4027ffffff] pre-allocated [ 0.000000] init_memory_mapping: [0x00004040000000-0x0000503fffffff] [ 0.000000] 4040000000 - 5040000000 page 1G [ 0.000000] kernel direct mapping tables up to 5040000000 @ [0x5027fff000-0x5027ffffff] pre-allocated [ 0.000000] init_memory_mapping: [0x00005040000000-0x0000603fffffff] [ 0.000000] 5040000000 - 6040000000 page 1G [ 0.000000] kernel direct mapping tables up to 6040000000 @ [0x6027fff000-0x6027ffffff] pre-allocated [ 0.000000] init_memory_mapping: [0x00006040000000-0x0000703fffffff] [ 0.000000] 6040000000 - 7040000000 page 1G [ 0.000000] kernel direct mapping tables up to 7040000000 @ [0x7027fff000-0x7027ffffff] pre-allocated [ 0.000000] init_memory_mapping: [0x00007040000000-0x00008027ffffff] [ 0.000000] 7040000000 - 8000000000 page 1G [ 0.000000] 8000000000 - 8028000000 page 2M [ 0.000000] kernel direct mapping tables up to 8028000000 @ [0x8027ffd000-0x8027ffffff] pre-allocated [ 0.000000] kernel direct mapping tables up to 8028000000 @ [0x8027ffd000-0x8027ffefff] final [ 0.000000] memblock_x86_reserve_range: [0x8027ffd000-0x8027ffefff] PGTABLE
So it fix the extra mapping problem.
-v2: Ingo is not happy with the #ifdef detection etc.
use page_size_mask instead of checking if gbpage is used or not.
--- arch/x86/mm/init_64.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
arch/x86/mm/init_64.c +11 -2
@@ -614,6 +614,7 @@ struct mapping_work_data {unsigned long start;unsigned long end;unsigned long pfn_mapped; +unsigned long align; };static int __init_refok
@@ -621,7 +622,14 @@ mapping_work_fn(unsigned long start_pfn, {struct mapping_work_data *data = datax;unsigned long pfn_mapped; -unsigned long final_start, final_end; +unsigned long final_start, final_end, tbl_end; + + tbl_end = end_pfn <<PAGE_SHIFT ; +/* need to align them to 1G or 2M boundary to avoid smaller mapping */ + start_pfn =round_down (start_pfn, data->align>>PAGE_SHIFT ); +if (start_pfn < data->pfn_mapped) + start_pfn = data->pfn_mapped; + end_pfn =round_up (end_pfn, data->align>>PAGE_SHIFT ); final_start = max_t(unsigned long, start_pfn<<PAGE_SHIFT, data->start); final_end = min_t(unsigned long, end_pfn<<PAGE_SHIFT, data->end);
@@ -629,7 +637,7 @@ mapping_work_fn(unsigned long start_pfn,if (final_end <= final_start)return 0 ; - pfn_mapped =init_memory_mapping (final_start, final_end); + pfn_mapped =init_memory_mapping_ext (final_start, final_end, tbl_end);if (pfn_mapped > data->pfn_mapped) data->pfn_mapped = pfn_mapped;
@@ -645,6 +653,7 @@ init_memory_mapping_active_regions(unsig data.start = start; data.end = end; data.pfn_mapped =0 ; + data.align = (page_size_mask & (1 <<PG_LEVEL_1G )) ?1UL <<30 :1UL <<21 ;work_with_active_regions (MAX_NUMNODES , mapping_work_fn, &data);
Re: [PATCH 3/3] x86,mm,64bit: Round up memory boundary for init_memory_mapping_high()
arch/x86/mm/init_64.c mapping_work_fn(unsigned long start_pfn, · line 632
Quoted 1 line · Yinghai Lu
arch/x86/mm/init_64.c +1
-0+ end_pfn =round_up (end_pfn, data->align>>PAGE_SHIFT );
And now you're mapping beyond max_pfn without even noting the behavior change _anywhere_. What the hell? It's not like this point hasn't been brought up before. It has been mentioned _twice_ in this very thread. Come on.
<rant>
Quoted 1 line
From the previous responses, I suppose you wouldn't care for this
advice but well I'll give it anyway. Spend time and effort documenting the changes and their rationales you make in the comments and changelog. Putting those things in words will force _yourself_ to think about whether the changes are accurate and justified in addition to helping other people understand and review the changes. And down the road, after several years, when someone, even yourself, needs to change the related code again, [s]he will be able to find out and understand how and why things are implemented much more easily.
In the second patch, you added @tbl_end and your explanation was @tbl_end could be shorter than @end. What is that? How is anyone supposed to understand what that means? You needed that change because the code currently depends on memory range to do NUMA affine allocation and when nodes are interleaved the rounding up may end up allocating page table from a different node. If you have put that in words, you would probably have recognized how lame and cryptic that piece of code is and other reviewers would also have much easier time understanding what that is doing and say no. And maybe this is too much to ask but why not add a nice docbook comment while you're adding an extra parameter?
At this point, I really find it difficult to take your patches seriously. They're cryptic, badly documented, and making behavior changes left and right and even when advised you don't even try to describe the changes and rationales.
</rant>
I know you know the code and hardware and have keen eyes for details. _Please_ give it a bit more effort.
For this one, I think I'll just redo the patches and rip out the memblock iteration code. The complexity doesn't really seem justified.
Thank you.
Signature · 1 line
tejun
Re: [PATCH 3/3] x86,mm,64bit: Round up memory boundary for init_memory_mapping_high()
Quoted 7 lines · Tejun Heo
arch/x86/mm/init_64.c mapping_work_fn(unsigned long start_pfn, · line 632
Quoted 1 line · Yinghai Lu
arch/x86/mm/init_64.c +1
-0+ end_pfn =round_up (end_pfn, data->align>>PAGE_SHIFT );And now you're mapping beyond
max_pfnwithout even noting the behavior change _anywhere_. What the hell? It's not like this point hasn't been brought up before. It has been mentioned _twice_ in this very thread. Come on.
Reading the code again, the range is capped by mapping_work_data->start/end, so it won't go over max_pfn. I apologize for the unwarranted rant about this part, but unfortunately the rest of the complaints still stand.
Thanks.
Signature · 1 line
tejun
Re: [PATCH 2/2] x86,mm,64bit: Round up memory boundary for init_memory_mapping_high()
In reply to Yinghai LuQuoted 11 lines · Yinghai Lu
tj pointed out:
when node does not have 1G aligned boundary, like 128M.init_memory_mapping_high()could render smaller mapping by 128M on one node, and 896M on next node with 2M pages instead of 1g page. that could increase TLB presure.So if gb page is used, try to align the boundary to 1G before calling
init_memory_mapping_ext(), to make sure only use one 1g entry for that cross node 1G.
Need toinit_meory_mapping_ext()to tabletbl_end, to make sure pgtable is on previous node instead of next node.
I don't know, Yinghai. The whole code seems overly complicated to me. Just ignore e820 map when building linear mapping. It doesn't matter. Why not just do something like the following? Also, can you please add some comments explaining how the NUMA affine allocation actually works for page tables? Or better, can you please make that explicit? It currently depends on memories being registered in ascending address order, right? The memblock code already is NUMA aware, I think it would be far better to make the node affine part explicit.
Thanks.
arch/x86/kernel/setup.c +5 -0
@@ -966,6 +966,11 @@ void __init setup_arch(char **cmdline_p) memblock.current_limit =get_max_mapped ();/* +* Add whole lot of comment explaining what's going on and WHY +* because as it currently stands, it's frigging cryptic. +*/ + +/* * NOTE: On x86-32, only from this point on, fixmaps are ready for use. */
arch/x86/mm/numa_64.c +5 -3
@@ -536,8 +536,6 @@ static int __init numa_register_memblks(struct numa_meminfo *mi)if (!numa_meminfo_cover_memory (mi))return -EINVAL ; -init_memory_mapping_high (); -/* Finally register nodes. */ for_each_node_mask (nid, node_possible_map) {u64 start = (u64 )max_pfn <<PAGE_SHIFT ;
@@ -550,8 +548,12 @@ static int __init numa_register_memblks(struct numa_meminfo *mi) end =max (mi->blk[i].end, end); } -if (start < end) +if (start < end) { +init_memory_mapping ( +ALIGN_DOWN_TO_MAX_MAP_SIZE_AND_CONVERT_TO_PFN (start), +ALIGN_UP_SIMILARY_BUT_DONT_GO_OVER_MAX_PFN (end));setup_node_bootmem (nid, start, end); + } }return 0 ;
Signature · 1 line
tejun
Re: [PATCH 2/2] x86,mm,64bit: Round up memory boundary for init_memory_mapping_high()
Quoted 13 lines · Tejun Heo
Quoted 11 lines · Yinghai Lu
tj pointed out:
when node does not have 1G aligned boundary, like 128M.init_memory_mapping_high()could render smaller mapping by 128M on one node, and 896M on next node with 2M pages instead of 1g page. that could increase TLB presure.So if gb page is used, try to align the boundary to 1G before calling
init_memory_mapping_ext(), to make sure only use one 1g entry for that cross node 1G.
Need toinit_meory_mapping_ext()to tabletbl_end, to make sure pgtable is on previous node instead of next node.
Quoted 5 lines
I don't know, Yinghai. The whole code seems overly complicated to me. Just ignore e820 map when building linear mapping. It doesn't matter. Why not just do something like the following? Also, can you please add some comments explaining how the NUMA affine allocation actually works for page tables?
yes, that could be done in separated patch.
Quoted 4 lines
Or better, can you please make that explicit?
It currently depends on memories being registered in ascending address order, right? The memblock code already is NUMA aware, I think it would be far better to make the node affine part explicit.
yes, memblock is numa aware after memblock_x86_register_active_regions(). and it rely on early_node_map[].
do you mean let init_memory_mapping to take node id like setup_node_bootmem? so find_early_table_space could take nodeid instead of tbl_end?
Quoted 33 lines
Thanks.
arch/x86/kernel/setup.c +5
-0@@ -966,6 +966,11 @@ void __init setup_arch(char **cmdline_p) memblock.current_limit =get_max_mapped ();/* +* Add whole lot of comment explaining what's going on and WHY +* because as it currently stands, it's frigging cryptic. +*/ + +/* * NOTE: On x86-32, only from this point on, fixmaps are ready for use. */ arch/x86/mm/numa_64.c +0
-2@@ -536,8 +536,6 @@ static int __init numa_register_memblks(struct numa_meminfo *mi)if (!numa_meminfo_cover_memory (mi))return -EINVAL ; -init_memory_mapping_high (); -/* Finally register nodes. */ for_each_node_mask (nid, node_possible_map) {u64 start = (u64 )max_pfn <<PAGE_SHIFT ;
arch/x86/mm/numa_64.c static int __init numa_register_memblks(struct numa_meminfo *mi) · line 556
Quoted 10 lines
arch/x86/mm/numa_64.c +5
-1@@ -550,8 +548,12 @@ static int __init numa_register_memblks(struct numa_meminfo *mi) end =max (mi->blk[i].end, end); } -if (start < end) +if (start < end) { +init_memory_mapping ( +ALIGN_DOWN_TO_MAX_MAP_SIZE_AND_CONVERT_TO_PFN (start), +ALIGN_UP_SIMILARY_BUT_DONT_GO_OVER_MAX_PFN (end));setup_node_bootmem (nid, start, end); + }
will have problem with cross node conf. like 0-4g, 8-12g on node0, 4g-8g, 12g-16g on node1.
arch/x86/mm/numa_64.c static int __init numa_register_memblks(struct numa_meminfo *mi) · line 559
Quoted 5 lines
arch/x86/mm/numa_64.c +0
-0}return 0 ;
Thanks
Yinghai Lu
Re: [PATCH 2/2] x86,mm,64bit: Round up memory boundary for init_memory_mapping_high()
Quoted 10 lines · Yinghai Lu
Quoted 4 lines
Or better, can you please make that explicit?
It currently depends on memories being registered in ascending address order, right? The memblock code already is NUMA aware, I think it would be far better to make the node affine part explicit.yes, memblock is numa aware after
memblock_x86_register_active_regions(). and it rely on early_node_map[].do you mean let
init_memory_mappingto take node id likesetup_node_bootmem? sofind_early_table_spacecould take nodeid instead oftbl_end?
Yeap.
Quoted 4 lines
Quoted 4 lines
arch/x86/mm/numa_64.c +0
-0@@ -550,8 +548,12 @@ static int __init numa_register_memblks(struct numa_meminfo *mi) end =max (mi->blk[i].end, end); }
Quoted 8 lines
arch/x86/mm/numa_64.c static int __init numa_register_memblks(struct numa_meminfo *mi) · line 556
Quoted 7 lines
arch/x86/mm/numa_64.c +5
-1-if (start < end) +if (start < end) { +init_memory_mapping ( +ALIGN_DOWN_TO_MAX_MAP_SIZE_AND_CONVERT_TO_PFN (start), +ALIGN_UP_SIMILARY_BUT_DONT_GO_OVER_MAX_PFN (end));setup_node_bootmem (nid, start, end); + }will have problem with cross node conf. like 0-4g, 8-12g on node0, 4g-8g, 12g-16g on node1.
And how common are they? This whole cruft is basically meaningless if 1GiB mapping is supported, IOW, basically on all AMD 64s and all post-nehalem intels. Why not just cite the limitation in the comment and stick to something simple?
Thanks.
Signature · 1 line
tejun
Re: [PATCH 2/2] x86,mm,64bit: Round up memory boundary for init_memory_mapping_high()
Quoted 6 lines · Tejun Heo
Quoted 2 lines
will have problem with cross node conf. like 0-4g, 8-12g on node0, 4g-8g, 12g-16g on node1.
And how common are they? This whole cruft is basically meaningless if 1GiB mapping is supported, IOW, basically on all AMD 64s and all post-nehalem intels. Why not just cite the limitation in the comment and stick to something simple?
Such complexity should be justified via very careful "perf stat --repeat" measurements.
I.e. showing 'before patch' and 'after patch' instruction, TLB miss and cycle counts, showing that a positive effect that goes beyond the noise of the measurement exists.
1GB mappings should be assumed as the common case - anything else probably does not matter from a future performance/scalability POV. For vmalloc() it might make sense
- but even for those precise measurements should be done about the positive effect.
Thanks,
Ingo
H. Peter Anvin
reviewerQuoted 15 lines · Tejun Heo
Hello, guys.
I've been looking at
init_memory_mapping_high()added by commit1411e0ec31(x86-64, numa: Put pgtable to local node memory) and I got curious about several things.1. The only rationale given in the commit description is that a RED-PEN is killed, which was the following.
/*
* RED-PEN putting page tables only on node 0 could
* cause a hotspot and fill upZONE_DMA. The page tables
* need roughly 0.5KB per GB.
*/
Quoted 9 lines
This already wasn't true with top-down memblock allocation.
The 0.5KB per GiB comment is for 32bit w/ 3 level mapping. On 64bit, it's ~4KiB per GiB when using 2MiB mappings and, well, very small per GiB if 1GiB mapping is used. Even with 2MiB mapping, 1TiB mapping would only be 4MiB. Under
ZONE_DMA, this could be problematic but with top-down this can't be a problem in any realistic way in foreseeable future.
It's true on 64 bits too when PAE is not available (e.g. with Xen.)
-hpa
Hey,
(sorry about the earlier empty reply, fat finger on my phone)
Quoted 19 lines · H. Peter Anvin
Quoted 9 lines
1. The only rationale given in the commit description is that a RED-PEN is killed, which was the following.
/*
* RED-PEN putting page tables only on node 0 could
* cause a hotspot and fill upZONE_DMA. The page tables
* need roughly 0.5KB per GB.
*/Quoted 9 lines
This already wasn't true with top-down memblock allocation.
The 0.5KB per GiB comment is for 32bit w/ 3 level mapping. On 64bit, it's ~4KiB per GiB when using 2MiB mappings and, well, very small per GiB if 1GiB mapping is used. Even with 2MiB mapping, 1TiB mapping would only be 4MiB. Under
ZONE_DMA, this could be problematic but with top-down this can't be a problem in any realistic way in foreseeable future.
Quoted 1 line
It's true on 64 bits too when PAE is not available (e.g. with Xen.)
Hmm... I don't follow. Can you elaborate? If PAE is not available for whatever reason, the physical memory is limited to 4GiB but I don't follow what that has to do with the above.
Thanks.
Signature · 1 line
tejun
H. Peter Anvin
reviewerQuoted 24 lines · Tejun Heo
Hey,
(sorry about the earlier empty reply, fat finger on my phone)
Quoted 19 lines · H. Peter Anvin
Quoted 18 lines
1. The only rationale given in the commit description is that a RED-PEN is killed, which was the following.
/*
* RED-PEN putting page tables only on node 0 could
* cause a hotspot and fill upZONE_DMA. The page tables
* need roughly 0.5KB per GB.
*/This already wasn't true with top-down memblock allocation.
The 0.5KB per GiB comment is for 32bit w/ 3 level mapping. On 64bit, it's ~4KiB per GiB when using 2MiB mappings and, well, very small per GiB if 1GiB mapping is used. Even with 2MiB mapping, 1TiB mapping would only be 4MiB. Under
ZONE_DMA, this could be problematic but with top-down this can't be a problem in any realistic way in foreseeable future.
Quoted 6 lines
Quoted 1 line
It's true on 64 bits too when PAE is not available (e.g. with Xen.)
Hmm... I don't follow. Can you elaborate? If PAE is not available for whatever reason, the physical memory is limited to 4GiB but I don't follow what that has to do with the above.
Sorry, PSE, not PAE.
-hpa