Re: [PATCH v2 4/5] mm: migrate: add isolate_folio_to_list()

[Miaohe Lin <linmiaohe@huawei.com>]

On 2024/8/20 17:46, Kefeng Wang wrote:
> 
> 
> On 2024/8/20 17:32, Miaohe Lin wrote:
>> 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@redhat.com>
>>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>>
>> Thanks for your patch.
>>>
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index d8298017bd99..64a145a0e29f 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -2671,6 +2671,7 @@ static int soft_offline_in_use_page(struct page *page)
>         struct folio *folio = page_folio(page);
>         char const *msg_page[] = {"page", "hugepage"};
>         bool huge = folio_test_hugetlb(folio);
> +       bool isolated;
>         LIST_HEAD(pagelist);
>         struct migration_target_control mtc = {
>                 .nid = NUMA_NO_NODE,
> @@ -2710,7 +2711,18 @@ static int soft_offline_in_use_page(struct page *page)
>                 return 0;
>         }
> 
> -       if (isolate_folio_to_list(folio, &pagelist)) {
> +       isolated = isolate_folio_to_list(folio, &pagelist);
> +
> +       /*
> +        * 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);
> +
> +       if (isolated) {
>                 ret = migrate_pages(&pagelist, alloc_migration_target, NULL,
>                         (unsigned long)&mtc, MIGRATE_SYNC, MR_MEMORY_FAILURE, NULL);
>                 if (!ret) {
> @@ -2733,15 +2745,6 @@ static int soft_offline_in_use_page(struct page *page)
>                 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);
> -
>         return ret;
>  }

This works to me.

Thanks.
.