Re: [PATCH v3 1/5] mm: add folio_mc_copy()

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

On 2024/5/28 22:17, Lance Yang wrote:
> Hi Kefeng,
> 
> On Tue, May 28, 2024 at 9:45 PM Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
>>
>> Add a #MC variant of folio_copy() which uses copy_mc_highpage() to
>> support #MC handled during folio copy, it will be used in folio
>> migration soon.
>>
>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>> ---
>>   include/linux/mm.h |  1 +
>>   mm/util.c          | 20 ++++++++++++++++++++
>>   2 files changed, 21 insertions(+)
>>
>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>> index 12bf20ebae5a..0c78d67f787d 100644
>> --- a/include/linux/mm.h
>> +++ b/include/linux/mm.h
>> @@ -1321,6 +1321,7 @@ void put_pages_list(struct list_head *pages);
>>
>>   void split_page(struct page *page, unsigned int order);
>>   void folio_copy(struct folio *dst, struct folio *src);
>> +int folio_mc_copy(struct folio *dst, struct folio *src);
>>
>>   unsigned long nr_free_buffer_pages(void);
>>
>> diff --git a/mm/util.c b/mm/util.c
>> index c9e519e6811f..9462dbf7ce02 100644
>> --- a/mm/util.c
>> +++ b/mm/util.c
>> @@ -828,6 +828,26 @@ void folio_copy(struct folio *dst, struct folio *src)
>>   }
>>   EXPORT_SYMBOL(folio_copy);
>>
>> +int folio_mc_copy(struct folio *dst, struct folio *src)
>> +{
>> +       long nr = folio_nr_pages(src);
>> +       long i = 0;
>> +       int ret = 0;
>> +
>> +       for (;;) {
>> +               if (copy_mc_highpage(folio_page(dst, i), folio_page(src, i))) {
>> +                       ret = -EFAULT;
>> +                       break;
>> +               }
>> +               if (++i == nr)
>> +                       break;
>> +               cond_resched();
>> +       }
>> +
>> +       return ret;
>> +}
>> +EXPORT_SYMBOL(folio_mc_copy);
> 
> The code below might be clearer:
> 
> +int folio_mc_copy(struct folio *dst, struct folio *src)
> +{
> +       long nr = folio_nr_pages(src);
> +       long i = 0;
> +
> +       for (;;) {
> +               if (copy_mc_highpage(folio_page(dst, i), folio_page(src, i)))
> +                       return -EFAULT;
> +               if (++i == nr)
> +                       break;
> +               cond_resched();
> +       }
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL(folio_mc_copy);
> 
> IMO, we can return the error code directly without needing to use the 'ret'.

Clearer, will update, thanks
> 
> Thanks,
> Lance
> 
>> +
>>   int sysctl_overcommit_memory __read_mostly = OVERCOMMIT_GUESS;
>>   int sysctl_overcommit_ratio __read_mostly = 50;
>>   unsigned long sysctl_overcommit_kbytes __read_mostly;
>> --
>> 2.27.0
>>
>>
>