* [PATCH 0/3] LoongArch: mm: Code cleanup with populate pte
@ 2023-07-12 3:16 Bibo Mao
2023-07-12 3:16 ` [PATCH 1/3] mm/percpu: Remove some local variables in pcpu_populate_pte Bibo Mao
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Bibo Mao @ 2023-07-12 3:16 UTC (permalink / raw)
To: Huacai Chen, Dennis Zhou, Tejun Heo, Christoph Lameter
Cc: Andrew Morton, loongarch, linux-kernel, linux-mm, WANG Xuerui
There are some confusion between pdg and p4d when populate pte for
kernel address space. This patch modifies this issue and adds unified
function for pcpu and fixmap populate pte.
Bibo Mao (3):
mm/percpu: Remove some local variables in pcpu_populate_pte
LoongArch: Code cleanup in function pcpu_populate_pte
LoongArch: mm: Add unified function populate_kernel_pte
arch/loongarch/include/asm/pgalloc.h | 1 +
arch/loongarch/kernel/numa.c | 35 ++-----------------
arch/loongarch/mm/init.c | 52 ++++++++++++++++------------
mm/percpu.c | 24 +++++--------
4 files changed, 42 insertions(+), 70 deletions(-)
--
2.27.0
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/3] mm/percpu: Remove some local variables in pcpu_populate_pte
2023-07-12 3:16 [PATCH 0/3] LoongArch: mm: Code cleanup with populate pte Bibo Mao
@ 2023-07-12 3:16 ` Bibo Mao
2023-07-27 23:08 ` Dennis Zhou
2023-07-12 3:16 ` [PATCH 2/3] LoongArch: Code cleanup in function pcpu_populate_pte Bibo Mao
` (2 subsequent siblings)
3 siblings, 1 reply; 16+ messages in thread
From: Bibo Mao @ 2023-07-12 3:16 UTC (permalink / raw)
To: Huacai Chen, Dennis Zhou, Tejun Heo, Christoph Lameter
Cc: Andrew Morton, loongarch, linux-kernel, linux-mm, WANG Xuerui
In function pcpu_populate_pte there are already variable defined,
it can be reused for later use, here remove duplicated local
variables.
Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
mm/percpu.c | 24 +++++++++---------------
1 file changed, 9 insertions(+), 15 deletions(-)
diff --git a/mm/percpu.c b/mm/percpu.c
index 28e07ede46f6..85e3f9b2a61f 100644
--- a/mm/percpu.c
+++ b/mm/percpu.c
@@ -3189,32 +3189,26 @@ void __init __weak pcpu_populate_pte(unsigned long addr)
pmd_t *pmd;
if (pgd_none(*pgd)) {
- p4d_t *new;
-
- new = memblock_alloc(P4D_TABLE_SIZE, P4D_TABLE_SIZE);
- if (!new)
+ p4d = memblock_alloc(P4D_TABLE_SIZE, P4D_TABLE_SIZE);
+ if (!p4d)
goto err_alloc;
- pgd_populate(&init_mm, pgd, new);
+ pgd_populate(&init_mm, pgd, p4d);
}
p4d = p4d_offset(pgd, addr);
if (p4d_none(*p4d)) {
- pud_t *new;
-
- new = memblock_alloc(PUD_TABLE_SIZE, PUD_TABLE_SIZE);
- if (!new)
+ pud = memblock_alloc(PUD_TABLE_SIZE, PUD_TABLE_SIZE);
+ if (!pud)
goto err_alloc;
- p4d_populate(&init_mm, p4d, new);
+ p4d_populate(&init_mm, p4d, pud);
}
pud = pud_offset(p4d, addr);
if (pud_none(*pud)) {
- pmd_t *new;
-
- new = memblock_alloc(PMD_TABLE_SIZE, PMD_TABLE_SIZE);
- if (!new)
+ pmd = memblock_alloc(PMD_TABLE_SIZE, PMD_TABLE_SIZE);
+ if (!pmd)
goto err_alloc;
- pud_populate(&init_mm, pud, new);
+ pud_populate(&init_mm, pud, pmd);
}
pmd = pmd_offset(pud, addr);
--
2.27.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/3] LoongArch: Code cleanup in function pcpu_populate_pte
2023-07-12 3:16 [PATCH 0/3] LoongArch: mm: Code cleanup with populate pte Bibo Mao
2023-07-12 3:16 ` [PATCH 1/3] mm/percpu: Remove some local variables in pcpu_populate_pte Bibo Mao
@ 2023-07-12 3:16 ` Bibo Mao
2023-07-31 14:15 ` Huacai Chen
2023-07-12 3:16 ` [PATCH 3/3] LoongArch: mm: Add unified function populate_kernel_pte Bibo Mao
2023-07-25 0:36 ` [PATCH 0/3] LoongArch: mm: Code cleanup with populate pte bibo mao
3 siblings, 1 reply; 16+ messages in thread
From: Bibo Mao @ 2023-07-12 3:16 UTC (permalink / raw)
To: Huacai Chen, Dennis Zhou, Tejun Heo, Christoph Lameter
Cc: Andrew Morton, loongarch, linux-kernel, linux-mm, WANG Xuerui
There are some code cleanups in function pcpu_populate_pte:
1. Replace memblock_alloc with memblock_alloc_raw for pud and pmd since
they will be reinitialized with pud_init and pmd_init.
2. Add memory allocation failure handling
3. Replace pgd_populate with p4d_populate, it will be useful if there
four-level page tables.
Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
arch/loongarch/kernel/numa.c | 33 ++++++++++++++++++++-------------
1 file changed, 20 insertions(+), 13 deletions(-)
diff --git a/arch/loongarch/kernel/numa.c b/arch/loongarch/kernel/numa.c
index 708665895b47..778e1c20bfb0 100644
--- a/arch/loongarch/kernel/numa.c
+++ b/arch/loongarch/kernel/numa.c
@@ -73,33 +73,40 @@ void __init pcpu_populate_pte(unsigned long addr)
pmd_t *pmd;
if (p4d_none(*p4d)) {
- pud_t *new;
-
- new = memblock_alloc(PAGE_SIZE, PAGE_SIZE);
- pgd_populate(&init_mm, pgd, new);
+ 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;
-
- new = memblock_alloc(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_present(*pmd)) {
- pte_t *new;
+ pte_t *pte;
- new = memblock_alloc(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;
+
+err_alloc:
+ panic("%s: Failed to allocate memory\n", __func__);
}
void __init setup_per_cpu_areas(void)
--
2.27.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/3] LoongArch: mm: Add unified function populate_kernel_pte
2023-07-12 3:16 [PATCH 0/3] LoongArch: mm: Code cleanup with populate pte Bibo Mao
2023-07-12 3:16 ` [PATCH 1/3] mm/percpu: Remove some local variables in pcpu_populate_pte Bibo Mao
2023-07-12 3:16 ` [PATCH 2/3] LoongArch: Code cleanup in function pcpu_populate_pte Bibo Mao
@ 2023-07-12 3:16 ` Bibo Mao
2023-07-31 14:15 ` Huacai Chen
2023-07-25 0:36 ` [PATCH 0/3] LoongArch: mm: Code cleanup with populate pte bibo mao
3 siblings, 1 reply; 16+ messages in thread
From: Bibo Mao @ 2023-07-12 3:16 UTC (permalink / raw)
To: Huacai Chen, Dennis Zhou, Tejun Heo, Christoph Lameter
Cc: Andrew Morton, loongarch, linux-kernel, linux-mm, WANG Xuerui
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);
+ 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
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 0/3] LoongArch: mm: Code cleanup with populate pte
2023-07-12 3:16 [PATCH 0/3] LoongArch: mm: Code cleanup with populate pte Bibo Mao
` (2 preceding siblings ...)
2023-07-12 3:16 ` [PATCH 3/3] LoongArch: mm: Add unified function populate_kernel_pte Bibo Mao
@ 2023-07-25 0:36 ` bibo mao
2023-07-25 2:33 ` Dennis Zhou
3 siblings, 1 reply; 16+ messages in thread
From: bibo mao @ 2023-07-25 0:36 UTC (permalink / raw)
To: Huacai Chen, Dennis Zhou, Tejun Heo, Christoph Lameter
Cc: Andrew Morton, loongarch, linux-kernel, linux-mm, WANG Xuerui
slightly ping.
在 2023/7/12 11:16, Bibo Mao 写道:
> There are some confusion between pdg and p4d when populate pte for
> kernel address space. This patch modifies this issue and adds unified
> function for pcpu and fixmap populate pte.
>
> Bibo Mao (3):
> mm/percpu: Remove some local variables in pcpu_populate_pte
> LoongArch: Code cleanup in function pcpu_populate_pte
> LoongArch: mm: Add unified function populate_kernel_pte
>
> arch/loongarch/include/asm/pgalloc.h | 1 +
> arch/loongarch/kernel/numa.c | 35 ++-----------------
> arch/loongarch/mm/init.c | 52 ++++++++++++++++------------
> mm/percpu.c | 24 +++++--------
> 4 files changed, 42 insertions(+), 70 deletions(-)
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/3] LoongArch: mm: Code cleanup with populate pte
2023-07-25 0:36 ` [PATCH 0/3] LoongArch: mm: Code cleanup with populate pte bibo mao
@ 2023-07-25 2:33 ` Dennis Zhou
0 siblings, 0 replies; 16+ messages in thread
From: Dennis Zhou @ 2023-07-25 2:33 UTC (permalink / raw)
To: bibo mao
Cc: Huacai Chen, Tejun Heo, Christoph Lameter, Andrew Morton,
loongarch, linux-kernel, linux-mm, WANG Xuerui
Hello,
On Tue, Jul 25, 2023 at 08:36:22AM +0800, bibo mao wrote:
> slightly ping.
>
Sorry, I'm not sure how I missed this. I'll take a look at this
tomorrow.
Thanks,
Dennis
> 在 2023/7/12 11:16, Bibo Mao 写道:
> > There are some confusion between pdg and p4d when populate pte for
> > kernel address space. This patch modifies this issue and adds unified
> > function for pcpu and fixmap populate pte.
> >
> > Bibo Mao (3):
> > mm/percpu: Remove some local variables in pcpu_populate_pte
> > LoongArch: Code cleanup in function pcpu_populate_pte
> > LoongArch: mm: Add unified function populate_kernel_pte
> >
> > arch/loongarch/include/asm/pgalloc.h | 1 +
> > arch/loongarch/kernel/numa.c | 35 ++-----------------
> > arch/loongarch/mm/init.c | 52 ++++++++++++++++------------
> > mm/percpu.c | 24 +++++--------
> > 4 files changed, 42 insertions(+), 70 deletions(-)
> >
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] mm/percpu: Remove some local variables in pcpu_populate_pte
2023-07-12 3:16 ` [PATCH 1/3] mm/percpu: Remove some local variables in pcpu_populate_pte Bibo Mao
@ 2023-07-27 23:08 ` Dennis Zhou
2023-07-28 1:13 ` bibo mao
0 siblings, 1 reply; 16+ messages in thread
From: Dennis Zhou @ 2023-07-27 23:08 UTC (permalink / raw)
To: Bibo Mao
Cc: Huacai Chen, Tejun Heo, Christoph Lameter, Andrew Morton,
loongarch, linux-kernel, linux-mm, WANG Xuerui
Hello,
On Wed, Jul 12, 2023 at 11:16:20AM +0800, Bibo Mao wrote:
> In function pcpu_populate_pte there are already variable defined,
> it can be reused for later use, here remove duplicated local
> variables.
>
> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> ---
> mm/percpu.c | 24 +++++++++---------------
> 1 file changed, 9 insertions(+), 15 deletions(-)
>
> diff --git a/mm/percpu.c b/mm/percpu.c
> index 28e07ede46f6..85e3f9b2a61f 100644
> --- a/mm/percpu.c
> +++ b/mm/percpu.c
> @@ -3189,32 +3189,26 @@ void __init __weak pcpu_populate_pte(unsigned long addr)
> pmd_t *pmd;
>
> if (pgd_none(*pgd)) {
> - p4d_t *new;
> -
> - new = memblock_alloc(P4D_TABLE_SIZE, P4D_TABLE_SIZE);
> - if (!new)
> + p4d = memblock_alloc(P4D_TABLE_SIZE, P4D_TABLE_SIZE);
> + if (!p4d)
> goto err_alloc;
> - pgd_populate(&init_mm, pgd, new);
> + pgd_populate(&init_mm, pgd, p4d);
> }
>
> p4d = p4d_offset(pgd, addr);
> if (p4d_none(*p4d)) {
> - pud_t *new;
> -
> - new = memblock_alloc(PUD_TABLE_SIZE, PUD_TABLE_SIZE);
> - if (!new)
> + pud = memblock_alloc(PUD_TABLE_SIZE, PUD_TABLE_SIZE);
> + if (!pud)
> goto err_alloc;
> - p4d_populate(&init_mm, p4d, new);
> + p4d_populate(&init_mm, p4d, pud);
> }
>
> pud = pud_offset(p4d, addr);
> if (pud_none(*pud)) {
> - pmd_t *new;
> -
> - new = memblock_alloc(PMD_TABLE_SIZE, PMD_TABLE_SIZE);
> - if (!new)
> + pmd = memblock_alloc(PMD_TABLE_SIZE, PMD_TABLE_SIZE);
> + if (!pmd)
> goto err_alloc;
> - pud_populate(&init_mm, pud, new);
> + pud_populate(&init_mm, pud, pmd);
> }
>
> pmd = pmd_offset(pud, addr);
> --
> 2.27.0
>
I've pulled this, but the other 2 are part of loongarch and should be
reviewed and pulled by those maintainers.
Thanks,
Dennis
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] mm/percpu: Remove some local variables in pcpu_populate_pte
2023-07-27 23:08 ` Dennis Zhou
@ 2023-07-28 1:13 ` bibo mao
0 siblings, 0 replies; 16+ messages in thread
From: bibo mao @ 2023-07-28 1:13 UTC (permalink / raw)
To: Dennis Zhou, Huacai Chen
Cc: Tejun Heo, Christoph Lameter, Andrew Morton, loongarch,
linux-kernel, linux-mm, WANG Xuerui
在 2023/7/28 07:08, Dennis Zhou 写道:
> Hello,
>
> On Wed, Jul 12, 2023 at 11:16:20AM +0800, Bibo Mao wrote:
>> In function pcpu_populate_pte there are already variable defined,
>> it can be reused for later use, here remove duplicated local
>> variables.
>>
>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>> ---
>> mm/percpu.c | 24 +++++++++---------------
>> 1 file changed, 9 insertions(+), 15 deletions(-)
>>
>> diff --git a/mm/percpu.c b/mm/percpu.c
>> index 28e07ede46f6..85e3f9b2a61f 100644
>> --- a/mm/percpu.c
>> +++ b/mm/percpu.c
>> @@ -3189,32 +3189,26 @@ void __init __weak pcpu_populate_pte(unsigned long addr)
>> pmd_t *pmd;
>>
>> if (pgd_none(*pgd)) {
>> - p4d_t *new;
>> -
>> - new = memblock_alloc(P4D_TABLE_SIZE, P4D_TABLE_SIZE);
>> - if (!new)
>> + p4d = memblock_alloc(P4D_TABLE_SIZE, P4D_TABLE_SIZE);
>> + if (!p4d)
>> goto err_alloc;
>> - pgd_populate(&init_mm, pgd, new);
>> + pgd_populate(&init_mm, pgd, p4d);
>> }
>>
>> p4d = p4d_offset(pgd, addr);
>> if (p4d_none(*p4d)) {
>> - pud_t *new;
>> -
>> - new = memblock_alloc(PUD_TABLE_SIZE, PUD_TABLE_SIZE);
>> - if (!new)
>> + pud = memblock_alloc(PUD_TABLE_SIZE, PUD_TABLE_SIZE);
>> + if (!pud)
>> goto err_alloc;
>> - p4d_populate(&init_mm, p4d, new);
>> + p4d_populate(&init_mm, p4d, pud);
>> }
>>
>> pud = pud_offset(p4d, addr);
>> if (pud_none(*pud)) {
>> - pmd_t *new;
>> -
>> - new = memblock_alloc(PMD_TABLE_SIZE, PMD_TABLE_SIZE);
>> - if (!new)
>> + pmd = memblock_alloc(PMD_TABLE_SIZE, PMD_TABLE_SIZE);
>> + if (!pmd)
>> goto err_alloc;
>> - pud_populate(&init_mm, pud, new);
>> + pud_populate(&init_mm, pud, pmd);
>> }
>>
>> pmd = pmd_offset(pud, addr);
>> --
>> 2.27.0
>>
>
> I've pulled this, but the other 2 are part of loongarch and should be
> reviewed and pulled by those maintainers.
Dennis,
Thanks for your feedback.
Huacai,
Could you give some review comments about the remaining 2 patches?
Regards
Bibo Mao
>
> Thanks,
> Dennis
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] LoongArch: Code cleanup in function pcpu_populate_pte
2023-07-12 3:16 ` [PATCH 2/3] LoongArch: Code cleanup in function pcpu_populate_pte Bibo Mao
@ 2023-07-31 14:15 ` Huacai Chen
2023-08-01 1:18 ` bibo mao
0 siblings, 1 reply; 16+ messages in thread
From: Huacai Chen @ 2023-07-31 14:15 UTC (permalink / raw)
To: Bibo Mao
Cc: Dennis Zhou, Tejun Heo, Christoph Lameter, Andrew Morton,
loongarch, linux-kernel, linux-mm, WANG Xuerui
On Wed, Jul 12, 2023 at 11:16 AM Bibo Mao <maobibo@loongson.cn> wrote:
>
> There are some code cleanups in function pcpu_populate_pte:
> 1. Replace memblock_alloc with memblock_alloc_raw for pud and pmd since
> they will be reinitialized with pud_init and pmd_init.
>
> 2. Add memory allocation failure handling
>
> 3. Replace pgd_populate with p4d_populate, it will be useful if there
> four-level page tables.
>
> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> ---
> arch/loongarch/kernel/numa.c | 33 ++++++++++++++++++++-------------
> 1 file changed, 20 insertions(+), 13 deletions(-)
>
> diff --git a/arch/loongarch/kernel/numa.c b/arch/loongarch/kernel/numa.c
> index 708665895b47..778e1c20bfb0 100644
> --- a/arch/loongarch/kernel/numa.c
> +++ b/arch/loongarch/kernel/numa.c
> @@ -73,33 +73,40 @@ void __init pcpu_populate_pte(unsigned long addr)
> pmd_t *pmd;
>
> if (p4d_none(*p4d)) {
> - pud_t *new;
> -
> - new = memblock_alloc(PAGE_SIZE, PAGE_SIZE);
> - pgd_populate(&init_mm, pgd, new);
> + pud = memblock_alloc_raw(PAGE_SIZE, PAGE_SIZE);
Don't use memblock_alloc_raw() here, it is better to keep consistency
with mm/percpu.c.
Huacai
> + 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;
> -
> - new = memblock_alloc(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_present(*pmd)) {
> - pte_t *new;
> + pte_t *pte;
>
> - new = memblock_alloc(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;
> +
> +err_alloc:
> + panic("%s: Failed to allocate memory\n", __func__);
> }
>
> void __init setup_per_cpu_areas(void)
> --
> 2.27.0
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] LoongArch: mm: Add unified function populate_kernel_pte
2023-07-12 3:16 ` [PATCH 3/3] LoongArch: mm: Add unified function populate_kernel_pte Bibo Mao
@ 2023-07-31 14:15 ` Huacai Chen
2023-08-01 1:22 ` bibo mao
0 siblings, 1 reply; 16+ messages in thread
From: Huacai Chen @ 2023-07-31 14:15 UTC (permalink / raw)
To: Bibo Mao
Cc: Dennis Zhou, Tejun Heo, Christoph Lameter, Andrew Morton,
loongarch, linux-kernel, linux-mm, WANG Xuerui
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
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] LoongArch: Code cleanup in function pcpu_populate_pte
2023-07-31 14:15 ` Huacai Chen
@ 2023-08-01 1:18 ` bibo mao
0 siblings, 0 replies; 16+ messages in thread
From: bibo mao @ 2023-08-01 1:18 UTC (permalink / raw)
To: Huacai Chen
Cc: Dennis Zhou, Tejun Heo, Christoph Lameter, Andrew Morton,
loongarch, linux-kernel, linux-mm, WANG Xuerui
在 2023/7/31 22:15, Huacai Chen 写道:
> On Wed, Jul 12, 2023 at 11:16 AM Bibo Mao <maobibo@loongson.cn> wrote:
>>
>> There are some code cleanups in function pcpu_populate_pte:
>> 1. Replace memblock_alloc with memblock_alloc_raw for pud and pmd since
>> they will be reinitialized with pud_init and pmd_init.
>>
>> 2. Add memory allocation failure handling
>>
>> 3. Replace pgd_populate with p4d_populate, it will be useful if there
>> four-level page tables.
>>
>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>> ---
>> arch/loongarch/kernel/numa.c | 33 ++++++++++++++++++++-------------
>> 1 file changed, 20 insertions(+), 13 deletions(-)
>>
>> diff --git a/arch/loongarch/kernel/numa.c b/arch/loongarch/kernel/numa.c
>> index 708665895b47..778e1c20bfb0 100644
>> --- a/arch/loongarch/kernel/numa.c
>> +++ b/arch/loongarch/kernel/numa.c
>> @@ -73,33 +73,40 @@ void __init pcpu_populate_pte(unsigned long addr)
>> pmd_t *pmd;
>>
>> if (p4d_none(*p4d)) {
>> - pud_t *new;
>> -
>> - new = memblock_alloc(PAGE_SIZE, PAGE_SIZE);
>> - pgd_populate(&init_mm, pgd, new);
>> + pud = memblock_alloc_raw(PAGE_SIZE, PAGE_SIZE);
> Don't use memblock_alloc_raw() here, it is better to keep consistency
> with mm/percpu.c.
memblock_alloc will clear the page, and there will be page table initialization
with the following pud_init(pud). memblock_alloc_raw will not clear the page.
I prefer memblock_alloc_raw for better performance however ok with both, it is
up to you to decide.
Regards
Bibo Mao
>
>
> Huacai
>> + 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;
>> -
>> - new = memblock_alloc(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_present(*pmd)) {
>> - pte_t *new;
>> + pte_t *pte;
>>
>> - new = memblock_alloc(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;
>> +
>> +err_alloc:
>> + panic("%s: Failed to allocate memory\n", __func__);
>> }
>>
>> void __init setup_per_cpu_areas(void)
>> --
>> 2.27.0
>>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] LoongArch: mm: Add unified function populate_kernel_pte
2023-07-31 14:15 ` Huacai Chen
@ 2023-08-01 1:22 ` bibo mao
2023-08-02 7:25 ` Huacai Chen
0 siblings, 1 reply; 16+ messages in thread
From: bibo mao @ 2023-08-01 1:22 UTC (permalink / raw)
To: Huacai Chen
Cc: Dennis Zhou, Tejun Heo, Christoph Lameter, Andrew Morton,
loongarch, linux-kernel, linux-mm, WANG Xuerui
在 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
>>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] LoongArch: mm: Add unified function populate_kernel_pte
2023-08-01 1:22 ` bibo mao
@ 2023-08-02 7:25 ` Huacai Chen
2023-08-10 4:08 ` bibo mao
0 siblings, 1 reply; 16+ messages in thread
From: Huacai Chen @ 2023-08-02 7:25 UTC (permalink / raw)
To: bibo mao
Cc: Dennis Zhou, Tejun Heo, Christoph Lameter, Andrew Morton,
loongarch, linux-kernel, linux-mm, WANG Xuerui
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
> >>
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] LoongArch: mm: Add unified function populate_kernel_pte
2023-08-02 7:25 ` Huacai Chen
@ 2023-08-10 4:08 ` bibo mao
2023-08-10 4:27 ` Huacai Chen
0 siblings, 1 reply; 16+ messages in thread
From: bibo mao @ 2023-08-10 4:08 UTC (permalink / raw)
To: Huacai Chen
Cc: Dennis Zhou, Tejun Heo, Christoph Lameter, Andrew Morton,
loongarch, linux-kernel, linux-mm, WANG Xuerui
在 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
>>>>
>>
>>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] LoongArch: mm: Add unified function populate_kernel_pte
2023-08-10 4:08 ` bibo mao
@ 2023-08-10 4:27 ` Huacai Chen
2023-08-10 4:42 ` bibo mao
0 siblings, 1 reply; 16+ messages in thread
From: Huacai Chen @ 2023-08-10 4:27 UTC (permalink / raw)
To: bibo mao
Cc: Dennis Zhou, Tejun Heo, Christoph Lameter, Andrew Morton,
loongarch, linux-kernel, linux-mm, WANG Xuerui
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
> >>>>
> >>
> >>
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] LoongArch: mm: Add unified function populate_kernel_pte
2023-08-10 4:27 ` Huacai Chen
@ 2023-08-10 4:42 ` bibo mao
0 siblings, 0 replies; 16+ messages in thread
From: bibo mao @ 2023-08-10 4:42 UTC (permalink / raw)
To: Huacai Chen
Cc: Dennis Zhou, Tejun Heo, Christoph Lameter, Andrew Morton,
loongarch, linux-kernel, linux-mm, WANG Xuerui
在 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
>>>>>>
>>>>
>>>>
>>
>>
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2023-08-10 4:43 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-12 3:16 [PATCH 0/3] LoongArch: mm: Code cleanup with populate pte Bibo Mao
2023-07-12 3:16 ` [PATCH 1/3] mm/percpu: Remove some local variables in pcpu_populate_pte Bibo Mao
2023-07-27 23:08 ` Dennis Zhou
2023-07-28 1:13 ` bibo mao
2023-07-12 3:16 ` [PATCH 2/3] LoongArch: Code cleanup in function pcpu_populate_pte Bibo Mao
2023-07-31 14:15 ` Huacai Chen
2023-08-01 1:18 ` bibo mao
2023-07-12 3:16 ` [PATCH 3/3] LoongArch: mm: Add unified function populate_kernel_pte Bibo Mao
2023-07-31 14:15 ` Huacai Chen
2023-08-01 1:22 ` bibo mao
2023-08-02 7:25 ` Huacai Chen
2023-08-10 4:08 ` bibo mao
2023-08-10 4:27 ` Huacai Chen
2023-08-10 4:42 ` bibo mao
2023-07-25 0:36 ` [PATCH 0/3] LoongArch: mm: Code cleanup with populate pte bibo mao
2023-07-25 2:33 ` Dennis Zhou
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).