diff mbox series

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

Message ID 20240528134513.2283548-2-wangkefeng.wang@huawei.com (mailing list archive)
State New
Headers show
Series mm: migrate: support poison recover from migrate folio | expand

Commit Message

Kefeng Wang May 28, 2024, 1:45 p.m. UTC 
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(+)

Comments

Lance Yang May 28, 2024, 2:17 p.m. UTC | #1 
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
>
>
Jane Chu May 28, 2024, 8:13 p.m. UTC | #2 
On 5/28/2024 6:45 AM, Kefeng Wang wrote:

>   
> +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;
> +		}
Why not be more specific by returning -EHWPOISON ?

-jane
Kefeng Wang May 29, 2024, 2:16 a.m. UTC | #3 
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
>>
>>
>
Kefeng Wang May 29, 2024, 2:16 a.m. UTC | #4 
On 2024/5/29 4:13, Jane Chu wrote:
> On 5/28/2024 6:45 AM, Kefeng Wang wrote:
> 
>> +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;
>> +        }
> Why not be more specific by returning -EHWPOISON ?

Yes, this is better, will update, thanks.

> 
> -jane
>
diff mbox series

Patch

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);
+
 int sysctl_overcommit_memory __read_mostly = OVERCOMMIT_GUESS;
 int sysctl_overcommit_ratio __read_mostly = 50;
 unsigned long sysctl_overcommit_kbytes __read_mostly;