Re: [PATCH V3 2/5] mm: update ptep_modify_prot_commit to take old pte value as arg

On 12/5/18 9:32 AM, Christophe LEROY wrote:

Le 05/12/2018 à 04:09, Aneesh Kumar K.V a écrit :

quoted

Architectures like ppc64 requires to do a conditional tlb flush based on the old
and new value of pte. Enable that by passing old pte value as the arg.

Signed-off-by: Aneesh Kumar K.V <redacted>
---
  arch/s390/include/asm/pgtable.h | 3 ++-
  arch/s390/mm/pgtable.c          | 2 +-
  arch/x86/include/asm/paravirt.h | 2 +-
  fs/proc/task_mmu.c              | 8 +++++---
  include/asm-generic/pgtable.h   | 2 +-
  mm/memory.c                     | 8 ++++----
  mm/mprotect.c                   | 6 +++---
  7 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/arch/s390/include/asm/pgtable.h b/arch/s390/include/asm/pgtable.h
index 5d730199e37b..76dc344edb8c 100644
--- a/arch/s390/include/asm/pgtable.h
+++ b/arch/s390/include/asm/pgtable.h

@@ -1070,7 +1070,8 @@ static inline pte_t ptep_get_and_clear(struct mm_struct *mm,

  #define __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION
  pte_t ptep_modify_prot_start(struct vm_area_struct *, unsigned long, pte_t *);
-void ptep_modify_prot_commit(struct vm_area_struct *, unsigned long, pte_t *, pte_t);
+void ptep_modify_prot_commit(struct vm_area_struct *, unsigned long,
+                 pte_t *, pte_t, pte_t);
  #define __HAVE_ARCH_PTEP_CLEAR_FLUSH
  static inline pte_t ptep_clear_flush(struct vm_area_struct *vma,

diff --git a/arch/s390/mm/pgtable.c b/arch/s390/mm/pgtable.c
index 29c0a21cd34a..b283b92722cc 100644
--- a/arch/s390/mm/pgtable.c
+++ b/arch/s390/mm/pgtable.c

@@ -322,7 +322,7 @@ pte_t ptep_modify_prot_start(struct vm_area_struct *vma, unsigned long addr,

  EXPORT_SYMBOL(ptep_modify_prot_start);
  void ptep_modify_prot_commit(struct vm_area_struct *vma, unsigned long addr,
-                 pte_t *ptep, pte_t pte)
+                 pte_t *ptep, pte_t old_pte, pte_t pte)
  {
      pgste_t pgste;
      struct mm_struct *mm = vma->vm_mm;

diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index 1154f154025d..0d75a4f60500 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h

@@ -429,7 +429,7 @@ static inline pte_t ptep_modify_prot_start(struct vm_area_struct *vma, unsigned

  }
  static inline void ptep_modify_prot_commit(struct vm_area_struct *vma, unsigned long addr,
-                       pte_t *ptep, pte_t pte)
+                       pte_t *ptep, pte_t old_pte, pte_t pte)
  {
      struct mm_struct *mm = vma->vm_mm;

diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 9952d7185170..8d62891d38a8 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c

@@ -940,10 +940,12 @@ static inline void clear_soft_dirty(struct vm_area_struct *vma,

      pte_t ptent = *pte;
      if (pte_present(ptent)) {
-        ptent = ptep_modify_prot_start(vma, addr, pte);
-        ptent = pte_wrprotect(ptent);
+        pte_t old_pte;
+
+        old_pte = ptep_modify_prot_start(vma, addr, pte);
+        ptent = pte_wrprotect(old_pte);

This change doesn't seem to fit with the commit description. Why write protecting in addition to clearing dirty ?

The hunk above use a new variable old_pte. There is no functional change in that hunk.

-aneesh