On Thu, Nov 28, 2024 at 6:52 PM David Wang <00107082@xxxxxxx> wrote:
>
> Current solution to adjust codetag references during page migration is
> done in 3 steps:
> 1. sets the codetag reference of the old page as empty (not pointing
> to any codetag);
> 2. subtracts counters of the new page to compensate for its own allocation;
> 3. sets codetag reference of the new page to point to the codetag of
> the old page.
> This does not work if CONFIG_MEM_ALLOC_PROFILING_DEBUG=n because
> set_codetag_empty() becomes NOOP. Instead, let's simply swap codetag
> references so that the new page is referencing the old codetag and the
> old page is referencing the new codetag. This way accounting stays
> valid and the logic makes more sense.
>
> Fixes: e0a955bf7f61 ("mm/codetag: add pgalloc_tag_copy()")
> Signed-off-by: David Wang <00107082@xxxxxxx>
> Closes: https://lore.kernel.org/lkml/20241124074318.399027-1-00107082@xxxxxxx/
> Acked-by: Suren Baghdasaryan <surenb@xxxxxxxxxx>
> Suggested-by: Suren Baghdasaryan <surenb@xxxxxxxxxx>
Thanks! This looks fine to me but IIRC I did not Ack this patch
before. In the future please do not add Acked-by until you get an
explicit Ack. That said,
Acked-by: Suren Baghdasaryan <surenb@xxxxxxxxxx>
> ---
> include/linux/pgalloc_tag.h | 4 ++--
> lib/alloc_tag.c | 36 ++++++++++++++++++++++--------------
> mm/migrate.c | 2 +-
> 3 files changed, 25 insertions(+), 17 deletions(-)
>
> diff --git a/include/linux/pgalloc_tag.h b/include/linux/pgalloc_tag.h
> index 0e43ab653ab6..3469c4b20105 100644
> --- a/include/linux/pgalloc_tag.h
> +++ b/include/linux/pgalloc_tag.h
> @@ -231,7 +231,7 @@ static inline void pgalloc_tag_sub_pages(struct alloc_tag *tag, unsigned int nr)
> }
>
> void pgalloc_tag_split(struct folio *folio, int old_order, int new_order);
> -void pgalloc_tag_copy(struct folio *new, struct folio *old);
> +void pgalloc_tag_swap(struct folio *new, struct folio *old);
>
> void __init alloc_tag_sec_init(void);
>
> @@ -245,7 +245,7 @@ static inline struct alloc_tag *pgalloc_tag_get(struct page *page) { return NULL
> static inline void pgalloc_tag_sub_pages(struct alloc_tag *tag, unsigned int nr) {}
> static inline void alloc_tag_sec_init(void) {}
> static inline void pgalloc_tag_split(struct folio *folio, int old_order, int new_order) {}
> -static inline void pgalloc_tag_copy(struct folio *new, struct folio *old) {}
> +static inline void pgalloc_tag_swap(struct folio *new, struct folio *old) {}
>
> #endif /* CONFIG_MEM_ALLOC_PROFILING */
>
> diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c
> index 2414a7ee7ec7..35f7560a309a 100644
> --- a/lib/alloc_tag.c
> +++ b/lib/alloc_tag.c
> @@ -189,26 +189,34 @@ void pgalloc_tag_split(struct folio *folio, int old_order, int new_order)
> }
> }
>
> -void pgalloc_tag_copy(struct folio *new, struct folio *old)
> +void pgalloc_tag_swap(struct folio *new, struct folio *old)
> {
> - union pgtag_ref_handle handle;
> - union codetag_ref ref;
> - struct alloc_tag *tag;
> + union pgtag_ref_handle handle_old, handle_new;
> + union codetag_ref ref_old, ref_new;
> + struct alloc_tag *tag_old, *tag_new;
>
> - tag = pgalloc_tag_get(&old->page);
> - if (!tag)
> + tag_old = pgalloc_tag_get(&old->page);
> + if (!tag_old)
> + return;
> + tag_new = pgalloc_tag_get(&new->page);
> + if (!tag_new)
> return;
>
> - if (!get_page_tag_ref(&new->page, &ref, &handle))
> + if (!get_page_tag_ref(&old->page, &ref_old, &handle_old))
> return;
> + if (!get_page_tag_ref(&new->page, &ref_new, &handle_new)) {
> + put_page_tag_ref(handle_old);
> + return;
> + }
> +
> + /* swap tags */
> + __alloc_tag_ref_set(&ref_old, tag_new);
> + update_page_tag_ref(handle_old, &ref_old);
> + __alloc_tag_ref_set(&ref_new, tag_old);
> + update_page_tag_ref(handle_new, &ref_new);
>
> - /* Clear the old ref to the original allocation tag. */
> - clear_page_tag_ref(&old->page);
> - /* Decrement the counters of the tag on get_new_folio. */
> - alloc_tag_sub(&ref, folio_size(new));
> - __alloc_tag_ref_set(&ref, tag);
> - update_page_tag_ref(handle, &ref);
> - put_page_tag_ref(handle);
> + put_page_tag_ref(handle_old);
> + put_page_tag_ref(handle_new);
> }
>
> static void shutdown_mem_profiling(bool remove_file)
> diff --git a/mm/migrate.c b/mm/migrate.c
> index 2ce6b4b814df..cc68583c86f9 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -745,7 +745,7 @@ void folio_migrate_flags(struct folio *newfolio, struct folio *folio)
> folio_set_readahead(newfolio);
>
> folio_copy_owner(newfolio, folio);
> - pgalloc_tag_copy(newfolio, folio);
> + pgalloc_tag_swap(newfolio, folio);
>
> mem_cgroup_migrate(folio, newfolio);
> }
> --
> 2.39.2
>
