x86/e820: break the loop when the region is less then current region
PATCH RESEND 1/1 x86/e820: break the loop when the region is less then current region
e820_all_mapped() iterates the e820 table to check whether a region is mapped or not. Since the e820 table is sorted, when the region is less than the current region, no need to continue the iteration.
The patch breaks the loop accordingly.
--- arch/x86/kernel/e820.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
arch/x86/kernel/e820.c +7 -1
@@ -86,10 +86,16 @@ int __init e820_all_mapped(u64 start, u64 end, unsigned type)for (i =0 ; i < e820->nr_map; i++) {struct e820entry *ei = &e820->map[i]; +/* Since the e820 table is sorted, when the region is less +* than the current region, break it. +*/ +if (ei->addr >= end) +break ; +if (type && ei->type != type)continue ;/* is the region (part) in overlap with the current region ?*/ -if (ei->addr >= end || ei->addr + ei->size <= start) +if (ei->addr + ei->size <= start)continue ;/* if the region is at the beginning of <start,end> we move
Signature · 1 line
2.5.0
Quoted 16 lines · Wei Yang
e820_all_mapped()iterates the e820 table to check whether a region is mapped or not. Since the e820 table is sorted, when the region is less than the current region, no need to continue the iteration.The patch breaks the loop accordingly.
Signed-off-by Wei Yang <[email protected]>--- arch/x86/kernel/e820.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)arch/x86/kernel/e820.c +0
-0
arch/x86/kernel/e820.c int __init e820_all_mapped(u64 start, u64 end, unsigned type) · line 93
Quoted 8 lines
arch/x86/kernel/e820.c +5
-0@@ -86,10 +86,16 @@ int __init e820_all_mapped(u64 start, u64 end, unsigned type)for (i =0 ; i < e820->nr_map; i++) {struct e820entry *ei = &e820->map[i]; +/* Since the e820 table is sorted, when the region is less +* than the current region, break it. +*/ +if (ei->addr >= end) +break ;
Please have a look at the relevant sections in Documentation/CodingStyle. (And yes, this file violates it in a number of ways, but that's no reason to add to the mess.)
But, more importantly, the reason I have not applied the patch before is that while it's true that the e820 map is _eventually_ sorted, have you made certain that all calls to e820_all_mapped() are done when the map is already sorted?
Thanks,
Ingo
Hi, Ingo
Glad to see your comments.
Quoted 22 lines · Ingo Molnar
Quoted 19 lines · Wei Yang
e820_all_mapped()iterates the e820 table to check whether a region is mapped or not. Since the e820 table is sorted, when the region is less than the current region, no need to continue the iteration.The patch breaks the loop accordingly.
Signed-off-by Wei Yang <[email protected]>--- arch/x86/kernel/e820.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)arch/x86/kernel/e820.c +0
-0@@ -86,10 +86,16 @@ int __init e820_all_mapped(u64 start, u64 end, unsigned type)for (i =0 ; i < e820->nr_map; i++) {struct e820entry *ei = &e820->map[i];
Quoted 10 lines
arch/x86/kernel/e820.c int __init e820_all_mapped(u64 start, u64 end, unsigned type) · line 93
Quoted 5 lines
arch/x86/kernel/e820.c +5
-0+/* Since the e820 table is sorted, when the region is less +* than the current region, break it. +*/ +if (ei->addr >= end) +break ;Please have a look at the relevant sections in
Documentation/CodingStyle. (And yes, this file violates it in a number of ways, but that's no reason to add to the mess.)
I took a look in the Documentation/CodingStyle, looks the comment style is not the preferred one. While I have checked this by scripts/checkpatch.pl, which prompts 0 error and 0 warning, so I thought it is fine.
I would change this if you like.
Quoted 4 lines
But, more importantly, the reason I have not applied the patch before is that while it's true that the e820 map is _eventually_ sorted, have you made certain that all calls to
e820_all_mapped()are done when the map is already sorted?
I think so, if not those caller really need to make sure not to do this at that moment.
The e820_all_mapped() can't work well if it is not sorted, even without this change.
For example, we have two ranges [0x1000, 0x1FFF] and [0x2000, 0x2FFF] but they are not sorted in e820. Then a range [0x1500, 0x2500] would be judge not all mapped, which actually is in range [0x1000, 0x2FFF].
BTW, I went through all the callers, and most of them are called from subsys_init() or arch_initcall() which is after setup_memory_map() called in setup_arch().
And two of them are called by e820_add_kernel_range() and efi_reserve_boot_services(), and both are after setup_memory_map().
Quoted 3 lines
Thanks,
Ingo
Signature · 2 lines
Wei Yang Help you, Help me