On 2024/8/17 16:49, Kefeng Wang wrote:
> Add isolate_folio_to_list() helper to try to isolate HugeTLB,
> no-LRU movable and LRU folios to a list, which will be reused by
> do_migrate_range() from memory hotplug soon, also drop the
> mf_isolate_folio() since we could directly use new helper in
> the soft_offline_in_use_page().
>
> Acked-by: David Hildenbrand <david@xxxxxxxxxx>
> Signed-off-by: Kefeng Wang <wangkefeng.wang@xxxxxxxxxx>
Thanks for your patch.
> ---
> include/linux/migrate.h | 3 +++
> mm/memory-failure.c | 46 ++++++++++-------------------------------
> mm/migrate.c | 27 ++++++++++++++++++++++++
> 3 files changed, 41 insertions(+), 35 deletions(-)
>
> diff --git a/include/linux/migrate.h b/include/linux/migrate.h
> index 644be30b69c8..002e49b2ebd9 100644
> --- a/include/linux/migrate.h
> +++ b/include/linux/migrate.h
> @@ -70,6 +70,7 @@ int migrate_pages(struct list_head *l, new_folio_t new, free_folio_t free,
> unsigned int *ret_succeeded);
> struct folio *alloc_migration_target(struct folio *src, unsigned long private);
> bool isolate_movable_page(struct page *page, isolate_mode_t mode);
> +bool isolate_folio_to_list(struct folio *folio, struct list_head *list);
>
> int migrate_huge_page_move_mapping(struct address_space *mapping,
> struct folio *dst, struct folio *src);
> @@ -91,6 +92,8 @@ static inline struct folio *alloc_migration_target(struct folio *src,
> { return NULL; }
> static inline bool isolate_movable_page(struct page *page, isolate_mode_t mode)
> { return false; }
> +static inline bool isolate_folio_to_list(struct folio *folio, struct list_head *list)
> + { return false; }
>
> static inline int migrate_huge_page_move_mapping(struct address_space *mapping,
> struct folio *dst, struct folio *src)
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index 93848330de1f..d8298017bd99 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -2659,40 +2659,6 @@ EXPORT_SYMBOL(unpoison_memory);
> #undef pr_fmt
> #define pr_fmt(fmt) "Soft offline: " fmt
>
> -static bool mf_isolate_folio(struct folio *folio, struct list_head *pagelist)
> -{
> - bool isolated = false;
> -
> - if (folio_test_hugetlb(folio)) {
> - isolated = isolate_hugetlb(folio, pagelist);
> - } else {
> - bool lru = !__folio_test_movable(folio);
> -
> - if (lru)
> - isolated = folio_isolate_lru(folio);
> - else
> - isolated = isolate_movable_page(&folio->page,
> - ISOLATE_UNEVICTABLE);
> -
> - if (isolated) {
> - list_add(&folio->lru, pagelist);
> - if (lru)
> - node_stat_add_folio(folio, NR_ISOLATED_ANON +
> - folio_is_file_lru(folio));
> - }
> - }
> -
> - /*
> - * If we succeed to isolate the folio, we grabbed another refcount on
> - * the folio, so we can safely drop the one we got from get_any_page().
> - * If we failed to isolate the folio, it means that we cannot go further
> - * and we will return an error, so drop the reference we got from
> - * get_any_page() as well.
> - */
> - folio_put(folio);
> - return isolated;
> -}
> -
> /*
> * soft_offline_in_use_page handles hugetlb-pages and non-hugetlb pages.
> * If the page is a non-dirty unmapped page-cache page, it simply invalidates.
> @@ -2744,7 +2710,7 @@ static int soft_offline_in_use_page(struct page *page)
> return 0;
> }
>
> - if (mf_isolate_folio(folio, &pagelist)) {
> + if (isolate_folio_to_list(folio, &pagelist)) {
> ret = migrate_pages(&pagelist, alloc_migration_target, NULL,
> (unsigned long)&mtc, MIGRATE_SYNC, MR_MEMORY_FAILURE, NULL);
> if (!ret) {
> @@ -2766,6 +2732,16 @@ static int soft_offline_in_use_page(struct page *page)
> pfn, msg_page[huge], page_count(page), &page->flags);
> ret = -EBUSY;
> }
> +
> + /*
> + * If we succeed to isolate the folio, we grabbed another refcount on
> + * the folio, so we can safely drop the one we got from get_any_page().
> + * If we failed to isolate the folio, it means that we cannot go further
> + * and we will return an error, so drop the reference we got from
> + * get_any_page() as well.
> + */
> + folio_put(folio);
Why folio_put() is deferred here? With this change, folio will have extra two refcnt when
calling migrate_pages() above. One is from get_any_page() and another one from folio_isolate_lru().
This would lead to migrate_pages() never success. And my many testcases failed due to this
change.
Thanks.
.
