diff mbox series

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

Message ID 20181205030931.12037-3-aneesh.kumar@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series NestMMU pte upgrade workaround for mprotect | expand

Commit Message

Aneesh Kumar K.V Dec. 5, 2018, 3:09 a.m. UTC
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 <aneesh.kumar@linux.ibm.com>
---
 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(-)

Comments

Christophe Leroy Dec. 5, 2018, 4:02 a.m. UTC | #1
Le 05/12/2018 à 04:09, Aneesh Kumar K.V a écrit :
> 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 <aneesh.kumar@linux.ibm.com>
> ---
>   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 ?

Christophe

>   		ptent = pte_clear_soft_dirty(ptent);
> -		ptep_modify_prot_commit(vma, addr, pte, ptent);
> +		ptep_modify_prot_commit(vma, addr, pte, old_pte, ptent);
>   	} else if (is_swap_pte(ptent)) {
>   		ptent = pte_swp_clear_soft_dirty(ptent);
>   		set_pte_at(vma->vm_mm, addr, pte, ptent);
> diff --git a/include/asm-generic/pgtable.h b/include/asm-generic/pgtable.h
> index c9897dcc46c4..37039e918f17 100644
> --- a/include/asm-generic/pgtable.h
> +++ b/include/asm-generic/pgtable.h
> @@ -619,7 +619,7 @@ static inline pte_t ptep_modify_prot_start(struct vm_area_struct *vma,
>    */
>   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)
>   {
>   	__ptep_modify_prot_commit(vma->vm_mm, addr, ptep, pte);
>   }
> diff --git a/mm/memory.c b/mm/memory.c
> index d36b0eaa7862..4f3ddaedc764 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -3568,7 +3568,7 @@ static vm_fault_t do_numa_page(struct vm_fault *vmf)
>   	int last_cpupid;
>   	int target_nid;
>   	bool migrated = false;
> -	pte_t pte;
> +	pte_t pte, old_pte;
>   	bool was_writable = pte_savedwrite(vmf->orig_pte);
>   	int flags = 0;
>   
> @@ -3588,12 +3588,12 @@ static vm_fault_t do_numa_page(struct vm_fault *vmf)
>   	 * Make it present again, Depending on how arch implementes non
>   	 * accessible ptes, some can allow access by kernel mode.
>   	 */
> -	pte = ptep_modify_prot_start(vma, vmf->address, vmf->pte);
> -	pte = pte_modify(pte, vma->vm_page_prot);
> +	old_pte = ptep_modify_prot_start(vma, vmf->address, vmf->pte);
> +	pte = pte_modify(old_pte, vma->vm_page_prot);
>   	pte = pte_mkyoung(pte);
>   	if (was_writable)
>   		pte = pte_mkwrite(pte);
> -	ptep_modify_prot_commit(vma, vmf->address, vmf->pte, pte);
> +	ptep_modify_prot_commit(vma, vmf->address, vmf->pte, old_pte, pte);
>   	update_mmu_cache(vma, vmf->address, vmf->pte);
>   
>   	page = vm_normal_page(vma, vmf->address, pte);
> diff --git a/mm/mprotect.c b/mm/mprotect.c
> index a301d4c83d3c..1b46b1b1248d 100644
> --- a/mm/mprotect.c
> +++ b/mm/mprotect.c
> @@ -110,8 +110,8 @@ static unsigned long change_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
>   					continue;
>   			}
>   
> -			ptent = ptep_modify_prot_start(vma, addr, pte);
> -			ptent = pte_modify(ptent, newprot);
> +			oldpte = ptep_modify_prot_start(vma, addr, pte);
> +			ptent = pte_modify(oldpte, newprot);
>   			if (preserve_write)
>   				ptent = pte_mk_savedwrite(ptent);
>   
> @@ -121,7 +121,7 @@ static unsigned long change_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
>   					 !(vma->vm_flags & VM_SOFTDIRTY))) {
>   				ptent = pte_mkwrite(ptent);
>   			}
> -			ptep_modify_prot_commit(vma, addr, pte, ptent);
> +			ptep_modify_prot_commit(vma, addr, pte, oldpte, ptent);
>   			pages++;
>   		} else if (IS_ENABLED(CONFIG_MIGRATION)) {
>   			swp_entry_t entry = pte_to_swp_entry(oldpte);
>
Aneesh Kumar K.V Dec. 5, 2018, 4:06 a.m. UTC | #2
On 12/5/18 9:32 AM, Christophe LEROY wrote:
> 
> 
> Le 05/12/2018 à 04:09, Aneesh Kumar K.V a écrit :
>> 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 <aneesh.kumar@linux.ibm.com>
>> ---
>>   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
Christophe Leroy Dec. 5, 2018, 4:42 a.m. UTC | #3
Le 05/12/2018 à 05:06, Aneesh Kumar K.V a écrit :
> On 12/5/18 9:32 AM, Christophe LEROY wrote:
>>
>>
>> Le 05/12/2018 à 04:09, Aneesh Kumar K.V a écrit :
>>> 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 <aneesh.kumar@linux.ibm.com>
>>> ---
>>>   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.
> 

Oops, sorry, I misread the patch, don't know why.

Christophe
diff mbox series

Patch

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);
 		ptent = pte_clear_soft_dirty(ptent);
-		ptep_modify_prot_commit(vma, addr, pte, ptent);
+		ptep_modify_prot_commit(vma, addr, pte, old_pte, ptent);
 	} else if (is_swap_pte(ptent)) {
 		ptent = pte_swp_clear_soft_dirty(ptent);
 		set_pte_at(vma->vm_mm, addr, pte, ptent);
diff --git a/include/asm-generic/pgtable.h b/include/asm-generic/pgtable.h
index c9897dcc46c4..37039e918f17 100644
--- a/include/asm-generic/pgtable.h
+++ b/include/asm-generic/pgtable.h
@@ -619,7 +619,7 @@  static inline pte_t ptep_modify_prot_start(struct vm_area_struct *vma,
  */
 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)
 {
 	__ptep_modify_prot_commit(vma->vm_mm, addr, ptep, pte);
 }
diff --git a/mm/memory.c b/mm/memory.c
index d36b0eaa7862..4f3ddaedc764 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3568,7 +3568,7 @@  static vm_fault_t do_numa_page(struct vm_fault *vmf)
 	int last_cpupid;
 	int target_nid;
 	bool migrated = false;
-	pte_t pte;
+	pte_t pte, old_pte;
 	bool was_writable = pte_savedwrite(vmf->orig_pte);
 	int flags = 0;
 
@@ -3588,12 +3588,12 @@  static vm_fault_t do_numa_page(struct vm_fault *vmf)
 	 * Make it present again, Depending on how arch implementes non
 	 * accessible ptes, some can allow access by kernel mode.
 	 */
-	pte = ptep_modify_prot_start(vma, vmf->address, vmf->pte);
-	pte = pte_modify(pte, vma->vm_page_prot);
+	old_pte = ptep_modify_prot_start(vma, vmf->address, vmf->pte);
+	pte = pte_modify(old_pte, vma->vm_page_prot);
 	pte = pte_mkyoung(pte);
 	if (was_writable)
 		pte = pte_mkwrite(pte);
-	ptep_modify_prot_commit(vma, vmf->address, vmf->pte, pte);
+	ptep_modify_prot_commit(vma, vmf->address, vmf->pte, old_pte, pte);
 	update_mmu_cache(vma, vmf->address, vmf->pte);
 
 	page = vm_normal_page(vma, vmf->address, pte);
diff --git a/mm/mprotect.c b/mm/mprotect.c
index a301d4c83d3c..1b46b1b1248d 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -110,8 +110,8 @@  static unsigned long change_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
 					continue;
 			}
 
-			ptent = ptep_modify_prot_start(vma, addr, pte);
-			ptent = pte_modify(ptent, newprot);
+			oldpte = ptep_modify_prot_start(vma, addr, pte);
+			ptent = pte_modify(oldpte, newprot);
 			if (preserve_write)
 				ptent = pte_mk_savedwrite(ptent);
 
@@ -121,7 +121,7 @@  static unsigned long change_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
 					 !(vma->vm_flags & VM_SOFTDIRTY))) {
 				ptent = pte_mkwrite(ptent);
 			}
-			ptep_modify_prot_commit(vma, addr, pte, ptent);
+			ptep_modify_prot_commit(vma, addr, pte, oldpte, ptent);
 			pages++;
 		} else if (IS_ENABLED(CONFIG_MIGRATION)) {
 			swp_entry_t entry = pte_to_swp_entry(oldpte);