diff mbox series

[3/3] LoongArch: mm: Add unified function populate_kernel_pte

Message ID 20230712031622.1888321-4-maobibo@loongson.cn (mailing list archive)
State New
Headers show
Series LoongArch: mm: Code cleanup with populate pte | expand

Commit Message

bibo mao July 12, 2023, 3:16 a.m. UTC
Function pcpu_populate_pte and fixmap_pte are similar, they populate
one page from kernel address space. And there is confusion between
pgd and p4d in function fixmap_pte, such as pgd_none always returns
zero. This patch adds unified function populate_kernel_pte and replaces
pcpu_populate_pte and fixmap_pte.

Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 arch/loongarch/include/asm/pgalloc.h |  1 +
 arch/loongarch/kernel/numa.c         | 40 +--------------------
 arch/loongarch/mm/init.c             | 52 ++++++++++++++++------------
 3 files changed, 32 insertions(+), 61 deletions(-)

Comments

Huacai Chen July 31, 2023, 2:15 p.m. UTC | #1
On Wed, Jul 12, 2023 at 11:16 AM Bibo Mao <maobibo@loongson.cn> wrote:
>
> Function pcpu_populate_pte and fixmap_pte are similar, they populate
> one page from kernel address space. And there is confusion between
> pgd and p4d in function fixmap_pte, such as pgd_none always returns
> zero. This patch adds unified function populate_kernel_pte and replaces
> pcpu_populate_pte and fixmap_pte.
>
> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> ---
>  arch/loongarch/include/asm/pgalloc.h |  1 +
>  arch/loongarch/kernel/numa.c         | 40 +--------------------
>  arch/loongarch/mm/init.c             | 52 ++++++++++++++++------------
>  3 files changed, 32 insertions(+), 61 deletions(-)
>
> diff --git a/arch/loongarch/include/asm/pgalloc.h b/arch/loongarch/include/asm/pgalloc.h
> index af1d1e4a6965..ca17b573dba6 100644
> --- a/arch/loongarch/include/asm/pgalloc.h
> +++ b/arch/loongarch/include/asm/pgalloc.h
> @@ -91,4 +91,5 @@ static inline pud_t *pud_alloc_one(struct mm_struct *mm, unsigned long address)
>
>  #endif /* __PAGETABLE_PUD_FOLDED */
>
> +extern pte_t * __init populate_kernel_pte(unsigned long addr);
>  #endif /* _ASM_PGALLOC_H */
> diff --git a/arch/loongarch/kernel/numa.c b/arch/loongarch/kernel/numa.c
> index 778e1c20bfb0..24a693b76873 100644
> --- a/arch/loongarch/kernel/numa.c
> +++ b/arch/loongarch/kernel/numa.c
> @@ -67,46 +67,8 @@ static int __init pcpu_cpu_distance(unsigned int from, unsigned int to)
>
>  void __init pcpu_populate_pte(unsigned long addr)
>  {
> -       pgd_t *pgd = pgd_offset_k(addr);
> -       p4d_t *p4d = p4d_offset(pgd, addr);
> -       pud_t *pud;
> -       pmd_t *pmd;
> -
> -       if (p4d_none(*p4d)) {
> -               pud = memblock_alloc_raw(PAGE_SIZE, PAGE_SIZE);
> -               if (!pud)
> -                       goto err_alloc;
> -               p4d_populate(&init_mm, p4d, pud);
> -#ifndef __PAGETABLE_PUD_FOLDED
> -               pud_init(pud);
> -#endif
> -       }
> -
> -       pud = pud_offset(p4d, addr);
> -       if (pud_none(*pud)) {
> -               pmd = memblock_alloc_raw(PAGE_SIZE, PAGE_SIZE);
> -               if (!pmd)
> -                       goto err_alloc;
> -               pud_populate(&init_mm, pud, pmd);
> -#ifndef __PAGETABLE_PMD_FOLDED
> -               pmd_init(pmd);
> -#endif
> -       }
> -
> -       pmd = pmd_offset(pud, addr);
> -       if (!pmd_present(*pmd)) {
> -               pte_t *pte;
> -
> -               pte = memblock_alloc(PAGE_SIZE, PAGE_SIZE);
> -               if (!pte)
> -                       goto err_alloc;
> -               pmd_populate_kernel(&init_mm, pmd, pte);
> -       }
> -
> +       populate_kernel_pte(addr);
>         return;
> -
> -err_alloc:
> -       panic("%s: Failed to allocate memory\n", __func__);
>  }
>
>  void __init setup_per_cpu_areas(void)
> diff --git a/arch/loongarch/mm/init.c b/arch/loongarch/mm/init.c
> index 3b7d8129570b..6cd2948373ae 100644
> --- a/arch/loongarch/mm/init.c
> +++ b/arch/loongarch/mm/init.c
> @@ -191,46 +191,49 @@ void vmemmap_free(unsigned long start, unsigned long end, struct vmem_altmap *al
>  #endif
>  #endif
>
> -static pte_t *fixmap_pte(unsigned long addr)
> +pte_t * __init populate_kernel_pte(unsigned long addr)
>  {
> -       pgd_t *pgd;
> -       p4d_t *p4d;
> +       pgd_t *pgd = pgd_offset_k(addr);
> +       p4d_t *p4d = p4d_offset(pgd, addr);
>         pud_t *pud;
>         pmd_t *pmd;
>
> -       pgd = pgd_offset_k(addr);
> -       p4d = p4d_offset(pgd, addr);
> -
> -       if (pgd_none(*pgd)) {
> -               pud_t *new __maybe_unused;
> -
> -               new = memblock_alloc_low(PAGE_SIZE, PAGE_SIZE);
> -               pgd_populate(&init_mm, pgd, new);
> +       if (p4d_none(*p4d)) {
> +               pud = memblock_alloc_raw(PAGE_SIZE, PAGE_SIZE);
> +               if (!pud)
> +                       goto err_alloc;
> +               p4d_populate(&init_mm, p4d, pud);
>  #ifndef __PAGETABLE_PUD_FOLDED
> -               pud_init(new);
> +               pud_init(pud);
>  #endif
>         }
>
>         pud = pud_offset(p4d, addr);
>         if (pud_none(*pud)) {
> -               pmd_t *new __maybe_unused;
> -
> -               new = memblock_alloc_low(PAGE_SIZE, PAGE_SIZE);
> -               pud_populate(&init_mm, pud, new);
> +               pmd = memblock_alloc_raw(PAGE_SIZE, PAGE_SIZE);
> +               if (!pmd)
> +                       goto err_alloc;
> +               pud_populate(&init_mm, pud, pmd);
>  #ifndef __PAGETABLE_PMD_FOLDED
> -               pmd_init(new);
> +               pmd_init(pmd);
>  #endif
>         }
>
>         pmd = pmd_offset(pud, addr);
> -       if (pmd_none(*pmd)) {
> -               pte_t *new __maybe_unused;
> +       if (!pmd_present(*pmd)) {
> +               pte_t *pte;
>
> -               new = memblock_alloc_low(PAGE_SIZE, PAGE_SIZE);
> -               pmd_populate_kernel(&init_mm, pmd, new);
> +               pte = memblock_alloc(PAGE_SIZE, PAGE_SIZE);
I don't think memblock_alloc_low() here can be replaced by memblock_alloc().


Huacai
> +               if (!pte)
> +                       goto err_alloc;
> +               pmd_populate_kernel(&init_mm, pmd, pte);
>         }
>
>         return pte_offset_kernel(pmd, addr);
> +
> +err_alloc:
> +       panic("%s: Failed to allocate memory\n", __func__);
> +       return NULL;
>  }
>
>  void __init __set_fixmap(enum fixed_addresses idx,
> @@ -241,7 +244,12 @@ void __init __set_fixmap(enum fixed_addresses idx,
>
>         BUG_ON(idx <= FIX_HOLE || idx >= __end_of_fixed_addresses);
>
> -       ptep = fixmap_pte(addr);
> +       /*
> +        * Now only FIX_EARLYCON_MEM_BASE fixed map is used
> +        * __set_fixmap must be called before mem_init since function
> +        * populate_kernel_pte allocates memory with memblock_alloc method.
> +        */
> +       ptep = populate_kernel_pte(addr);
>         if (!pte_none(*ptep)) {
>                 pte_ERROR(*ptep);
>                 return;
> --
> 2.27.0
>
bibo mao Aug. 1, 2023, 1:22 a.m. UTC | #2
在 2023/7/31 22:15, Huacai Chen 写道:
> On Wed, Jul 12, 2023 at 11:16 AM Bibo Mao <maobibo@loongson.cn> wrote:
>>
>> Function pcpu_populate_pte and fixmap_pte are similar, they populate
>> one page from kernel address space. And there is confusion between
>> pgd and p4d in function fixmap_pte, such as pgd_none always returns
>> zero. This patch adds unified function populate_kernel_pte and replaces
>> pcpu_populate_pte and fixmap_pte.
>>
>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>> ---
>>  arch/loongarch/include/asm/pgalloc.h |  1 +
>>  arch/loongarch/kernel/numa.c         | 40 +--------------------
>>  arch/loongarch/mm/init.c             | 52 ++++++++++++++++------------
>>  3 files changed, 32 insertions(+), 61 deletions(-)
>>
>> diff --git a/arch/loongarch/include/asm/pgalloc.h b/arch/loongarch/include/asm/pgalloc.h
>> index af1d1e4a6965..ca17b573dba6 100644
>> --- a/arch/loongarch/include/asm/pgalloc.h
>> +++ b/arch/loongarch/include/asm/pgalloc.h
>> @@ -91,4 +91,5 @@ static inline pud_t *pud_alloc_one(struct mm_struct *mm, unsigned long address)
>>
>>  #endif /* __PAGETABLE_PUD_FOLDED */
>>
>> +extern pte_t * __init populate_kernel_pte(unsigned long addr);
>>  #endif /* _ASM_PGALLOC_H */
>> diff --git a/arch/loongarch/kernel/numa.c b/arch/loongarch/kernel/numa.c
>> index 778e1c20bfb0..24a693b76873 100644
>> --- a/arch/loongarch/kernel/numa.c
>> +++ b/arch/loongarch/kernel/numa.c
>> @@ -67,46 +67,8 @@ static int __init pcpu_cpu_distance(unsigned int from, unsigned int to)
>>
>>  void __init pcpu_populate_pte(unsigned long addr)
>>  {
>> -       pgd_t *pgd = pgd_offset_k(addr);
>> -       p4d_t *p4d = p4d_offset(pgd, addr);
>> -       pud_t *pud;
>> -       pmd_t *pmd;
>> -
>> -       if (p4d_none(*p4d)) {
>> -               pud = memblock_alloc_raw(PAGE_SIZE, PAGE_SIZE);
>> -               if (!pud)
>> -                       goto err_alloc;
>> -               p4d_populate(&init_mm, p4d, pud);
>> -#ifndef __PAGETABLE_PUD_FOLDED
>> -               pud_init(pud);
>> -#endif
>> -       }
>> -
>> -       pud = pud_offset(p4d, addr);
>> -       if (pud_none(*pud)) {
>> -               pmd = memblock_alloc_raw(PAGE_SIZE, PAGE_SIZE);
>> -               if (!pmd)
>> -                       goto err_alloc;
>> -               pud_populate(&init_mm, pud, pmd);
>> -#ifndef __PAGETABLE_PMD_FOLDED
>> -               pmd_init(pmd);
>> -#endif
>> -       }
>> -
>> -       pmd = pmd_offset(pud, addr);
>> -       if (!pmd_present(*pmd)) {
>> -               pte_t *pte;
>> -
>> -               pte = memblock_alloc(PAGE_SIZE, PAGE_SIZE);
>> -               if (!pte)
>> -                       goto err_alloc;
>> -               pmd_populate_kernel(&init_mm, pmd, pte);
>> -       }
>> -
>> +       populate_kernel_pte(addr);
>>         return;
>> -
>> -err_alloc:
>> -       panic("%s: Failed to allocate memory\n", __func__);
>>  }
>>
>>  void __init setup_per_cpu_areas(void)
>> diff --git a/arch/loongarch/mm/init.c b/arch/loongarch/mm/init.c
>> index 3b7d8129570b..6cd2948373ae 100644
>> --- a/arch/loongarch/mm/init.c
>> +++ b/arch/loongarch/mm/init.c
>> @@ -191,46 +191,49 @@ void vmemmap_free(unsigned long start, unsigned long end, struct vmem_altmap *al
>>  #endif
>>  #endif
>>
>> -static pte_t *fixmap_pte(unsigned long addr)
>> +pte_t * __init populate_kernel_pte(unsigned long addr)
>>  {
>> -       pgd_t *pgd;
>> -       p4d_t *p4d;
>> +       pgd_t *pgd = pgd_offset_k(addr);
>> +       p4d_t *p4d = p4d_offset(pgd, addr);
>>         pud_t *pud;
>>         pmd_t *pmd;
>>
>> -       pgd = pgd_offset_k(addr);
>> -       p4d = p4d_offset(pgd, addr);
>> -
>> -       if (pgd_none(*pgd)) {
>> -               pud_t *new __maybe_unused;
>> -
>> -               new = memblock_alloc_low(PAGE_SIZE, PAGE_SIZE);
>> -               pgd_populate(&init_mm, pgd, new);
>> +       if (p4d_none(*p4d)) {
>> +               pud = memblock_alloc_raw(PAGE_SIZE, PAGE_SIZE);
>> +               if (!pud)
>> +                       goto err_alloc;
>> +               p4d_populate(&init_mm, p4d, pud);
>>  #ifndef __PAGETABLE_PUD_FOLDED
>> -               pud_init(new);
>> +               pud_init(pud);
>>  #endif
>>         }
>>
>>         pud = pud_offset(p4d, addr);
>>         if (pud_none(*pud)) {
>> -               pmd_t *new __maybe_unused;
>> -
>> -               new = memblock_alloc_low(PAGE_SIZE, PAGE_SIZE);
>> -               pud_populate(&init_mm, pud, new);
>> +               pmd = memblock_alloc_raw(PAGE_SIZE, PAGE_SIZE);
>> +               if (!pmd)
>> +                       goto err_alloc;
>> +               pud_populate(&init_mm, pud, pmd);
>>  #ifndef __PAGETABLE_PMD_FOLDED
>> -               pmd_init(new);
>> +               pmd_init(pmd);
>>  #endif
>>         }
>>
>>         pmd = pmd_offset(pud, addr);
>> -       if (pmd_none(*pmd)) {
>> -               pte_t *new __maybe_unused;
>> +       if (!pmd_present(*pmd)) {
>> +               pte_t *pte;
>>
>> -               new = memblock_alloc_low(PAGE_SIZE, PAGE_SIZE);
>> -               pmd_populate_kernel(&init_mm, pmd, new);
>> +               pte = memblock_alloc(PAGE_SIZE, PAGE_SIZE);
> I don't think memblock_alloc_low() here can be replaced by memblock_alloc().
Can you share me the points that pte table must be allocated with memblock_alloc_low
in this place?

Regards
Bibo Mao
> 
> 
> Huacai
>> +               if (!pte)
>> +                       goto err_alloc;
>> +               pmd_populate_kernel(&init_mm, pmd, pte);
>>         }
>>
>>         return pte_offset_kernel(pmd, addr);
>> +
>> +err_alloc:
>> +       panic("%s: Failed to allocate memory\n", __func__);
>> +       return NULL;
>>  }
>>
>>  void __init __set_fixmap(enum fixed_addresses idx,
>> @@ -241,7 +244,12 @@ void __init __set_fixmap(enum fixed_addresses idx,
>>
>>         BUG_ON(idx <= FIX_HOLE || idx >= __end_of_fixed_addresses);
>>
>> -       ptep = fixmap_pte(addr);
>> +       /*
>> +        * Now only FIX_EARLYCON_MEM_BASE fixed map is used
>> +        * __set_fixmap must be called before mem_init since function
>> +        * populate_kernel_pte allocates memory with memblock_alloc method.
>> +        */
>> +       ptep = populate_kernel_pte(addr);
>>         if (!pte_none(*ptep)) {
>>                 pte_ERROR(*ptep);
>>                 return;
>> --
>> 2.27.0
>>
Huacai Chen Aug. 2, 2023, 7:25 a.m. UTC | #3
On Tue, Aug 1, 2023 at 9:22 AM bibo mao <maobibo@loongson.cn> wrote:
>
>
>
> 在 2023/7/31 22:15, Huacai Chen 写道:
> > On Wed, Jul 12, 2023 at 11:16 AM Bibo Mao <maobibo@loongson.cn> wrote:
> >>
> >> Function pcpu_populate_pte and fixmap_pte are similar, they populate
> >> one page from kernel address space. And there is confusion between
> >> pgd and p4d in function fixmap_pte, such as pgd_none always returns
> >> zero. This patch adds unified function populate_kernel_pte and replaces
> >> pcpu_populate_pte and fixmap_pte.
> >>
> >> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> >> ---
> >>  arch/loongarch/include/asm/pgalloc.h |  1 +
> >>  arch/loongarch/kernel/numa.c         | 40 +--------------------
> >>  arch/loongarch/mm/init.c             | 52 ++++++++++++++++------------
> >>  3 files changed, 32 insertions(+), 61 deletions(-)
> >>
> >> diff --git a/arch/loongarch/include/asm/pgalloc.h b/arch/loongarch/include/asm/pgalloc.h
> >> index af1d1e4a6965..ca17b573dba6 100644
> >> --- a/arch/loongarch/include/asm/pgalloc.h
> >> +++ b/arch/loongarch/include/asm/pgalloc.h
> >> @@ -91,4 +91,5 @@ static inline pud_t *pud_alloc_one(struct mm_struct *mm, unsigned long address)
> >>
> >>  #endif /* __PAGETABLE_PUD_FOLDED */
> >>
> >> +extern pte_t * __init populate_kernel_pte(unsigned long addr);
> >>  #endif /* _ASM_PGALLOC_H */
> >> diff --git a/arch/loongarch/kernel/numa.c b/arch/loongarch/kernel/numa.c
> >> index 778e1c20bfb0..24a693b76873 100644
> >> --- a/arch/loongarch/kernel/numa.c
> >> +++ b/arch/loongarch/kernel/numa.c
> >> @@ -67,46 +67,8 @@ static int __init pcpu_cpu_distance(unsigned int from, unsigned int to)
> >>
> >>  void __init pcpu_populate_pte(unsigned long addr)
> >>  {
> >> -       pgd_t *pgd = pgd_offset_k(addr);
> >> -       p4d_t *p4d = p4d_offset(pgd, addr);
> >> -       pud_t *pud;
> >> -       pmd_t *pmd;
> >> -
> >> -       if (p4d_none(*p4d)) {
> >> -               pud = memblock_alloc_raw(PAGE_SIZE, PAGE_SIZE);
> >> -               if (!pud)
> >> -                       goto err_alloc;
> >> -               p4d_populate(&init_mm, p4d, pud);
> >> -#ifndef __PAGETABLE_PUD_FOLDED
> >> -               pud_init(pud);
> >> -#endif
> >> -       }
> >> -
> >> -       pud = pud_offset(p4d, addr);
> >> -       if (pud_none(*pud)) {
> >> -               pmd = memblock_alloc_raw(PAGE_SIZE, PAGE_SIZE);
> >> -               if (!pmd)
> >> -                       goto err_alloc;
> >> -               pud_populate(&init_mm, pud, pmd);
> >> -#ifndef __PAGETABLE_PMD_FOLDED
> >> -               pmd_init(pmd);
> >> -#endif
> >> -       }
> >> -
> >> -       pmd = pmd_offset(pud, addr);
> >> -       if (!pmd_present(*pmd)) {
> >> -               pte_t *pte;
> >> -
> >> -               pte = memblock_alloc(PAGE_SIZE, PAGE_SIZE);
> >> -               if (!pte)
> >> -                       goto err_alloc;
> >> -               pmd_populate_kernel(&init_mm, pmd, pte);
> >> -       }
> >> -
> >> +       populate_kernel_pte(addr);
> >>         return;
> >> -
> >> -err_alloc:
> >> -       panic("%s: Failed to allocate memory\n", __func__);
> >>  }
> >>
> >>  void __init setup_per_cpu_areas(void)
> >> diff --git a/arch/loongarch/mm/init.c b/arch/loongarch/mm/init.c
> >> index 3b7d8129570b..6cd2948373ae 100644
> >> --- a/arch/loongarch/mm/init.c
> >> +++ b/arch/loongarch/mm/init.c
> >> @@ -191,46 +191,49 @@ void vmemmap_free(unsigned long start, unsigned long end, struct vmem_altmap *al
> >>  #endif
> >>  #endif
> >>
> >> -static pte_t *fixmap_pte(unsigned long addr)
> >> +pte_t * __init populate_kernel_pte(unsigned long addr)
> >>  {
> >> -       pgd_t *pgd;
> >> -       p4d_t *p4d;
> >> +       pgd_t *pgd = pgd_offset_k(addr);
> >> +       p4d_t *p4d = p4d_offset(pgd, addr);
> >>         pud_t *pud;
> >>         pmd_t *pmd;
> >>
> >> -       pgd = pgd_offset_k(addr);
> >> -       p4d = p4d_offset(pgd, addr);
> >> -
> >> -       if (pgd_none(*pgd)) {
> >> -               pud_t *new __maybe_unused;
> >> -
> >> -               new = memblock_alloc_low(PAGE_SIZE, PAGE_SIZE);
> >> -               pgd_populate(&init_mm, pgd, new);
> >> +       if (p4d_none(*p4d)) {
> >> +               pud = memblock_alloc_raw(PAGE_SIZE, PAGE_SIZE);
> >> +               if (!pud)
> >> +                       goto err_alloc;
> >> +               p4d_populate(&init_mm, p4d, pud);
> >>  #ifndef __PAGETABLE_PUD_FOLDED
> >> -               pud_init(new);
> >> +               pud_init(pud);
> >>  #endif
> >>         }
> >>
> >>         pud = pud_offset(p4d, addr);
> >>         if (pud_none(*pud)) {
> >> -               pmd_t *new __maybe_unused;
> >> -
> >> -               new = memblock_alloc_low(PAGE_SIZE, PAGE_SIZE);
> >> -               pud_populate(&init_mm, pud, new);
> >> +               pmd = memblock_alloc_raw(PAGE_SIZE, PAGE_SIZE);
> >> +               if (!pmd)
> >> +                       goto err_alloc;
> >> +               pud_populate(&init_mm, pud, pmd);
> >>  #ifndef __PAGETABLE_PMD_FOLDED
> >> -               pmd_init(new);
> >> +               pmd_init(pmd);
> >>  #endif
> >>         }
> >>
> >>         pmd = pmd_offset(pud, addr);
> >> -       if (pmd_none(*pmd)) {
> >> -               pte_t *new __maybe_unused;
> >> +       if (!pmd_present(*pmd)) {
> >> +               pte_t *pte;
> >>
> >> -               new = memblock_alloc_low(PAGE_SIZE, PAGE_SIZE);
> >> -               pmd_populate_kernel(&init_mm, pmd, new);
> >> +               pte = memblock_alloc(PAGE_SIZE, PAGE_SIZE);
> > I don't think memblock_alloc_low() here can be replaced by memblock_alloc().
> Can you share me the points that pte table must be allocated with memblock_alloc_low
> in this place?
I forget the reason now, so if you confirm memblock_alloc() works well
here, you can use it. But please don't use memblock_alloc_raw().

Huacai
>
> Regards
> Bibo Mao
> >
> >
> > Huacai
> >> +               if (!pte)
> >> +                       goto err_alloc;
> >> +               pmd_populate_kernel(&init_mm, pmd, pte);
> >>         }
> >>
> >>         return pte_offset_kernel(pmd, addr);
> >> +
> >> +err_alloc:
> >> +       panic("%s: Failed to allocate memory\n", __func__);
> >> +       return NULL;
> >>  }
> >>
> >>  void __init __set_fixmap(enum fixed_addresses idx,
> >> @@ -241,7 +244,12 @@ void __init __set_fixmap(enum fixed_addresses idx,
> >>
> >>         BUG_ON(idx <= FIX_HOLE || idx >= __end_of_fixed_addresses);
> >>
> >> -       ptep = fixmap_pte(addr);
> >> +       /*
> >> +        * Now only FIX_EARLYCON_MEM_BASE fixed map is used
> >> +        * __set_fixmap must be called before mem_init since function
> >> +        * populate_kernel_pte allocates memory with memblock_alloc method.
> >> +        */
> >> +       ptep = populate_kernel_pte(addr);
> >>         if (!pte_none(*ptep)) {
> >>                 pte_ERROR(*ptep);
> >>                 return;
> >> --
> >> 2.27.0
> >>
>
>
bibo mao Aug. 10, 2023, 4:08 a.m. UTC | #4
在 2023/8/2 15:25, Huacai Chen 写道:
> On Tue, Aug 1, 2023 at 9:22 AM bibo mao <maobibo@loongson.cn> wrote:
>>
>>
>>
>> 在 2023/7/31 22:15, Huacai Chen 写道:
>>> On Wed, Jul 12, 2023 at 11:16 AM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>
>>>> Function pcpu_populate_pte and fixmap_pte are similar, they populate
>>>> one page from kernel address space. And there is confusion between
>>>> pgd and p4d in function fixmap_pte, such as pgd_none always returns
>>>> zero. This patch adds unified function populate_kernel_pte and replaces
>>>> pcpu_populate_pte and fixmap_pte.
>>>>
>>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>>> ---
>>>>  arch/loongarch/include/asm/pgalloc.h |  1 +
>>>>  arch/loongarch/kernel/numa.c         | 40 +--------------------
>>>>  arch/loongarch/mm/init.c             | 52 ++++++++++++++++------------
>>>>  3 files changed, 32 insertions(+), 61 deletions(-)
>>>>
>>>> diff --git a/arch/loongarch/include/asm/pgalloc.h b/arch/loongarch/include/asm/pgalloc.h
>>>> index af1d1e4a6965..ca17b573dba6 100644
>>>> --- a/arch/loongarch/include/asm/pgalloc.h
>>>> +++ b/arch/loongarch/include/asm/pgalloc.h
>>>> @@ -91,4 +91,5 @@ static inline pud_t *pud_alloc_one(struct mm_struct *mm, unsigned long address)
>>>>
>>>>  #endif /* __PAGETABLE_PUD_FOLDED */
>>>>
>>>> +extern pte_t * __init populate_kernel_pte(unsigned long addr);
>>>>  #endif /* _ASM_PGALLOC_H */
>>>> diff --git a/arch/loongarch/kernel/numa.c b/arch/loongarch/kernel/numa.c
>>>> index 778e1c20bfb0..24a693b76873 100644
>>>> --- a/arch/loongarch/kernel/numa.c
>>>> +++ b/arch/loongarch/kernel/numa.c
>>>> @@ -67,46 +67,8 @@ static int __init pcpu_cpu_distance(unsigned int from, unsigned int to)
>>>>
>>>>  void __init pcpu_populate_pte(unsigned long addr)
>>>>  {
>>>> -       pgd_t *pgd = pgd_offset_k(addr);
>>>> -       p4d_t *p4d = p4d_offset(pgd, addr);
>>>> -       pud_t *pud;
>>>> -       pmd_t *pmd;
>>>> -
>>>> -       if (p4d_none(*p4d)) {
>>>> -               pud = memblock_alloc_raw(PAGE_SIZE, PAGE_SIZE);
>>>> -               if (!pud)
>>>> -                       goto err_alloc;
>>>> -               p4d_populate(&init_mm, p4d, pud);
>>>> -#ifndef __PAGETABLE_PUD_FOLDED
>>>> -               pud_init(pud);
>>>> -#endif
>>>> -       }
>>>> -
>>>> -       pud = pud_offset(p4d, addr);
>>>> -       if (pud_none(*pud)) {
>>>> -               pmd = memblock_alloc_raw(PAGE_SIZE, PAGE_SIZE);
>>>> -               if (!pmd)
>>>> -                       goto err_alloc;
>>>> -               pud_populate(&init_mm, pud, pmd);
>>>> -#ifndef __PAGETABLE_PMD_FOLDED
>>>> -               pmd_init(pmd);
>>>> -#endif
>>>> -       }
>>>> -
>>>> -       pmd = pmd_offset(pud, addr);
>>>> -       if (!pmd_present(*pmd)) {
>>>> -               pte_t *pte;
>>>> -
>>>> -               pte = memblock_alloc(PAGE_SIZE, PAGE_SIZE);
>>>> -               if (!pte)
>>>> -                       goto err_alloc;
>>>> -               pmd_populate_kernel(&init_mm, pmd, pte);
>>>> -       }
>>>> -
>>>> +       populate_kernel_pte(addr);
>>>>         return;
>>>> -
>>>> -err_alloc:
>>>> -       panic("%s: Failed to allocate memory\n", __func__);
>>>>  }
>>>>
>>>>  void __init setup_per_cpu_areas(void)
>>>> diff --git a/arch/loongarch/mm/init.c b/arch/loongarch/mm/init.c
>>>> index 3b7d8129570b..6cd2948373ae 100644
>>>> --- a/arch/loongarch/mm/init.c
>>>> +++ b/arch/loongarch/mm/init.c
>>>> @@ -191,46 +191,49 @@ void vmemmap_free(unsigned long start, unsigned long end, struct vmem_altmap *al
>>>>  #endif
>>>>  #endif
>>>>
>>>> -static pte_t *fixmap_pte(unsigned long addr)
>>>> +pte_t * __init populate_kernel_pte(unsigned long addr)
>>>>  {
>>>> -       pgd_t *pgd;
>>>> -       p4d_t *p4d;
>>>> +       pgd_t *pgd = pgd_offset_k(addr);
>>>> +       p4d_t *p4d = p4d_offset(pgd, addr);
>>>>         pud_t *pud;
>>>>         pmd_t *pmd;
>>>>
>>>> -       pgd = pgd_offset_k(addr);
>>>> -       p4d = p4d_offset(pgd, addr);
>>>> -
>>>> -       if (pgd_none(*pgd)) {
>>>> -               pud_t *new __maybe_unused;
>>>> -
>>>> -               new = memblock_alloc_low(PAGE_SIZE, PAGE_SIZE);
>>>> -               pgd_populate(&init_mm, pgd, new);
>>>> +       if (p4d_none(*p4d)) {
>>>> +               pud = memblock_alloc_raw(PAGE_SIZE, PAGE_SIZE);
>>>> +               if (!pud)
>>>> +                       goto err_alloc;
>>>> +               p4d_populate(&init_mm, p4d, pud);
>>>>  #ifndef __PAGETABLE_PUD_FOLDED
>>>> -               pud_init(new);
>>>> +               pud_init(pud);
>>>>  #endif
>>>>         }
>>>>
>>>>         pud = pud_offset(p4d, addr);
>>>>         if (pud_none(*pud)) {
>>>> -               pmd_t *new __maybe_unused;
>>>> -
>>>> -               new = memblock_alloc_low(PAGE_SIZE, PAGE_SIZE);
>>>> -               pud_populate(&init_mm, pud, new);
>>>> +               pmd = memblock_alloc_raw(PAGE_SIZE, PAGE_SIZE);
>>>> +               if (!pmd)
>>>> +                       goto err_alloc;
>>>> +               pud_populate(&init_mm, pud, pmd);
>>>>  #ifndef __PAGETABLE_PMD_FOLDED
>>>> -               pmd_init(new);
>>>> +               pmd_init(pmd);
>>>>  #endif
>>>>         }
>>>>
>>>>         pmd = pmd_offset(pud, addr);
>>>> -       if (pmd_none(*pmd)) {
>>>> -               pte_t *new __maybe_unused;
>>>> +       if (!pmd_present(*pmd)) {
>>>> +               pte_t *pte;
>>>>
>>>> -               new = memblock_alloc_low(PAGE_SIZE, PAGE_SIZE);
>>>> -               pmd_populate_kernel(&init_mm, pmd, new);
>>>> +               pte = memblock_alloc(PAGE_SIZE, PAGE_SIZE);
>>> I don't think memblock_alloc_low() here can be replaced by memblock_alloc().
>> Can you share me the points that pte table must be allocated with memblock_alloc_low
>> in this place?
> I forget the reason now, so if you confirm memblock_alloc() works well
> here, you can use it. But please don't use memblock_alloc_raw().
what a mess, there is more comments if there is special reason, else everyone can
forgot by elapsed time.

why the function memblock_alloc_raw can not be use? there is one useless page copy.

Regards
Bibo Mao


> 
> Huacai
>>
>> Regards
>> Bibo Mao
>>>
>>>
>>> Huacai
>>>> +               if (!pte)
>>>> +                       goto err_alloc;
>>>> +               pmd_populate_kernel(&init_mm, pmd, pte);
>>>>         }
>>>>
>>>>         return pte_offset_kernel(pmd, addr);
>>>> +
>>>> +err_alloc:
>>>> +       panic("%s: Failed to allocate memory\n", __func__);
>>>> +       return NULL;
>>>>  }
>>>>
>>>>  void __init __set_fixmap(enum fixed_addresses idx,
>>>> @@ -241,7 +244,12 @@ void __init __set_fixmap(enum fixed_addresses idx,
>>>>
>>>>         BUG_ON(idx <= FIX_HOLE || idx >= __end_of_fixed_addresses);
>>>>
>>>> -       ptep = fixmap_pte(addr);
>>>> +       /*
>>>> +        * Now only FIX_EARLYCON_MEM_BASE fixed map is used
>>>> +        * __set_fixmap must be called before mem_init since function
>>>> +        * populate_kernel_pte allocates memory with memblock_alloc method.
>>>> +        */
>>>> +       ptep = populate_kernel_pte(addr);
>>>>         if (!pte_none(*ptep)) {
>>>>                 pte_ERROR(*ptep);
>>>>                 return;
>>>> --
>>>> 2.27.0
>>>>
>>
>>
Huacai Chen Aug. 10, 2023, 4:27 a.m. UTC | #5
On Thu, Aug 10, 2023 at 12:09 PM bibo mao <maobibo@loongson.cn> wrote:
>
>
>
> 在 2023/8/2 15:25, Huacai Chen 写道:
> > On Tue, Aug 1, 2023 at 9:22 AM bibo mao <maobibo@loongson.cn> wrote:
> >>
> >>
> >>
> >> 在 2023/7/31 22:15, Huacai Chen 写道:
> >>> On Wed, Jul 12, 2023 at 11:16 AM Bibo Mao <maobibo@loongson.cn> wrote:
> >>>>
> >>>> Function pcpu_populate_pte and fixmap_pte are similar, they populate
> >>>> one page from kernel address space. And there is confusion between
> >>>> pgd and p4d in function fixmap_pte, such as pgd_none always returns
> >>>> zero. This patch adds unified function populate_kernel_pte and replaces
> >>>> pcpu_populate_pte and fixmap_pte.
> >>>>
> >>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> >>>> ---
> >>>>  arch/loongarch/include/asm/pgalloc.h |  1 +
> >>>>  arch/loongarch/kernel/numa.c         | 40 +--------------------
> >>>>  arch/loongarch/mm/init.c             | 52 ++++++++++++++++------------
> >>>>  3 files changed, 32 insertions(+), 61 deletions(-)
> >>>>
> >>>> diff --git a/arch/loongarch/include/asm/pgalloc.h b/arch/loongarch/include/asm/pgalloc.h
> >>>> index af1d1e4a6965..ca17b573dba6 100644
> >>>> --- a/arch/loongarch/include/asm/pgalloc.h
> >>>> +++ b/arch/loongarch/include/asm/pgalloc.h
> >>>> @@ -91,4 +91,5 @@ static inline pud_t *pud_alloc_one(struct mm_struct *mm, unsigned long address)
> >>>>
> >>>>  #endif /* __PAGETABLE_PUD_FOLDED */
> >>>>
> >>>> +extern pte_t * __init populate_kernel_pte(unsigned long addr);
> >>>>  #endif /* _ASM_PGALLOC_H */
> >>>> diff --git a/arch/loongarch/kernel/numa.c b/arch/loongarch/kernel/numa.c
> >>>> index 778e1c20bfb0..24a693b76873 100644
> >>>> --- a/arch/loongarch/kernel/numa.c
> >>>> +++ b/arch/loongarch/kernel/numa.c
> >>>> @@ -67,46 +67,8 @@ static int __init pcpu_cpu_distance(unsigned int from, unsigned int to)
> >>>>
> >>>>  void __init pcpu_populate_pte(unsigned long addr)
> >>>>  {
> >>>> -       pgd_t *pgd = pgd_offset_k(addr);
> >>>> -       p4d_t *p4d = p4d_offset(pgd, addr);
> >>>> -       pud_t *pud;
> >>>> -       pmd_t *pmd;
> >>>> -
> >>>> -       if (p4d_none(*p4d)) {
> >>>> -               pud = memblock_alloc_raw(PAGE_SIZE, PAGE_SIZE);
> >>>> -               if (!pud)
> >>>> -                       goto err_alloc;
> >>>> -               p4d_populate(&init_mm, p4d, pud);
> >>>> -#ifndef __PAGETABLE_PUD_FOLDED
> >>>> -               pud_init(pud);
> >>>> -#endif
> >>>> -       }
> >>>> -
> >>>> -       pud = pud_offset(p4d, addr);
> >>>> -       if (pud_none(*pud)) {
> >>>> -               pmd = memblock_alloc_raw(PAGE_SIZE, PAGE_SIZE);
> >>>> -               if (!pmd)
> >>>> -                       goto err_alloc;
> >>>> -               pud_populate(&init_mm, pud, pmd);
> >>>> -#ifndef __PAGETABLE_PMD_FOLDED
> >>>> -               pmd_init(pmd);
> >>>> -#endif
> >>>> -       }
> >>>> -
> >>>> -       pmd = pmd_offset(pud, addr);
> >>>> -       if (!pmd_present(*pmd)) {
> >>>> -               pte_t *pte;
> >>>> -
> >>>> -               pte = memblock_alloc(PAGE_SIZE, PAGE_SIZE);
> >>>> -               if (!pte)
> >>>> -                       goto err_alloc;
> >>>> -               pmd_populate_kernel(&init_mm, pmd, pte);
> >>>> -       }
> >>>> -
> >>>> +       populate_kernel_pte(addr);
> >>>>         return;
> >>>> -
> >>>> -err_alloc:
> >>>> -       panic("%s: Failed to allocate memory\n", __func__);
> >>>>  }
> >>>>
> >>>>  void __init setup_per_cpu_areas(void)
> >>>> diff --git a/arch/loongarch/mm/init.c b/arch/loongarch/mm/init.c
> >>>> index 3b7d8129570b..6cd2948373ae 100644
> >>>> --- a/arch/loongarch/mm/init.c
> >>>> +++ b/arch/loongarch/mm/init.c
> >>>> @@ -191,46 +191,49 @@ void vmemmap_free(unsigned long start, unsigned long end, struct vmem_altmap *al
> >>>>  #endif
> >>>>  #endif
> >>>>
> >>>> -static pte_t *fixmap_pte(unsigned long addr)
> >>>> +pte_t * __init populate_kernel_pte(unsigned long addr)
> >>>>  {
> >>>> -       pgd_t *pgd;
> >>>> -       p4d_t *p4d;
> >>>> +       pgd_t *pgd = pgd_offset_k(addr);
> >>>> +       p4d_t *p4d = p4d_offset(pgd, addr);
> >>>>         pud_t *pud;
> >>>>         pmd_t *pmd;
> >>>>
> >>>> -       pgd = pgd_offset_k(addr);
> >>>> -       p4d = p4d_offset(pgd, addr);
> >>>> -
> >>>> -       if (pgd_none(*pgd)) {
> >>>> -               pud_t *new __maybe_unused;
> >>>> -
> >>>> -               new = memblock_alloc_low(PAGE_SIZE, PAGE_SIZE);
> >>>> -               pgd_populate(&init_mm, pgd, new);
> >>>> +       if (p4d_none(*p4d)) {
> >>>> +               pud = memblock_alloc_raw(PAGE_SIZE, PAGE_SIZE);
> >>>> +               if (!pud)
> >>>> +                       goto err_alloc;
> >>>> +               p4d_populate(&init_mm, p4d, pud);
> >>>>  #ifndef __PAGETABLE_PUD_FOLDED
> >>>> -               pud_init(new);
> >>>> +               pud_init(pud);
> >>>>  #endif
> >>>>         }
> >>>>
> >>>>         pud = pud_offset(p4d, addr);
> >>>>         if (pud_none(*pud)) {
> >>>> -               pmd_t *new __maybe_unused;
> >>>> -
> >>>> -               new = memblock_alloc_low(PAGE_SIZE, PAGE_SIZE);
> >>>> -               pud_populate(&init_mm, pud, new);
> >>>> +               pmd = memblock_alloc_raw(PAGE_SIZE, PAGE_SIZE);
> >>>> +               if (!pmd)
> >>>> +                       goto err_alloc;
> >>>> +               pud_populate(&init_mm, pud, pmd);
> >>>>  #ifndef __PAGETABLE_PMD_FOLDED
> >>>> -               pmd_init(new);
> >>>> +               pmd_init(pmd);
> >>>>  #endif
> >>>>         }
> >>>>
> >>>>         pmd = pmd_offset(pud, addr);
> >>>> -       if (pmd_none(*pmd)) {
> >>>> -               pte_t *new __maybe_unused;
> >>>> +       if (!pmd_present(*pmd)) {
> >>>> +               pte_t *pte;
> >>>>
> >>>> -               new = memblock_alloc_low(PAGE_SIZE, PAGE_SIZE);
> >>>> -               pmd_populate_kernel(&init_mm, pmd, new);
> >>>> +               pte = memblock_alloc(PAGE_SIZE, PAGE_SIZE);
> >>> I don't think memblock_alloc_low() here can be replaced by memblock_alloc().
> >> Can you share me the points that pte table must be allocated with memblock_alloc_low
> >> in this place?
> > I forget the reason now, so if you confirm memblock_alloc() works well
> > here, you can use it. But please don't use memblock_alloc_raw().
> what a mess, there is more comments if there is special reason, else everyone can
> forgot by elapsed time.
>
> why the function memblock_alloc_raw can not be use? there is one useless page copy.
This is not a performance critical path, keeping consistency with
mm/percpu.c can make life easier.

Huacai

>
> Regards
> Bibo Mao
>
>
> >
> > Huacai
> >>
> >> Regards
> >> Bibo Mao
> >>>
> >>>
> >>> Huacai
> >>>> +               if (!pte)
> >>>> +                       goto err_alloc;
> >>>> +               pmd_populate_kernel(&init_mm, pmd, pte);
> >>>>         }
> >>>>
> >>>>         return pte_offset_kernel(pmd, addr);
> >>>> +
> >>>> +err_alloc:
> >>>> +       panic("%s: Failed to allocate memory\n", __func__);
> >>>> +       return NULL;
> >>>>  }
> >>>>
> >>>>  void __init __set_fixmap(enum fixed_addresses idx,
> >>>> @@ -241,7 +244,12 @@ void __init __set_fixmap(enum fixed_addresses idx,
> >>>>
> >>>>         BUG_ON(idx <= FIX_HOLE || idx >= __end_of_fixed_addresses);
> >>>>
> >>>> -       ptep = fixmap_pte(addr);
> >>>> +       /*
> >>>> +        * Now only FIX_EARLYCON_MEM_BASE fixed map is used
> >>>> +        * __set_fixmap must be called before mem_init since function
> >>>> +        * populate_kernel_pte allocates memory with memblock_alloc method.
> >>>> +        */
> >>>> +       ptep = populate_kernel_pte(addr);
> >>>>         if (!pte_none(*ptep)) {
> >>>>                 pte_ERROR(*ptep);
> >>>>                 return;
> >>>> --
> >>>> 2.27.0
> >>>>
> >>
> >>
>
>
bibo mao Aug. 10, 2023, 4:42 a.m. UTC | #6
在 2023/8/10 12:27, Huacai Chen 写道:
> On Thu, Aug 10, 2023 at 12:09 PM bibo mao <maobibo@loongson.cn> wrote:
>>
>>
>>
>> 在 2023/8/2 15:25, Huacai Chen 写道:
>>> On Tue, Aug 1, 2023 at 9:22 AM bibo mao <maobibo@loongson.cn> wrote:
>>>>
>>>>
>>>>
>>>> 在 2023/7/31 22:15, Huacai Chen 写道:
>>>>> On Wed, Jul 12, 2023 at 11:16 AM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>>>
>>>>>> Function pcpu_populate_pte and fixmap_pte are similar, they populate
>>>>>> one page from kernel address space. And there is confusion between
>>>>>> pgd and p4d in function fixmap_pte, such as pgd_none always returns
>>>>>> zero. This patch adds unified function populate_kernel_pte and replaces
>>>>>> pcpu_populate_pte and fixmap_pte.
>>>>>>
>>>>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>>>>> ---
>>>>>>  arch/loongarch/include/asm/pgalloc.h |  1 +
>>>>>>  arch/loongarch/kernel/numa.c         | 40 +--------------------
>>>>>>  arch/loongarch/mm/init.c             | 52 ++++++++++++++++------------
>>>>>>  3 files changed, 32 insertions(+), 61 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/loongarch/include/asm/pgalloc.h b/arch/loongarch/include/asm/pgalloc.h
>>>>>> index af1d1e4a6965..ca17b573dba6 100644
>>>>>> --- a/arch/loongarch/include/asm/pgalloc.h
>>>>>> +++ b/arch/loongarch/include/asm/pgalloc.h
>>>>>> @@ -91,4 +91,5 @@ static inline pud_t *pud_alloc_one(struct mm_struct *mm, unsigned long address)
>>>>>>
>>>>>>  #endif /* __PAGETABLE_PUD_FOLDED */
>>>>>>
>>>>>> +extern pte_t * __init populate_kernel_pte(unsigned long addr);
>>>>>>  #endif /* _ASM_PGALLOC_H */
>>>>>> diff --git a/arch/loongarch/kernel/numa.c b/arch/loongarch/kernel/numa.c
>>>>>> index 778e1c20bfb0..24a693b76873 100644
>>>>>> --- a/arch/loongarch/kernel/numa.c
>>>>>> +++ b/arch/loongarch/kernel/numa.c
>>>>>> @@ -67,46 +67,8 @@ static int __init pcpu_cpu_distance(unsigned int from, unsigned int to)
>>>>>>
>>>>>>  void __init pcpu_populate_pte(unsigned long addr)
>>>>>>  {
>>>>>> -       pgd_t *pgd = pgd_offset_k(addr);
>>>>>> -       p4d_t *p4d = p4d_offset(pgd, addr);
>>>>>> -       pud_t *pud;
>>>>>> -       pmd_t *pmd;
>>>>>> -
>>>>>> -       if (p4d_none(*p4d)) {
>>>>>> -               pud = memblock_alloc_raw(PAGE_SIZE, PAGE_SIZE);
>>>>>> -               if (!pud)
>>>>>> -                       goto err_alloc;
>>>>>> -               p4d_populate(&init_mm, p4d, pud);
>>>>>> -#ifndef __PAGETABLE_PUD_FOLDED
>>>>>> -               pud_init(pud);
>>>>>> -#endif
>>>>>> -       }
>>>>>> -
>>>>>> -       pud = pud_offset(p4d, addr);
>>>>>> -       if (pud_none(*pud)) {
>>>>>> -               pmd = memblock_alloc_raw(PAGE_SIZE, PAGE_SIZE);
>>>>>> -               if (!pmd)
>>>>>> -                       goto err_alloc;
>>>>>> -               pud_populate(&init_mm, pud, pmd);
>>>>>> -#ifndef __PAGETABLE_PMD_FOLDED
>>>>>> -               pmd_init(pmd);
>>>>>> -#endif
>>>>>> -       }
>>>>>> -
>>>>>> -       pmd = pmd_offset(pud, addr);
>>>>>> -       if (!pmd_present(*pmd)) {
>>>>>> -               pte_t *pte;
>>>>>> -
>>>>>> -               pte = memblock_alloc(PAGE_SIZE, PAGE_SIZE);
>>>>>> -               if (!pte)
>>>>>> -                       goto err_alloc;
>>>>>> -               pmd_populate_kernel(&init_mm, pmd, pte);
>>>>>> -       }
>>>>>> -
>>>>>> +       populate_kernel_pte(addr);
>>>>>>         return;
>>>>>> -
>>>>>> -err_alloc:
>>>>>> -       panic("%s: Failed to allocate memory\n", __func__);
>>>>>>  }
>>>>>>
>>>>>>  void __init setup_per_cpu_areas(void)
>>>>>> diff --git a/arch/loongarch/mm/init.c b/arch/loongarch/mm/init.c
>>>>>> index 3b7d8129570b..6cd2948373ae 100644
>>>>>> --- a/arch/loongarch/mm/init.c
>>>>>> +++ b/arch/loongarch/mm/init.c
>>>>>> @@ -191,46 +191,49 @@ void vmemmap_free(unsigned long start, unsigned long end, struct vmem_altmap *al
>>>>>>  #endif
>>>>>>  #endif
>>>>>>
>>>>>> -static pte_t *fixmap_pte(unsigned long addr)
>>>>>> +pte_t * __init populate_kernel_pte(unsigned long addr)
>>>>>>  {
>>>>>> -       pgd_t *pgd;
>>>>>> -       p4d_t *p4d;
>>>>>> +       pgd_t *pgd = pgd_offset_k(addr);
>>>>>> +       p4d_t *p4d = p4d_offset(pgd, addr);
>>>>>>         pud_t *pud;
>>>>>>         pmd_t *pmd;
>>>>>>
>>>>>> -       pgd = pgd_offset_k(addr);
>>>>>> -       p4d = p4d_offset(pgd, addr);
>>>>>> -
>>>>>> -       if (pgd_none(*pgd)) {
>>>>>> -               pud_t *new __maybe_unused;
>>>>>> -
>>>>>> -               new = memblock_alloc_low(PAGE_SIZE, PAGE_SIZE);
>>>>>> -               pgd_populate(&init_mm, pgd, new);
>>>>>> +       if (p4d_none(*p4d)) {
>>>>>> +               pud = memblock_alloc_raw(PAGE_SIZE, PAGE_SIZE);
>>>>>> +               if (!pud)
>>>>>> +                       goto err_alloc;
>>>>>> +               p4d_populate(&init_mm, p4d, pud);
>>>>>>  #ifndef __PAGETABLE_PUD_FOLDED
>>>>>> -               pud_init(new);
>>>>>> +               pud_init(pud);
>>>>>>  #endif
>>>>>>         }
>>>>>>
>>>>>>         pud = pud_offset(p4d, addr);
>>>>>>         if (pud_none(*pud)) {
>>>>>> -               pmd_t *new __maybe_unused;
>>>>>> -
>>>>>> -               new = memblock_alloc_low(PAGE_SIZE, PAGE_SIZE);
>>>>>> -               pud_populate(&init_mm, pud, new);
>>>>>> +               pmd = memblock_alloc_raw(PAGE_SIZE, PAGE_SIZE);
>>>>>> +               if (!pmd)
>>>>>> +                       goto err_alloc;
>>>>>> +               pud_populate(&init_mm, pud, pmd);
>>>>>>  #ifndef __PAGETABLE_PMD_FOLDED
>>>>>> -               pmd_init(new);
>>>>>> +               pmd_init(pmd);
>>>>>>  #endif
>>>>>>         }
>>>>>>
>>>>>>         pmd = pmd_offset(pud, addr);
>>>>>> -       if (pmd_none(*pmd)) {
>>>>>> -               pte_t *new __maybe_unused;
>>>>>> +       if (!pmd_present(*pmd)) {
>>>>>> +               pte_t *pte;
>>>>>>
>>>>>> -               new = memblock_alloc_low(PAGE_SIZE, PAGE_SIZE);
>>>>>> -               pmd_populate_kernel(&init_mm, pmd, new);
>>>>>> +               pte = memblock_alloc(PAGE_SIZE, PAGE_SIZE);
>>>>> I don't think memblock_alloc_low() here can be replaced by memblock_alloc().
>>>> Can you share me the points that pte table must be allocated with memblock_alloc_low
>>>> in this place?
>>> I forget the reason now, so if you confirm memblock_alloc() works well
>>> here, you can use it. But please don't use memblock_alloc_raw().
>> what a mess, there is more comments if there is special reason, else everyone can
>> forgot by elapsed time.
>>
>> why the function memblock_alloc_raw can not be use? there is one useless page copy.
> This is not a performance critical path, keeping consistency with
> mm/percpu.c can make life easier.
yes, it is not critical path, it influences boot speed. Can we make it better? else it
will be just so so.

Regards
Bibo Mao
> 
> Huacai
> 
>>
>> Regards
>> Bibo Mao
>>
>>
>>>
>>> Huacai
>>>>
>>>> Regards
>>>> Bibo Mao
>>>>>
>>>>>
>>>>> Huacai
>>>>>> +               if (!pte)
>>>>>> +                       goto err_alloc;
>>>>>> +               pmd_populate_kernel(&init_mm, pmd, pte);
>>>>>>         }
>>>>>>
>>>>>>         return pte_offset_kernel(pmd, addr);
>>>>>> +
>>>>>> +err_alloc:
>>>>>> +       panic("%s: Failed to allocate memory\n", __func__);
>>>>>> +       return NULL;
>>>>>>  }
>>>>>>
>>>>>>  void __init __set_fixmap(enum fixed_addresses idx,
>>>>>> @@ -241,7 +244,12 @@ void __init __set_fixmap(enum fixed_addresses idx,
>>>>>>
>>>>>>         BUG_ON(idx <= FIX_HOLE || idx >= __end_of_fixed_addresses);
>>>>>>
>>>>>> -       ptep = fixmap_pte(addr);
>>>>>> +       /*
>>>>>> +        * Now only FIX_EARLYCON_MEM_BASE fixed map is used
>>>>>> +        * __set_fixmap must be called before mem_init since function
>>>>>> +        * populate_kernel_pte allocates memory with memblock_alloc method.
>>>>>> +        */
>>>>>> +       ptep = populate_kernel_pte(addr);
>>>>>>         if (!pte_none(*ptep)) {
>>>>>>                 pte_ERROR(*ptep);
>>>>>>                 return;
>>>>>> --
>>>>>> 2.27.0
>>>>>>
>>>>
>>>>
>>
>>
diff mbox series

Patch

diff --git a/arch/loongarch/include/asm/pgalloc.h b/arch/loongarch/include/asm/pgalloc.h
index af1d1e4a6965..ca17b573dba6 100644
--- a/arch/loongarch/include/asm/pgalloc.h
+++ b/arch/loongarch/include/asm/pgalloc.h
@@ -91,4 +91,5 @@  static inline pud_t *pud_alloc_one(struct mm_struct *mm, unsigned long address)
 
 #endif /* __PAGETABLE_PUD_FOLDED */
 
+extern pte_t * __init populate_kernel_pte(unsigned long addr);
 #endif /* _ASM_PGALLOC_H */
diff --git a/arch/loongarch/kernel/numa.c b/arch/loongarch/kernel/numa.c
index 778e1c20bfb0..24a693b76873 100644
--- a/arch/loongarch/kernel/numa.c
+++ b/arch/loongarch/kernel/numa.c
@@ -67,46 +67,8 @@  static int __init pcpu_cpu_distance(unsigned int from, unsigned int to)
 
 void __init pcpu_populate_pte(unsigned long addr)
 {
-	pgd_t *pgd = pgd_offset_k(addr);
-	p4d_t *p4d = p4d_offset(pgd, addr);
-	pud_t *pud;
-	pmd_t *pmd;
-
-	if (p4d_none(*p4d)) {
-		pud = memblock_alloc_raw(PAGE_SIZE, PAGE_SIZE);
-		if (!pud)
-			goto err_alloc;
-		p4d_populate(&init_mm, p4d, pud);
-#ifndef __PAGETABLE_PUD_FOLDED
-		pud_init(pud);
-#endif
-	}
-
-	pud = pud_offset(p4d, addr);
-	if (pud_none(*pud)) {
-		pmd = memblock_alloc_raw(PAGE_SIZE, PAGE_SIZE);
-		if (!pmd)
-			goto err_alloc;
-		pud_populate(&init_mm, pud, pmd);
-#ifndef __PAGETABLE_PMD_FOLDED
-		pmd_init(pmd);
-#endif
-	}
-
-	pmd = pmd_offset(pud, addr);
-	if (!pmd_present(*pmd)) {
-		pte_t *pte;
-
-		pte = memblock_alloc(PAGE_SIZE, PAGE_SIZE);
-		if (!pte)
-			goto err_alloc;
-		pmd_populate_kernel(&init_mm, pmd, pte);
-	}
-
+	populate_kernel_pte(addr);
 	return;
-
-err_alloc:
-	panic("%s: Failed to allocate memory\n", __func__);
 }
 
 void __init setup_per_cpu_areas(void)
diff --git a/arch/loongarch/mm/init.c b/arch/loongarch/mm/init.c
index 3b7d8129570b..6cd2948373ae 100644
--- a/arch/loongarch/mm/init.c
+++ b/arch/loongarch/mm/init.c
@@ -191,46 +191,49 @@  void vmemmap_free(unsigned long start, unsigned long end, struct vmem_altmap *al
 #endif
 #endif
 
-static pte_t *fixmap_pte(unsigned long addr)
+pte_t * __init populate_kernel_pte(unsigned long addr)
 {
-	pgd_t *pgd;
-	p4d_t *p4d;
+	pgd_t *pgd = pgd_offset_k(addr);
+	p4d_t *p4d = p4d_offset(pgd, addr);
 	pud_t *pud;
 	pmd_t *pmd;
 
-	pgd = pgd_offset_k(addr);
-	p4d = p4d_offset(pgd, addr);
-
-	if (pgd_none(*pgd)) {
-		pud_t *new __maybe_unused;
-
-		new = memblock_alloc_low(PAGE_SIZE, PAGE_SIZE);
-		pgd_populate(&init_mm, pgd, new);
+	if (p4d_none(*p4d)) {
+		pud = memblock_alloc_raw(PAGE_SIZE, PAGE_SIZE);
+		if (!pud)
+			goto err_alloc;
+		p4d_populate(&init_mm, p4d, pud);
 #ifndef __PAGETABLE_PUD_FOLDED
-		pud_init(new);
+		pud_init(pud);
 #endif
 	}
 
 	pud = pud_offset(p4d, addr);
 	if (pud_none(*pud)) {
-		pmd_t *new __maybe_unused;
-
-		new = memblock_alloc_low(PAGE_SIZE, PAGE_SIZE);
-		pud_populate(&init_mm, pud, new);
+		pmd = memblock_alloc_raw(PAGE_SIZE, PAGE_SIZE);
+		if (!pmd)
+			goto err_alloc;
+		pud_populate(&init_mm, pud, pmd);
 #ifndef __PAGETABLE_PMD_FOLDED
-		pmd_init(new);
+		pmd_init(pmd);
 #endif
 	}
 
 	pmd = pmd_offset(pud, addr);
-	if (pmd_none(*pmd)) {
-		pte_t *new __maybe_unused;
+	if (!pmd_present(*pmd)) {
+		pte_t *pte;
 
-		new = memblock_alloc_low(PAGE_SIZE, PAGE_SIZE);
-		pmd_populate_kernel(&init_mm, pmd, new);
+		pte = memblock_alloc(PAGE_SIZE, PAGE_SIZE);
+		if (!pte)
+			goto err_alloc;
+		pmd_populate_kernel(&init_mm, pmd, pte);
 	}
 
 	return pte_offset_kernel(pmd, addr);
+
+err_alloc:
+	panic("%s: Failed to allocate memory\n", __func__);
+	return NULL;
 }
 
 void __init __set_fixmap(enum fixed_addresses idx,
@@ -241,7 +244,12 @@  void __init __set_fixmap(enum fixed_addresses idx,
 
 	BUG_ON(idx <= FIX_HOLE || idx >= __end_of_fixed_addresses);
 
-	ptep = fixmap_pte(addr);
+	/*
+	 * Now only FIX_EARLYCON_MEM_BASE fixed map is used
+	 * __set_fixmap must be called before mem_init since function
+	 * populate_kernel_pte allocates memory with memblock_alloc method.
+	 */
+	ptep = populate_kernel_pte(addr);
 	if (!pte_none(*ptep)) {
 		pte_ERROR(*ptep);
 		return;