arch/loongarch/include/asm/pgtable.h | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-)
In the current pte_modify operation, _PAGE_DIRTY might be cleared. Since
the hardware-page-walk does not have a predefined _PAGE_MODIFIED flag,
this could lead to loss of valid data in certain scenarios.
The new modification involves checking whether the original PTE has the
_PAGE_DIRTY flag. If it exists, the _PAGE_MODIFIED bit is set, ensuring
that the pte_dirty interface can return accurate information.
Co-developed-by: Liupu Wang <wangliupu@loongson.cn>
Signed-off-by: Liupu Wang <wangliupu@loongson.cn>
Signed-off-by: Tianyang Zhang <zhangtianyang@loongson.cn>
---
arch/loongarch/include/asm/pgtable.h | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/arch/loongarch/include/asm/pgtable.h b/arch/loongarch/include/asm/pgtable.h
index bd128696e96d..106abfa5183b 100644
--- a/arch/loongarch/include/asm/pgtable.h
+++ b/arch/loongarch/include/asm/pgtable.h
@@ -424,8 +424,13 @@ static inline unsigned long pte_accessible(struct mm_struct *mm, pte_t a)
static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
{
- return __pte((pte_val(pte) & _PAGE_CHG_MASK) |
- (pgprot_val(newprot) & ~_PAGE_CHG_MASK));
+ unsigned long val = (pte_val(pte) & _PAGE_CHG_MASK) |
+ (pgprot_val(newprot) & ~_PAGE_CHG_MASK);
+
+ if (pte_val(pte) & _PAGE_DIRTY)
+ val |= _PAGE_MODIFIED;
+
+ return __pte(val);
}
extern void __update_tlb(struct vm_area_struct *vma,
@@ -547,9 +552,13 @@ static inline struct page *pmd_page(pmd_t pmd)
static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
{
- pmd_val(pmd) = (pmd_val(pmd) & _HPAGE_CHG_MASK) |
+ unsigned long val = (pmd_val(pmd) & _HPAGE_CHG_MASK) |
(pgprot_val(newprot) & ~_HPAGE_CHG_MASK);
- return pmd;
+
+ if (pmd_val(pmd) & _PAGE_DIRTY)
+ val |= _PAGE_MODIFIED;
+
+ return __pmd(val);
}
static inline pmd_t pmd_mkinvalid(pmd_t pmd)
--
2.41.0
On 2025/11/4 下午3:30, Tianyang Zhang wrote:
> In the current pte_modify operation, _PAGE_DIRTY might be cleared. Since
> the hardware-page-walk does not have a predefined _PAGE_MODIFIED flag,
> this could lead to loss of valid data in certain scenarios.
>
> The new modification involves checking whether the original PTE has the
> _PAGE_DIRTY flag. If it exists, the _PAGE_MODIFIED bit is set, ensuring
> that the pte_dirty interface can return accurate information.
>
> Co-developed-by: Liupu Wang <wangliupu@loongson.cn>
> Signed-off-by: Liupu Wang <wangliupu@loongson.cn>
> Signed-off-by: Tianyang Zhang <zhangtianyang@loongson.cn>
> ---
> arch/loongarch/include/asm/pgtable.h | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/arch/loongarch/include/asm/pgtable.h b/arch/loongarch/include/asm/pgtable.h
> index bd128696e96d..106abfa5183b 100644
> --- a/arch/loongarch/include/asm/pgtable.h
> +++ b/arch/loongarch/include/asm/pgtable.h
> @@ -424,8 +424,13 @@ static inline unsigned long pte_accessible(struct mm_struct *mm, pte_t a)
>
> static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
> {
> - return __pte((pte_val(pte) & _PAGE_CHG_MASK) |
> - (pgprot_val(newprot) & ~_PAGE_CHG_MASK));
> + unsigned long val = (pte_val(pte) & _PAGE_CHG_MASK) |
> + (pgprot_val(newprot) & ~_PAGE_CHG_MASK);
> +
> + if (pte_val(pte) & _PAGE_DIRTY)
> + val |= _PAGE_MODIFIED;
Since ptep_get_and_clear() is not atomic operation on LoongArch like
other architectures, considering this scenery with HW PTW enabled:
CPU 0: CPU1
old_pte = ptep_modify_prot_start();
old_pte = ptep_get(ptep);
write(buf);
*HW will set _PAGE_DIRTY bit*
pte_clear(mm, address, ptep);
^^^^^^^^^^ For CPU1, bit _PAGE_DIRTY is no set in old_pte, _PAGE_DIRTY
will be lost also. ^^^^^^^^^^^
pte = pte_modify(old_pte,)
ptep_modify_prot_commit(.., pte)
Regards
Bibo Mao
> +
> + return __pte(val);
> }
>
> extern void __update_tlb(struct vm_area_struct *vma,
> @@ -547,9 +552,13 @@ static inline struct page *pmd_page(pmd_t pmd)
>
> static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
> {
> - pmd_val(pmd) = (pmd_val(pmd) & _HPAGE_CHG_MASK) |
> + unsigned long val = (pmd_val(pmd) & _HPAGE_CHG_MASK) |
> (pgprot_val(newprot) & ~_HPAGE_CHG_MASK);
> - return pmd;
> +
> + if (pmd_val(pmd) & _PAGE_DIRTY)
> + val |= _PAGE_MODIFIED;
> +
> + return __pmd(val);
> }
>
> static inline pmd_t pmd_mkinvalid(pmd_t pmd)
>
在 2025/11/6 下午3:07, Bibo Mao 写道:
>
>
> On 2025/11/4 下午3:30, Tianyang Zhang wrote:
>> In the current pte_modify operation, _PAGE_DIRTY might be cleared. Since
>> the hardware-page-walk does not have a predefined _PAGE_MODIFIED flag,
>> this could lead to loss of valid data in certain scenarios.
>>
>> The new modification involves checking whether the original PTE has the
>> _PAGE_DIRTY flag. If it exists, the _PAGE_MODIFIED bit is set, ensuring
>> that the pte_dirty interface can return accurate information.
>>
>> Co-developed-by: Liupu Wang <wangliupu@loongson.cn>
>> Signed-off-by: Liupu Wang <wangliupu@loongson.cn>
>> Signed-off-by: Tianyang Zhang <zhangtianyang@loongson.cn>
>> ---
>> arch/loongarch/include/asm/pgtable.h | 17 +++++++++++++----
>> 1 file changed, 13 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/loongarch/include/asm/pgtable.h
>> b/arch/loongarch/include/asm/pgtable.h
>> index bd128696e96d..106abfa5183b 100644
>> --- a/arch/loongarch/include/asm/pgtable.h
>> +++ b/arch/loongarch/include/asm/pgtable.h
>> @@ -424,8 +424,13 @@ static inline unsigned long
>> pte_accessible(struct mm_struct *mm, pte_t a)
>> static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
>> {
>> - return __pte((pte_val(pte) & _PAGE_CHG_MASK) |
>> - (pgprot_val(newprot) & ~_PAGE_CHG_MASK));
>> + unsigned long val = (pte_val(pte) & _PAGE_CHG_MASK) |
>> + (pgprot_val(newprot) & ~_PAGE_CHG_MASK);
>> +
>> + if (pte_val(pte) & _PAGE_DIRTY)
>> + val |= _PAGE_MODIFIED;
> Since ptep_get_and_clear() is not atomic operation on LoongArch like
> other architectures, considering this scenery with HW PTW enabled:
> CPU 0: CPU1
> old_pte = ptep_modify_prot_start();
> old_pte = ptep_get(ptep);
>
> write(buf);
> *HW will set _PAGE_DIRTY bit*
> pte_clear(mm, address, ptep);
> ^^^^^^^^^^ For CPU1, bit _PAGE_DIRTY is no set in old_pte, _PAGE_DIRTY
> will be lost also. ^^^^^^^^^^^
> pte = pte_modify(old_pte,)
> ptep_modify_prot_commit(.., pte)
There does appear to be an issue here. It seems we should use
`__HAVE_ARCH_PTEP_GET_AND_CLEAR` and implement ptep_get_and_clear via
`atomic_xchg`.
However, I believe this change should be submitted in a new patch.
Thanks
Tianyang
>
> Regards
> Bibo Mao
>> +
>> + return __pte(val);
>> }
>> extern void __update_tlb(struct vm_area_struct *vma,
>> @@ -547,9 +552,13 @@ static inline struct page *pmd_page(pmd_t pmd)
>> static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
>> {
>> - pmd_val(pmd) = (pmd_val(pmd) & _HPAGE_CHG_MASK) |
>> + unsigned long val = (pmd_val(pmd) & _HPAGE_CHG_MASK) |
>> (pgprot_val(newprot) & ~_HPAGE_CHG_MASK);
>> - return pmd;
>> +
>> + if (pmd_val(pmd) & _PAGE_DIRTY)
>> + val |= _PAGE_MODIFIED;
>> +
>> + return __pmd(val);
>> }
>> static inline pmd_t pmd_mkinvalid(pmd_t pmd)
>>
Hi, Tianyang,
The subject line can be:
LoongArch: Let {pte,pmd}_modify() record the status of _PAGE_DIRTY (If
I'm right in the later comments).
On Tue, Nov 4, 2025 at 3:30 PM Tianyang Zhang <zhangtianyang@loongson.cn> wrote:
>
> In the current pte_modify operation, _PAGE_DIRTY might be cleared. Since
> the hardware-page-walk does not have a predefined _PAGE_MODIFIED flag,
> this could lead to loss of valid data in certain scenarios.
>
> The new modification involves checking whether the original PTE has the
> _PAGE_DIRTY flag. If it exists, the _PAGE_MODIFIED bit is set, ensuring
> that the pte_dirty interface can return accurate information.
The description may be wrong here. Because pte_dirty() returns
pte_val(pte) & (_PAGE_DIRTY | _PAGE_MODIFIED).
If _PAGE_DIRTY isn't lost, pte_dirty() is always right, no matter
whether there is or isn't _PAGE_MODIFIED.
I think the real reason is we need to set _PAGE_MODIFIED in
pte/pmd_modify to record the status of _PAGE_DIRTY, so that we can
recover _PAGE_DIRTY afterwards, such as in pte/pmd_mkwrite().
>
> Co-developed-by: Liupu Wang <wangliupu@loongson.cn>
> Signed-off-by: Liupu Wang <wangliupu@loongson.cn>
> Signed-off-by: Tianyang Zhang <zhangtianyang@loongson.cn>
> ---
> arch/loongarch/include/asm/pgtable.h | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/arch/loongarch/include/asm/pgtable.h b/arch/loongarch/include/asm/pgtable.h
> index bd128696e96d..106abfa5183b 100644
> --- a/arch/loongarch/include/asm/pgtable.h
> +++ b/arch/loongarch/include/asm/pgtable.h
> @@ -424,8 +424,13 @@ static inline unsigned long pte_accessible(struct mm_struct *mm, pte_t a)
>
> static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
> {
> - return __pte((pte_val(pte) & _PAGE_CHG_MASK) |
> - (pgprot_val(newprot) & ~_PAGE_CHG_MASK));
> + unsigned long val = (pte_val(pte) & _PAGE_CHG_MASK) |
> + (pgprot_val(newprot) & ~_PAGE_CHG_MASK);
> +
> + if (pte_val(pte) & _PAGE_DIRTY)
> + val |= _PAGE_MODIFIED;
> +
> + return __pte(val);
> }
>
> extern void __update_tlb(struct vm_area_struct *vma,
> @@ -547,9 +552,13 @@ static inline struct page *pmd_page(pmd_t pmd)
>
> static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
> {
> - pmd_val(pmd) = (pmd_val(pmd) & _HPAGE_CHG_MASK) |
> + unsigned long val = (pmd_val(pmd) & _HPAGE_CHG_MASK) |
> (pgprot_val(newprot) & ~_HPAGE_CHG_MASK);
> - return pmd;
> +
> + if (pmd_val(pmd) & _PAGE_DIRTY)
> + val |= _PAGE_MODIFIED;
> +
> + return __pmd(val);
> }
A minimal modification can be:
diff --git a/arch/loongarch/include/asm/pgtable.h
b/arch/loongarch/include/asm/pgtable.h
index 1f20e9280062..907ece0199e0 100644
--- a/arch/loongarch/include/asm/pgtable.h
+++ b/arch/loongarch/include/asm/pgtable.h
@@ -448,8 +448,13 @@ static inline unsigned long pte_accessible(struct
mm_struct *mm, pte_t a)
static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
{
- return __pte((pte_val(pte) & _PAGE_CHG_MASK) |
- (pgprot_val(newprot) & ~_PAGE_CHG_MASK));
+ pte_val(pte) = (pte_val(pte) & _PAGE_CHG_MASK) |
+ (pgprot_val(newprot) & ~_PAGE_CHG_MASK);
+
+ if (pte_val(pte) & _PAGE_DIRTY)
+ pte_val(pte) |= _PAGE_MODIFIED;
+
+ return pte;
}
extern void __update_tlb(struct vm_area_struct *vma,
@@ -583,7 +588,11 @@ static inline struct page *pmd_page(pmd_t pmd)
static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
{
pmd_val(pmd) = (pmd_val(pmd) & _HPAGE_CHG_MASK) |
- (pgprot_val(newprot) & ~_HPAGE_CHG_MASK);
+ (pgprot_val(newprot) & ~_HPAGE_CHG_MASK);
+
+ if (pmd_val(pmd) & _PAGE_DIRTY)
+ pmd_val(pmd) |= _PAGE_MODIFIED;
+
return pmd;
}
You needn't define a new variable.
Huacai
>
> static inline pmd_t pmd_mkinvalid(pmd_t pmd)
> --
> 2.41.0
>
>
Hi, Huacai
在 2025/11/4 下午4:00, Huacai Chen 写道:
> Hi, Tianyang,
>
> The subject line can be:
> LoongArch: Let {pte,pmd}_modify() record the status of _PAGE_DIRTY (If
> I'm right in the later comments).
Ok. I got it
>
> On Tue, Nov 4, 2025 at 3:30 PM Tianyang Zhang <zhangtianyang@loongson.cn> wrote:
>> In the current pte_modify operation, _PAGE_DIRTY might be cleared. Since
>> the hardware-page-walk does not have a predefined _PAGE_MODIFIED flag,
>> this could lead to loss of valid data in certain scenarios.
>>
>> The new modification involves checking whether the original PTE has the
>> _PAGE_DIRTY flag. If it exists, the _PAGE_MODIFIED bit is set, ensuring
>> that the pte_dirty interface can return accurate information.
> The description may be wrong here. Because pte_dirty() returns
> pte_val(pte) & (_PAGE_DIRTY | _PAGE_MODIFIED).
> If _PAGE_DIRTY isn't lost, pte_dirty() is always right, no matter
> whether there is or isn't _PAGE_MODIFIED.
>
> I think the real reason is we need to set _PAGE_MODIFIED in
> pte/pmd_modify to record the status of _PAGE_DIRTY, so that we can
> recover _PAGE_DIRTY afterwards, such as in pte/pmd_mkwrite().
Ok, I will adjust the description
>
>> Co-developed-by: Liupu Wang <wangliupu@loongson.cn>
>> Signed-off-by: Liupu Wang <wangliupu@loongson.cn>
>> Signed-off-by: Tianyang Zhang <zhangtianyang@loongson.cn>
>> ---
>> arch/loongarch/include/asm/pgtable.h | 17 +++++++++++++----
>> 1 file changed, 13 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/loongarch/include/asm/pgtable.h b/arch/loongarch/include/asm/pgtable.h
>> index bd128696e96d..106abfa5183b 100644
>> --- a/arch/loongarch/include/asm/pgtable.h
>> +++ b/arch/loongarch/include/asm/pgtable.h
>> @@ -424,8 +424,13 @@ static inline unsigned long pte_accessible(struct mm_struct *mm, pte_t a)
>>
>> static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
>> {
>> - return __pte((pte_val(pte) & _PAGE_CHG_MASK) |
>> - (pgprot_val(newprot) & ~_PAGE_CHG_MASK));
>> + unsigned long val = (pte_val(pte) & _PAGE_CHG_MASK) |
>> + (pgprot_val(newprot) & ~_PAGE_CHG_MASK);
>> +
>> + if (pte_val(pte) & _PAGE_DIRTY)
>> + val |= _PAGE_MODIFIED;
>> +
>> + return __pte(val);
>> }
>>
>> extern void __update_tlb(struct vm_area_struct *vma,
>> @@ -547,9 +552,13 @@ static inline struct page *pmd_page(pmd_t pmd)
>>
>> static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
>> {
>> - pmd_val(pmd) = (pmd_val(pmd) & _HPAGE_CHG_MASK) |
>> + unsigned long val = (pmd_val(pmd) & _HPAGE_CHG_MASK) |
>> (pgprot_val(newprot) & ~_HPAGE_CHG_MASK);
>> - return pmd;
>> +
>> + if (pmd_val(pmd) & _PAGE_DIRTY)
>> + val |= _PAGE_MODIFIED;
>> +
>> + return __pmd(val);
>> }
> A minimal modification can be:
> diff --git a/arch/loongarch/include/asm/pgtable.h
> b/arch/loongarch/include/asm/pgtable.h
> index 1f20e9280062..907ece0199e0 100644
> --- a/arch/loongarch/include/asm/pgtable.h
> +++ b/arch/loongarch/include/asm/pgtable.h
> @@ -448,8 +448,13 @@ static inline unsigned long pte_accessible(struct
> mm_struct *mm, pte_t a)
>
> static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
> {
> - return __pte((pte_val(pte) & _PAGE_CHG_MASK) |
> - (pgprot_val(newprot) & ~_PAGE_CHG_MASK));
> + pte_val(pte) = (pte_val(pte) & _PAGE_CHG_MASK) |
> + (pgprot_val(newprot) & ~_PAGE_CHG_MASK);
> +
> + if (pte_val(pte) & _PAGE_DIRTY)
> + pte_val(pte) |= _PAGE_MODIFIED;
> +
> + return pte;
> }
+ pte_val(pte) = (pte_val(pte) & _PAGE_CHG_MASK) |
+ (pgprot_val(newprot) & ~_PAGE_CHG_MASK);
After this step, _PAGE_DIRTY may have already disappeared,
If no new variables are added, they can be modified in follow way:
static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
{
+ if (pte_val(pte) & _PAGE_DIRTY)
+ pte_val(pte) |= _PAGE_MODIFIED;
+
return __pte((pte_val(pte) & _PAGE_CHG_MASK) |
(pgprot_val(newprot) & ~_PAGE_CHG_MASK));
}
>
> extern void __update_tlb(struct vm_area_struct *vma,
> @@ -583,7 +588,11 @@ static inline struct page *pmd_page(pmd_t pmd)
> static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
> {
> pmd_val(pmd) = (pmd_val(pmd) & _HPAGE_CHG_MASK) |
> - (pgprot_val(newprot) & ~_HPAGE_CHG_MASK);
> + (pgprot_val(newprot) & ~_HPAGE_CHG_MASK);
> +
> + if (pmd_val(pmd) & _PAGE_DIRTY)
> + pmd_val(pmd) |= _PAGE_MODIFIED;
> +
> return pmd;
> }
>
> You needn't define a new variable.
>
>
> Huacai
>
>> static inline pmd_t pmd_mkinvalid(pmd_t pmd)
>> --
>> 2.41.0
>>
>>
Thanks
Tianyang
On Wed, Nov 5, 2025 at 8:57 AM Tianyang Zhang <zhangtianyang@loongson.cn> wrote:
>
> Hi, Huacai
>
> 在 2025/11/4 下午4:00, Huacai Chen 写道:
> > Hi, Tianyang,
> >
> > The subject line can be:
> > LoongArch: Let {pte,pmd}_modify() record the status of _PAGE_DIRTY (If
> > I'm right in the later comments).
> Ok. I got it
> >
> > On Tue, Nov 4, 2025 at 3:30 PM Tianyang Zhang <zhangtianyang@loongson.cn> wrote:
> >> In the current pte_modify operation, _PAGE_DIRTY might be cleared. Since
> >> the hardware-page-walk does not have a predefined _PAGE_MODIFIED flag,
> >> this could lead to loss of valid data in certain scenarios.
> >>
> >> The new modification involves checking whether the original PTE has the
> >> _PAGE_DIRTY flag. If it exists, the _PAGE_MODIFIED bit is set, ensuring
> >> that the pte_dirty interface can return accurate information.
> > The description may be wrong here. Because pte_dirty() returns
> > pte_val(pte) & (_PAGE_DIRTY | _PAGE_MODIFIED).
> > If _PAGE_DIRTY isn't lost, pte_dirty() is always right, no matter
> > whether there is or isn't _PAGE_MODIFIED.
> >
> > I think the real reason is we need to set _PAGE_MODIFIED in
> > pte/pmd_modify to record the status of _PAGE_DIRTY, so that we can
> > recover _PAGE_DIRTY afterwards, such as in pte/pmd_mkwrite().
> Ok, I will adjust the description
After some thinking, your original description may be right. Without
this patch the scenario maybe like this:
The pte is dirty _PAGE_DIRTY but without _PAGE_MODIFIED, after
pte_modify() we lose _PAGE_DIRTY, then pte_dirty() returns false. So
we need _PAGE_MODIFIED to record _PAGE_DIRTY here.
But the description also needs to be updated.
> >
> >> Co-developed-by: Liupu Wang <wangliupu@loongson.cn>
> >> Signed-off-by: Liupu Wang <wangliupu@loongson.cn>
> >> Signed-off-by: Tianyang Zhang <zhangtianyang@loongson.cn>
> >> ---
> >> arch/loongarch/include/asm/pgtable.h | 17 +++++++++++++----
> >> 1 file changed, 13 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/arch/loongarch/include/asm/pgtable.h b/arch/loongarch/include/asm/pgtable.h
> >> index bd128696e96d..106abfa5183b 100644
> >> --- a/arch/loongarch/include/asm/pgtable.h
> >> +++ b/arch/loongarch/include/asm/pgtable.h
> >> @@ -424,8 +424,13 @@ static inline unsigned long pte_accessible(struct mm_struct *mm, pte_t a)
> >>
> >> static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
> >> {
> >> - return __pte((pte_val(pte) & _PAGE_CHG_MASK) |
> >> - (pgprot_val(newprot) & ~_PAGE_CHG_MASK));
> >> + unsigned long val = (pte_val(pte) & _PAGE_CHG_MASK) |
> >> + (pgprot_val(newprot) & ~_PAGE_CHG_MASK);
> >> +
> >> + if (pte_val(pte) & _PAGE_DIRTY)
> >> + val |= _PAGE_MODIFIED;
> >> +
> >> + return __pte(val);
> >> }
> >>
> >> extern void __update_tlb(struct vm_area_struct *vma,
> >> @@ -547,9 +552,13 @@ static inline struct page *pmd_page(pmd_t pmd)
> >>
> >> static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
> >> {
> >> - pmd_val(pmd) = (pmd_val(pmd) & _HPAGE_CHG_MASK) |
> >> + unsigned long val = (pmd_val(pmd) & _HPAGE_CHG_MASK) |
> >> (pgprot_val(newprot) & ~_HPAGE_CHG_MASK);
> >> - return pmd;
> >> +
> >> + if (pmd_val(pmd) & _PAGE_DIRTY)
> >> + val |= _PAGE_MODIFIED;
> >> +
> >> + return __pmd(val);
> >> }
> > A minimal modification can be:
> > diff --git a/arch/loongarch/include/asm/pgtable.h
> > b/arch/loongarch/include/asm/pgtable.h
> > index 1f20e9280062..907ece0199e0 100644
> > --- a/arch/loongarch/include/asm/pgtable.h
> > +++ b/arch/loongarch/include/asm/pgtable.h
> > @@ -448,8 +448,13 @@ static inline unsigned long pte_accessible(struct
> > mm_struct *mm, pte_t a)
> >
> > static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
> > {
> > - return __pte((pte_val(pte) & _PAGE_CHG_MASK) |
> > - (pgprot_val(newprot) & ~_PAGE_CHG_MASK));
> > + pte_val(pte) = (pte_val(pte) & _PAGE_CHG_MASK) |
> > + (pgprot_val(newprot) & ~_PAGE_CHG_MASK);
> > +
> > + if (pte_val(pte) & _PAGE_DIRTY)
> > + pte_val(pte) |= _PAGE_MODIFIED;
> > +
> > + return pte;
> > }
>
> + pte_val(pte) = (pte_val(pte) & _PAGE_CHG_MASK) |
> + (pgprot_val(newprot) & ~_PAGE_CHG_MASK);
>
> After this step, _PAGE_DIRTY may have already disappeared,
> If no new variables are added, they can be modified in follow way:
>
> static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
> {
> + if (pte_val(pte) & _PAGE_DIRTY)
> + pte_val(pte) |= _PAGE_MODIFIED;
> +
> return __pte((pte_val(pte) & _PAGE_CHG_MASK) |
> (pgprot_val(newprot) & ~_PAGE_CHG_MASK));
>
> }
OK, it makes sense.
Huacai
>
> >
> > extern void __update_tlb(struct vm_area_struct *vma,
> > @@ -583,7 +588,11 @@ static inline struct page *pmd_page(pmd_t pmd)
> > static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
> > {
> > pmd_val(pmd) = (pmd_val(pmd) & _HPAGE_CHG_MASK) |
> > - (pgprot_val(newprot) & ~_HPAGE_CHG_MASK);
> > + (pgprot_val(newprot) & ~_HPAGE_CHG_MASK);
> > +
> > + if (pmd_val(pmd) & _PAGE_DIRTY)
> > + pmd_val(pmd) |= _PAGE_MODIFIED;
> > +
> > return pmd;
> > }
> >
> > You needn't define a new variable.
> >
> >
> > Huacai
> >
> >> static inline pmd_t pmd_mkinvalid(pmd_t pmd)
> >> --
> >> 2.41.0
> >>
> >>
> Thanks
>
> Tianyang
>
On 2025/11/5 上午9:07, Huacai Chen wrote:
> On Wed, Nov 5, 2025 at 8:57 AM Tianyang Zhang <zhangtianyang@loongson.cn> wrote:
>>
>> Hi, Huacai
>>
>> 在 2025/11/4 下午4:00, Huacai Chen 写道:
>>> Hi, Tianyang,
>>>
>>> The subject line can be:
>>> LoongArch: Let {pte,pmd}_modify() record the status of _PAGE_DIRTY (If
>>> I'm right in the later comments).
>> Ok. I got it
>>>
>>> On Tue, Nov 4, 2025 at 3:30 PM Tianyang Zhang <zhangtianyang@loongson.cn> wrote:
>>>> In the current pte_modify operation, _PAGE_DIRTY might be cleared. Since
>>>> the hardware-page-walk does not have a predefined _PAGE_MODIFIED flag,
>>>> this could lead to loss of valid data in certain scenarios.
>>>>
>>>> The new modification involves checking whether the original PTE has the
>>>> _PAGE_DIRTY flag. If it exists, the _PAGE_MODIFIED bit is set, ensuring
>>>> that the pte_dirty interface can return accurate information.
>>> The description may be wrong here. Because pte_dirty() returns
>>> pte_val(pte) & (_PAGE_DIRTY | _PAGE_MODIFIED).
>>> If _PAGE_DIRTY isn't lost, pte_dirty() is always right, no matter
>>> whether there is or isn't _PAGE_MODIFIED.
>>>
>>> I think the real reason is we need to set _PAGE_MODIFIED in
>>> pte/pmd_modify to record the status of _PAGE_DIRTY, so that we can
>>> recover _PAGE_DIRTY afterwards, such as in pte/pmd_mkwrite().
>> Ok, I will adjust the description
> After some thinking, your original description may be right. Without
> this patch the scenario maybe like this:
> The pte is dirty _PAGE_DIRTY but without _PAGE_MODIFIED, after
> pte_modify() we lose _PAGE_DIRTY, then pte_dirty() returns false. So
> we need _PAGE_MODIFIED to record _PAGE_DIRTY here.
In theory pte_modify() is to modify RWX attribute. I think that it is a
tricky to remove _PAGE_DIRTY and add _PAGE_MODIFIED with HW PTW system.
Also _PAGE_ACCESSED is lost with pte_modify() API, is there any
influence with HW PTW system, or wait until possible problems coming out.
Regards
Bibo Mao
>
> But the description also needs to be updated.
>
>>>
>>>> Co-developed-by: Liupu Wang <wangliupu@loongson.cn>
>>>> Signed-off-by: Liupu Wang <wangliupu@loongson.cn>
>>>> Signed-off-by: Tianyang Zhang <zhangtianyang@loongson.cn>
>>>> ---
>>>> arch/loongarch/include/asm/pgtable.h | 17 +++++++++++++----
>>>> 1 file changed, 13 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/arch/loongarch/include/asm/pgtable.h b/arch/loongarch/include/asm/pgtable.h
>>>> index bd128696e96d..106abfa5183b 100644
>>>> --- a/arch/loongarch/include/asm/pgtable.h
>>>> +++ b/arch/loongarch/include/asm/pgtable.h
>>>> @@ -424,8 +424,13 @@ static inline unsigned long pte_accessible(struct mm_struct *mm, pte_t a)
>>>>
>>>> static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
>>>> {
>>>> - return __pte((pte_val(pte) & _PAGE_CHG_MASK) |
>>>> - (pgprot_val(newprot) & ~_PAGE_CHG_MASK));
>>>> + unsigned long val = (pte_val(pte) & _PAGE_CHG_MASK) |
>>>> + (pgprot_val(newprot) & ~_PAGE_CHG_MASK);
>>>> +
>>>> + if (pte_val(pte) & _PAGE_DIRTY)
>>>> + val |= _PAGE_MODIFIED;
>>>> +
>>>> + return __pte(val);
>>>> }
>>>>
>>>> extern void __update_tlb(struct vm_area_struct *vma,
>>>> @@ -547,9 +552,13 @@ static inline struct page *pmd_page(pmd_t pmd)
>>>>
>>>> static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
>>>> {
>>>> - pmd_val(pmd) = (pmd_val(pmd) & _HPAGE_CHG_MASK) |
>>>> + unsigned long val = (pmd_val(pmd) & _HPAGE_CHG_MASK) |
>>>> (pgprot_val(newprot) & ~_HPAGE_CHG_MASK);
>>>> - return pmd;
>>>> +
>>>> + if (pmd_val(pmd) & _PAGE_DIRTY)
>>>> + val |= _PAGE_MODIFIED;
>>>> +
>>>> + return __pmd(val);
>>>> }
>>> A minimal modification can be:
>>> diff --git a/arch/loongarch/include/asm/pgtable.h
>>> b/arch/loongarch/include/asm/pgtable.h
>>> index 1f20e9280062..907ece0199e0 100644
>>> --- a/arch/loongarch/include/asm/pgtable.h
>>> +++ b/arch/loongarch/include/asm/pgtable.h
>>> @@ -448,8 +448,13 @@ static inline unsigned long pte_accessible(struct
>>> mm_struct *mm, pte_t a)
>>>
>>> static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
>>> {
>>> - return __pte((pte_val(pte) & _PAGE_CHG_MASK) |
>>> - (pgprot_val(newprot) & ~_PAGE_CHG_MASK));
>>> + pte_val(pte) = (pte_val(pte) & _PAGE_CHG_MASK) |
>>> + (pgprot_val(newprot) & ~_PAGE_CHG_MASK);
>>> +
>>> + if (pte_val(pte) & _PAGE_DIRTY)
>>> + pte_val(pte) |= _PAGE_MODIFIED;
>>> +
>>> + return pte;
>>> }
>>
>> + pte_val(pte) = (pte_val(pte) & _PAGE_CHG_MASK) |
>> + (pgprot_val(newprot) & ~_PAGE_CHG_MASK);
>>
>> After this step, _PAGE_DIRTY may have already disappeared,
>> If no new variables are added, they can be modified in follow way:
>>
>> static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
>> {
>> + if (pte_val(pte) & _PAGE_DIRTY)
>> + pte_val(pte) |= _PAGE_MODIFIED;
>> +
>> return __pte((pte_val(pte) & _PAGE_CHG_MASK) |
>> (pgprot_val(newprot) & ~_PAGE_CHG_MASK));
>>
>> }
> OK, it makes sense.
>
> Huacai
>>
>>>
>>> extern void __update_tlb(struct vm_area_struct *vma,
>>> @@ -583,7 +588,11 @@ static inline struct page *pmd_page(pmd_t pmd)
>>> static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
>>> {
>>> pmd_val(pmd) = (pmd_val(pmd) & _HPAGE_CHG_MASK) |
>>> - (pgprot_val(newprot) & ~_HPAGE_CHG_MASK);
>>> + (pgprot_val(newprot) & ~_HPAGE_CHG_MASK);
>>> +
>>> + if (pmd_val(pmd) & _PAGE_DIRTY)
>>> + pmd_val(pmd) |= _PAGE_MODIFIED;
>>> +
>>> return pmd;
>>> }
>>>
>>> You needn't define a new variable.
>>>
>>>
>>> Huacai
>>>
>>>> static inline pmd_t pmd_mkinvalid(pmd_t pmd)
>>>> --
>>>> 2.41.0
>>>>
>>>>
>> Thanks
>>
>> Tianyang
>>
Hi ,Bibao
在 2025/11/5 上午9:18, Bibo Mao 写道:
>
>
> On 2025/11/5 上午9:07, Huacai Chen wrote:
>> On Wed, Nov 5, 2025 at 8:57 AM Tianyang Zhang
>> <zhangtianyang@loongson.cn> wrote:
>>>
>>> Hi, Huacai
>>>
>>> 在 2025/11/4 下午4:00, Huacai Chen 写道:
>>>> Hi, Tianyang,
>>>>
>>>> The subject line can be:
>>>> LoongArch: Let {pte,pmd}_modify() record the status of _PAGE_DIRTY (If
>>>> I'm right in the later comments).
>>> Ok. I got it
>>>>
>>>> On Tue, Nov 4, 2025 at 3:30 PM Tianyang Zhang
>>>> <zhangtianyang@loongson.cn> wrote:
>>>>> In the current pte_modify operation, _PAGE_DIRTY might be cleared.
>>>>> Since
>>>>> the hardware-page-walk does not have a predefined _PAGE_MODIFIED
>>>>> flag,
>>>>> this could lead to loss of valid data in certain scenarios.
>>>>>
>>>>> The new modification involves checking whether the original PTE
>>>>> has the
>>>>> _PAGE_DIRTY flag. If it exists, the _PAGE_MODIFIED bit is set,
>>>>> ensuring
>>>>> that the pte_dirty interface can return accurate information.
>>>> The description may be wrong here. Because pte_dirty() returns
>>>> pte_val(pte) & (_PAGE_DIRTY | _PAGE_MODIFIED).
>>>> If _PAGE_DIRTY isn't lost, pte_dirty() is always right, no matter
>>>> whether there is or isn't _PAGE_MODIFIED.
>>>>
>>>> I think the real reason is we need to set _PAGE_MODIFIED in
>>>> pte/pmd_modify to record the status of _PAGE_DIRTY, so that we can
>>>> recover _PAGE_DIRTY afterwards, such as in pte/pmd_mkwrite().
>>> Ok, I will adjust the description
>> After some thinking, your original description may be right. Without
>> this patch the scenario maybe like this:
>> The pte is dirty _PAGE_DIRTY but without _PAGE_MODIFIED, after
>> pte_modify() we lose _PAGE_DIRTY, then pte_dirty() returns false. So
>> we need _PAGE_MODIFIED to record _PAGE_DIRTY here.
> In theory pte_modify() is to modify RWX attribute. I think that it is
> a tricky to remove _PAGE_DIRTY and add _PAGE_MODIFIED with HW PTW system.
>
> Also _PAGE_ACCESSED is lost with pte_modify() API, is there any
> influence with HW PTW system, or wait until possible problems coming out.
static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
{
return __pte((pte_val(pte) & _PAGE_CHG_MASK) |
(pgprot_val(newprot) & ~_PAGE_CHG_MASK));
}
In my understand, During the pte_modify process, it is essential to
ensure that specific bits are inherited from the original PTE rather
than simply replaced(as set_pte),
this guarantees the coherent operation of the memory management system.
Since _PAGE_CHG_MASK explicitly requires preserving pte_modified, and
there is an inherent correlation between pte_dirty and pte_modified,
these attributes must be evaluated and handled accordingly.
The pte_valid attribute, being a hardware property, is inherently the
target of modification in the pte_modify interface. Therefore, it is
reasonable not to preserve it.
Thanks
Tianyang
>
> Regards
> Bibo Mao
>>
>> But the description also needs to be updated.
>>
>>>>
>>>>> Co-developed-by: Liupu Wang <wangliupu@loongson.cn>
>>>>> Signed-off-by: Liupu Wang <wangliupu@loongson.cn>
>>>>> Signed-off-by: Tianyang Zhang <zhangtianyang@loongson.cn>
>>>>> ---
>>>>> arch/loongarch/include/asm/pgtable.h | 17 +++++++++++++----
>>>>> 1 file changed, 13 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/arch/loongarch/include/asm/pgtable.h
>>>>> b/arch/loongarch/include/asm/pgtable.h
>>>>> index bd128696e96d..106abfa5183b 100644
>>>>> --- a/arch/loongarch/include/asm/pgtable.h
>>>>> +++ b/arch/loongarch/include/asm/pgtable.h
>>>>> @@ -424,8 +424,13 @@ static inline unsigned long
>>>>> pte_accessible(struct mm_struct *mm, pte_t a)
>>>>>
>>>>> static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
>>>>> {
>>>>> - return __pte((pte_val(pte) & _PAGE_CHG_MASK) |
>>>>> - (pgprot_val(newprot) & ~_PAGE_CHG_MASK));
>>>>> + unsigned long val = (pte_val(pte) & _PAGE_CHG_MASK) |
>>>>> + (pgprot_val(newprot) & ~_PAGE_CHG_MASK);
>>>>> +
>>>>> + if (pte_val(pte) & _PAGE_DIRTY)
>>>>> + val |= _PAGE_MODIFIED;
>>>>> +
>>>>> + return __pte(val);
>>>>> }
>>>>>
>>>>> extern void __update_tlb(struct vm_area_struct *vma,
>>>>> @@ -547,9 +552,13 @@ static inline struct page *pmd_page(pmd_t pmd)
>>>>>
>>>>> static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
>>>>> {
>>>>> - pmd_val(pmd) = (pmd_val(pmd) & _HPAGE_CHG_MASK) |
>>>>> + unsigned long val = (pmd_val(pmd) & _HPAGE_CHG_MASK) |
>>>>> (pgprot_val(newprot) &
>>>>> ~_HPAGE_CHG_MASK);
>>>>> - return pmd;
>>>>> +
>>>>> + if (pmd_val(pmd) & _PAGE_DIRTY)
>>>>> + val |= _PAGE_MODIFIED;
>>>>> +
>>>>> + return __pmd(val);
>>>>> }
>>>> A minimal modification can be:
>>>> diff --git a/arch/loongarch/include/asm/pgtable.h
>>>> b/arch/loongarch/include/asm/pgtable.h
>>>> index 1f20e9280062..907ece0199e0 100644
>>>> --- a/arch/loongarch/include/asm/pgtable.h
>>>> +++ b/arch/loongarch/include/asm/pgtable.h
>>>> @@ -448,8 +448,13 @@ static inline unsigned long pte_accessible(struct
>>>> mm_struct *mm, pte_t a)
>>>>
>>>> static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
>>>> {
>>>> - return __pte((pte_val(pte) & _PAGE_CHG_MASK) |
>>>> - (pgprot_val(newprot) & ~_PAGE_CHG_MASK));
>>>> + pte_val(pte) = (pte_val(pte) & _PAGE_CHG_MASK) |
>>>> + (pgprot_val(newprot) & ~_PAGE_CHG_MASK);
>>>> +
>>>> + if (pte_val(pte) & _PAGE_DIRTY)
>>>> + pte_val(pte) |= _PAGE_MODIFIED;
>>>> +
>>>> + return pte;
>>>> }
>>>
>>> + pte_val(pte) = (pte_val(pte) & _PAGE_CHG_MASK) |
>>> + (pgprot_val(newprot) & ~_PAGE_CHG_MASK);
>>>
>>> After this step, _PAGE_DIRTY may have already disappeared,
>>> If no new variables are added, they can be modified in follow way:
>>>
>>> static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
>>> {
>>> + if (pte_val(pte) & _PAGE_DIRTY)
>>> + pte_val(pte) |= _PAGE_MODIFIED;
>>> +
>>> return __pte((pte_val(pte) & _PAGE_CHG_MASK) |
>>> (pgprot_val(newprot) & ~_PAGE_CHG_MASK));
>>>
>>> }
>> OK, it makes sense.
>>
>> Huacai
>>>
>>>>
>>>> extern void __update_tlb(struct vm_area_struct *vma,
>>>> @@ -583,7 +588,11 @@ static inline struct page *pmd_page(pmd_t pmd)
>>>> static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
>>>> {
>>>> pmd_val(pmd) = (pmd_val(pmd) & _HPAGE_CHG_MASK) |
>>>> - (pgprot_val(newprot) &
>>>> ~_HPAGE_CHG_MASK);
>>>> + (pgprot_val(newprot) & ~_HPAGE_CHG_MASK);
>>>> +
>>>> + if (pmd_val(pmd) & _PAGE_DIRTY)
>>>> + pmd_val(pmd) |= _PAGE_MODIFIED;
>>>> +
>>>> return pmd;
>>>> }
>>>>
>>>> You needn't define a new variable.
>>>>
>>>>
>>>> Huacai
>>>>
>>>>> static inline pmd_t pmd_mkinvalid(pmd_t pmd)
>>>>> --
>>>>> 2.41.0
>>>>>
>>>>>
>>> Thanks
>>>
>>> Tianyang
>>>
On 2025/11/6 上午9:55, Tianyang Zhang wrote:
> Hi ,Bibao
>
> 在 2025/11/5 上午9:18, Bibo Mao 写道:
>>
>>
>> On 2025/11/5 上午9:07, Huacai Chen wrote:
>>> On Wed, Nov 5, 2025 at 8:57 AM Tianyang Zhang
>>> <zhangtianyang@loongson.cn> wrote:
>>>>
>>>> Hi, Huacai
>>>>
>>>> 在 2025/11/4 下午4:00, Huacai Chen 写道:
>>>>> Hi, Tianyang,
>>>>>
>>>>> The subject line can be:
>>>>> LoongArch: Let {pte,pmd}_modify() record the status of _PAGE_DIRTY (If
>>>>> I'm right in the later comments).
>>>> Ok. I got it
>>>>>
>>>>> On Tue, Nov 4, 2025 at 3:30 PM Tianyang Zhang
>>>>> <zhangtianyang@loongson.cn> wrote:
>>>>>> In the current pte_modify operation, _PAGE_DIRTY might be cleared.
>>>>>> Since
>>>>>> the hardware-page-walk does not have a predefined _PAGE_MODIFIED
>>>>>> flag,
>>>>>> this could lead to loss of valid data in certain scenarios.
>>>>>>
>>>>>> The new modification involves checking whether the original PTE
>>>>>> has the
>>>>>> _PAGE_DIRTY flag. If it exists, the _PAGE_MODIFIED bit is set,
>>>>>> ensuring
>>>>>> that the pte_dirty interface can return accurate information.
>>>>> The description may be wrong here. Because pte_dirty() returns
>>>>> pte_val(pte) & (_PAGE_DIRTY | _PAGE_MODIFIED).
>>>>> If _PAGE_DIRTY isn't lost, pte_dirty() is always right, no matter
>>>>> whether there is or isn't _PAGE_MODIFIED.
>>>>>
>>>>> I think the real reason is we need to set _PAGE_MODIFIED in
>>>>> pte/pmd_modify to record the status of _PAGE_DIRTY, so that we can
>>>>> recover _PAGE_DIRTY afterwards, such as in pte/pmd_mkwrite().
>>>> Ok, I will adjust the description
>>> After some thinking, your original description may be right. Without
>>> this patch the scenario maybe like this:
>>> The pte is dirty _PAGE_DIRTY but without _PAGE_MODIFIED, after
>>> pte_modify() we lose _PAGE_DIRTY, then pte_dirty() returns false. So
>>> we need _PAGE_MODIFIED to record _PAGE_DIRTY here.
>> In theory pte_modify() is to modify RWX attribute. I think that it is
>> a tricky to remove _PAGE_DIRTY and add _PAGE_MODIFIED with HW PTW system.
>>
>> Also _PAGE_ACCESSED is lost with pte_modify() API, is there any
>> influence with HW PTW system, or wait until possible problems coming out.
>
> static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
> {
> return __pte((pte_val(pte) & _PAGE_CHG_MASK) |
> (pgprot_val(newprot) & ~_PAGE_CHG_MASK));
> }
> In my understand, During the pte_modify process, it is essential to
> ensure that specific bits are inherited from the original PTE rather
> than simply replaced(as set_pte),
>
> this guarantees the coherent operation of the memory management system.
>
> Since _PAGE_CHG_MASK explicitly requires preserving pte_modified, and
The problem is how _PAGE_CHG_MASK should be defined, do you check with
other architectures?
> there is an inherent correlation between pte_dirty and pte_modified,
> these attributes must be evaluated and handled accordingly.
>
> The pte_valid attribute, being a hardware property, is inherently the
> target of modification in the pte_modify interface. Therefore, it is
> reasonable not to preserve it.
On HW PTW system, _PAGE_PRESENT will control whether trigger page fault
rather than pte_valid/_PAGE_ACCESSED. For simple, do you think the
following code is ok or not ?
static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
{
- return __pte((pte_val(pte) & _PAGE_CHG_MASK) |
- (pgprot_val(newprot) & ~_PAGE_CHG_MASK));
+ unsigned long mask = _PAGE_CHG_MASK;
+
+ if (cpu_has_ptw)
+ mask |= _PAGE_DIRTY | _PAGE_ACCESSED;
+ return __pte((pte_val(pte) & mask) |
+ (pgprot_val(newprot) & ~mask));
}
Regards
Bibo Mao
>
> Thanks
>
> Tianyang
>
>>
>> Regards
>> Bibo Mao
>>>
>>> But the description also needs to be updated.
>>>
>>>>>
>>>>>> Co-developed-by: Liupu Wang <wangliupu@loongson.cn>
>>>>>> Signed-off-by: Liupu Wang <wangliupu@loongson.cn>
>>>>>> Signed-off-by: Tianyang Zhang <zhangtianyang@loongson.cn>
>>>>>> ---
>>>>>> arch/loongarch/include/asm/pgtable.h | 17 +++++++++++++----
>>>>>> 1 file changed, 13 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/loongarch/include/asm/pgtable.h
>>>>>> b/arch/loongarch/include/asm/pgtable.h
>>>>>> index bd128696e96d..106abfa5183b 100644
>>>>>> --- a/arch/loongarch/include/asm/pgtable.h
>>>>>> +++ b/arch/loongarch/include/asm/pgtable.h
>>>>>> @@ -424,8 +424,13 @@ static inline unsigned long
>>>>>> pte_accessible(struct mm_struct *mm, pte_t a)
>>>>>>
>>>>>> static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
>>>>>> {
>>>>>> - return __pte((pte_val(pte) & _PAGE_CHG_MASK) |
>>>>>> - (pgprot_val(newprot) & ~_PAGE_CHG_MASK));
>>>>>> + unsigned long val = (pte_val(pte) & _PAGE_CHG_MASK) |
>>>>>> + (pgprot_val(newprot) & ~_PAGE_CHG_MASK);
>>>>>> +
>>>>>> + if (pte_val(pte) & _PAGE_DIRTY)
>>>>>> + val |= _PAGE_MODIFIED;
>>>>>> +
>>>>>> + return __pte(val);
>>>>>> }
>>>>>>
>>>>>> extern void __update_tlb(struct vm_area_struct *vma,
>>>>>> @@ -547,9 +552,13 @@ static inline struct page *pmd_page(pmd_t pmd)
>>>>>>
>>>>>> static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
>>>>>> {
>>>>>> - pmd_val(pmd) = (pmd_val(pmd) & _HPAGE_CHG_MASK) |
>>>>>> + unsigned long val = (pmd_val(pmd) & _HPAGE_CHG_MASK) |
>>>>>> (pgprot_val(newprot) &
>>>>>> ~_HPAGE_CHG_MASK);
>>>>>> - return pmd;
>>>>>> +
>>>>>> + if (pmd_val(pmd) & _PAGE_DIRTY)
>>>>>> + val |= _PAGE_MODIFIED;
>>>>>> +
>>>>>> + return __pmd(val);
>>>>>> }
>>>>> A minimal modification can be:
>>>>> diff --git a/arch/loongarch/include/asm/pgtable.h
>>>>> b/arch/loongarch/include/asm/pgtable.h
>>>>> index 1f20e9280062..907ece0199e0 100644
>>>>> --- a/arch/loongarch/include/asm/pgtable.h
>>>>> +++ b/arch/loongarch/include/asm/pgtable.h
>>>>> @@ -448,8 +448,13 @@ static inline unsigned long pte_accessible(struct
>>>>> mm_struct *mm, pte_t a)
>>>>>
>>>>> static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
>>>>> {
>>>>> - return __pte((pte_val(pte) & _PAGE_CHG_MASK) |
>>>>> - (pgprot_val(newprot) & ~_PAGE_CHG_MASK));
>>>>> + pte_val(pte) = (pte_val(pte) & _PAGE_CHG_MASK) |
>>>>> + (pgprot_val(newprot) & ~_PAGE_CHG_MASK);
>>>>> +
>>>>> + if (pte_val(pte) & _PAGE_DIRTY)
>>>>> + pte_val(pte) |= _PAGE_MODIFIED;
>>>>> +
>>>>> + return pte;
>>>>> }
>>>>
>>>> + pte_val(pte) = (pte_val(pte) & _PAGE_CHG_MASK) |
>>>> + (pgprot_val(newprot) & ~_PAGE_CHG_MASK);
>>>>
>>>> After this step, _PAGE_DIRTY may have already disappeared,
>>>> If no new variables are added, they can be modified in follow way:
>>>>
>>>> static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
>>>> {
>>>> + if (pte_val(pte) & _PAGE_DIRTY)
>>>> + pte_val(pte) |= _PAGE_MODIFIED;
>>>> +
>>>> return __pte((pte_val(pte) & _PAGE_CHG_MASK) |
>>>> (pgprot_val(newprot) & ~_PAGE_CHG_MASK));
>>>>
>>>> }
>>> OK, it makes sense.
>>>
>>> Huacai
>>>>
>>>>>
>>>>> extern void __update_tlb(struct vm_area_struct *vma,
>>>>> @@ -583,7 +588,11 @@ static inline struct page *pmd_page(pmd_t pmd)
>>>>> static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
>>>>> {
>>>>> pmd_val(pmd) = (pmd_val(pmd) & _HPAGE_CHG_MASK) |
>>>>> - (pgprot_val(newprot) &
>>>>> ~_HPAGE_CHG_MASK);
>>>>> + (pgprot_val(newprot) & ~_HPAGE_CHG_MASK);
>>>>> +
>>>>> + if (pmd_val(pmd) & _PAGE_DIRTY)
>>>>> + pmd_val(pmd) |= _PAGE_MODIFIED;
>>>>> +
>>>>> return pmd;
>>>>> }
>>>>>
>>>>> You needn't define a new variable.
>>>>>
>>>>>
>>>>> Huacai
>>>>>
>>>>>> static inline pmd_t pmd_mkinvalid(pmd_t pmd)
>>>>>> --
>>>>>> 2.41.0
>>>>>>
>>>>>>
>>>> Thanks
>>>>
>>>> Tianyang
>>>>
在 2025/11/6 上午10:10, Bibo Mao 写道:
>
>
> On 2025/11/6 上午9:55, Tianyang Zhang wrote:
>> Hi ,Bibao
>>
>> 在 2025/11/5 上午9:18, Bibo Mao 写道:
>>>
>>>
>>> On 2025/11/5 上午9:07, Huacai Chen wrote:
>>>> On Wed, Nov 5, 2025 at 8:57 AM Tianyang Zhang
>>>> <zhangtianyang@loongson.cn> wrote:
>>>>>
>>>>> Hi, Huacai
>>>>>
>>>>> 在 2025/11/4 下午4:00, Huacai Chen 写道:
>>>>>> Hi, Tianyang,
>>>>>>
>>>>>> The subject line can be:
>>>>>> LoongArch: Let {pte,pmd}_modify() record the status of
>>>>>> _PAGE_DIRTY (If
>>>>>> I'm right in the later comments).
>>>>> Ok. I got it
>>>>>>
>>>>>> On Tue, Nov 4, 2025 at 3:30 PM Tianyang Zhang
>>>>>> <zhangtianyang@loongson.cn> wrote:
>>>>>>> In the current pte_modify operation, _PAGE_DIRTY might be
>>>>>>> cleared. Since
>>>>>>> the hardware-page-walk does not have a predefined _PAGE_MODIFIED
>>>>>>> flag,
>>>>>>> this could lead to loss of valid data in certain scenarios.
>>>>>>>
>>>>>>> The new modification involves checking whether the original PTE
>>>>>>> has the
>>>>>>> _PAGE_DIRTY flag. If it exists, the _PAGE_MODIFIED bit is set,
>>>>>>> ensuring
>>>>>>> that the pte_dirty interface can return accurate information.
>>>>>> The description may be wrong here. Because pte_dirty() returns
>>>>>> pte_val(pte) & (_PAGE_DIRTY | _PAGE_MODIFIED).
>>>>>> If _PAGE_DIRTY isn't lost, pte_dirty() is always right, no matter
>>>>>> whether there is or isn't _PAGE_MODIFIED.
>>>>>>
>>>>>> I think the real reason is we need to set _PAGE_MODIFIED in
>>>>>> pte/pmd_modify to record the status of _PAGE_DIRTY, so that we can
>>>>>> recover _PAGE_DIRTY afterwards, such as in pte/pmd_mkwrite().
>>>>> Ok, I will adjust the description
>>>> After some thinking, your original description may be right. Without
>>>> this patch the scenario maybe like this:
>>>> The pte is dirty _PAGE_DIRTY but without _PAGE_MODIFIED, after
>>>> pte_modify() we lose _PAGE_DIRTY, then pte_dirty() returns false. So
>>>> we need _PAGE_MODIFIED to record _PAGE_DIRTY here.
>>> In theory pte_modify() is to modify RWX attribute. I think that it
>>> is a tricky to remove _PAGE_DIRTY and add _PAGE_MODIFIED with HW PTW
>>> system.
>>>
>>> Also _PAGE_ACCESSED is lost with pte_modify() API, is there any
>>> influence with HW PTW system, or wait until possible problems coming
>>> out.
>>
>> static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
>> {
>> return __pte((pte_val(pte) & _PAGE_CHG_MASK) |
>> (pgprot_val(newprot) & ~_PAGE_CHG_MASK));
>> }
>> In my understand, During the pte_modify process, it is essential to
>> ensure that specific bits are inherited from the original PTE rather
>> than simply replaced(as set_pte),
>>
>> this guarantees the coherent operation of the memory management system.
>>
>> Since _PAGE_CHG_MASK explicitly requires preserving pte_modified, and
> The problem is how _PAGE_CHG_MASK should be defined, do you check with
> other architectures?
Under mainstream architectures(like ARM64/X86 ), if the pte_modify
interface clears the hard-dirty flag, it will set the soft-dirty flag
through some mechanism.
Thus, at least from the perspective of PAGE_DIRTY logic, this approach
is right.
>> there is an inherent correlation between pte_dirty and pte_modified,
>> these attributes must be evaluated and handled accordingly.
>>
>> The pte_valid attribute, being a hardware property, is inherently the
>> target of modification in the pte_modify interface. Therefore, it is
>> reasonable not to preserve it.
> On HW PTW system, _PAGE_PRESENT will control whether trigger page
> fault rather than pte_valid/_PAGE_ACCESSED. For simple, do you think
> the following code is ok or not ?
"On HW PTW system, _PAGE_PRESENT will control whether trigger page fault
rather than pte_valid/_PAGE_ACCESSED"
Yes, indeed, in many cases, PAGE_PRESENT is precisely the cleanup target
of pte_modify.
>
> static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
> {
> - return __pte((pte_val(pte) & _PAGE_CHG_MASK) |
> - (pgprot_val(newprot) & ~_PAGE_CHG_MASK));
> + unsigned long mask = _PAGE_CHG_MASK;
> +
> + if (cpu_has_ptw)
> + mask |= _PAGE_DIRTY | _PAGE_ACCESSED;
> + return __pte((pte_val(pte) & mask) |
> + (pgprot_val(newprot) & ~mask));
> }
>
This modification is inappropriate.
Firstly, _PAGE_ACCESSED(bit 0, as _PAGE_PRESENT) and _PAGE_DIRTY bits
are inherently the targets of pte_modify operations. Some
sub-memory-system like numa_balance precisely rely on
clearing these bits to trigger hardware exceptions and complete
subsequent processes, this appears to be unrelated to hardware-ptw
And, under hardware-ptw scenarios, the WRITE=0 && DIRTY=1 condition
should never occur, therefore, we cannot preserve the DIRTY bit in advance.
Thanks
Tianyang
> Regards
> Bibo Mao
>
>>
>> Thanks
>>
>> Tianyang
>>
>>>
>>> Regards
>>> Bibo Mao
>>>>
>>>> But the description also needs to be updated.
>>>>
>>>>>>
>>>>>>> Co-developed-by: Liupu Wang <wangliupu@loongson.cn>
>>>>>>> Signed-off-by: Liupu Wang <wangliupu@loongson.cn>
>>>>>>> Signed-off-by: Tianyang Zhang <zhangtianyang@loongson.cn>
>>>>>>> ---
>>>>>>> arch/loongarch/include/asm/pgtable.h | 17 +++++++++++++----
>>>>>>> 1 file changed, 13 insertions(+), 4 deletions(-)
>>>>>>>
>>>>>>> diff --git a/arch/loongarch/include/asm/pgtable.h
>>>>>>> b/arch/loongarch/include/asm/pgtable.h
>>>>>>> index bd128696e96d..106abfa5183b 100644
>>>>>>> --- a/arch/loongarch/include/asm/pgtable.h
>>>>>>> +++ b/arch/loongarch/include/asm/pgtable.h
>>>>>>> @@ -424,8 +424,13 @@ static inline unsigned long
>>>>>>> pte_accessible(struct mm_struct *mm, pte_t a)
>>>>>>>
>>>>>>> static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
>>>>>>> {
>>>>>>> - return __pte((pte_val(pte) & _PAGE_CHG_MASK) |
>>>>>>> - (pgprot_val(newprot) & ~_PAGE_CHG_MASK));
>>>>>>> + unsigned long val = (pte_val(pte) & _PAGE_CHG_MASK) |
>>>>>>> + (pgprot_val(newprot) & ~_PAGE_CHG_MASK);
>>>>>>> +
>>>>>>> + if (pte_val(pte) & _PAGE_DIRTY)
>>>>>>> + val |= _PAGE_MODIFIED;
>>>>>>> +
>>>>>>> + return __pte(val);
>>>>>>> }
>>>>>>>
>>>>>>> extern void __update_tlb(struct vm_area_struct *vma,
>>>>>>> @@ -547,9 +552,13 @@ static inline struct page *pmd_page(pmd_t pmd)
>>>>>>>
>>>>>>> static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
>>>>>>> {
>>>>>>> - pmd_val(pmd) = (pmd_val(pmd) & _HPAGE_CHG_MASK) |
>>>>>>> + unsigned long val = (pmd_val(pmd) & _HPAGE_CHG_MASK) |
>>>>>>> (pgprot_val(newprot) &
>>>>>>> ~_HPAGE_CHG_MASK);
>>>>>>> - return pmd;
>>>>>>> +
>>>>>>> + if (pmd_val(pmd) & _PAGE_DIRTY)
>>>>>>> + val |= _PAGE_MODIFIED;
>>>>>>> +
>>>>>>> + return __pmd(val);
>>>>>>> }
>>>>>> A minimal modification can be:
>>>>>> diff --git a/arch/loongarch/include/asm/pgtable.h
>>>>>> b/arch/loongarch/include/asm/pgtable.h
>>>>>> index 1f20e9280062..907ece0199e0 100644
>>>>>> --- a/arch/loongarch/include/asm/pgtable.h
>>>>>> +++ b/arch/loongarch/include/asm/pgtable.h
>>>>>> @@ -448,8 +448,13 @@ static inline unsigned long
>>>>>> pte_accessible(struct
>>>>>> mm_struct *mm, pte_t a)
>>>>>>
>>>>>> static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
>>>>>> {
>>>>>> - return __pte((pte_val(pte) & _PAGE_CHG_MASK) |
>>>>>> - (pgprot_val(newprot) & ~_PAGE_CHG_MASK));
>>>>>> + pte_val(pte) = (pte_val(pte) & _PAGE_CHG_MASK) |
>>>>>> + (pgprot_val(newprot) & ~_PAGE_CHG_MASK);
>>>>>> +
>>>>>> + if (pte_val(pte) & _PAGE_DIRTY)
>>>>>> + pte_val(pte) |= _PAGE_MODIFIED;
>>>>>> +
>>>>>> + return pte;
>>>>>> }
>>>>>
>>>>> + pte_val(pte) = (pte_val(pte) & _PAGE_CHG_MASK) |
>>>>> + (pgprot_val(newprot) & ~_PAGE_CHG_MASK);
>>>>>
>>>>> After this step, _PAGE_DIRTY may have already disappeared,
>>>>> If no new variables are added, they can be modified in follow way:
>>>>>
>>>>> static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
>>>>> {
>>>>> + if (pte_val(pte) & _PAGE_DIRTY)
>>>>> + pte_val(pte) |= _PAGE_MODIFIED;
>>>>> +
>>>>> return __pte((pte_val(pte) & _PAGE_CHG_MASK) |
>>>>> (pgprot_val(newprot) & ~_PAGE_CHG_MASK));
>>>>>
>>>>> }
>>>> OK, it makes sense.
>>>>
>>>> Huacai
>>>>>
>>>>>>
>>>>>> extern void __update_tlb(struct vm_area_struct *vma,
>>>>>> @@ -583,7 +588,11 @@ static inline struct page *pmd_page(pmd_t pmd)
>>>>>> static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
>>>>>> {
>>>>>> pmd_val(pmd) = (pmd_val(pmd) & _HPAGE_CHG_MASK) |
>>>>>> - (pgprot_val(newprot) &
>>>>>> ~_HPAGE_CHG_MASK);
>>>>>> + (pgprot_val(newprot) & ~_HPAGE_CHG_MASK);
>>>>>> +
>>>>>> + if (pmd_val(pmd) & _PAGE_DIRTY)
>>>>>> + pmd_val(pmd) |= _PAGE_MODIFIED;
>>>>>> +
>>>>>> return pmd;
>>>>>> }
>>>>>>
>>>>>> You needn't define a new variable.
>>>>>>
>>>>>>
>>>>>> Huacai
>>>>>>
>>>>>>> static inline pmd_t pmd_mkinvalid(pmd_t pmd)
>>>>>>> --
>>>>>>> 2.41.0
>>>>>>>
>>>>>>>
>>>>> Thanks
>>>>>
>>>>> Tianyang
>>>>>
On 2025/11/6 下午4:50, Tianyang Zhang wrote:
>
> 在 2025/11/6 上午10:10, Bibo Mao 写道:
>>
>>
>> On 2025/11/6 上午9:55, Tianyang Zhang wrote:
>>> Hi ,Bibao
>>>
>>> 在 2025/11/5 上午9:18, Bibo Mao 写道:
>>>>
>>>>
>>>> On 2025/11/5 上午9:07, Huacai Chen wrote:
>>>>> On Wed, Nov 5, 2025 at 8:57 AM Tianyang Zhang
>>>>> <zhangtianyang@loongson.cn> wrote:
>>>>>>
>>>>>> Hi, Huacai
>>>>>>
>>>>>> 在 2025/11/4 下午4:00, Huacai Chen 写道:
>>>>>>> Hi, Tianyang,
>>>>>>>
>>>>>>> The subject line can be:
>>>>>>> LoongArch: Let {pte,pmd}_modify() record the status of
>>>>>>> _PAGE_DIRTY (If
>>>>>>> I'm right in the later comments).
>>>>>> Ok. I got it
>>>>>>>
>>>>>>> On Tue, Nov 4, 2025 at 3:30 PM Tianyang Zhang
>>>>>>> <zhangtianyang@loongson.cn> wrote:
>>>>>>>> In the current pte_modify operation, _PAGE_DIRTY might be
>>>>>>>> cleared. Since
>>>>>>>> the hardware-page-walk does not have a predefined _PAGE_MODIFIED
>>>>>>>> flag,
>>>>>>>> this could lead to loss of valid data in certain scenarios.
>>>>>>>>
>>>>>>>> The new modification involves checking whether the original PTE
>>>>>>>> has the
>>>>>>>> _PAGE_DIRTY flag. If it exists, the _PAGE_MODIFIED bit is set,
>>>>>>>> ensuring
>>>>>>>> that the pte_dirty interface can return accurate information.
>>>>>>> The description may be wrong here. Because pte_dirty() returns
>>>>>>> pte_val(pte) & (_PAGE_DIRTY | _PAGE_MODIFIED).
>>>>>>> If _PAGE_DIRTY isn't lost, pte_dirty() is always right, no matter
>>>>>>> whether there is or isn't _PAGE_MODIFIED.
>>>>>>>
>>>>>>> I think the real reason is we need to set _PAGE_MODIFIED in
>>>>>>> pte/pmd_modify to record the status of _PAGE_DIRTY, so that we can
>>>>>>> recover _PAGE_DIRTY afterwards, such as in pte/pmd_mkwrite().
>>>>>> Ok, I will adjust the description
>>>>> After some thinking, your original description may be right. Without
>>>>> this patch the scenario maybe like this:
>>>>> The pte is dirty _PAGE_DIRTY but without _PAGE_MODIFIED, after
>>>>> pte_modify() we lose _PAGE_DIRTY, then pte_dirty() returns false. So
>>>>> we need _PAGE_MODIFIED to record _PAGE_DIRTY here.
>>>> In theory pte_modify() is to modify RWX attribute. I think that it
>>>> is a tricky to remove _PAGE_DIRTY and add _PAGE_MODIFIED with HW PTW
>>>> system.
>>>>
>>>> Also _PAGE_ACCESSED is lost with pte_modify() API, is there any
>>>> influence with HW PTW system, or wait until possible problems coming
>>>> out.
>>>
>>> static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
>>> {
>>> return __pte((pte_val(pte) & _PAGE_CHG_MASK) |
>>> (pgprot_val(newprot) & ~_PAGE_CHG_MASK));
>>> }
>>> In my understand, During the pte_modify process, it is essential to
>>> ensure that specific bits are inherited from the original PTE rather
>>> than simply replaced(as set_pte),
>>>
>>> this guarantees the coherent operation of the memory management system.
>>>
>>> Since _PAGE_CHG_MASK explicitly requires preserving pte_modified, and
>> The problem is how _PAGE_CHG_MASK should be defined, do you check with
>> other architectures?
>
> Under mainstream architectures(like ARM64/X86 ), if the pte_modify
> interface clears the hard-dirty flag, it will set the soft-dirty flag
> through some mechanism.
>
> Thus, at least from the perspective of PAGE_DIRTY logic, this approach
> is right.
>
>>> there is an inherent correlation between pte_dirty and pte_modified,
>>> these attributes must be evaluated and handled accordingly.
>>>
>>> The pte_valid attribute, being a hardware property, is inherently the
>>> target of modification in the pte_modify interface. Therefore, it is
>>> reasonable not to preserve it.
>> On HW PTW system, _PAGE_PRESENT will control whether trigger page
>> fault rather than pte_valid/_PAGE_ACCESSED. For simple, do you think
>> the following code is ok or not ?
>
> "On HW PTW system, _PAGE_PRESENT will control whether trigger page fault
> rather than pte_valid/_PAGE_ACCESSED"
>
> Yes, indeed, in many cases, PAGE_PRESENT is precisely the cleanup target
> of pte_modify.
Ok, I have no any question more :) Just do it.
Regards
Bibo Mao
>
>>
>> static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
>> {
>> - return __pte((pte_val(pte) & _PAGE_CHG_MASK) |
>> - (pgprot_val(newprot) & ~_PAGE_CHG_MASK));
>> + unsigned long mask = _PAGE_CHG_MASK;
>> +
>> + if (cpu_has_ptw)
>> + mask |= _PAGE_DIRTY | _PAGE_ACCESSED;
>> + return __pte((pte_val(pte) & mask) |
>> + (pgprot_val(newprot) & ~mask));
>> }
>>
> This modification is inappropriate.
>
> Firstly, _PAGE_ACCESSED(bit 0, as _PAGE_PRESENT) and _PAGE_DIRTY bits
> are inherently the targets of pte_modify operations. Some
> sub-memory-system like numa_balance precisely rely on
>
> clearing these bits to trigger hardware exceptions and complete
> subsequent processes, this appears to be unrelated to hardware-ptw
>
> And, under hardware-ptw scenarios, the WRITE=0 && DIRTY=1 condition
> should never occur, therefore, we cannot preserve the DIRTY bit in advance.
>
> Thanks
>
> Tianyang
>
>> Regards
>> Bibo Mao
>>
>>>
>>> Thanks
>>>
>>> Tianyang
>>>
>>>>
>>>> Regards
>>>> Bibo Mao
>>>>>
>>>>> But the description also needs to be updated.
>>>>>
>>>>>>>
>>>>>>>> Co-developed-by: Liupu Wang <wangliupu@loongson.cn>
>>>>>>>> Signed-off-by: Liupu Wang <wangliupu@loongson.cn>
>>>>>>>> Signed-off-by: Tianyang Zhang <zhangtianyang@loongson.cn>
>>>>>>>> ---
>>>>>>>> arch/loongarch/include/asm/pgtable.h | 17 +++++++++++++----
>>>>>>>> 1 file changed, 13 insertions(+), 4 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/arch/loongarch/include/asm/pgtable.h
>>>>>>>> b/arch/loongarch/include/asm/pgtable.h
>>>>>>>> index bd128696e96d..106abfa5183b 100644
>>>>>>>> --- a/arch/loongarch/include/asm/pgtable.h
>>>>>>>> +++ b/arch/loongarch/include/asm/pgtable.h
>>>>>>>> @@ -424,8 +424,13 @@ static inline unsigned long
>>>>>>>> pte_accessible(struct mm_struct *mm, pte_t a)
>>>>>>>>
>>>>>>>> static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
>>>>>>>> {
>>>>>>>> - return __pte((pte_val(pte) & _PAGE_CHG_MASK) |
>>>>>>>> - (pgprot_val(newprot) & ~_PAGE_CHG_MASK));
>>>>>>>> + unsigned long val = (pte_val(pte) & _PAGE_CHG_MASK) |
>>>>>>>> + (pgprot_val(newprot) & ~_PAGE_CHG_MASK);
>>>>>>>> +
>>>>>>>> + if (pte_val(pte) & _PAGE_DIRTY)
>>>>>>>> + val |= _PAGE_MODIFIED;
>>>>>>>> +
>>>>>>>> + return __pte(val);
>>>>>>>> }
>>>>>>>>
>>>>>>>> extern void __update_tlb(struct vm_area_struct *vma,
>>>>>>>> @@ -547,9 +552,13 @@ static inline struct page *pmd_page(pmd_t pmd)
>>>>>>>>
>>>>>>>> static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
>>>>>>>> {
>>>>>>>> - pmd_val(pmd) = (pmd_val(pmd) & _HPAGE_CHG_MASK) |
>>>>>>>> + unsigned long val = (pmd_val(pmd) & _HPAGE_CHG_MASK) |
>>>>>>>> (pgprot_val(newprot) &
>>>>>>>> ~_HPAGE_CHG_MASK);
>>>>>>>> - return pmd;
>>>>>>>> +
>>>>>>>> + if (pmd_val(pmd) & _PAGE_DIRTY)
>>>>>>>> + val |= _PAGE_MODIFIED;
>>>>>>>> +
>>>>>>>> + return __pmd(val);
>>>>>>>> }
>>>>>>> A minimal modification can be:
>>>>>>> diff --git a/arch/loongarch/include/asm/pgtable.h
>>>>>>> b/arch/loongarch/include/asm/pgtable.h
>>>>>>> index 1f20e9280062..907ece0199e0 100644
>>>>>>> --- a/arch/loongarch/include/asm/pgtable.h
>>>>>>> +++ b/arch/loongarch/include/asm/pgtable.h
>>>>>>> @@ -448,8 +448,13 @@ static inline unsigned long
>>>>>>> pte_accessible(struct
>>>>>>> mm_struct *mm, pte_t a)
>>>>>>>
>>>>>>> static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
>>>>>>> {
>>>>>>> - return __pte((pte_val(pte) & _PAGE_CHG_MASK) |
>>>>>>> - (pgprot_val(newprot) & ~_PAGE_CHG_MASK));
>>>>>>> + pte_val(pte) = (pte_val(pte) & _PAGE_CHG_MASK) |
>>>>>>> + (pgprot_val(newprot) & ~_PAGE_CHG_MASK);
>>>>>>> +
>>>>>>> + if (pte_val(pte) & _PAGE_DIRTY)
>>>>>>> + pte_val(pte) |= _PAGE_MODIFIED;
>>>>>>> +
>>>>>>> + return pte;
>>>>>>> }
>>>>>>
>>>>>> + pte_val(pte) = (pte_val(pte) & _PAGE_CHG_MASK) |
>>>>>> + (pgprot_val(newprot) & ~_PAGE_CHG_MASK);
>>>>>>
>>>>>> After this step, _PAGE_DIRTY may have already disappeared,
>>>>>> If no new variables are added, they can be modified in follow way:
>>>>>>
>>>>>> static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
>>>>>> {
>>>>>> + if (pte_val(pte) & _PAGE_DIRTY)
>>>>>> + pte_val(pte) |= _PAGE_MODIFIED;
>>>>>> +
>>>>>> return __pte((pte_val(pte) & _PAGE_CHG_MASK) |
>>>>>> (pgprot_val(newprot) & ~_PAGE_CHG_MASK));
>>>>>>
>>>>>> }
>>>>> OK, it makes sense.
>>>>>
>>>>> Huacai
>>>>>>
>>>>>>>
>>>>>>> extern void __update_tlb(struct vm_area_struct *vma,
>>>>>>> @@ -583,7 +588,11 @@ static inline struct page *pmd_page(pmd_t pmd)
>>>>>>> static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
>>>>>>> {
>>>>>>> pmd_val(pmd) = (pmd_val(pmd) & _HPAGE_CHG_MASK) |
>>>>>>> - (pgprot_val(newprot) &
>>>>>>> ~_HPAGE_CHG_MASK);
>>>>>>> + (pgprot_val(newprot) & ~_HPAGE_CHG_MASK);
>>>>>>> +
>>>>>>> + if (pmd_val(pmd) & _PAGE_DIRTY)
>>>>>>> + pmd_val(pmd) |= _PAGE_MODIFIED;
>>>>>>> +
>>>>>>> return pmd;
>>>>>>> }
>>>>>>>
>>>>>>> You needn't define a new variable.
>>>>>>>
>>>>>>>
>>>>>>> Huacai
>>>>>>>
>>>>>>>> static inline pmd_t pmd_mkinvalid(pmd_t pmd)
>>>>>>>> --
>>>>>>>> 2.41.0
>>>>>>>>
>>>>>>>>
>>>>>> Thanks
>>>>>>
>>>>>> Tianyang
>>>>>>
>
© 2016 - 2025 Red Hat, Inc.