On Thu, Oct 18, 2012 at 01:50:14PM -0700, Yinghai Lu wrote:
> After we add code use buffer in BRK to pre-map page table,
                   ^- to

So .. which patch is that? Can you include the title of the
patch here?

> it should be safe to remove early_memmap for page table accessing.
> Instead we get panic with that.
> 
> It turns out we clear the initial page table wrongly for next range that is
              ^- that

> separated by holes.
> And it only happens when we are trying to map range one by one range 
> separately.
                                                     ^-s

> 
> We need to check if the range is ram before clearing page table.

Ok, so that sounds like a bug-fix... but
> 
> Signed-off-by: Yinghai Lu <ying...@kernel.org>
> ---
>  arch/x86/mm/init_64.c |   37 ++++++++++++++++---------------------
>  1 files changed, 16 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
> index f40f383..61b3c44 100644
> --- a/arch/x86/mm/init_64.c
> +++ b/arch/x86/mm/init_64.c
> @@ -363,20 +363,19 @@ static unsigned long __meminit
>  phys_pte_init(pte_t *pte_page, unsigned long addr, unsigned long end,
>             pgprot_t prot)
>  {
> -     unsigned pages = 0;
> +     unsigned long pages = 0, next;
>       unsigned long last_map_addr = end;
>       int i;
>  
>       pte_t *pte = pte_page + pte_index(addr);
>  
> -     for(i = pte_index(addr); i < PTRS_PER_PTE; i++, addr += PAGE_SIZE, 
> pte++) {
> -
> +     for (i = pte_index(addr); i < PTRS_PER_PTE; i++, addr = next, pte++) {
> +             next = (addr & PAGE_MASK) + PAGE_SIZE;
>               if (addr >= end) {
> -                     if (!after_bootmem) {
> -                             for(; i < PTRS_PER_PTE; i++, pte++)
> -                                     set_pte(pte, __pte(0));
> -                     }
> -                     break;
> +                     if (!after_bootmem &&
> +                         !e820_any_mapped(addr & PAGE_MASK, next, 0))
> +                             set_pte(pte, __pte(0));
> +                     continue;

.. Interestingly, you also removed the extra loop. How come? Why not
retain the little loop? (which could call e820_any_mapped?) Is that
an improvement and cleanup? If so, I would think you should at least
explain in the git commit:

"And while we are at it, also axe the extra loop and instead depend on
the top loop which we can safely piggyback on."

>               }
>  
>               /*
> @@ -418,16 +417,14 @@ phys_pmd_init(pmd_t *pmd_page, unsigned long address, 
> unsigned long end,
>               pte_t *pte;
>               pgprot_t new_prot = prot;
>  
> +             next = (address & PMD_MASK) + PMD_SIZE;
>               if (address >= end) {
> -                     if (!after_bootmem) {
> -                             for (; i < PTRS_PER_PMD; i++, pmd++)
> -                                     set_pmd(pmd, __pmd(0));
> -                     }
> -                     break;
> +                     if (!after_bootmem &&
> +                         !e820_any_mapped(address & PMD_MASK, next, 0))
> +                             set_pmd(pmd, __pmd(0));
> +                     continue;
>               }
>  
> -             next = (address & PMD_MASK) + PMD_SIZE;
> -
>               if (pmd_val(*pmd)) {
>                       if (!pmd_large(*pmd)) {
>                               spin_lock(&init_mm.page_table_lock);
> @@ -494,13 +491,11 @@ phys_pud_init(pud_t *pud_page, unsigned long addr, 
> unsigned long end,
>               pmd_t *pmd;
>               pgprot_t prot = PAGE_KERNEL;
>  
> -             if (addr >= end)
> -                     break;
> -
>               next = (addr & PUD_MASK) + PUD_SIZE;
> -
> -             if (!after_bootmem && !e820_any_mapped(addr, next, 0)) {
> -                     set_pud(pud, __pud(0));
> +             if (addr >= end) {
> +                     if (!after_bootmem &&
> +                         !e820_any_mapped(addr & PUD_MASK, next, 0))
> +                             set_pud(pud, __pud(0));
>                       continue;
>               }
>  
> -- 
> 1.7.7
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to