On 9/13/24 10:13 AM, Catalin Marinas wrote:
On Thu, Sep 12, 2024 at 01:41:28PM -0700, Yang Shi wrote:
diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
index a7bb20055ce0..c8687ccc2633 100644
--- a/arch/arm64/mm/copypage.c
+++ b/arch/arm64/mm/copypage.c
@@ -18,17 +18,41 @@ void copy_highpage(struct page *to, struct page *from)
{
void *kto = page_address(to);
void *kfrom = page_address(from);
+ struct folio *src = page_folio(from);
+ struct folio *dst = page_folio(to);
+ unsigned int i, nr_pages;
copy_page(kto, kfrom);
if (kasan_hw_tags_enabled())
page_kasan_tag_reset(to);
- if (system_supports_mte() && page_mte_tagged(from)) {
- /* It's a new page, shouldn't have been tagged yet */
- WARN_ON_ONCE(!try_page_mte_tagging(to));
- mte_copy_page_tags(kto, kfrom);
- set_page_mte_tagged(to);
+ if (system_supports_mte()) {
+ if (folio_test_hugetlb(src) &&
+ folio_test_hugetlb_mte_tagged(src)) {
+ if (!try_folio_hugetlb_mte_tagging(dst))
+ return;
+
+ /*
+ * Populate tags for all subpages.
+ *
+ * Don't assume the first page is head page since
+ * huge page copy may start from any subpage.
+ */
+ nr_pages = folio_nr_pages(src);
+ for (i = 0; i < nr_pages; i++) {
+ kfrom = page_address(folio_page(src, i));
+ kto = page_address(folio_page(dst, i));
+ mte_copy_page_tags(kto, kfrom);
+ }
+ folio_set_hugetlb_mte_tagged(dst);
+ } else if (page_mte_tagged(from)) {
+ /* It's a new page, shouldn't have been tagged yet */
+ WARN_ON_ONCE(!try_page_mte_tagging(to));
+
+ mte_copy_page_tags(kto, kfrom);
+ set_page_mte_tagged(to);
+ }
}
}
A nitpick here: I don't like that much indentation, so just do an early
return if !system_supports_mte() in this function.
Sure.
Otherwise the patch looks fine to me. I agree with David's point on an
earlier version of this patch, the naming of these functions isn't
great. So, as per David's suggestion (at least for the first two):
folio_test_hugetlb_mte_tagged()
folio_set_hugetlb_mte_tagged()
folio_try_hugetlb_mte_tagging()
I already incorporated the first two in this version. But I kept
try_folio_hugetlb_mte_tagging(). Will change to
folio_try_hugetlb_mte_tagging().
I will spin a new version and send out soon since the change is trivial
and I'm going to travel to LPC on Monday.
As for "try" vs "test_and_set_.*_lock", the original name was picked to
mimic spin_trylock() since this function is waiting/spinning. It's not
great but the alternative naming is closer to test_and_set_bit_lock().
This has different behaviour, it only sets a bit with acquire semantics,
no waiting/spinning.
