Re: [PATCH v4 4/6] mm: migrate: support poisoned recover from migrate folio

[Jane Chu <jane.chu@oracle.com>]

On 6/6/2024 2:27 PM, Jane Chu wrote:

> On 6/3/2024 2:24 AM, Kefeng Wang wrote:
>> diff --git a/mm/migrate.c b/mm/migrate.c
>> index e930376c261a..28aa9da95781 100644
>> --- a/mm/migrate.c
>> +++ b/mm/migrate.c
>> @@ -663,16 +663,29 @@ static int __migrate_folio(struct address_space 
>> *mapping, struct folio *dst,
>>                  struct folio *src, void *src_private,
>>                  enum migrate_mode mode)
>>   {
>> -    int rc;
>> +    int ret, expected_cnt = folio_expected_refs(mapping, src);
>>   -    rc = folio_migrate_mapping(mapping, dst, src, 0);
>> -    if (rc != MIGRATEPAGE_SUCCESS)
>> -        return rc;
>> +    if (!mapping) {
>> +        if (folio_ref_count(src) != expected_cnt)
>> +            return -EAGAIN;
>> +    } else {
>> +        if (!folio_ref_freeze(src, expected_cnt))
>> +            return -EAGAIN;
>> +    }
>> +
>
> Let me take a guess, the reason you split up folio_migrate_copy() is that
>
> folio_mc_copy() should be done before the 'src' folio's ->flags is 
> changed, right?
>
> Is there any other reason?  Could you add a comment please?

I see, both the clearing of the 'dirty' bit in the source folio, and the 
xas_store of the

new folio to the mapping, these need to be done after folio_mc_copy 
considering in the

event of UE, memory_failure() is called to handle the poison in the 
source page.

That said, since the poisoned page was queued up and handling is 
asynchronous, so in

theory, there is an extremely unlikely chance that memory_failure() is 
invoked after

folio_migrate_mapping(), do you think things would still be cool?


thanks,

-jane

>
>> +    ret = folio_mc_copy(dst, src);
>> +    if (unlikely(ret)) {
>> +        if (mapping)
>> +            folio_ref_unfreeze(src, expected_cnt);
>> +        return ret;
>> +    }
>> +
>> +    __folio_migrate_mapping(mapping, dst, src, expected_cnt);
>>         if (src_private)
>>           folio_attach_private(dst, folio_detach_private(src));
>>   -    folio_migrate_copy(dst, src);
>> +    folio_migrate_flags(dst, src);
>>       return MIGRATEPAGE_SUCCESS;
>>   }
>
> thanks,
>
> -jane
>
>