diff mbox series

riscv: mm: Remove the copy operation of pmd

Message ID 20220127100622.11856-1-hanchuanhua@oppo.com (mailing list archive)
State New, archived
Headers show
Series riscv: mm: Remove the copy operation of pmd | expand

Commit Message

Chuanhua Han Jan. 27, 2022, 10:06 a.m. UTC
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: hanchuanhua <hanchuanhua@oppo.com>
---
 arch/riscv/mm/fault.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

--
2.7.4

Comments

Palmer Dabbelt Feb. 22, 2022, 9:25 p.m. UTC | #1
On Thu, 27 Jan 2022 02:06:22 PST (-0800), hanchuanhua@oppo.com wrote:
> 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: hanchuanhua <hanchuanhua@oppo.com>
> ---
>  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 4e9efbe..40694f0 100644
> --- a/arch/riscv/mm/fault.c
> +++ b/arch/riscv/mm/fault.c
> @@ -102,9 +102,9 @@ static inline void bad_area(struct pt_regs *regs, struct mm_struct *mm, int code
>  static inline void vmalloc_fault(struct pt_regs *regs, int code, unsigned long addr)
>  {
>         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;
>         unsigned long pfn;
> @@ -132,14 +132,12 @@ static inline void vmalloc_fault(struct pt_regs *regs, int code, unsigned long a
>         }
>         set_pgd(pgd, *pgd_k);
>
> -       p4d = p4d_offset(pgd, addr);
>         p4d_k = p4d_offset(pgd_k, addr);
>         if (!p4d_present(*p4d_k)) {
>                 no_context(regs, addr);
>                 return;
>         }
>
> -       pud = pud_offset(p4d, addr);
>         pud_k = pud_offset(p4d_k, addr);
>         if (!pud_present(*pud_k)) {
>                 no_context(regs, addr);
> @@ -150,13 +148,11 @@ static inline void vmalloc_fault(struct pt_regs *regs, int code, unsigned long a
>          * 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)) {
>                 no_context(regs, addr);
>                 return;
>         }
> -       set_pmd(pmd, *pmd_k);
>
>         /*
>          * Make sure the actual PTE exists as well to

This looks good, but "Signed off by" lines are supposed to have real 
names associated with them and this looks like a handle to me.  There 
also seems to be something broken with the email, as this is causing git 
to blow up, but for a small patch like this that's OK as I can resurrect 
it by hand.

Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com> # aside from the name

Please send a v2 with the proper Signed off by line, as that's not 
something I'm supposed to just fix up myself.

Thanks!
Chuanhua Han Feb. 23, 2022, 2:58 a.m. UTC | #2
On 2022/2/23 5:25, Palmer Dabbelt wrote:
> On Thu, 27 Jan 2022 02:06:22 PST (-0800), hanchuanhua@oppo.com wrote:
>> 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: hanchuanhua <hanchuanhua@oppo.com>
>> ---
>>   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 4e9efbe..40694f0 100644
>> --- a/arch/riscv/mm/fault.c
>> +++ b/arch/riscv/mm/fault.c
>> @@ -102,9 +102,9 @@ static inline void bad_area(struct pt_regs *regs, struct mm_struct *mm, int code
>>   static inline void vmalloc_fault(struct pt_regs *regs, int code, unsigned long addr)
>>   {
>>          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;
>>          unsigned long pfn;
>> @@ -132,14 +132,12 @@ static inline void vmalloc_fault(struct pt_regs *regs, int code, unsigned long a
>>          }
>>          set_pgd(pgd, *pgd_k);
>>
>> -       p4d = p4d_offset(pgd, addr);
>>          p4d_k = p4d_offset(pgd_k, addr);
>>          if (!p4d_present(*p4d_k)) {
>>                  no_context(regs, addr);
>>                  return;
>>          }
>>
>> -       pud = pud_offset(p4d, addr);
>>          pud_k = pud_offset(p4d_k, addr);
>>          if (!pud_present(*pud_k)) {
>>                  no_context(regs, addr);
>> @@ -150,13 +148,11 @@ static inline void vmalloc_fault(struct pt_regs *regs, int code, unsigned long a
>>           * 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)) {
>>                  no_context(regs, addr);
>>                  return;
>>          }
>> -       set_pmd(pmd, *pmd_k);
>>
>>          /*
>>           * Make sure the actual PTE exists as well to
> This looks good, but "Signed off by" lines are supposed to have real
> names associated with them and this looks like a handle to me.  There
> also seems to be something broken with the email, as this is causing git
> to blow up, but for a small patch like this that's OK as I can resurrect
> it by hand.
>
> Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com> # aside from the name
>
> Please send a v2 with the proper Signed off by line, as that's not
> something I'm supposed to just fix up myself.
>
> Thanks!

First of all, thank you for your review and reply. I don't quite
understand what's wrong with the line "Signed off by"?

Thanks!
diff mbox series

Patch

diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
index 4e9efbe..40694f0 100644
--- a/arch/riscv/mm/fault.c
+++ b/arch/riscv/mm/fault.c
@@ -102,9 +102,9 @@  static inline void bad_area(struct pt_regs *regs, struct mm_struct *mm, int code
 static inline void vmalloc_fault(struct pt_regs *regs, int code, unsigned long addr)
 {
        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;
        unsigned long pfn;
@@ -132,14 +132,12 @@  static inline void vmalloc_fault(struct pt_regs *regs, int code, unsigned long a
        }
        set_pgd(pgd, *pgd_k);

-       p4d = p4d_offset(pgd, addr);
        p4d_k = p4d_offset(pgd_k, addr);
        if (!p4d_present(*p4d_k)) {
                no_context(regs, addr);
                return;
        }

-       pud = pud_offset(p4d, addr);
        pud_k = pud_offset(p4d_k, addr);
        if (!pud_present(*pud_k)) {
                no_context(regs, addr);
@@ -150,13 +148,11 @@  static inline void vmalloc_fault(struct pt_regs *regs, int code, unsigned long a
         * 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)) {
                no_context(regs, addr);
                return;
        }
-       set_pmd(pmd, *pmd_k);

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