| Message ID | 20230512235755.1589034-3-pcc@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: Fix bug affecting swapping in MTE tagged pages | expand |
On 13.05.23 01:57, Peter Collingbourne wrote: > The previous patch made it possible for MTE to restore tags before they > are freed by hooking arch_do_swap_page(). > > However, the arch_do_swap_page() hook API is incompatible with swap > restoration in circumstances where we do not have an mm or a vma, > such as swapoff with swapped out shmem, and I expect that ADI will > currently fail to restore tags in these circumstances. This implies that > arch-specific metadata stores ought to be indexed by swap index, as MTE > does, rather than by mm and vma, as ADI does, and we should discourage > hooking arch_do_swap_page(), preferring to hook arch_swap_restore() > instead, as MTE already does. > > Therefore, instead of directly hooking arch_do_swap_page() for > MTE, deprecate that hook, change its default implementation to call > arch_swap_restore() and rely on the existing implementation of the latter > for MTE. > > Fixes: c145e0b47c77 ("mm: streamline COW logic in do_swap_page()") Can you enlighten me how this change fixes that commit? I'm afraid I am missing something important. What is the user-visible impact of the problem, how was it caused by c145e0b47c77, and how does your change fix it?
diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h index c63cd44777ec..fc0259cf60fb 100644 --- a/include/linux/pgtable.h +++ b/include/linux/pgtable.h @@ -740,6 +740,12 @@ static inline int pgd_same(pgd_t pgd_a, pgd_t pgd_b) set_pgd(pgdp, pgd); \ }) +#ifndef __HAVE_ARCH_SWAP_RESTORE +static inline void arch_swap_restore(swp_entry_t entry, struct folio *folio) +{ +} +#endif + #ifndef __HAVE_ARCH_DO_SWAP_PAGE /* * Some architectures support metadata associated with a page. When a @@ -748,14 +754,14 @@ static inline int pgd_same(pgd_t pgd_a, pgd_t pgd_b) * processors support an ADI (Application Data Integrity) tag for the * page as metadata for the page. arch_do_swap_page() can restore this * metadata when a page is swapped back in. + * + * This hook is deprecated. Architectures should hook arch_swap_restore() + * instead, because this hook is not called on all code paths that can + * swap in a page, particularly those where mm and vma are not available + * (e.g. swapoff for shmem pages). */ -static inline void arch_do_swap_page(struct mm_struct *mm, - struct vm_area_struct *vma, - unsigned long addr, - pte_t pte, pte_t oldpte) -{ - -} +#define arch_do_swap_page(mm, vma, addr, pte, oldpte) \ + arch_swap_restore(pte_to_swp_entry(oldpte), page_folio(pte_page(pte))) #endif #ifndef __HAVE_ARCH_UNMAP_ONE @@ -798,12 +804,6 @@ static inline void arch_swap_invalidate_area(int type) } #endif -#ifndef __HAVE_ARCH_SWAP_RESTORE -static inline void arch_swap_restore(swp_entry_t entry, struct folio *folio) -{ -} -#endif - #ifndef __HAVE_ARCH_PGD_OFFSET_GATE #define pgd_offset_gate(mm, addr) pgd_offset(mm, addr) #endif
The previous patch made it possible for MTE to restore tags before they are freed by hooking arch_do_swap_page(). However, the arch_do_swap_page() hook API is incompatible with swap restoration in circumstances where we do not have an mm or a vma, such as swapoff with swapped out shmem, and I expect that ADI will currently fail to restore tags in these circumstances. This implies that arch-specific metadata stores ought to be indexed by swap index, as MTE does, rather than by mm and vma, as ADI does, and we should discourage hooking arch_do_swap_page(), preferring to hook arch_swap_restore() instead, as MTE already does. Therefore, instead of directly hooking arch_do_swap_page() for MTE, deprecate that hook, change its default implementation to call arch_swap_restore() and rely on the existing implementation of the latter for MTE. Fixes: c145e0b47c77 ("mm: streamline COW logic in do_swap_page()") Link: https://linux-review.googlesource.com/id/Id2f1ad76eaf606ae210e1d2dd0b7fe287e5f7d87 Signed-off-by: Peter Collingbourne <pcc@google.com> Reported-by: Qun-wei Lin (林群崴) <Qun-wei.Lin@mediatek.com> Link: https://lore.kernel.org/all/5050805753ac469e8d727c797c2218a9d780d434.camel@mediatek.com/ Cc: <stable@vger.kernel.org> # 6.1 --- include/linux/pgtable.h | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-)