Re: [PATCH 01/19] x86, mm: Align start address to correct big page size

[Konrad Rzeszutek Wilk <konrad@kernel.org>]

>On Thu, Oct 18, 2012 at 01:50:10PM -0700, Yinghai Lu wrote:


I am pretty sure I gave you some ideas of how to fix up the commit
description in earlier reviews, but it looks like you missed them.

Let me write them here once more.

> We are going to use buffer in BRK to pre-map page table buffer.

What buffer? Is buffer the same thing as page table?
> 
> Page table buffer could be only page aligned, but range around it are

.. ranges
> ram too, we could use bigger page to map it to avoid small pages.
> 
> We will adjust page_size_mask in next patch to use big page size for

Instead of saying "next patch" - include the title of the patch
so that one can search for it.

> small ram range.
> 
> Before that, this patch will make start address to be aligned down

s/will make/made/

> according to bigger page size, otherwise entry in page page will
> not have correct value.


I would structure this git commit description to first introduce
the problem.

Say at the start of the patch:

"Before this patch, the start address was aligned down according
to bigger a page size (1GB, 2MB). This is a problem b/c an
entry in the page table will not have correct value. "

Here can you explain why it does not have the correct value?

> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> ---
>  arch/x86/mm/init_32.c |    1 +
>  arch/x86/mm/init_64.c |    5 +++--
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
> index 11a5800..27f7fc6 100644
> --- a/arch/x86/mm/init_32.c
> +++ b/arch/x86/mm/init_32.c
> @@ -310,6 +310,7 @@ repeat:
>  					__pgprot(PTE_IDENT_ATTR |
>  						 _PAGE_PSE);
>  
> +				pfn &= PMD_MASK >> PAGE_SHIFT;
>  				addr2 = (pfn + PTRS_PER_PTE-1) * PAGE_SIZE +
>  					PAGE_OFFSET + PAGE_SIZE-1;
>  
> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
> index ab558eb..f40f383 100644
> --- a/arch/x86/mm/init_64.c
> +++ b/arch/x86/mm/init_64.c
> @@ -461,7 +461,7 @@ phys_pmd_init(pmd_t *pmd_page, unsigned long address, unsigned long end,
>  			pages++;
>  			spin_lock(&init_mm.page_table_lock);
>  			set_pte((pte_t *)pmd,
> -				pfn_pte(address >> PAGE_SHIFT,
> +				pfn_pte((address & PMD_MASK) >> PAGE_SHIFT,
>  					__pgprot(pgprot_val(prot) | _PAGE_PSE)));
>  			spin_unlock(&init_mm.page_table_lock);
>  			last_map_addr = next;
> @@ -536,7 +536,8 @@ phys_pud_init(pud_t *pud_page, unsigned long addr, unsigned long end,
>  			pages++;
>  			spin_lock(&init_mm.page_table_lock);
>  			set_pte((pte_t *)pud,
> -				pfn_pte(addr >> PAGE_SHIFT, PAGE_KERNEL_LARGE));
> +				pfn_pte((addr & PUD_MASK) >> PAGE_SHIFT,
> +					PAGE_KERNEL_LARGE));
>  			spin_unlock(&init_mm.page_table_lock);
>  			last_map_addr = next;
>  			continue;
> -- 
> 1.7.7
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>