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

[Miaohe Lin <linmiaohe@huawei.com>]

On 2024/8/21 10:14, Kefeng Wang wrote:
> 
> 
> On 2024/8/21 10:00, Miaohe Lin wrote:
>> 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.
> 
> Good, my bad for break.
> 
> Andrew，please help to squash above changes, thanks.

With above changes added, this patch looks good to me.

Acked-by: Miaohe Lin <linmiaohe@huawei.com>
Tested-by: Miaohe Lin <linmiaohe@huawei.com>

Thanks.
.