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

[Lance Yang <ioworker0@gmail.com>]

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'.

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
>
>