2020-03-30 11:55:01

by Chuanhua Han

Subject: [PATCH] riscv: mm: Remove the copy operation of pmd

Since all processes share the kernel address space,
we only need to copy pgd in case of a vmalloc page
fault exception, the other levels of page tables are
shared, so the operation of copying pmd is unnecessary.

Signed-off-by: Chuanhua Han <[email protected]>
---
arch/riscv/mm/fault.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
index be84e32adc4c..24f4ebfd2df8 100644
--- a/arch/riscv/mm/fault.c
+++ b/arch/riscv/mm/fault.c
@@ -208,9 +208,9 @@ asmlinkage void do_page_fault(struct pt_regs *regs)
vmalloc_fault:
{
pgd_t *pgd, *pgd_k;
- pud_t *pud, *pud_k;
- p4d_t *p4d, *p4d_k;
- pmd_t *pmd, *pmd_k;
+ pud_t *pud_k;
+ p4d_t *p4d_k;
+ pmd_t *pmd_k;
pte_t *pte_k;
int index;

@@ -234,12 +234,10 @@ asmlinkage void do_page_fault(struct pt_regs *regs)
goto no_context;
set_pgd(pgd, *pgd_k);

- p4d = p4d_offset(pgd, addr);
p4d_k = p4d_offset(pgd_k, addr);
if (!p4d_present(*p4d_k))
goto no_context;

- pud = pud_offset(p4d, addr);
pud_k = pud_offset(p4d_k, addr);
if (!pud_present(*pud_k))
goto no_context;
@@ -248,11 +246,9 @@ asmlinkage void do_page_fault(struct pt_regs *regs)
* Since the vmalloc area is global, it is unnecessary
* to copy individual PTEs
*/
- pmd = pmd_offset(pud, addr);
pmd_k = pmd_offset(pud_k, addr);
if (!pmd_present(*pmd_k))
goto no_context;
- set_pmd(pmd, *pmd_k);

/*
* Make sure the actual PTE exists as well to
--
2.17.1

2021-02-24 13:18:35

by Alexandre Ghiti

Subject: Re: [PATCH] riscv: mm: Remove the copy operation of pmd

Le 3/30/20 ? 7:53 AM, Chuanhua Han a ?crit?:
> Since all processes share the kernel address space,
> we only need to copy pgd in case of a vmalloc page
> fault exception, the other levels of page tables are
> shared, so the operation of copying pmd is unnecessary.
>
> Signed-off-by: Chuanhua Han <[email protected]>
> ---
> arch/riscv/mm/fault.c | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
> index be84e32adc4c..24f4ebfd2df8 100644
> --- a/arch/riscv/mm/fault.c
> +++ b/arch/riscv/mm/fault.c
> @@ -208,9 +208,9 @@ asmlinkage void do_page_fault(struct pt_regs *regs)
> vmalloc_fault:
> {
> pgd_t *pgd, *pgd_k;
> - pud_t *pud, *pud_k;
> - p4d_t *p4d, *p4d_k;
> - pmd_t *pmd, *pmd_k;
> + pud_t *pud_k;
> + p4d_t *p4d_k;
> + pmd_t *pmd_k;
> pte_t *pte_k;
> int index;
>
> @@ -234,12 +234,10 @@ asmlinkage void do_page_fault(struct pt_regs *regs)
> goto no_context;
> set_pgd(pgd, *pgd_k);
>
> - p4d = p4d_offset(pgd, addr);
> p4d_k = p4d_offset(pgd_k, addr);
> if (!p4d_present(*p4d_k))
> goto no_context;
>
> - pud = pud_offset(p4d, addr);
> pud_k = pud_offset(p4d_k, addr);
> if (!pud_present(*pud_k))
> goto no_context;
> @@ -248,11 +246,9 @@ asmlinkage void do_page_fault(struct pt_regs *regs)
> * Since the vmalloc area is global, it is unnecessary
> * to copy individual PTEs
> */
> - pmd = pmd_offset(pud, addr);
> pmd_k = pmd_offset(pud_k, addr);
> if (!pmd_present(*pmd_k))
> goto no_context;
> - set_pmd(pmd, *pmd_k);
>
> /*
> * Make sure the actual PTE exists as well to
>

Better late than never: I do agree with this patch, once the PGD is
copied into the user page table, it "comes with all its mappings" so
there is no need to copy PMD, the only thing left to do is to make sure
the mapping does exists in the kernel page table.

So feel free to add:

Reviewed-by: Alexandre Ghiti <[email protected]>
Tested-by: Alexandre Ghiti <[email protected]>

Thanks,

Alex