On 05.09.24 10:11, Barry Song wrote:
From: Barry Song <v-songbaohua@xxxxxxxx>
According to David and Ryan, there isn't a bug here, even though we
don't advance the PTE entry, because __ptep_set_access_flags() only
uses the access flags from the entry.
However, we always check pte_same(pte, entry) using the first entry
in __ptep_set_access_flags(). This means that the checks from 1 to
nr - 1 are not comparing the same PTE indexes (thus, they always
return false), which can be a bit confusing. To clarify the code, let's
add some comments.
Reviewed-by: Ryan Roberts <ryan.roberts@xxxxxxx>
Signed-off-by: Barry Song <v-songbaohua@xxxxxxxx>
Cc: Ard Biesheuvel <ardb@xxxxxxxxxx>
Cc: John Hubbard <jhubbard@xxxxxxxxxx>
Cc: Mark Rutland <mark.rutland@xxxxxxx>
Cc: Catalin Marinas <catalin.marinas@xxxxxxx>
Cc: David Hildenbrand <david@xxxxxxxxxx>
Cc: Will Deacon <will@xxxxxxxxxx>
---
-v2:
* collect Ryan's rb, thanks!
* doc why but not advance entry
* refine changelog and subject
-v1:
https://lore.kernel.org/linux-mm/20240831083537.62111-1-21cnbao@xxxxxxxxx/
arch/arm64/mm/contpte.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/arm64/mm/contpte.c b/arch/arm64/mm/contpte.c
index a3edced29ac1..55107d27d3f8 100644
--- a/arch/arm64/mm/contpte.c
+++ b/arch/arm64/mm/contpte.c
@@ -421,6 +421,12 @@ int contpte_ptep_set_access_flags(struct vm_area_struct *vma,
ptep = contpte_align_down(ptep);
start_addr = addr = ALIGN_DOWN(addr, CONT_PTE_SIZE);
+ /*
+ * We are not advancing entry because __ptep_set_access_flags()
+ * only consumes access flags from entry. And since we have checked
+ * for the whole contpte block and returned early, pte_same()
+ * within __ptep_set_access_flags() is likely false.
+ */
for (i = 0; i < CONT_PTES; i++, ptep++, addr += PAGE_SIZE)
__ptep_set_access_flags(vma, addr, ptep, entry, 0);
Reviewed-by: David Hildenbrand <david@xxxxxxxxxx>
--
Cheers,
David / dhildenb
