Re: [PATCH] Loongarch:Make pte/pmd_modify can set _PAGE_MODIFIED
From: Bibo Mao
Date: Thu Nov 06 2025 - 04:05:19 EST
On 2025/11/6 下午4:50, Tianyang Zhang wrote:
Ok, I have no any question more :) Just do it.
在 2025/11/6 上午10:10, Bibo Mao 写道:
On 2025/11/6 上午9:55, Tianyang Zhang wrote:
Hi ,BibaoThe problem is how _PAGE_CHG_MASK should be defined, do you check with other architectures?
在 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@xxxxxxxxxxx> wrote: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.After some thinking, your original description may be right. Without
Hi, Huacai
在 2025/11/4 下午4:00, Huacai Chen 写道:
Hi, Tianyang,Ok. I got it
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 will adjust the description
On Tue, Nov 4, 2025 at 3:30 PM Tianyang Zhang <zhangtianyang@xxxxxxxxxxx> wrote:In the current pte_modify operation, _PAGE_DIRTY might be cleared. SinceThe description may be wrong here. Because pte_dirty() returns
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.
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().
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.
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
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.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 ?
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"
Yes, indeed, in many cases, PAGE_PRESENT is precisely the cleanup target of pte_modify.
Regards
Bibo Mao
This modification is inappropriate.
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));
}
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.
OK, it makes sense.
Co-developed-by: Liupu Wang <wangliupu@xxxxxxxxxxx>A minimal modification can be:
Signed-off-by: Liupu Wang <wangliupu@xxxxxxxxxxx>
Signed-off-by: Tianyang Zhang <zhangtianyang@xxxxxxxxxxx>
---
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);
}
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));
}
Huacai
Thanks
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
Tianyang