Re: [PATCH 04/10] x86, mm: Don't clear page table if next range is ram

2012-10-09 Thread Yinghai Lu
On Tue, Oct 9, 2012 at 9:04 AM, Konrad Rzeszutek Wilk  wrote:
> How do we clean it wrongly?
>> And it only happens when we are trying to map range one by one range 
>> separately.
>>
>> After we add checking before clearing the related page table, that panic will
>> not happen anymore.
>
> So we do not clean the pages anymore. Is the cleaning of the pages
> addressed somewhere?

that is related calling sequence.

old design assume: we should call that several times under 1G.
--
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/


Re: [PATCH 04/10] x86, mm: Don't clear page table if next range is ram

2012-10-09 Thread Konrad Rzeszutek Wilk
On Mon, Oct 08, 2012 at 09:39:12PM -0700, Yinghai Lu wrote:
> After we add code use BRK to map buffer for final page table,

.. mention the name of the patch that adds this.

What is 'final page table'? Isn't this just the existing
bootup tables modified to cover more memory.
> 
> It should be safe to remove early_memmap for page table accessing.
> 
> But we get panic with that.

Can you rewrite that please. For example it can be:
"With that it should have been safe to remove the call to early_memmap
but sadly it panics. The panic is due to .."
> 
> It turns out we clear the initial page table wrongly for next range that is
> separated by holes.

How do we clean it wrongly?
> And it only happens when we are trying to map range one by one range 
> separately.
> 
> After we add checking before clearing the related page table, that panic will
> not happen anymore.

So we do not clean the pages anymore. Is the cleaning of the pages
addressed somewhere?
> 
> Signed-off-by: Yinghai Lu 
> ---
>  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;
>   }
>  
>   /*
> @@ -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(_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/


Re: [PATCH 04/10] x86, mm: Don't clear page table if next range is ram

2012-10-09 Thread Konrad Rzeszutek Wilk
On Mon, Oct 08, 2012 at 09:39:12PM -0700, Yinghai Lu wrote:
 After we add code use BRK to map buffer for final page table,

.. mention the name of the patch that adds this.

What is 'final page table'? Isn't this just the existing
bootup tables modified to cover more memory.
 
 It should be safe to remove early_memmap for page table accessing.
 
 But we get panic with that.

Can you rewrite that please. For example it can be:
With that it should have been safe to remove the call to early_memmap
but sadly it panics. The panic is due to ..
 
 It turns out we clear the initial page table wrongly for next range that is
 separated by holes.

How do we clean it wrongly?
 And it only happens when we are trying to map range one by one range 
 separately.
 
 After we add checking before clearing the related page table, that panic will
 not happen anymore.

So we do not clean the pages anymore. Is the cleaning of the pages
addressed somewhere?
 
 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;
   }
  
   /*
 @@ -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/


Re: [PATCH 04/10] x86, mm: Don't clear page table if next range is ram

2012-10-09 Thread Yinghai Lu
On Tue, Oct 9, 2012 at 9:04 AM, Konrad Rzeszutek Wilk kon...@kernel.org wrote:
 How do we clean it wrongly?
 And it only happens when we are trying to map range one by one range 
 separately.

 After we add checking before clearing the related page table, that panic will
 not happen anymore.

 So we do not clean the pages anymore. Is the cleaning of the pages
 addressed somewhere?

that is related calling sequence.

old design assume: we should call that several times under 1G.
--
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/