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

[Kefeng Wang <wangkefeng.wang@huawei.com>]

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.

> 
> Thanks.
> .