mm/thp: fix MTE tag mismatch when replacing zero-filled subpages
Subsystems (7)
Lance Yang
reviewerPATCH 1/1 mm/thp: fix MTE tag mismatch when replacing zero-filled subpages
From: Lance Yang <[email protected]>
When both THP and MTE are enabled, splitting a THP and replacing its zero-filled subpages with the shared zeropage can cause MTE tag mismatch faults in userspace.
Remapping zero-filled subpages to the shared zeropage is unsafe, as the zeropage has a fixed tag of zero, which may not match the tag expected by the userspace pointer.
KSM already avoids this problem by using memcmp_pages(), which on arm64 intentionally reports MTE-tagged pages as non-identical to prevent unsafe merging.
As suggested by David[1], this patch adopts the same pattern, replacing the memchr_inv() byte-level check with a call to pages_identical(). This leverages existing architecture-specific logic to determine if a page is truly identical to the shared zeropage.
Having both the THP shrinker and KSM rely on pages_identical() makes the design more future-proof, IMO. Instead of handling quirks in generic code, we just let the architecture decide what makes two pages identical.
[1] https://lore.kernel.org/all/[email protected]
---
Tested on x86_64 and on QEMU for arm64 (with and without MTE support), and the fix works as expected.
mm/huge_memory.c | 15 +++------------ mm/migrate.c | 8 +------- 2 files changed, 4 insertions(+), 19 deletions(-)
mm/huge_memory.c +3 -12
@@ -4104,29 +4104,20 @@ static unsigned long deferred_split_count(struct shrinker *shrink,static bool thp_underused (struct folio *folio) {int num_zero_pages =0 , num_filled_pages =0 ; -void *kaddr;int i;for (i =0 ; i <folio_nr_pages (folio); i++) { - kaddr =kmap_local_folio (folio, i *PAGE_SIZE ); -if (!memchr_inv (kaddr,0 ,PAGE_SIZE )) { - num_zero_pages++; -if (num_zero_pages > khugepaged_max_ptes_none) { -kunmap_local (kaddr); +if (pages_identical (folio_page (folio, i),ZERO_PAGE (0 ))) { +if (++num_zero_pages > khugepaged_max_ptes_none)return true; - } }else {/* * Another path for early exit once the number * of non-zero filled pages exceeds threshold. */ - num_filled_pages++; -if (num_filled_pages >=HPAGE_PMD_NR - khugepaged_max_ptes_none) { -kunmap_local (kaddr); +if (++num_filled_pages >=HPAGE_PMD_NR - khugepaged_max_ptes_none)return false; - } } -kunmap_local (kaddr); }return false; }
mm/migrate.c +1 -7
@@ -300,9 +300,7 @@ static bool try_to_map_unused_to_zeropage(struct page_vma_mapped_walk *pvmw, unsigned long idx) {struct page *page =folio_page (folio, idx); -bool contains_data;pte_t newpte; -void *addr;if (PageCompound (page))return false;
@@ -319,11 +317,7 @@ static bool try_to_map_unused_to_zeropage(struct page_vma_mapped_walk *pvmw,* this subpage has been non present. If the subpage is only zero-filled * then map it to the shared zeropage. */ - addr =kmap_local_page (page); - contains_data =memchr_inv (addr,0 ,PAGE_SIZE ); -kunmap_local (addr); - -if (contains_data) +if (!pages_identical (page,ZERO_PAGE (0 )))return false; newpte = pte_mkspecial(pfn_pte(my_zero_pfn(pvmw->address),
Signature · 1 line
2.49.0
Zi Yan
reviewerQuoted 31 lines · Lance Yang
From: Lance Yang <[email protected]>
When both THP and MTE are enabled, splitting a THP and replacing its zero-filled subpages with the shared zeropage can cause MTE tag mismatch faults in userspace.
Remapping zero-filled subpages to the shared zeropage is unsafe, as the zeropage has a fixed tag of zero, which may not match the tag expected by the userspace pointer.
KSM already avoids this problem by using
memcmp_pages(), which on arm64 intentionally reports MTE-tagged pages as non-identical to prevent unsafe merging.As suggested by David[1], this patch adopts the same pattern, replacing the
memchr_inv()byte-level check with a call topages_identical(). This leverages existing architecture-specific logic to determine if a page is truly identical to the shared zeropage.Having both the THP shrinker and KSM rely on
pages_identical()makes the design more future-proof, IMO. Instead of handling quirks in generic code, we just let the architecture decide what makes two pages identical.[1] https://lore.kernel.org/all/[email protected]
Cc <[email protected]>Reported-by Qun-wei Lin <[email protected]>Fixes b1f202060afe ("mm: remap unused subpages to shared zeropage when splitting isolated thp")Suggested-by David Hildenbrand <[email protected]>Signed-off-by Lance Yang <[email protected]>
Quoted 3 lines
---
Tested onx86_64and on QEMU for arm64 (with and without MTE support), and the fix works as expected.
From [1], I see you mentioned RISC-V also has the address masking feature. Is it affected by this? And memcmp_pages() is only implemented by ARM64 for MTE. Should any arch with address masking always implement it to avoid the same issue?
Quoted 71 lines
mm/huge_memory.c | 15 +++------------ mm/migrate.c | 8 +------- 2 files changed, 4 insertions(+), 19 deletions(-)mm/huge_memory.c +3
-12@@ -4104,29 +4104,20 @@ static unsigned long deferred_split_count(struct shrinker *shrink,static bool thp_underused (struct folio *folio) {int num_zero_pages =0 , num_filled_pages =0 ; -void *kaddr;int i;for (i =0 ; i <folio_nr_pages (folio); i++) { - kaddr =kmap_local_folio (folio, i *PAGE_SIZE ); -if (!memchr_inv (kaddr,0 ,PAGE_SIZE )) { - num_zero_pages++; -if (num_zero_pages > khugepaged_max_ptes_none) { -kunmap_local (kaddr); +if (pages_identical (folio_page (folio, i),ZERO_PAGE (0 ))) { +if (++num_zero_pages > khugepaged_max_ptes_none)return true; - } }else {/* * Another path for early exit once the number * of non-zero filled pages exceeds threshold. */ - num_filled_pages++; -if (num_filled_pages >=HPAGE_PMD_NR - khugepaged_max_ptes_none) { -kunmap_local (kaddr); +if (++num_filled_pages >=HPAGE_PMD_NR - khugepaged_max_ptes_none)return false; - } } -kunmap_local (kaddr); }return false; }mm/migrate.c +1
-7@@ -300,9 +300,7 @@ static bool try_to_map_unused_to_zeropage(struct page_vma_mapped_walk *pvmw, unsigned long idx) {struct page *page =folio_page (folio, idx); -bool contains_data;pte_t newpte; -void *addr;if (PageCompound (page))return false;@@ -319,11 +317,7 @@ static bool try_to_map_unused_to_zeropage(struct page_vma_mapped_walk *pvmw,* this subpage has been non present. If the subpage is only zero-filled * then map it to the shared zeropage. */ - addr =kmap_local_page (page); - contains_data =memchr_inv (addr,0 ,PAGE_SIZE ); -kunmap_local (addr); - -if (contains_data) +if (!pages_identical (page,ZERO_PAGE (0 )))return false; newpte = pte_mkspecial(pfn_pte(my_zero_pfn(pvmw->address),Signature · 1 line
2.49.0
The changes look good to me. Thanks. Acked-by: Zi Yan <[email protected]>
--
Best Regards,
Yan, Zi
Lance Yang
reviewerQuoted 37 lines · Zi Yan
Quoted 31 lines · Lance Yang
From: Lance Yang <[email protected]>
When both THP and MTE are enabled, splitting a THP and replacing its zero-filled subpages with the shared zeropage can cause MTE tag mismatch faults in userspace.
Remapping zero-filled subpages to the shared zeropage is unsafe, as the zeropage has a fixed tag of zero, which may not match the tag expected by the userspace pointer.
KSM already avoids this problem by using
memcmp_pages(), which on arm64 intentionally reports MTE-tagged pages as non-identical to prevent unsafe merging.As suggested by David[1], this patch adopts the same pattern, replacing the
memchr_inv()byte-level check with a call topages_identical(). This leverages existing architecture-specific logic to determine if a page is truly identical to the shared zeropage.Having both the THP shrinker and KSM rely on
pages_identical()makes the design more future-proof, IMO. Instead of handling quirks in generic code, we just let the architecture decide what makes two pages identical.[1] https://lore.kernel.org/all/[email protected]
Cc <[email protected]>Reported-by Qun-wei Lin <[email protected]>Fixes b1f202060afe ("mm: remap unused subpages to shared zeropage when splitting isolated thp")Suggested-by David Hildenbrand <[email protected]>Signed-off-by Lance Yang <[email protected]>Quoted 3 lines
---
Tested onx86_64and on QEMU for arm64 (with and without MTE support), and the fix works as expected.
Quoted 4 lines
From [1], I see you mentioned RISC-V also has the address masking feature.
Is it affected by this? Andmemcmp_pages()is only implemented by ARM64 for MTE. Should any arch with address masking always implement it to avoid the same issue?
Yeah, I'm new to RISC-V, seems like RISC-V has a similar feature as described in Documentation/arch/riscv/uabi.rst, which is the Supm (Supervisor-mode Pointer Masking) extension.
``` Pointer masking --------------- Support for pointer masking in userspace (the Supm extension) is provided via the ``PR_SET_TAGGED_ADDR_CTRL`` and ``PR_GET_TAGGED_ADDR_CTRL`` ``prctl()`` operations. Pointer masking is disabled by default. To enable it, userspace must call ``PR_SET_TAGGED_ADDR_CTRL`` with the ``PR_PMLEN`` field set to the number of mask/tag bits needed by the application. ``PR_PMLEN`` is interpreted as a lower bound; if the kernel is unable to satisfy the request, the ``PR_SET_TAGGED_ADDR_CTRL`` operation will fail. The actual number of tag bits is returned in ``PR_PMLEN`` by the ``PR_GET_TAGGED_ADDR_CTRL`` operation. ```
But, IIUC, Supm by itself only ensures that the upper bits are ignored on memory access :)
So, RISC-V today would likely not be affected. However, once it implements full hardware tag checking, it will face the exact same zero-page problem.
Anyway, any architecture with a feature like MTE in the future will need its own memcmp_pages() to prevent unsafe merges ;)
Quoted 69 lines
Quoted 69 lines
mm/huge_memory.c | 15 +++------------ mm/migrate.c | 8 +------- 2 files changed, 4 insertions(+), 19 deletions(-)mm/huge_memory.c +3
-12@@ -4104,29 +4104,20 @@ static unsigned long deferred_split_count(struct shrinker *shrink,static bool thp_underused (struct folio *folio) {int num_zero_pages =0 , num_filled_pages =0 ; -void *kaddr;int i;for (i =0 ; i <folio_nr_pages (folio); i++) { - kaddr =kmap_local_folio (folio, i *PAGE_SIZE ); -if (!memchr_inv (kaddr,0 ,PAGE_SIZE )) { - num_zero_pages++; -if (num_zero_pages > khugepaged_max_ptes_none) { -kunmap_local (kaddr); +if (pages_identical (folio_page (folio, i),ZERO_PAGE (0 ))) { +if (++num_zero_pages > khugepaged_max_ptes_none)return true; - } }else {/* * Another path for early exit once the number * of non-zero filled pages exceeds threshold. */ - num_filled_pages++; -if (num_filled_pages >=HPAGE_PMD_NR - khugepaged_max_ptes_none) { -kunmap_local (kaddr); +if (++num_filled_pages >=HPAGE_PMD_NR - khugepaged_max_ptes_none)return false; - } } -kunmap_local (kaddr); }return false; }mm/migrate.c +1
-7@@ -300,9 +300,7 @@ static bool try_to_map_unused_to_zeropage(struct page_vma_mapped_walk *pvmw, unsigned long idx) {struct page *page =folio_page (folio, idx); -bool contains_data;pte_t newpte; -void *addr;if (PageCompound (page))return false;@@ -319,11 +317,7 @@ static bool try_to_map_unused_to_zeropage(struct page_vma_mapped_walk *pvmw,* this subpage has been non present. If the subpage is only zero-filled * then map it to the shared zeropage. */ - addr =kmap_local_page (page); - contains_data =memchr_inv (addr,0 ,PAGE_SIZE ); -kunmap_local (addr); - -if (contains_data) +if (!pages_identical (page,ZERO_PAGE (0 )))return false;
Quoted 5 lines
Quoted 3 lines
mm/migrate.c +0
-0newpte = pte_mkspecial(pfn_pte(my_zero_pfn(pvmw->address),Signature · 1 line
2.49.0The changes look good to me. Thanks. Acked-by: Zi Yan <[email protected]>
Cheers!
Quoted 25 lines · Lance Yang
From: Lance Yang <[email protected]>
When both THP and MTE are enabled, splitting a THP and replacing its zero-filled subpages with the shared zeropage can cause MTE tag mismatch faults in userspace.
Remapping zero-filled subpages to the shared zeropage is unsafe, as the zeropage has a fixed tag of zero, which may not match the tag expected by the userspace pointer.
KSM already avoids this problem by using
memcmp_pages(), which on arm64 intentionally reports MTE-tagged pages as non-identical to prevent unsafe merging.As suggested by David[1], this patch adopts the same pattern, replacing the
memchr_inv()byte-level check with a call topages_identical(). This leverages existing architecture-specific logic to determine if a page is truly identical to the shared zeropage.Having both the THP shrinker and KSM rely on
pages_identical()makes the design more future-proof, IMO. Instead of handling quirks in generic code, we just let the architecture decide what makes two pages identical.
Quoted 7 lines
Cc <[email protected]>Reported-by Qun-wei Lin <[email protected]>Fixes b1f202060afe ("mm: remap unused subpages to shared zeropage when splitting isolated thp")Suggested-by David Hildenbrand <[email protected]>Signed-off-by Lance Yang <[email protected]>---
LGTM, thanks
Signature · 3 lines
Cheers David / dhildenb
Quoted 31 lines · Lance Yang
From: Lance Yang <[email protected]>
When both THP and MTE are enabled, splitting a THP and replacing its zero-filled subpages with the shared zeropage can cause MTE tag mismatch faults in userspace.
Remapping zero-filled subpages to the shared zeropage is unsafe, as the zeropage has a fixed tag of zero, which may not match the tag expected by the userspace pointer.
KSM already avoids this problem by using
memcmp_pages(), which on arm64 intentionally reports MTE-tagged pages as non-identical to prevent unsafe merging.As suggested by David[1], this patch adopts the same pattern, replacing the
memchr_inv()byte-level check with a call topages_identical(). This leverages existing architecture-specific logic to determine if a page is truly identical to the shared zeropage.Having both the THP shrinker and KSM rely on
pages_identical()makes the design more future-proof, IMO. Instead of handling quirks in generic code, we just let the architecture decide what makes two pages identical.[1] https://lore.kernel.org/all/[email protected]
Cc <[email protected]>Reported-by Qun-wei Lin <[email protected]>Fixes b1f202060afe ("mm: remap unused subpages to shared zeropage when splitting isolated thp")Suggested-by David Hildenbrand <[email protected]>Signed-off-by Lance Yang <[email protected]>
Quoted 8 lines
---
Tested onx86_64and on QEMU for arm64 (with and without MTE support), and the fix works as expected.mm/huge_memory.c | 15 +++------------ mm/migrate.c | 8 +------- 2 files changed, 4 insertions(+), 19 deletions(-)
Thanks for the fix!
Quoted 23 lines · Lance Yang
From: Lance Yang <[email protected]>
When both THP and MTE are enabled, splitting a THP and replacing its zero-filled subpages with the shared zeropage can cause MTE tag mismatch faults in userspace.
Remapping zero-filled subpages to the shared zeropage is unsafe, as the zeropage has a fixed tag of zero, which may not match the tag expected by the userspace pointer.
KSM already avoids this problem by using
memcmp_pages(), which on arm64 intentionally reports MTE-tagged pages as non-identical to prevent unsafe merging.As suggested by David[1], this patch adopts the same pattern, replacing the
memchr_inv()byte-level check with a call topages_identical(). This leverages existing architecture-specific logic to determine if a page is truly identical to the shared zeropage.Having both the THP shrinker and KSM rely on
pages_identical()makes the design more future-proof, IMO. Instead of handling quirks in generic code, we just let the architecture decide what makes two pages identical.
Quoted 8 lines
[1] https://lore.kernel.org/all/[email protected]
Cc <[email protected]>Reported-by Qun-wei Lin <[email protected]>Fixes b1f202060afe ("mm: remap unused subpages to shared zeropage when splitting isolated thp")Suggested-by David Hildenbrand <[email protected]>Signed-off-by Lance Yang <[email protected]>
Functionally, the patch looks fine, both with and without MTE.
Quoted 10 lines
mm/huge_memory.c +0
-1@@ -4104,29 +4104,20 @@ static unsigned long deferred_split_count(struct shrinker *shrink,static bool thp_underused (struct folio *folio) {int num_zero_pages =0 , num_filled_pages =0 ; -void *kaddr;int i;
mm/huge_memory.c static unsigned long deferred_split_count(struct shrinker *shrink, · line 4112
Quoted 10 lines
mm/huge_memory.c +2
-5for (i =0 ; i <folio_nr_pages (folio); i++) { - kaddr =kmap_local_folio (folio, i *PAGE_SIZE ); -if (!memchr_inv (kaddr,0 ,PAGE_SIZE )) { - num_zero_pages++; -if (num_zero_pages > khugepaged_max_ptes_none) { -kunmap_local (kaddr); +if (pages_identical (folio_page (folio, i),ZERO_PAGE (0 ))) { +if (++num_zero_pages > khugepaged_max_ptes_none)return true;
I wonder what the overhead of doing a memcmp() vs memchr_inv() is. The former will need to read from two places. If it's noticeable, it would affect architectures that don't have an MTE equivalent.
Alternatively we could introduce something like folio_has_metadata() which on arm64 simply checks PG_mte_tagged.
Signature · 1 line
Catalin
Quoted 62 lines · Catalin Marinas
Quoted 23 lines · Lance Yang
From: Lance Yang <[email protected]>
When both THP and MTE are enabled, splitting a THP and replacing its zero-filled subpages with the shared zeropage can cause MTE tag mismatch faults in userspace.
Remapping zero-filled subpages to the shared zeropage is unsafe, as the zeropage has a fixed tag of zero, which may not match the tag expected by the userspace pointer.
KSM already avoids this problem by using
memcmp_pages(), which on arm64 intentionally reports MTE-tagged pages as non-identical to prevent unsafe merging.As suggested by David[1], this patch adopts the same pattern, replacing the
memchr_inv()byte-level check with a call topages_identical(). This leverages existing architecture-specific logic to determine if a page is truly identical to the shared zeropage.Having both the THP shrinker and KSM rely on
pages_identical()makes the design more future-proof, IMO. Instead of handling quirks in generic code, we just let the architecture decide what makes two pages identical.Quoted 8 lines
[1] https://lore.kernel.org/all/[email protected]
Cc <[email protected]>Reported-by Qun-wei Lin <[email protected]>Fixes b1f202060afe ("mm: remap unused subpages to shared zeropage when splitting isolated thp")Suggested-by David Hildenbrand <[email protected]>Signed-off-by Lance Yang <[email protected]>Functionally, the patch looks fine, both with and without MTE.
Reviewed-by Catalin Marinas <[email protected]>Quoted 10 lines
mm/huge_memory.c +0
-1@@ -4104,29 +4104,20 @@ static unsigned long deferred_split_count(struct shrinker *shrink,static bool thp_underused (struct folio *folio) {int num_zero_pages =0 , num_filled_pages =0 ; -void *kaddr;int i;mm/huge_memory.c static unsigned long deferred_split_count(struct shrinker *shrink, · line 4112
Quoted 10 lines
mm/huge_memory.c +2
-5for (i =0 ; i <folio_nr_pages (folio); i++) { - kaddr =kmap_local_folio (folio, i *PAGE_SIZE ); -if (!memchr_inv (kaddr,0 ,PAGE_SIZE )) { - num_zero_pages++; -if (num_zero_pages > khugepaged_max_ptes_none) { -kunmap_local (kaddr); +if (pages_identical (folio_page (folio, i),ZERO_PAGE (0 ))) { +if (++num_zero_pages > khugepaged_max_ptes_none)return true;I wonder what the overhead of doing a
memcmp()vsmemchr_inv()is. The former will need to read from two places. If it's noticeable, it would affect architectures that don't have an MTE equivalent.
Quoted 2 lines
Alternatively we could introduce something like
folio_has_metadata()which on arm64 simply checksPG_mte_tagged.
We discussed something similar in the other thread (I suggested page_is_mergable()). I'd prefer to use pages_identical() for now, so we have the same logic here and in ksm code.
(this patch here almost looks like a cleanup :) )
If this becomes a problem, what we could do is in pages_identical() would be simply doing the memchr_inv() in case is_zero_pfn(). KSM might benefit from that as well when merging with the shared zeropage through try_to_merge_with_zero_page().
Signature · 3 lines
Cheers David / dhildenb
Lance Yang
reviewerQuoted 34 lines · David Hildenbrand
Quoted 33 lines · Catalin Marinas
Quoted 31 lines · Lance Yang
From: Lance Yang <[email protected]>
When both THP and MTE are enabled, splitting a THP and replacing its zero-filled subpages with the shared zeropage can cause MTE tag mismatch faults in userspace.
Remapping zero-filled subpages to the shared zeropage is unsafe, as the zeropage has a fixed tag of zero, which may not match the tag expected by the userspace pointer.
KSM already avoids this problem by using
memcmp_pages(), which on arm64 intentionally reports MTE-tagged pages as non-identical to prevent unsafe merging.As suggested by David[1], this patch adopts the same pattern, replacing the
memchr_inv()byte-level check with a call topages_identical(). This leverages existing architecture-specific logic to determine if a page is truly identical to the shared zeropage.Having both the THP shrinker and KSM rely on
pages_identical()makes the design more future-proof, IMO. Instead of handling quirks in generic code, we just let the architecture decide what makes two pages identical.[1] https://lore.kernel.org/all/ [email protected]
Cc <[email protected]>Reported-by Qun-wei Lin <[email protected]>Fixes b1f202060afe ("mm: remap unused subpages to shared zeropage when splitting isolated thp")Suggested-by David Hildenbrand <[email protected]>Signed-off-by Lance Yang <[email protected]>
Quoted 3 lines
Quoted 3 lines
Functionally, the patch looks fine, both with and without MTE.
Reviewed-by Catalin Marinas <[email protected]>
Thanks for taking time to review!
Quoted 26 lines
Quoted 26 lines
Quoted 11 lines
mm/huge_memory.c +0
-1@@ -4104,29 +4104,20 @@ static unsigned long deferred_split_count(struct shrinker *shrink, static bool thp_underused(struct folio *folio) { int num_zero_pages = 0, num_filled_pages = 0; - void *kaddr;mm/huge_memory.c static unsigned long · line 4112
Quoted 10 lines
mm/huge_memory.c +2
-5int i; for (i = 0; i < folio_nr_pages(folio); i++) { - kaddr = kmap_local_folio(folio, i * PAGE_SIZE); - if (!memchr_inv(kaddr, 0, PAGE_SIZE)) { - num_zero_pages++; - if (num_zero_pages > khugepaged_max_ptes_none) { - kunmap_local(kaddr); + if (pages_identical(folio_page(folio, i), ZERO_PAGE(0))) { + if (++num_zero_pages > khugepaged_max_ptes_none) return true;I wonder what the overhead of doing a
memcmp()vsmemchr_inv()is. The former will need to read from two places. If it's noticeable, it would affect architectures that don't have an MTE equivalent.
Quoted 6 lines
Quoted 2 lines
Alternatively we could introduce something like
folio_has_metadata()which on arm64 simply checksPG_mte_tagged.We discussed something similar in the other thread (I suggested
page_is_mergable()). I'd prefer to usepages_identical()for now, so we have the same logic here and in ksm code.(this patch here almost looks like a cleanup :) )
Yeah, let's keep it as-is for now.
Using the same pages_identical() pattern as KSM makes the logic consistent.
And it's simple enough to be easily backported to stable trees ;)
Quoted 2 lines
If this becomes a problem, what we could do is in
pages_identical()would be simply doing thememchr_inv()in caseis_zero_pfn(). KSM might benefit from that as well when merging with the shared zeropage throughtry_to_merge_with_zero_page().
Right, there is room for that optimization. I will look into it as a follow-up patch after this one is settled and backported, especially if the performance overhead turns out to be a real concern :)
Cheers,
Lance
Catalin Marinas
maintainerQuoted 33 lines · David Hildenbrand
Quoted 25 lines · Catalin Marinas
Quoted 9 lines · Lance Yang
mm/huge_memory.c +0
-1@@ -4104,29 +4104,20 @@ static unsigned long deferred_split_count(struct shrinker *shrink,static bool thp_underused (struct folio *folio) {int num_zero_pages =0 , num_filled_pages =0 ; -void *kaddr;mm/huge_memory.c static unsigned long deferred_split_count(struct shrinker *shrink, · line 4111
Quoted 10 lines
mm/huge_memory.c +2
-5int i;for (i =0 ; i <folio_nr_pages (folio); i++) { - kaddr =kmap_local_folio (folio, i *PAGE_SIZE ); -if (!memchr_inv (kaddr,0 ,PAGE_SIZE )) { - num_zero_pages++; -if (num_zero_pages > khugepaged_max_ptes_none) { -kunmap_local (kaddr); +if (pages_identical (folio_page (folio, i),ZERO_PAGE (0 ))) { +if (++num_zero_pages > khugepaged_max_ptes_none)return true;I wonder what the overhead of doing a
memcmp()vsmemchr_inv()is. The former will need to read from two places. If it's noticeable, it would affect architectures that don't have an MTE equivalent.Quoted 2 lines
Alternatively we could introduce something like
folio_has_metadata()which on arm64 simply checksPG_mte_tagged.We discussed something similar in the other thread (I suggested
page_is_mergable()). I'd prefer to usepages_identical()for now, so we have the same logic here and in ksm code.
Quoted 6 lines
(this patch here almost looks like a cleanup :) )
If this becomes a problem, what we could do is in
pages_identical()would be simply doing thememchr_inv()in caseis_zero_pfn(). KSM might benefit from that as well when merging with the shared zeropage throughtry_to_merge_with_zero_page().
Yes, we can always optimise it later.
I just realised that on arm64 with MTE we won't get any merging with the zero page even if the user page isn't mapped with PROT_MTE. In cpu_enable_mte() we zero the tags in the zero page and set PG_mte_tagged. The reason is that we want to use the zero page with PROT_MTE mappings (until tag setting causes CoW). Hmm, the arm64 memcmp_pages() messed up KSM merging with the zero page even before this patch.
The MTE tag setting evolved a bit over time with some locking using PG_* flags to avoid a set_pte_at() race trying to initialise the tags on the same page. We also moved the swap restoring to arch_swap_restore() rather than the set_pte_at() path. So it is safe now to merge with the zero page if the other page isn't tagged. A subsequent set_pte_at() attempting to clear the tags would notice that the zero page is already tagged.
We could go a step further and add tag comparison (I had some code around) but I think the quick fix is to just not treat the zero page as tagged. Not fully tested yet:
arch/arm64/kernel/mte.c +5 -4
@@ -73,6 +73,8 @@ int memcmp_pages(struct page *page1, struct page *page2) {char *addr1, *addr2;int ret; +bool page1_tagged =page_mte_tagged (page1) && !is_zero_page (page1); +bool page2_tagged =page_mte_tagged (page2) && !is_zero_page (page2); addr1 =page_address (page1); addr2 =page_address (page2);
@@ -83,11 +85,10 @@ int memcmp_pages(struct page *page1, struct page *page2)/* * If the page content is identical but at least one of the pages is -* tagged, return non-zero to avoid KSM merging. If only one of the -* pages is tagged, __set_ptes() may zero or change the tags of the -* other page via mte_sync_tags(). +* tagged, return non-zero to avoid KSM merging. Ignore the zero page +* since it is always tagged with the tags cleared. */ -if (page_mte_tagged (page1) ||page_mte_tagged (page2)) +if (page1_tagged || page2_tagged)return addr1 != addr2;return ret;
Signature · 1 line
Catalin
Quoted 59 lines · Catalin Marinas
Quoted 33 lines · David Hildenbrand
Quoted 25 lines · Catalin Marinas
Quoted 9 lines · Lance Yang
mm/huge_memory.c +0
-1@@ -4104,29 +4104,20 @@ static unsigned long deferred_split_count(struct shrinker *shrink,static bool thp_underused (struct folio *folio) {int num_zero_pages =0 , num_filled_pages =0 ; -void *kaddr;mm/huge_memory.c static unsigned long deferred_split_count(struct shrinker *shrink, · line 4111
Quoted 10 lines
mm/huge_memory.c +2
-5int i;for (i =0 ; i <folio_nr_pages (folio); i++) { - kaddr =kmap_local_folio (folio, i *PAGE_SIZE ); -if (!memchr_inv (kaddr,0 ,PAGE_SIZE )) { - num_zero_pages++; -if (num_zero_pages > khugepaged_max_ptes_none) { -kunmap_local (kaddr); +if (pages_identical (folio_page (folio, i),ZERO_PAGE (0 ))) { +if (++num_zero_pages > khugepaged_max_ptes_none)return true;I wonder what the overhead of doing a
memcmp()vsmemchr_inv()is. The former will need to read from two places. If it's noticeable, it would affect architectures that don't have an MTE equivalent.Quoted 2 lines
Alternatively we could introduce something like
folio_has_metadata()which on arm64 simply checksPG_mte_tagged.We discussed something similar in the other thread (I suggested
page_is_mergable()). I'd prefer to usepages_identical()for now, so we have the same logic here and in ksm code.Quoted 6 lines
(this patch here almost looks like a cleanup :) )
If this becomes a problem, what we could do is in
pages_identical()would be simply doing thememchr_inv()in caseis_zero_pfn(). KSM might benefit from that as well when merging with the shared zeropage throughtry_to_merge_with_zero_page().Yes, we can always optimise it later.
I just realised that on arm64 with MTE we won't get any merging with the zero page even if the user page isn't mapped with
PROT_MTE. Incpu_enable_mte()we zero the tags in the zero page and setPG_mte_tagged. The reason is that we want to use the zero page withPROT_MTEmappings (until tag setting causes CoW). Hmm, the arm64memcmp_pages()messed up KSM merging with the zero page even before this patch.The MTE tag setting evolved a bit over time with some locking using PG_* flags to avoid a
set_pte_at()race trying to initialise the tags on the same page. We also moved the swap restoring toarch_swap_restore()rather than theset_pte_at()path. So it is safe now to merge with the zero page if the other page isn't tagged. A subsequentset_pte_at()attempting to clear the tags would notice that the zero page is already tagged.
Quoted 3 lines
We could go a step further and add tag comparison (I had some code around) but I think the quick fix is to just not treat the zero page as tagged.
I assume any tag changes would result in CoW.
It would be interesting to know if there are use cases with VMs or other workloads where that could be beneficial with KSM.
Quoted 18 lines
Not fully tested yet:
arch/arm64/kernel/mte.c +2
-0@@ -73,6 +73,8 @@ int memcmp_pages(struct page *page1, struct page *page2) {char *addr1, *addr2;int ret; +bool page1_tagged =page_mte_tagged (page1) && !is_zero_page (page1); +bool page2_tagged =page_mte_tagged (page2) && !is_zero_page (page2); addr1 =page_address (page1); addr2 =page_address (page2);@@ -83,11 +85,10 @@ int memcmp_pages(struct page *page1, struct page *page2)/*
arch/arm64/kernel/mte.c int memcmp_pages(struct page *page1, struct page *page2) · line 92
Quoted 10 lines
arch/arm64/kernel/mte.c +3
-4* If the page content is identical but at least one of the pages is -* tagged, return non-zero to avoid KSM merging. If only one of the -* pages is tagged, __set_ptes() may zero or change the tags of the -* other page via mte_sync_tags(). +* tagged, return non-zero to avoid KSM merging. Ignore the zero page +* since it is always tagged with the tags cleared. */ -if (page_mte_tagged (page1) ||page_mte_tagged (page2)) +if (page1_tagged || page2_tagged)return addr1 != addr2;
That looks reasonable to me.
@Lance as you had a test setup, could you give this a try as well with KSM shared zeropage deduplication enabled whether it now works as expected as well?
Then, this should likely be an independent fix.
For KSM you likely have to enable it first through /sys/kernel/mm/ksm/use_zero_pages.
Signature · 3 lines
Cheers David / dhildenb
Lance Yang
reviewerQuoted 92 lines · David Hildenbrand
Quoted 60 lines · Catalin Marinas
Quoted 34 lines · David Hildenbrand
Quoted 26 lines · Catalin Marinas
Quoted 10 lines · Lance Yang
mm/huge_memory.c +0
-1@@ -4104,29 +4104,20 @@ static unsigned long deferred_split_count(struct shrinker *shrink, static bool thp_underused(struct folio *folio) { int num_zero_pages = 0, num_filled_pages = 0; - void *kaddr;mm/huge_memory.c static unsigned long · line 4112
Quoted 10 lines
mm/huge_memory.c +2
-5int i; for (i = 0; i < folio_nr_pages(folio); i++) { - kaddr = kmap_local_folio(folio, i * PAGE_SIZE); - if (!memchr_inv(kaddr, 0, PAGE_SIZE)) { - num_zero_pages++; - if (num_zero_pages > khugepaged_max_ptes_none) { - kunmap_local(kaddr); + if (pages_identical(folio_page(folio, i), ZERO_PAGE(0))) { + if (++num_zero_pages > khugepaged_max_ptes_none) return true;I wonder what the overhead of doing a
memcmp()vsmemchr_inv()is. The former will need to read from two places. If it's noticeable, it would affect architectures that don't have an MTE equivalent.Quoted 2 lines
Alternatively we could introduce something like
folio_has_metadata()which on arm64 simply checksPG_mte_tagged.We discussed something similar in the other thread (I suggested
page_is_mergable()). I'd prefer to usepages_identical()for now, so we have the same logic here and in ksm code.Quoted 6 lines
(this patch here almost looks like a cleanup :) )
If this becomes a problem, what we could do is in
pages_identical()would be simply doing thememchr_inv()in caseis_zero_pfn(). KSM might benefit from that as well when merging with the shared zeropage throughtry_to_merge_with_zero_page().Yes, we can always optimise it later.
I just realised that on arm64 with MTE we won't get any merging with the zero page even if the user page isn't mapped with
PROT_MTE. Incpu_enable_mte()we zero the tags in the zero page and setPG_mte_tagged. The reason is that we want to use the zero page withPROT_MTEmappings (until tag setting causes CoW). Hmm, the arm64memcmp_pages()messed up KSM merging with the zero page even before this patch.The MTE tag setting evolved a bit over time with some locking using PG_* flags to avoid a
set_pte_at()race trying to initialise the tags on the same page. We also moved the swap restoring toarch_swap_restore()rather than theset_pte_at()path. So it is safe now to merge with the zero page if the other page isn't tagged. A subsequentset_pte_at()attempting to clear the tags would notice that the zero page is already tagged.Quoted 3 lines
We could go a step further and add tag comparison (I had some code around) but I think the quick fix is to just not treat the zero page as tagged.
I assume any tag changes would result in CoW.
It would be interesting to know if there are use cases with VMs or other workloads where that could be beneficial with KSM.
Quoted 23 lines
Not fully tested yet:
arch/arm64/kernel/mte.c +3
-3@@ -73,6 +73,8 @@ int memcmp_pages(struct page *page1, struct page *page2) { char *addr1, *addr2; int ret; + bool page1_tagged = page_mte_tagged(page1) && !is_zero_page(page1); + bool page2_tagged = page_mte_tagged(page2) && !is_zero_page(page2); addr1 = page_address(page1); addr2 = page_address(page2);@@ -83,11 +85,10 @@ int memcmp_pages(struct page *page1, struct page *page2) /** If the page content is identical but at least one of the pages is -* tagged, return non-zero to avoid KSM merging. If only one of the -* pages is tagged, __set_ptes() may zero or change the tags of the -* other page via mte_sync_tags(). +* tagged, return non-zero to avoid KSM merging. Ignore the zero
Quoted 8 lines
Quoted 6 lines
page
+ * since it is always tagged with the tags cleared. */
- if (page_mte_tagged(page1)||page_mte_tagged(page2)) + if (page1_tagged||page2_tagged)
return addr1 != addr2;That looks reasonable to me.
Yeah, looks good to me as well.
Quoted 3 lines
@Lance as you had a test setup, could you give this a try as well with KSM shared zeropage deduplication enabled whether it now works as expected as well?
Sure. I'll test that and get back to you ;)
Quoted 4 lines
Then, this should likely be an independent fix.
For KSM you likely have to enable it first through
/sys/kernel/mm/ksm/use_zero_pages.
Got it.
Catalin Marinas
maintainerQuoted 17 lines · David Hildenbrand
Quoted 16 lines · Catalin Marinas
I just realised that on arm64 with MTE we won't get any merging with the zero page even if the user page isn't mapped with
PROT_MTE. Incpu_enable_mte()we zero the tags in the zero page and setPG_mte_tagged. The reason is that we want to use the zero page withPROT_MTEmappings (until tag setting causes CoW). Hmm, the arm64memcmp_pages()messed up KSM merging with the zero page even before this patch.The MTE tag setting evolved a bit over time with some locking using PG_* flags to avoid a
set_pte_at()race trying to initialise the tags on the same page. We also moved the swap restoring toarch_swap_restore()rather than theset_pte_at()path. So it is safe now to merge with the zero page if the other page isn't tagged. A subsequentset_pte_at()attempting to clear the tags would notice that the zero page is already tagged.
Quoted 5 lines
Quoted 3 lines
We could go a step further and add tag comparison (I had some code around) but I think the quick fix is to just not treat the zero page as tagged.
I assume any tag changes would result in CoW.
Yes.
Quoted 2 lines
It would be interesting to know if there are use cases with VMs or other workloads where that could be beneficial with KSM.
With VMs, if MTE is allowed in the guest, we currently treat any VM page as tagged. In the initial version of the MTE spec, we did not have any fine-rained control at the stage 2 page table over whether MTE is in use by the guest (just a big knob in a control register). We later got FEAT_MTE_PERM which allows stage 2 to trap tag accesses in a VM on a page by page basis, though we haven't added KVM support for it yet.
If we add full tag comparison, VMs may be able to share more pages. For example, code pages are never tagged in a VM but the hypervisor doesn't know this, so it just avoids sharing. I posted tag comparison some years ago but dropped it eventually to keep things simple:
https://lore.kernel.org/all/[email protected]/
However, it needs a bit of tidying up since at the time we assumed everything was tagged. I can respin the above (on top of the fix below), though I don't see many vendors rushing to deploy MTE in a multi-VM scenario (Android + pKVM maybe but not sure they enable KSM due to power constraints).
Quoted 27 lines
Quoted 14 lines
arch/arm64/kernel/mte.c +2
-0@@ -73,6 +73,8 @@ int memcmp_pages(struct page *page1, struct page *page2) {char *addr1, *addr2;int ret; +bool page1_tagged =page_mte_tagged (page1) && !is_zero_page (page1); +bool page2_tagged =page_mte_tagged (page2) && !is_zero_page (page2); addr1 =page_address (page1); addr2 =page_address (page2);@@ -83,11 +85,10 @@ int memcmp_pages(struct page *page1, struct page *page2)/* arch/arm64/kernel/mte.c int memcmp_pages(struct page *page1, struct page *page2) · line 91
Quoted 10 lines
arch/arm64/kernel/mte.c +3
-4* If the page content is identical but at least one of the pages is -* tagged, return non-zero to avoid KSM merging. If only one of the -* pages is tagged, __set_ptes() may zero or change the tags of the -* other page via mte_sync_tags(). +* tagged, return non-zero to avoid KSM merging. Ignore the zero page +* since it is always tagged with the tags cleared. */ -if (page_mte_tagged (page1) ||page_mte_tagged (page2)) +if (page1_tagged || page2_tagged)return addr1 != addr2;That looks reasonable to me.
Quoted 3 lines
@Lance as you had a test setup, could you give this a try as well with KSM shared zeropage deduplication enabled whether it now works as expected as well?
Thanks!
Quoted 1 line
Then, this should likely be an independent fix.
Yes, I'll add a proper commit message. We could do a cc stable, though it's more of an optimisation.
Signature · 1 line
Catalin
Lance Yang
reviewerQuoted 61 lines · David Hildenbrand
Quoted 60 lines · Catalin Marinas
Quoted 34 lines · David Hildenbrand
Quoted 26 lines · Catalin Marinas
Quoted 10 lines · Lance Yang
mm/huge_memory.c +0
-1@@ -4104,29 +4104,20 @@ static unsigned long deferred_split_count(struct shrinker *shrink, static bool thp_underused(struct folio *folio) { int num_zero_pages = 0, num_filled_pages = 0; - void *kaddr;mm/huge_memory.c static unsigned long · line 4112
Quoted 10 lines
mm/huge_memory.c +2
-5int i; for (i = 0; i < folio_nr_pages(folio); i++) { - kaddr = kmap_local_folio(folio, i * PAGE_SIZE); - if (!memchr_inv(kaddr, 0, PAGE_SIZE)) { - num_zero_pages++; - if (num_zero_pages > khugepaged_max_ptes_none) { - kunmap_local(kaddr); + if (pages_identical(folio_page(folio, i), ZERO_PAGE(0))) { + if (++num_zero_pages > khugepaged_max_ptes_none) return true;I wonder what the overhead of doing a
memcmp()vsmemchr_inv()is. The former will need to read from two places. If it's noticeable, it would affect architectures that don't have an MTE equivalent.Quoted 2 lines
Alternatively we could introduce something like
folio_has_metadata()which on arm64 simply checksPG_mte_tagged.We discussed something similar in the other thread (I suggested
page_is_mergable()). I'd prefer to usepages_identical()for now, so we have the same logic here and in ksm code.Quoted 6 lines
(this patch here almost looks like a cleanup :) )
If this becomes a problem, what we could do is in
pages_identical()would be simply doing thememchr_inv()in caseis_zero_pfn(). KSM might benefit from that as well when merging with the shared zeropage throughtry_to_merge_with_zero_page().Yes, we can always optimise it later.
I just realised that on arm64 with MTE we won't get any merging with the zero page even if the user page isn't mapped with
PROT_MTE. Incpu_enable_mte()we zero the tags in the zero page and setPG_mte_tagged. The reason is that we want to use the zero page withPROT_MTEmappings (until tag setting causes CoW). Hmm, the arm64memcmp_pages()messed up KSM merging with the zero page even before this patch.The MTE tag setting evolved a bit over time with some locking using PG_* flags to avoid a
set_pte_at()race trying to initialise the tags on the same page. We also moved the swap restoring toarch_swap_restore()rather than theset_pte_at()path. So it is safe now to merge with the zero page if the other page isn't tagged. A subsequentset_pte_at()attempting to clear the tags would notice that the zero page is already tagged.
Quoted 5 lines
Quoted 3 lines
We could go a step further and add tag comparison (I had some code around) but I think the quick fix is to just not treat the zero page as tagged.
I assume any tag changes would result in CoW.
Right, I did a test and confirmed that any attempt to change tags on the shared zero page results in a Copy-on-Write, as expected ;)
Quoted 35 lines
It would be interesting to know if there are use cases with VMs or other workloads where that could be beneficial with KSM.
Quoted 23 lines
Not fully tested yet:
arch/arm64/kernel/mte.c +3
-3@@ -73,6 +73,8 @@ int memcmp_pages(struct page *page1, struct page *page2) { char *addr1, *addr2; int ret; + bool page1_tagged = page_mte_tagged(page1) && !is_zero_page(page1); + bool page2_tagged = page_mte_tagged(page2) && !is_zero_page(page2); addr1 = page_address(page1); addr2 = page_address(page2);@@ -83,11 +85,10 @@ int memcmp_pages(struct page *page1, struct page *page2) /** If the page content is identical but at least one of the pages is -* tagged, return non-zero to avoid KSM merging. If only one of the -* pages is tagged, __set_ptes() may zero or change the tags of the -* other page via mte_sync_tags(). +* tagged, return non-zero to avoid KSM merging. Ignore the zero Quoted 6 lines
page
+ * since it is always tagged with the tags cleared. */
- if (page_mte_tagged(page1)||page_mte_tagged(page2)) + if (page1_tagged||page2_tagged)
return addr1 != addr2;That looks reasonable to me.
Quoted 2 lines
@Lance as you had a test setup, could you give this a try as well with KSM shared zeropage deduplication enabled whether it now works as expected as well?
This works as expected. Both KSM (with use_zero_pages enabled) and the THP shrinker are now able to successfully merge zero-filled pages with the shared zero page, as long as those pages are not mapped with PROT_MTE.
Quoted 2 lines
Then, this should likely be an independent fix.
@Catalin could you send a real patch?
Catalin Marinas
maintainerQuoted 7 lines · Catalin Marinas
I just realised that on arm64 with MTE we won't get any merging with the zero page even if the user page isn't mapped with
PROT_MTE. Incpu_enable_mte()we zero the tags in the zero page and setPG_mte_tagged. The reason is that we want to use the zero page withPROT_MTEmappings (until tag setting causes CoW). Hmm, the arm64memcmp_pages()messed up KSM merging with the zero page even before this patch.
[...]
Quoted 18 lines
arch/arm64/kernel/mte.c +2
-1@@ -73,6 +73,8 @@ int memcmp_pages(struct page *page1, struct page *page2) {char *addr1, *addr2;int ret; +bool page1_tagged =page_mte_tagged (page1) && !is_zero_page (page1); +bool page2_tagged =page_mte_tagged (page2) && !is_zero_page (page2); addr1 =page_address (page1); addr2 =page_address (page2);@@ -83,11 +85,10 @@ int memcmp_pages(struct page *page1, struct page *page2)/* * If the page content is identical but at least one of the pages is -* tagged, return non-zero to avoid KSM merging. If only one of the
arch/arm64/kernel/mte.c int memcmp_pages(struct page *page1, struct page *page2) · line 94
Quoted 10 lines
arch/arm64/kernel/mte.c +3
-3-* pages is tagged, __set_ptes() may zero or change the tags of the -* other page via mte_sync_tags(). +* tagged, return non-zero to avoid KSM merging. Ignore the zero page +* since it is always tagged with the tags cleared. */ -if (page_mte_tagged (page1) ||page_mte_tagged (page2)) +if (page1_tagged || page2_tagged)return addr1 != addr2;return ret;
Unrelated to this discussion, I got an internal report that Linux hangs during boot with CONFIG_DEFERRED_STRUCT_PAGE_INIT because try_page_mte_tagging() locks up on uninitialised page flags.
Since we (always?) map the zero page as pte_special(), set_pte_at() won't check if the tags have to be initialised, so we can skip the PG_mte_tagged altogether. We actually had this code for some time until we introduced the pte_special() check in set_pte_at().
So alternative patch that also fixes the deferred struct page init (on the assumptions that the zero page is always mapped as pte_special():
arch/arm64/kernel/cpufeature.c +7 -3
@@ -2419,17 +2419,21 @@ static void bti_enable(const struct arm64_cpu_capabilities *__unused)#ifdef CONFIG_ARM64_MTE static void cpu_enable_mte (struct arm64_cpu_capabilities const *cap) { +static bool cleared_zero_page = false; +sysreg_clear_set (sctlr_el1,0 , SCTLR_ELx_ATA |SCTLR_EL1_ATA0 );mte_cpu_setup ();/* * Clear the tags in the zero page. This needs to be done via the -* linear map which has the Tagged attribute. +* linear map which has the Tagged attribute. Since this page is +* always mapped as pte_special(), set_pte_at() will not attempt to +* clear the tags or set PG_mte_tagged. */ -if (try_page_mte_tagging (ZERO_PAGE (0 ))) { +if (!cleared_zero_page) { + cleared_zero_page = true;mte_clear_page_tags (lm_alias (empty_zero_page)); -set_page_mte_tagged (ZERO_PAGE (0 )); }kasan_init_hw_tags_cpu ();
Signature · 1 line
Catalin
Quoted 36 lines · Catalin Marinas
Quoted 7 lines · Catalin Marinas
I just realised that on arm64 with MTE we won't get any merging with the zero page even if the user page isn't mapped with
PROT_MTE. Incpu_enable_mte()we zero the tags in the zero page and setPG_mte_tagged. The reason is that we want to use the zero page withPROT_MTEmappings (until tag setting causes CoW). Hmm, the arm64memcmp_pages()messed up KSM merging with the zero page even before this patch.[...]
Quoted 27 lines
arch/arm64/kernel/mte.c +5
-4@@ -73,6 +73,8 @@ int memcmp_pages(struct page *page1, struct page *page2) {char *addr1, *addr2;int ret; +bool page1_tagged =page_mte_tagged (page1) && !is_zero_page (page1); +bool page2_tagged =page_mte_tagged (page2) && !is_zero_page (page2); addr1 =page_address (page1); addr2 =page_address (page2);@@ -83,11 +85,10 @@ int memcmp_pages(struct page *page1, struct page *page2)/* * If the page content is identical but at least one of the pages is -* tagged, return non-zero to avoid KSM merging. If only one of the -* pages is tagged, __set_ptes() may zero or change the tags of the -* other page via mte_sync_tags(). +* tagged, return non-zero to avoid KSM merging. Ignore the zero page +* since it is always tagged with the tags cleared. */ -if (page_mte_tagged (page1) ||page_mte_tagged (page2)) +if (page1_tagged || page2_tagged)return addr1 != addr2;
Quoted 7 lines
arch/arm64/kernel/mte.c int memcmp_pages(struct page *page1, struct page *page2) · line 94
Quoted 1 line
arch/arm64/kernel/mte.c +0
-0return ret;Unrelated to this discussion, I got an internal report that Linux hangs during boot with
CONFIG_DEFERRED_STRUCT_PAGE_INITbecausetry_page_mte_tagging()locks up on uninitialised page flags.Since we (always?) map the zero page as
pte_special(),set_pte_at()
Yes. (if pte_special is implemented by the arch of course)
Quoted 7 lines
won't check if the tags have to be initialised, so we can skip the
PG_mte_taggedaltogether. We actually had this code for some time until we introduced thepte_special()check inset_pte_at().So alternative patch that also fixes the deferred struct page init (on the assumptions that the zero page is always mapped as
pte_special():
LGTM!
Signature · 3 lines
Cheers David / dhildenb
Lance Yang
reviewerQuoted 42 lines · Catalin Marinas
Quoted 7 lines · Catalin Marinas
I just realised that on arm64 with MTE we won't get any merging with the zero page even if the user page isn't mapped with
PROT_MTE. Incpu_enable_mte()we zero the tags in the zero page and setPG_mte_tagged. The reason is that we want to use the zero page withPROT_MTEmappings (until tag setting causes CoW). Hmm, the arm64memcmp_pages()messed up KSM merging with the zero page even before this patch.[...]
Quoted 18 lines
arch/arm64/kernel/mte.c +2
-1@@ -73,6 +73,8 @@ int memcmp_pages(struct page *page1, struct page *page2) {char *addr1, *addr2;int ret; +bool page1_tagged =page_mte_tagged (page1) && !is_zero_page (page1); +bool page2_tagged =page_mte_tagged (page2) && !is_zero_page (page2); addr1 =page_address (page1); addr2 =page_address (page2);@@ -83,11 +85,10 @@ int memcmp_pages(struct page *page1, struct page *page2)/* * If the page content is identical but at least one of the pages is -* tagged, return non-zero to avoid KSM merging. If only one of the arch/arm64/kernel/mte.c int memcmp_pages(struct page *page1, struct page *page2) · line 94
Quoted 10 lines
arch/arm64/kernel/mte.c +3
-3-* pages is tagged, __set_ptes() may zero or change the tags of the -* other page via mte_sync_tags(). +* tagged, return non-zero to avoid KSM merging. Ignore the zero page +* since it is always tagged with the tags cleared. */ -if (page_mte_tagged (page1) ||page_mte_tagged (page2)) +if (page1_tagged || page2_tagged)return addr1 != addr2;return ret;Unrelated to this discussion, I got an internal report that Linux hangs during boot with
CONFIG_DEFERRED_STRUCT_PAGE_INITbecausetry_page_mte_tagging()locks up on uninitialised page flags.
Quoted 7 lines
Since we (always?) map the zero page as
pte_special(),set_pte_at()won't check if the tags have to be initialised, so we can skip thePG_mte_taggedaltogether. We actually had this code for some time until we introduced thepte_special()check inset_pte_at().So alternative patch that also fixes the deferred struct page init (on the assumptions that the zero page is always mapped as
pte_special():
I can confirm that this alternative patch also works correctly; my tests for MTE all pass ;)
This looks like a better fix since it solves the boot hang issue too.
Quoted 31 lines
arch/arm64/kernel/cpufeature.c +7
-3@@ -2419,17 +2419,21 @@ static void bti_enable(const struct arm64_cpu_capabilities *__unused)#ifdef CONFIG_ARM64_MTE static void cpu_enable_mte (struct arm64_cpu_capabilities const *cap) { +static bool cleared_zero_page = false; +sysreg_clear_set (sctlr_el1,0 , SCTLR_ELx_ATA |SCTLR_EL1_ATA0 );mte_cpu_setup ();/* * Clear the tags in the zero page. This needs to be done via the -* linear map which has the Tagged attribute. +* linear map which has the Tagged attribute. Since this page is +* always mapped as pte_special(), set_pte_at() will not attempt to +* clear the tags or set PG_mte_tagged. */ -if (try_page_mte_tagging (ZERO_PAGE (0 ))) { +if (!cleared_zero_page) { + cleared_zero_page = true;mte_clear_page_tags (lm_alias (empty_zero_page)); -set_page_mte_tagged (ZERO_PAGE (0 )); }kasan_init_hw_tags_cpu ();
Catalin Marinas
maintainerQuoted 6 lines · Lance Yang
Quoted 2 lines · Catalin Marinas
So alternative patch that also fixes the deferred struct page init (on the assumptions that the zero page is always mapped as
pte_special():I can confirm that this alternative patch also works correctly; my tests for MTE all pass ;)
Thanks Lance for testing. I'll post one of the variants today.
Quoted 1 line
This looks like a better fix since it solves the boot hang issue too.
In principle, yes, until I tracked down why I changed it in the first place - 68d54ceeec0e ("arm64: mte: Allow PTRACE_PEEKMTETAGS access to the zero page"). ptrace() can read tags from PROT_MTE mappings and we want to allow reading zeroes as well if the page points to the zero page. Not flagging the page as PG_mte_tagged caused issues.
I can change the logic in the ptrace() code, I just need to figure out what happens to the huge zero page. Ideally we should treat both in the same way but, AFAICT, we don't use pmd_mkspecial() on the huge zero page, so it gets flagged with PG_mte_tagged.
If I go with the first fix for the page merging, I'll look to defer the zero page initialisation post page_alloc_init_late() in a separate patch.
Signature · 1 line
Catalin
Quoted 18 lines · Catalin Marinas
Quoted 6 lines · Lance Yang
Quoted 2 lines · Catalin Marinas
So alternative patch that also fixes the deferred struct page init (on the assumptions that the zero page is always mapped as
pte_special():I can confirm that this alternative patch also works correctly; my tests for MTE all pass ;)
Thanks Lance for testing. I'll post one of the variants today.
Quoted 1 line
This looks like a better fix since it solves the boot hang issue too.
In principle, yes, until I tracked down why I changed it in the first place -
68d54ceeec0e("arm64: mte: AllowPTRACE_PEEKMTETAGSaccess to the zero page").ptrace()can read tags fromPROT_MTEmappings and we want to allow reading zeroes as well if the page points to the zero page. Not flagging the page asPG_mte_taggedcaused issues.
Quoted 4 lines
I can change the logic in the
ptrace()code, I just need to figure out what happens to the huge zero page. Ideally we should treat both in the same way but, AFAICT, we don't usepmd_mkspecial()on the huge zero page, so it gets flagged withPG_mte_tagged.
I changed that recently :) The huge zero folio will now always have pmd_special() set.
Signature · 3 lines
Cheers David / dhildenb
Catalin Marinas
maintainerQuoted 19 lines · David Hildenbrand
Quoted 18 lines · Catalin Marinas
Quoted 6 lines · Lance Yang
Quoted 2 lines · Catalin Marinas
So alternative patch that also fixes the deferred struct page init (on the assumptions that the zero page is always mapped as
pte_special():I can confirm that this alternative patch also works correctly; my tests for MTE all pass ;)
Thanks Lance for testing. I'll post one of the variants today.
Quoted 1 line
This looks like a better fix since it solves the boot hang issue too.
In principle, yes, until I tracked down why I changed it in the first place -
68d54ceeec0e("arm64: mte: AllowPTRACE_PEEKMTETAGSaccess to the zero page").ptrace()can read tags fromPROT_MTEmappings and we want to allow reading zeroes as well if the page points to the zero page. Not flagging the page asPG_mte_taggedcaused issues.
Quoted 7 lines
Quoted 4 lines
I can change the logic in the
ptrace()code, I just need to figure out what happens to the huge zero page. Ideally we should treat both in the same way but, AFAICT, we don't usepmd_mkspecial()on the huge zero page, so it gets flagged withPG_mte_tagged.I changed that recently :) The huge zero folio will now always have
pmd_special()set.
Oh, which commit was this? It means that we can end up with uninitialised tags if we have a PROT_MTE huge zero page since set_pmd_at/set_pte_at() skips mte_sync_tags().
Signature · 1 line
Catalin
Quoted 25 lines · Catalin Marinas
Quoted 24 lines · David Hildenbrand
Quoted 22 lines · Catalin Marinas
Quoted 6 lines · Lance Yang
Quoted 2 lines · Catalin Marinas
So alternative patch that also fixes the deferred struct page init (on the assumptions that the zero page is always mapped as
pte_special():I can confirm that this alternative patch also works correctly; my tests for MTE all pass ;)
Thanks Lance for testing. I'll post one of the variants today.
Quoted 1 line
This looks like a better fix since it solves the boot hang issue too.
In principle, yes, until I tracked down why I changed it in the first place -
68d54ceeec0e("arm64: mte: AllowPTRACE_PEEKMTETAGSaccess to the zero page").ptrace()can read tags fromPROT_MTEmappings and we want to allow reading zeroes as well if the page points to the zero page. Not flagging the page asPG_mte_taggedcaused issues.I can change the logic in the
ptrace()code, I just need to figure out what happens to the huge zero page. Ideally we should treat both in the same way but, AFAICT, we don't usepmd_mkspecial()on the huge zero page, so it gets flagged withPG_mte_tagged.
Quoted 7 lines
Quoted 2 lines
I changed that recently :) The huge zero folio will now always have
pmd_special()set.Oh, which commit was this? It means that we can end up with uninitialised tags if we have a
PROT_MTEhuge zero page since set_pmd_at/set_pte_at() skipsmte_sync_tags().
This one:
commit d82d09e482199e6bbc204df10b2082f764cbe1f4
Author: David Hildenbrand <[email protected]> Date: Mon Aug 11 13:26:25 2025 +0200
mm/huge_memory: mark PMD mappings of the huge zero folio special
The huge zero folio is refcounted (+mapcounted -- is that a word?) differently than "normal" folios, similarly (but different) to the ordinary shared zeropage.
It should be in mm-stable, to go upstream in the upcoming merge window. It's been lurking in -next for a while now.
As it behaves just like the ordinary shared zeropage now, would we have to zero/initialize the tags after allocating it?
Signature · 3 lines
Cheers David / dhildenb
Catalin Marinas
maintainerQuoted 41 lines · David Hildenbrand
Quoted 25 lines · Catalin Marinas
Quoted 24 lines · David Hildenbrand
Quoted 22 lines · Catalin Marinas
Quoted 6 lines · Lance Yang
Quoted 2 lines · Catalin Marinas
So alternative patch that also fixes the deferred struct page init (on the assumptions that the zero page is always mapped as
pte_special():I can confirm that this alternative patch also works correctly; my tests for MTE all pass ;)
Thanks Lance for testing. I'll post one of the variants today.
Quoted 1 line
This looks like a better fix since it solves the boot hang issue too.
In principle, yes, until I tracked down why I changed it in the first place -
68d54ceeec0e("arm64: mte: AllowPTRACE_PEEKMTETAGSaccess to the zero page").ptrace()can read tags fromPROT_MTEmappings and we want to allow reading zeroes as well if the page points to the zero page. Not flagging the page asPG_mte_taggedcaused issues.I can change the logic in the
ptrace()code, I just need to figure out what happens to the huge zero page. Ideally we should treat both in the same way but, AFAICT, we don't usepmd_mkspecial()on the huge zero page, so it gets flagged withPG_mte_tagged.Quoted 6 lines
Quoted 2 lines
I changed that recently :) The huge zero folio will now always have
pmd_special()set.Oh, which commit was this? It means that we can end up with uninitialised tags if we have a
PROT_MTEhuge zero page since set_pmd_at/set_pte_at() skipsmte_sync_tags().This one:
commit
d82d09e482199e6bbc204df10b2082f764cbe1f4Author: David Hildenbrand <[email protected]> Date: Mon Aug 11 13:26:25 2025 +0200
mm/huge_memory: mark PMD mappings of the huge zero folio special
Quoted 7 lines
The huge zero folio is refcounted (+mapcounted -- is that a word?) differently than "normal" folios, similarly (but different) to the ordinary shared zeropage.
It should be in mm-stable, to go upstream in the upcoming merge window. It's been lurking in -next for a while now.
Thanks. At least it's something to address in the next kernel version. I need to improve the MTE kselftests to catch the zero page scenarios.
Quoted 2 lines
As it behaves just like the ordinary shared zeropage now, would we have to zero/initialize the tags after allocating it?
Yes. Before pmd_special(), it was be done lazily via set_pmd_at(). I think it just needs a __GFP_ZEROTAGS. The only other place we use this flag is in vma_alloc_zeroed_movable_folio(), as an optimisation to avoid a separate loop for zeroing the tags after data.
Signature · 1 line
Catalin
Quoted 23 lines · Lance Yang
From: Lance Yang <[email protected]>
When both THP and MTE are enabled, splitting a THP and replacing its zero-filled subpages with the shared zeropage can cause MTE tag mismatch faults in userspace.
Remapping zero-filled subpages to the shared zeropage is unsafe, as the zeropage has a fixed tag of zero, which may not match the tag expected by the userspace pointer.
KSM already avoids this problem by using
memcmp_pages(), which on arm64 intentionally reports MTE-tagged pages as non-identical to prevent unsafe merging.As suggested by David[1], this patch adopts the same pattern, replacing the
memchr_inv()byte-level check with a call topages_identical(). This leverages existing architecture-specific logic to determine if a page is truly identical to the shared zeropage.Having both the THP shrinker and KSM rely on
pages_identical()makes the design more future-proof, IMO. Instead of handling quirks in generic code, we just let the architecture decide what makes two pages identical.
Quoted 8 lines
[1] https://lore.kernel.org/all/[email protected]
Cc <[email protected]>Reported-by Qun-wei Lin <[email protected]>Fixes b1f202060afe ("mm: remap unused subpages to shared zeropage when splitting isolated thp")Suggested-by David Hildenbrand <[email protected]>Signed-off-by Lance Yang <[email protected]>
Nice catch.
Signature · 2 lines
Wei Yang Help you, Help me