On Mon, Sep 2, 2013 at 10:38 PM, Tang Chen <tangchen@xxxxxxxxxxxxxx> wrote: > On 09/03/2013 10:48 AM, Yinghai Lu wrote: >> >> On Mon, Sep 2, 2013 at 6:06 PM, Tang Chen<tangchen@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html