mm: Fix bug affecting swapping in MTE tagged pages
Subsystems (5)
v3 cover mm: Fix bug affecting swapping in MTE tagged pages
This patch series reworks the logic that handles swapping in page metadata to fix a reported bug [1] where metadata can sometimes not be swapped in correctly after commit c145e0b47c77 ("mm: streamline COW logic in do_swap_page()").
- Patch 1 fixes the bug itself, but still requires architectures to restore metadata in both arch_swap_restore() and set_pte_at().
- Patch 2 makes it so that architectures only need to restore metadata in arch_swap_restore().
- Patch 3 changes arm64 to remove support for restoring metadata in set_pte_at().
[1] https://lore.kernel.org/all/[email protected]/
v3:
- Added patch to call arch_swap_restore() from unuse_pte()
- Rebased onto arm64/for-next/fixes
v2:
- Call arch_swap_restore() directly instead of via arch_do_swap_page()
Peter Collingbourne (3):
mm: Call arch_swap_restore() from do_swap_page()
mm: Call arch_swap_restore() from unuse_pte()
arm64: mte: Simplify swap tag restoration logic
arch/arm64/include/asm/mte.h | 4 ++-- arch/arm64/include/asm/pgtable.h | 14 ++---------- arch/arm64/kernel/mte.c | 37 ++++++-------------------------- arch/arm64/mm/mteswap.c | 7 +++--- mm/memory.c | 7 ++++++ mm/swapfile.c | 7 ++++++ 6 files changed, 28 insertions(+), 48 deletions(-)
Signature · 1 line
2.40.1.606.ga4b1b128d6-goog
PATCH v3 1/3 mm: Call arch_swap_restore() from do_swap_page()
Commit c145e0b47c77 ("mm: streamline COW logic in do_swap_page()") moved the call to swap_free() before the call to set_pte_at(), which meant that the MTE tags could end up being freed before set_pte_at() had a chance to restore them. Fix it by adding a call to the arch_swap_restore() hook before the call to swap_free().
---
v2:
- Call arch_swap_restore() directly instead of via arch_do_swap_page()
mm/memory.c | 7 +++++++ 1 file changed, 7 insertions(+)
mm/memory.c +7 -0
@@ -3932,6 +3932,13 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) } } +/* +* Some architectures may have to restore extra metadata to the page +* when reading from swap. This metadata may be indexed by swap entry +* so this must be called before swap_free(). +*/ +arch_swap_restore (entry, folio); +/* * Remove the swap entry and conditionally try to free up the swapcache. * We're already holding a reference on the page but haven't mapped it
Signature · 1 line
2.40.1.606.ga4b1b128d6-goog
Re: [PATCH v3 1/3] mm: Call arch_swap_restore() from do_swap_page()
Quoted 27 lines · Peter Collingbourne
Commit
c145e0b47c77("mm: streamline COW logic indo_swap_page()") moved the call toswap_free()before the call toset_pte_at(), which meant that the MTE tags could end up being freed beforeset_pte_at()had a chance to restore them. Fix it by adding a call to thearch_swap_restore()hook before the call toswap_free().Signed-off-by Peter Collingbourne <[email protected]>Cc <[email protected]> # 6.1Fixes c145e0b47c77 ("mm: streamline COW logic in do_swap_page()")Reported-by Qun-wei Lin (林群崴) <[email protected]>---
v2:
- Callarch_swap_restore()directly instead of viaarch_do_swap_page()mm/memory.c | 7 +++++++ 1 file changed, 7 insertions(+)mm/memory.c +0
-0@@ -3932,6 +3932,13 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) } }
mm/memory.c vm_fault_t do_swap_page(struct vm_fault *vmf) · line 3944
Quoted 10 lines
mm/memory.c +7
-0+/* +* Some architectures may have to restore extra metadata to the page +* when reading from swap. This metadata may be indexed by swap entry +* so this must be called before swap_free(). +*/ +arch_swap_restore (entry, folio); +/* * Remove the swap entry and conditionally try to free up the swapcache. * We're already holding a reference on the page but haven't mapped it
Should you add
for 1/3 and 2/3.
It looks good for me for swap code related part. Feel free to add
to 1/3 and 2/3.
Best Regards,
Huang, Ying
David Hildenbrand
Acked-byRe: [PATCH v3 1/3] mm: Call arch_swap_restore() from do_swap_page()
Quoted 42 lines · Ying
Quoted 27 lines · Peter Collingbourne
Commit
c145e0b47c77("mm: streamline COW logic indo_swap_page()") moved the call toswap_free()before the call toset_pte_at(), which meant that the MTE tags could end up being freed beforeset_pte_at()had a chance to restore them. Fix it by adding a call to thearch_swap_restore()hook before the call toswap_free().Signed-off-by Peter Collingbourne <[email protected]>Cc <[email protected]> # 6.1Fixes c145e0b47c77 ("mm: streamline COW logic in do_swap_page()")Reported-by Qun-wei Lin (林群崴) <[email protected]>---
v2:
- Callarch_swap_restore()directly instead of viaarch_do_swap_page()mm/memory.c | 7 +++++++ 1 file changed, 7 insertions(+)mm/memory.c +0
-0@@ -3932,6 +3932,13 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) } }mm/memory.c vm_fault_t do_swap_page(struct vm_fault *vmf) · line 3944
Quoted 10 lines
mm/memory.c +7
-0+/* +* Some architectures may have to restore extra metadata to the page +* when reading from swap. This metadata may be indexed by swap entry +* so this must be called before swap_free(). +*/ +arch_swap_restore (entry, folio); +/* * Remove the swap entry and conditionally try to free up the swapcache. * We're already holding a reference on the page but haven't mapped it Should you add
Quoted 3 lines
Suggested-by David Hildenbrand <[email protected]>for 1/3 and 2/3.
For 1/3, I think I rather only explained the problem in the first patch and didn't really suggest this.
Signature · 3 lines
Thanks, David / dhildenb
Re: [PATCH v3 1/3] mm: Call arch_swap_restore() from do_swap_page()
In reply to Peter CollingbourneQuoted 6 lines · Peter Collingbourne
Commit
c145e0b47c77("mm: streamline COW logic indo_swap_page()") moved the call toswap_free()before the call toset_pte_at(), which meant that the MTE tags could end up being freed beforeset_pte_at()had a chance to restore them. Fix it by adding a call to thearch_swap_restore()hook before the call toswap_free().
Quoted 6 lines
Signed-off-by Peter Collingbourne <[email protected]>Cc <[email protected]> # 6.1Fixes c145e0b47c77 ("mm: streamline COW logic in do_swap_page()")Reported-by Qun-wei Lin (林群崴) <[email protected]>
Quoted 25 lines
---
v2:
- Callarch_swap_restore()directly instead of viaarch_do_swap_page()mm/memory.c | 7 +++++++ 1 file changed, 7 insertions(+)mm/memory.c +7
-0@@ -3932,6 +3932,13 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) } } +/* +* Some architectures may have to restore extra metadata to the page +* when reading from swap. This metadata may be indexed by swap entry +* so this must be called before swap_free(). +*/ +arch_swap_restore (entry, folio); +/* * Remove the swap entry and conditionally try to free up the swapcache. * We're already holding a reference on the page but haven't mapped it
Re: [PATCH v3 1/3] mm: Call arch_swap_restore() from do_swap_page()
In reply to Peter CollingbourneQuoted 6 lines · Peter Collingbourne
Commit
c145e0b47c77("mm: streamline COW logic indo_swap_page()") moved the call toswap_free()before the call toset_pte_at(), which meant that the MTE tags could end up being freed beforeset_pte_at()had a chance to restore them. Fix it by adding a call to thearch_swap_restore()hook before the call toswap_free().
Quoted 6 lines
Signed-off-by Peter Collingbourne <[email protected]>Cc <[email protected]> # 6.1Fixes c145e0b47c77 ("mm: streamline COW logic in do_swap_page()")Reported-by Qun-wei Lin (林群崴) <[email protected]>
PATCH v3 2/3 mm: Call arch_swap_restore() from unuse_pte()
In reply to Peter CollingbourneWe would like to move away from requiring architectures to restore metadata from swap in the set_pte_at() implementation, as this is not only error-prone but adds complexity to the arch-specific code. This requires us to call arch_swap_restore() before calling swap_free() whenever pages are restored from swap. We are currently doing so everywhere except in unuse_pte(); do so there as well.
--- mm/swapfile.c | 7 +++++++ 1 file changed, 7 insertions(+)
mm/swapfile.c +7 -0
@@ -1794,6 +1794,13 @@ static int unuse_pte(struct vm_area_struct *vma, pmd_t *pmd, goto setpte; } +/* +* Some architectures may have to restore extra metadata to the page +* when reading from swap. This metadata may be indexed by swap entry +* so this must be called before swap_free(). +*/ +arch_swap_restore (entry,page_folio (page)); +/* See do_swap_page() */ BUG_ON (!PageAnon (page) &&PageMappedToDisk (page));BUG_ON (PageAnon (page) &&PageAnonExclusive (page));
Signature · 1 line
2.40.1.606.ga4b1b128d6-goog
Re: [PATCH v3 2/3] mm: Call arch_swap_restore() from unuse_pte()
Quoted 21 lines · Peter Collingbourne
We would like to move away from requiring architectures to restore metadata from swap in the
set_pte_at()implementation, as this is not only error-prone but adds complexity to the arch-specific code. This requires us to callarch_swap_restore()before callingswap_free()whenever pages are restored from swap. We are currently doing so everywhere except inunuse_pte(); do so there as well.Signed-off-by Peter Collingbourne <[email protected]>--- mm/swapfile.c | 7 +++++++ 1 file changed, 7 insertions(+)mm/swapfile.c +0
-0@@ -1794,6 +1794,13 @@ static int unuse_pte(struct vm_area_struct *vma, pmd_t *pmd, goto setpte; }
mm/swapfile.c static int unuse_pte(struct vm_area_struct *vma, pmd_t *pmd, · line 1806
Quoted 10 lines
mm/swapfile.c +7
-0+/* +* Some architectures may have to restore extra metadata to the page +* when reading from swap. This metadata may be indexed by swap entry +* so this must be called before swap_free(). +*/ +arch_swap_restore (entry,page_folio (page)); +/* See do_swap_page() */ BUG_ON (!PageAnon (page) &&PageMappedToDisk (page));BUG_ON (PageAnon (page) &&PageAnonExclusive (page));
Signature · 9 lines
Thanks, David / dhildenb _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH v3 2/3] mm: Call arch_swap_restore() from unuse_pte()
In reply to Peter CollingbourneQuoted 9 lines · Peter Collingbourne
We would like to move away from requiring architectures to restore metadata from swap in the
set_pte_at()implementation, as this is not only error-prone but adds complexity to the arch-specific code. This requires us to callarch_swap_restore()before callingswap_free()whenever pages are restored from swap. We are currently doing so everywhere except inunuse_pte(); do so there as well.
Quoted 22 lines
--- mm/swapfile.c | 7 +++++++ 1 file changed, 7 insertions(+)mm/swapfile.c +7
-0@@ -1794,6 +1794,13 @@ static int unuse_pte(struct vm_area_struct *vma, pmd_t *pmd, goto setpte; } +/* +* Some architectures may have to restore extra metadata to the page +* when reading from swap. This metadata may be indexed by swap entry +* so this must be called before swap_free(). +*/ +arch_swap_restore (entry,page_folio (page)); +/* See do_swap_page() */ BUG_ON (!PageAnon (page) &&PageMappedToDisk (page));BUG_ON (PageAnon (page) &&PageAnonExclusive (page));
Re: [PATCH v3 2/3] mm: Call arch_swap_restore() from unuse_pte()
In reply to Peter CollingbourneQuoted 9 lines · Peter Collingbourne
We would like to move away from requiring architectures to restore metadata from swap in the
set_pte_at()implementation, as this is not only error-prone but adds complexity to the arch-specific code. This requires us to callarch_swap_restore()before callingswap_free()whenever pages are restored from swap. We are currently doing so everywhere except inunuse_pte(); do so there as well.
PATCH v3 3/3 arm64: mte: Simplify swap tag restoration logic
In reply to Peter CollingbourneAs a result of the previous two patches, there are no circumstances in which a swapped-in page is installed in a page table without first having arch_swap_restore() called on it. Therefore, we no longer need the logic in set_pte_at() that restores the tags, so remove it.
Because we can now rely on the page being locked, we no longer need to handle the case where a page is having its tags restored by multiple tasks concurrently, so we can slightly simplify the logic in mte_restore_tags().
---
v3:
- Rebased onto arm64/for-next/fixes, which already has a fix for the issue previously tagged, therefore removed Fixes:
tag
arch/arm64/include/asm/mte.h | 4 ++-- arch/arm64/include/asm/pgtable.h | 14 ++---------- arch/arm64/kernel/mte.c | 37 ++++++-------------------------- arch/arm64/mm/mteswap.c | 7 +++--- 4 files changed, 14 insertions(+), 48 deletions(-)
arch/arm64/include/asm/mte.h +2 -2
@@ -90,7 +90,7 @@ static inline bool try_page_mte_tagging(struct page *page) }void mte_zero_clear_page_tags (void *addr); -void mte_sync_tags (pte_t old_pte,pte_t pte); +void mte_sync_tags (pte_t pte);void mte_copy_page_tags (void *kto,const void *kfrom);void mte_thread_init_user (void );void mte_thread_switch (struct task_struct *next);
@@ -122,7 +122,7 @@ static inline bool try_page_mte_tagging(struct page *page)static inline void mte_zero_clear_page_tags (void *addr) { } -static inline void mte_sync_tags (pte_t old_pte,pte_t pte) +static inline void mte_sync_tags (pte_t pte) { }static inline void mte_copy_page_tags (void *kto,const void *kfrom)
arch/arm64/include/asm/pgtable.h +2 -12
@@ -337,18 +337,8 @@ static inline void __set_pte_at(struct mm_struct *mm, unsigned long addr,* don't expose tags (instruction fetches don't check tags). */ if (system_supports_mte () &&pte_access_permitted (pte, false) && - !pte_special (pte)) { -pte_t old_pte =READ_ONCE (*ptep); -/* -* We only need to synchronise if the new PTE has tags enabled -* or if swapping in (in which case another mapping may have -* set tags in the past even if this PTE isn't tagged). -* (!pte_none() && !pte_present()) is an open coded version of -* is_swap_pte() -*/ -if (pte_tagged (pte) || (!pte_none (old_pte) && !pte_present (old_pte))) -mte_sync_tags (old_pte, pte); - } + !pte_special (pte) &&pte_tagged (pte)) +mte_sync_tags (pte);__check_safe_pte_update (mm, ptep, pte);
arch/arm64/kernel/mte.c +7 -30
@@ -35,41 +35,18 @@ DEFINE_STATIC_KEY_FALSE(mte_async_or_asymm_mode);EXPORT_SYMBOL_GPL (mte_async_or_asymm_mode);#endif -static void mte_sync_page_tags (struct page *page,pte_t old_pte, -bool check_swap,bool pte_is_tagged) -{ -if (check_swap &&is_swap_pte (old_pte)) { -swp_entry_t entry =pte_to_swp_entry (old_pte); - -if (!non_swap_entry (entry)) -mte_restore_tags (entry, page); - } - -if (!pte_is_tagged) -return ; - -if (try_page_mte_tagging (page)) { -mte_clear_page_tags (page_address (page)); -set_page_mte_tagged (page); - } -} - -void mte_sync_tags (pte_t old_pte,pte_t pte) +void mte_sync_tags (pte_t pte) {struct page *page =pte_page (pte);long i, nr_pages =compound_nr (page); -bool check_swap = nr_pages ==1 ; -bool pte_is_tagged =pte_tagged (pte); - -/* Early out if there's nothing to do */ -if (!check_swap && !pte_is_tagged) -return ;/* if PG_mte_tagged is set, tags have already been initialised */ -for (i =0 ; i < nr_pages; i++, page++) -if (!page_mte_tagged (page)) -mte_sync_page_tags (page, old_pte, check_swap, - pte_is_tagged); +for (i =0 ; i < nr_pages; i++, page++) { +if (try_page_mte_tagging (page)) { +mte_clear_page_tags (page_address (page)); +set_page_mte_tagged (page); + } + }/* ensure the tags are visible before the PTE is set */ smp_wmb ();
arch/arm64/mm/mteswap.c +3 -4
@@ -53,10 +53,9 @@ void mte_restore_tags(swp_entry_t entry, struct page *page)if (!tags)return ; -if (try_page_mte_tagging (page)) { -mte_restore_page_tags (page_address (page), tags); -set_page_mte_tagged (page); - } +WARN_ON_ONCE (!try_page_mte_tagging (page)); +mte_restore_page_tags (page_address (page), tags); +set_page_mte_tagged (page); }void mte_invalidate_tags (int type,pgoff_t offset)
Signature · 1 line
2.40.1.606.ga4b1b128d6-goog
Re: [PATCH v3 3/3] arm64: mte: Simplify swap tag restoration logic
Quoted 11 lines · Peter Collingbourne
As a result of the previous two patches, there are no circumstances in which a swapped-in page is installed in a page table without first having
arch_swap_restore()called on it. Therefore, we no longer need the logic inset_pte_at()that restores the tags, so remove it.Because we can now rely on the page being locked, we no longer need to handle the case where a page is having its tags restored by multiple tasks concurrently, so we can slightly simplify the logic in
mte_restore_tags().
This is much neater, thanks for figuring out a better way of implementing it. The set_pte_at() thing always felt like a hack, but it was always there for the non-swap case and I obviously never figured out a better solution.
Quoted 130 lines
---
v3:
- Rebased onto arm64/for-next/fixes, which already has a fix for the issue previously tagged, therefore removed Fixes:
tagarch/arm64/include/asm/mte.h | 4 ++-- arch/arm64/include/asm/pgtable.h | 14 ++---------- arch/arm64/kernel/mte.c | 37 ++++++-------------------------- arch/arm64/mm/mteswap.c | 7 +++--- 4 files changed, 14 insertions(+), 48 deletions(-)arch/arm64/include/asm/mte.h +2
-2@@ -90,7 +90,7 @@ static inline bool try_page_mte_tagging(struct page *page) }void mte_zero_clear_page_tags (void *addr); -void mte_sync_tags (pte_t old_pte,pte_t pte); +void mte_sync_tags (pte_t pte);void mte_copy_page_tags (void *kto,const void *kfrom);void mte_thread_init_user (void );void mte_thread_switch (struct task_struct *next);@@ -122,7 +122,7 @@ static inline bool try_page_mte_tagging(struct page *page)static inline void mte_zero_clear_page_tags (void *addr) { } -static inline void mte_sync_tags (pte_t old_pte,pte_t pte) +static inline void mte_sync_tags (pte_t pte) { }static inline void mte_copy_page_tags (void *kto,const void *kfrom)arch/arm64/include/asm/pgtable.h +2
-12@@ -337,18 +337,8 @@ static inline void __set_pte_at(struct mm_struct *mm, unsigned long addr,* don't expose tags (instruction fetches don't check tags). */ if (system_supports_mte () &&pte_access_permitted (pte, false) && - !pte_special (pte)) { -pte_t old_pte =READ_ONCE (*ptep); -/* -* We only need to synchronise if the new PTE has tags enabled -* or if swapping in (in which case another mapping may have -* set tags in the past even if this PTE isn't tagged). -* (!pte_none() && !pte_present()) is an open coded version of -* is_swap_pte() -*/ -if (pte_tagged (pte) || (!pte_none (old_pte) && !pte_present (old_pte))) -mte_sync_tags (old_pte, pte); - } + !pte_special (pte) &&pte_tagged (pte)) +mte_sync_tags (pte);__check_safe_pte_update (mm, ptep, pte);arch/arm64/kernel/mte.c +7
-30@@ -35,41 +35,18 @@ DEFINE_STATIC_KEY_FALSE(mte_async_or_asymm_mode);EXPORT_SYMBOL_GPL (mte_async_or_asymm_mode);#endif -static void mte_sync_page_tags (struct page *page,pte_t old_pte, -bool check_swap,bool pte_is_tagged) -{ -if (check_swap &&is_swap_pte (old_pte)) { -swp_entry_t entry =pte_to_swp_entry (old_pte); - -if (!non_swap_entry (entry)) -mte_restore_tags (entry, page); - } - -if (!pte_is_tagged) -return ; - -if (try_page_mte_tagging (page)) { -mte_clear_page_tags (page_address (page)); -set_page_mte_tagged (page); - } -} - -void mte_sync_tags (pte_t old_pte,pte_t pte) +void mte_sync_tags (pte_t pte) {struct page *page =pte_page (pte);long i, nr_pages =compound_nr (page); -bool check_swap = nr_pages ==1 ; -bool pte_is_tagged =pte_tagged (pte); - -/* Early out if there's nothing to do */ -if (!check_swap && !pte_is_tagged) -return ;/* if PG_mte_tagged is set, tags have already been initialised */ -for (i =0 ; i < nr_pages; i++, page++) -if (!page_mte_tagged (page)) -mte_sync_page_tags (page, old_pte, check_swap, - pte_is_tagged); +for (i =0 ; i < nr_pages; i++, page++) { +if (try_page_mte_tagging (page)) { +mte_clear_page_tags (page_address (page)); +set_page_mte_tagged (page); + } + }/* ensure the tags are visible before the PTE is set */ smp_wmb ();arch/arm64/mm/mteswap.c +3
-4@@ -53,10 +53,9 @@ void mte_restore_tags(swp_entry_t entry, struct page *page)if (!tags)return ; -if (try_page_mte_tagging (page)) { -mte_restore_page_tags (page_address (page), tags); -set_page_mte_tagged (page); - } +WARN_ON_ONCE (!try_page_mte_tagging (page)); +mte_restore_page_tags (page_address (page), tags); +set_page_mte_tagged (page); }void mte_invalidate_tags (int type,pgoff_t offset)
Re: [PATCH v3 3/3] arm64: mte: Simplify swap tag restoration logic
In reply to Peter CollingbourneQuoted 8 lines · Peter Collingbourne
As a result of the previous two patches, there are no circumstances in which a swapped-in page is installed in a page table without first having
arch_swap_restore()called on it. Therefore, we no longer need the logic inset_pte_at()that restores the tags, so remove it.Because we can now rely on the page being locked, we no longer need to handle the case where a page is having its tags restored by multiple tasks concurrently, so we can slightly simplify the logic in
mte_restore_tags().
[...]
Quoted 6 lines
arch/arm64/mm/mteswap.c +0
-0@@ -53,10 +53,9 @@ void mte_restore_tags(swp_entry_t entry, struct page *page)if (!tags)
arch/arm64/mm/mteswap.c void mte_restore_tags(swp_entry_t entry, struct page *page) · line 59
Quoted 10 lines
arch/arm64/mm/mteswap.c +3
-4return ; -if (try_page_mte_tagging (page)) { -mte_restore_page_tags (page_address (page), tags); -set_page_mte_tagged (page); - } +WARN_ON_ONCE (!try_page_mte_tagging (page)); +mte_restore_page_tags (page_address (page), tags); +set_page_mte_tagged (page); }
Can we have a situation where two processes share the same swap pte (CoW) and they both enter the do_swap_page() or the unuse_pte() paths triggering this warning?
Other than that, the looks nice, it simplifies the logic and probably saves a few cycles as well on the set_pte_at() path.
Re: [PATCH v3 3/3] arm64: mte: Simplify swap tag restoration logic
Hi Catalin,
Quoted 19 lines · Catalin Marinas
Quoted 8 lines · Peter Collingbourne
As a result of the previous two patches, there are no circumstances in which a swapped-in page is installed in a page table without first having
arch_swap_restore()called on it. Therefore, we no longer need the logic inset_pte_at()that restores the tags, so remove it.Because we can now rely on the page being locked, we no longer need to handle the case where a page is having its tags restored by multiple tasks concurrently, so we can slightly simplify the logic in
mte_restore_tags().[...]
Quoted 8 lines
arch/arm64/mm/mteswap.c +0
-0@@ -53,10 +53,9 @@ void mte_restore_tags(swp_entry_t entry, struct page *page)if (!tags)return ;
Quoted 12 lines
arch/arm64/mm/mteswap.c void mte_restore_tags(swp_entry_t entry, struct page *page) · line 59
Quoted 8 lines
arch/arm64/mm/mteswap.c +3
-4-if (try_page_mte_tagging (page)) { -mte_restore_page_tags (page_address (page), tags); -set_page_mte_tagged (page); - } +WARN_ON_ONCE (!try_page_mte_tagging (page)); +mte_restore_page_tags (page_address (page), tags); +set_page_mte_tagged (page); }Can we have a situation where two processes share the same swap pte (CoW) and they both enter the
do_swap_page()or theunuse_pte()paths triggering this warning?
Having examined the code more closely, I realized that this is possible with two do_swap_page() calls on CoW shared pages (or do_swap_page() followed by unuse_pte()), because the swapcache page will be shared between the tasks and so they will both call arch_swap_restore() on the same page. I was able to provoke the warning with the following program:
#include <sys/mman.h>
#include <unistd.h>
int main() {
char *p = mmap(0, 4096, PROT_READ|PROT_WRITE|PROT_MTE,
MAP_ANON|MAP_PRIVATE, -1, 0);
p[0] = 1;
madvise(p, 4096, MADV_PAGEOUT);
fork();
return p[0];
}
I will send a v4 with this hunk removed.
Quoted 4 lines
Other than that, the looks nice, it simplifies the logic and probably saves a few cycles as well on the
set_pte_at()path.Reviewed-by Catalin Marinas <[email protected]>
Thanks for the review!
Peter