| Message ID | 1378117829-9095-3-git-send-email-tangchen@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Mon, Sep 2, 2013 at 3:30 AM, Tang Chen <tangchen@cn.fujitsu.com> wrote: > In current kernel, we update min_pfn_mapped and max_pfn_mapped like this: > > init_mem_mapping() > { > while ( a loop iterates all memory ranges ) { > init_range_memory_mapping(); > |->init_memory_mapping() > |->kernel_physical_mapping_init() > |->add_pfn_range_mapped() > |-> update max_pfn_mapped; > > update min_pfn_mapped; > } > } > > max_pfn_mapped is updated in add_pfn_range_mapped() when a range of memory > is mapped. But min_pfn_mapped is updated in init_mem_mapping(). We can also > update min_pfn_mapped in add_pfn_range_mapped(). > > Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com> > --- > arch/x86/mm/init.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c > index 5b2eaca..a97749f 100644 > --- a/arch/x86/mm/init.c > +++ b/arch/x86/mm/init.c > @@ -313,6 +313,7 @@ static void add_pfn_range_mapped(unsigned long start_pfn, unsigned long end_pfn) > nr_pfn_mapped = clean_sort_range(pfn_mapped, E820_X_MAX); > > max_pfn_mapped = max(max_pfn_mapped, end_pfn); > + min_pfn_mapped = min(min_pfn_mapped, start_pfn); > } > > bool pfn_range_is_mapped(unsigned long start_pfn, unsigned long end_pfn) > @@ -442,7 +443,6 @@ void __init init_mem_mapping(void) > new_mapped_ram_size = init_range_memory_mapping(start, > last_start); > last_start = start; > - min_pfn_mapped = last_start >> PAGE_SHIFT; > /* only increase step_size after big range get mapped */ > if (new_mapped_ram_size > mapped_ram_size) > step_size <<= STEP_SIZE_SHIFT; Nak, you can not move that. min_pfn_mapped should not be updated before init_range_memory_mapping is returned. as it need to refer old min_pfn_mapped. and init_range_memory_mapping still init mapping from low to high locally. min_pfn_mapped can not be updated too early. Yinghai -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Yinghai, On 09/03/2013 02:41 AM, Yinghai Lu wrote: ...... > > Nak, you can not move that. > > min_pfn_mapped should not be updated before init_range_memory_mapping > is returned. as it need to refer old min_pfn_mapped. > and init_range_memory_mapping still init mapping from low to high locally. > min_pfn_mapped can not be updated too early. The current code is like this: init_mem_mapping() { while (from high to low) { init_range_memory_mapping() { /* Here is from low to high */ for (from low to high) { init_memory_mapping() { for () { /* Need to refer min_pfn_mapped here */ kernel_physical_mapping_init(); } /* So if updating min_pfn_mapped here, it is too low */ add_pfn_range_mapped(); } } } } } How about change the "for (from low to high)" in init_range_memory_mapping() to "for_rev(from high to low)" ? Then we can update min_pfn_mapped in add_pfn_range_mapped(). And also, the outer loop is from high to low, we can change the inner loop to be from high to low too. I think updating min_pfn_mapped in init_mem_mapping() is less readable. And min_pfn_mapped and max_pfn_mapped should be updated together. Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Sep 2, 2013 at 6:06 PM, Tang Chen <tangchen@cn.fujitsu.com> wrote: > Hi Yinghai, > > On 09/03/2013 02:41 AM, Yinghai Lu wrote: > How about change the "for (from low to high)" in init_range_memory_mapping() > to > "for_rev(from high to low)" ? > Then we can update min_pfn_mapped in add_pfn_range_mapped(). > > And also, the outer loop is from high to low, we can change the inner loop > to be from high > to low too. No. there is other reason for doing local from low to high. kernel_physical_mapping_init() could clear some mapping near the end of PUG/PMD entries but not the head. > > I think updating min_pfn_mapped in init_mem_mapping() is less readable. And > min_pfn_mapped > and max_pfn_mapped should be updated together. min_pfn_mapped is early local variable to control allocation in alloc_low_pages. put it in init_mem_mapping is more readable. Yinghai -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/03/2013 10:48 AM, Yinghai Lu wrote: > On Mon, Sep 2, 2013 at 6:06 PM, Tang Chen<tangchen@cn.fujitsu.com> wrote: >> Hi Yinghai, >> >> On 09/03/2013 02:41 AM, Yinghai Lu wrote: > >> How about change the "for (from low to high)" in init_range_memory_mapping() >> to >> "for_rev(from high to low)" ? >> Then we can update min_pfn_mapped in add_pfn_range_mapped(). >> >> And also, the outer loop is from high to low, we can change the inner loop >> to be from high >> to low too. > > No. there is other reason for doing local from low to high. > > kernel_physical_mapping_init() could clear some mapping near the end > of PUG/PMD entries but not the head. Thanks for your explanation. But sorry, I'd like to understand it more clearly. Are you talking about the following code ? phys_pud_init() { if (addr >= end) { if (!after_bootmem && !e820_any_mapped(addr & PUD_MASK, next, E820_RAM) && !e820_any_mapped(addr & PUD_MASK, next, E820_RESERVED_KERN)) set_pud(pud, __pud(0)); continue; } } It will clear the PUD/PMD out of range. But, init_mem_mapping() { while (from high to low) { init_range_memory_mapping() { for (from low to high) { /* I'm saying changing this loop */ init_memory_mapping() { for () { /* Not this one */ kernel_physical_mapping_init(); } add_pfn_range_mapped(); } } } } } I'm saying changing the outer loop in init_range_memory_mapping(), not the one in init_memory_mapping(). I think it is OK to call init_memory_mapping() with any order. The loop is out of init_memory_mapping(), right ? In init_memory_mapping(), it is still from low to high. But when the kernel_physical_mapping_init() finished, we can update min_pfn_mapped in add_pfn_range_mapped() because the outer loop is from high to low. Am I missing something here ? Please tell me. > >> >> I think updating min_pfn_mapped in init_mem_mapping() is less readable. And >> min_pfn_mapped >> and max_pfn_mapped should be updated together. > > min_pfn_mapped is early local variable to control allocation in alloc_low_pages. > put it in init_mem_mapping is more readable. > But add_pfn_range_mapped() is in the same file with init_mem_mapping(). I think it is OK to update min_pfn_mapped in it. Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Sep 2, 2013 at 10:38 PM, Tang Chen <tangchen@cn.fujitsu.com> wrote: > On 09/03/2013 10:48 AM, Yinghai Lu wrote: >> >> On Mon, Sep 2, 2013 at 6:06 PM, Tang Chen<tangchen@cn.fujitsu.com> wrote: >>> >>> Hi Yinghai, >>> >>> On 09/03/2013 02:41 AM, Yinghai Lu wrote: >> >> >>> How about change the "for (from low to high)" in >>> init_range_memory_mapping() >>> to >>> "for_rev(from high to low)" ? >>> Then we can update min_pfn_mapped in add_pfn_range_mapped(). >>> >>> And also, the outer loop is from high to low, we can change the inner >>> loop >>> to be from high >>> to low too. >> >> >> No. there is other reason for doing local from low to high. >> >> kernel_physical_mapping_init() could clear some mapping near the end >> of PUG/PMD entries but not the head. > > > Thanks for your explanation. But sorry, I'd like to understand it more > clearly. > > Are you talking about the following code ? > phys_pud_init() > { > if (addr >= end) { > if (!after_bootmem && > !e820_any_mapped(addr & PUD_MASK, next, > E820_RAM) && > !e820_any_mapped(addr & PUD_MASK, next, > E820_RESERVED_KERN)) > set_pud(pud, __pud(0)); > continue; > } > } > It will clear the PUD/PMD out of range. > > > But, > > init_mem_mapping() > { > while (from high to low) { > init_range_memory_mapping() > { > for (from low to high) { > /* I'm saying changing this loop */ > init_memory_mapping() > { > for () { > /* Not this one */ > kernel_physical_mapping_init(); > } > add_pfn_range_mapped(); > } > } > } > } > } > > I'm saying changing the outer loop in init_range_memory_mapping(), not the > one in init_memory_mapping(). > I think it is OK to call init_memory_mapping() with any order. The loop is > out of init_memory_mapping(), right ? > > In init_memory_mapping(), it is still from low to high. But when the > kernel_physical_mapping_init() finished, > we can update min_pfn_mapped in add_pfn_range_mapped() because the outer > loop is from high to low. > > Am I missing something here ? Please tell me. Yes, that looks ok, but will make the code more hard to understand, aka more dependency. the only purpose for min_pfn_mapped is for control allocation for alloc_low_pages. so put it's updating in init_mem_mapping is clear and less twisting. also in my patchset that put page table in local node, min_pfn_mapped is replaced by local_min_pfn_mapped per node. Yinghai -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c index 5b2eaca..a97749f 100644 --- a/arch/x86/mm/init.c +++ b/arch/x86/mm/init.c @@ -313,6 +313,7 @@ static void add_pfn_range_mapped(unsigned long start_pfn, unsigned long end_pfn) nr_pfn_mapped = clean_sort_range(pfn_mapped, E820_X_MAX); max_pfn_mapped = max(max_pfn_mapped, end_pfn); + min_pfn_mapped = min(min_pfn_mapped, start_pfn); } bool pfn_range_is_mapped(unsigned long start_pfn, unsigned long end_pfn) @@ -442,7 +443,6 @@ void __init init_mem_mapping(void) new_mapped_ram_size = init_range_memory_mapping(start, last_start); last_start = start; - min_pfn_mapped = last_start >> PAGE_SHIFT; /* only increase step_size after big range get mapped */ if (new_mapped_ram_size > mapped_ram_size) step_size <<= STEP_SIZE_SHIFT;
In current kernel, we update min_pfn_mapped and max_pfn_mapped like this: init_mem_mapping() { while ( a loop iterates all memory ranges ) { init_range_memory_mapping(); |->init_memory_mapping() |->kernel_physical_mapping_init() |->add_pfn_range_mapped() |-> update max_pfn_mapped; update min_pfn_mapped; } } max_pfn_mapped is updated in add_pfn_range_mapped() when a range of memory is mapped. But min_pfn_mapped is updated in init_mem_mapping(). We can also update min_pfn_mapped in add_pfn_range_mapped(). Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com> --- arch/x86/mm/init.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)