linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] mm: hwpoison: two more poison recovery
@ 2024-09-06  2:41 Kefeng Wang
  2024-09-06  2:42 ` [PATCH 1/2] mm: support poison recovery from do_cow_fault() Kefeng Wang
  2024-09-06  2:42 ` [PATCH 2/2] mm: support poison recovery from copy_present_page() Kefeng Wang
  0 siblings, 2 replies; 12+ messages in thread
From: Kefeng Wang @ 2024-09-06  2:41 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, Jane Chu, Miaohe Lin, Naoya Horiguchi, Tony Luck,
	Jiaqi Yan, David Hildenbrand, Kefeng Wang

One more CoW path to support poison recorvery in do_cow_fault(), and
the last copy_user_highpage() user is replaced to copy_mc_user_highpage()
from copy_present_page() during fork to support poison recorvery too.

Kefeng Wang (2):
  mm: support poison recovery from do_cow_fault()
  mm: support poison recovery from copy_present_page()

 mm/memory.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

-- 
2.27.0



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] mm: support poison recovery from do_cow_fault()
  2024-09-06  2:41 [PATCH 0/2] mm: hwpoison: two more poison recovery Kefeng Wang
@ 2024-09-06  2:42 ` Kefeng Wang
  2024-09-06 22:17   ` jane.chu
                     ` (2 more replies)
  2024-09-06  2:42 ` [PATCH 2/2] mm: support poison recovery from copy_present_page() Kefeng Wang
  1 sibling, 3 replies; 12+ messages in thread
From: Kefeng Wang @ 2024-09-06  2:42 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, Jane Chu, Miaohe Lin, Naoya Horiguchi, Tony Luck,
	Jiaqi Yan, David Hildenbrand, Kefeng Wang

Like commit a873dfe1032a ("mm, hwpoison: try to recover from copy-on
write faults"), there is another path which could crash because it does
not have recovery code where poison is consumed by the kernel in
do_cow_fault(), a crash calltrace shown below on old kernel, but it
could be happened in the lastest mainline code,

  CPU: 7 PID: 3248 Comm: mpi Kdump: loaded Tainted: G           OE     5.10.0 #1
  pc : copy_page+0xc/0xbc
  lr : copy_user_highpage+0x50/0x9c
  Call trace:
    copy_page+0xc/0xbc
    do_cow_fault+0x118/0x2bc
    do_fault+0x40/0x1a4
    handle_pte_fault+0x154/0x230
    __handle_mm_fault+0x1a8/0x38c
    handle_mm_fault+0xf0/0x250
    do_page_fault+0x184/0x454
    do_translation_fault+0xac/0xd4
    do_mem_abort+0x44/0xbc

Fix it by using copy_mc_user_highpage() to handle this case and return
VM_FAULT_HWPOISON for cow fault.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 mm/memory.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/mm/memory.c b/mm/memory.c
index 42674c0748cb..d310c073a1b3 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -5089,7 +5089,10 @@ static vm_fault_t do_cow_fault(struct vm_fault *vmf)
 	if (ret & VM_FAULT_DONE_COW)
 		return ret;
 
-	copy_user_highpage(vmf->cow_page, vmf->page, vmf->address, vma);
+	if (copy_mc_user_highpage(vmf->cow_page, vmf->page, vmf->address, vma)) {
+		ret = VM_FAULT_HWPOISON;
+		goto uncharge_out;
+	}
 	__folio_mark_uptodate(folio);
 
 	ret |= finish_fault(vmf);
-- 
2.27.0



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/2] mm: support poison recovery from copy_present_page()
  2024-09-06  2:41 [PATCH 0/2] mm: hwpoison: two more poison recovery Kefeng Wang
  2024-09-06  2:42 ` [PATCH 1/2] mm: support poison recovery from do_cow_fault() Kefeng Wang
@ 2024-09-06  2:42 ` Kefeng Wang
  2024-09-06 23:14   ` jane.chu
  2024-09-10  2:19   ` Miaohe Lin
  1 sibling, 2 replies; 12+ messages in thread
From: Kefeng Wang @ 2024-09-06  2:42 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, Jane Chu, Miaohe Lin, Naoya Horiguchi, Tony Luck,
	Jiaqi Yan, David Hildenbrand, Kefeng Wang

Similar to other poison recovery, use copy_mc_user_highpage() to
avoid potentially kernel panic during copy page in copy_present_page()
from fork, once copy failed due to hwpoison in source page, we need
to break out of copy in copy_pte_range() and release prealloc folio,
so copy_mc_user_highpage() is moved ahead before set *prealloc to NULL.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 mm/memory.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index d310c073a1b3..6e7b78e49d1a 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -926,8 +926,11 @@ copy_present_page(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma
 	 * We have a prealloc page, all good!  Take it
 	 * over and copy the page & arm it.
 	 */
+
+	if (copy_mc_user_highpage(&new_folio->page, page, addr, src_vma))
+		return -EHWPOISON;
+
 	*prealloc = NULL;
-	copy_user_highpage(&new_folio->page, page, addr, src_vma);
 	__folio_mark_uptodate(new_folio);
 	folio_add_new_anon_rmap(new_folio, dst_vma, addr, RMAP_EXCLUSIVE);
 	folio_add_lru_vma(new_folio, dst_vma);
@@ -1166,8 +1169,9 @@ copy_pte_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,
 		/*
 		 * If we need a pre-allocated page for this pte, drop the
 		 * locks, allocate, and try again.
+		 * If copy failed due to hwpoison in source page, break out.
 		 */
-		if (unlikely(ret == -EAGAIN))
+		if (unlikely(ret == -EAGAIN || ret == -EHWPOISON))
 			break;
 		if (unlikely(prealloc)) {
 			/*
@@ -1197,7 +1201,7 @@ copy_pte_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,
 			goto out;
 		}
 		entry.val = 0;
-	} else if (ret == -EBUSY) {
+	} else if (ret == -EBUSY || unlikely(ret == -EHWPOISON)) {
 		goto out;
 	} else if (ret ==  -EAGAIN) {
 		prealloc = folio_prealloc(src_mm, src_vma, addr, false);
-- 
2.27.0



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] mm: support poison recovery from do_cow_fault()
  2024-09-06  2:42 ` [PATCH 1/2] mm: support poison recovery from do_cow_fault() Kefeng Wang
@ 2024-09-06 22:17   ` jane.chu
  2024-09-10  1:58   ` Miaohe Lin
  2024-09-10  2:15   ` [PATCH] mm: support poison recovery from do_cow_fault() fix Kefeng Wang
  2 siblings, 0 replies; 12+ messages in thread
From: jane.chu @ 2024-09-06 22:17 UTC (permalink / raw)
  To: Kefeng Wang, Andrew Morton
  Cc: linux-mm, Miaohe Lin, Naoya Horiguchi, Tony Luck, Jiaqi Yan,
	David Hildenbrand

On 9/5/2024 7:42 PM, Kefeng Wang wrote:

> Like commit a873dfe1032a ("mm, hwpoison: try to recover from copy-on
> write faults"), there is another path which could crash because it does
> not have recovery code where poison is consumed by the kernel in
> do_cow_fault(), a crash calltrace shown below on old kernel, but it
> could be happened in the lastest mainline code,
>
>    CPU: 7 PID: 3248 Comm: mpi Kdump: loaded Tainted: G           OE     5.10.0 #1
>    pc : copy_page+0xc/0xbc
>    lr : copy_user_highpage+0x50/0x9c
>    Call trace:
>      copy_page+0xc/0xbc
>      do_cow_fault+0x118/0x2bc
>      do_fault+0x40/0x1a4
>      handle_pte_fault+0x154/0x230
>      __handle_mm_fault+0x1a8/0x38c
>      handle_mm_fault+0xf0/0x250
>      do_page_fault+0x184/0x454
>      do_translation_fault+0xac/0xd4
>      do_mem_abort+0x44/0xbc
>
> Fix it by using copy_mc_user_highpage() to handle this case and return
> VM_FAULT_HWPOISON for cow fault.
>
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
>   mm/memory.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/mm/memory.c b/mm/memory.c
> index 42674c0748cb..d310c073a1b3 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -5089,7 +5089,10 @@ static vm_fault_t do_cow_fault(struct vm_fault *vmf)
>   	if (ret & VM_FAULT_DONE_COW)
>   		return ret;
>   
> -	copy_user_highpage(vmf->cow_page, vmf->page, vmf->address, vma);
> +	if (copy_mc_user_highpage(vmf->cow_page, vmf->page, vmf->address, vma)) {
> +		ret = VM_FAULT_HWPOISON;
> +		goto uncharge_out;
> +	}
>   	__folio_mark_uptodate(folio);
>   
>   	ret |= finish_fault(vmf);

Thanks for catching it!

Reviewed-by: Jane Chu <jane.chu@oracle.com>

-jane



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] mm: support poison recovery from copy_present_page()
  2024-09-06  2:42 ` [PATCH 2/2] mm: support poison recovery from copy_present_page() Kefeng Wang
@ 2024-09-06 23:14   ` jane.chu
  2024-09-10  2:19   ` Miaohe Lin
  1 sibling, 0 replies; 12+ messages in thread
From: jane.chu @ 2024-09-06 23:14 UTC (permalink / raw)
  To: Kefeng Wang, Andrew Morton
  Cc: linux-mm, Miaohe Lin, Naoya Horiguchi, Tony Luck, Jiaqi Yan,
	David Hildenbrand

On 9/5/2024 7:42 PM, Kefeng Wang wrote:

> Similar to other poison recovery, use copy_mc_user_highpage() to
> avoid potentially kernel panic during copy page in copy_present_page()
> from fork, once copy failed due to hwpoison in source page, we need
> to break out of copy in copy_pte_range() and release prealloc folio,
> so copy_mc_user_highpage() is moved ahead before set *prealloc to NULL.
>
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
>   mm/memory.c | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/mm/memory.c b/mm/memory.c
> index d310c073a1b3..6e7b78e49d1a 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -926,8 +926,11 @@ copy_present_page(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma
>   	 * We have a prealloc page, all good!  Take it
>   	 * over and copy the page & arm it.
>   	 */
> +
> +	if (copy_mc_user_highpage(&new_folio->page, page, addr, src_vma))
> +		return -EHWPOISON;
> +
>   	*prealloc = NULL;
> -	copy_user_highpage(&new_folio->page, page, addr, src_vma);
>   	__folio_mark_uptodate(new_folio);
>   	folio_add_new_anon_rmap(new_folio, dst_vma, addr, RMAP_EXCLUSIVE);
>   	folio_add_lru_vma(new_folio, dst_vma);
> @@ -1166,8 +1169,9 @@ copy_pte_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,
>   		/*
>   		 * If we need a pre-allocated page for this pte, drop the
>   		 * locks, allocate, and try again.
> +		 * If copy failed due to hwpoison in source page, break out.
>   		 */
> -		if (unlikely(ret == -EAGAIN))
> +		if (unlikely(ret == -EAGAIN || ret == -EHWPOISON))
>   			break;
>   		if (unlikely(prealloc)) {
>   			/*
> @@ -1197,7 +1201,7 @@ copy_pte_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,
>   			goto out;
>   		}
>   		entry.val = 0;
> -	} else if (ret == -EBUSY) {
> +	} else if (ret == -EBUSY || unlikely(ret == -EHWPOISON)) {
>   		goto out;
>   	} else if (ret ==  -EAGAIN) {
>   		prealloc = folio_prealloc(src_mm, src_vma, addr, false);

Looks good.

Reviewed-by: Jane Chu <jane.chu@oracle.com>

-jane



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] mm: support poison recovery from do_cow_fault()
  2024-09-06  2:42 ` [PATCH 1/2] mm: support poison recovery from do_cow_fault() Kefeng Wang
  2024-09-06 22:17   ` jane.chu
@ 2024-09-10  1:58   ` Miaohe Lin
  2024-09-10  2:13     ` Kefeng Wang
  2024-09-10  2:15   ` [PATCH] mm: support poison recovery from do_cow_fault() fix Kefeng Wang
  2 siblings, 1 reply; 12+ messages in thread
From: Miaohe Lin @ 2024-09-10  1:58 UTC (permalink / raw)
  To: Kefeng Wang, Andrew Morton
  Cc: linux-mm, Jane Chu, Naoya Horiguchi, Tony Luck, Jiaqi Yan,
	David Hildenbrand

On 2024/9/6 10:42, Kefeng Wang wrote:
> Like commit a873dfe1032a ("mm, hwpoison: try to recover from copy-on
> write faults"), there is another path which could crash because it does
> not have recovery code where poison is consumed by the kernel in
> do_cow_fault(), a crash calltrace shown below on old kernel, but it
> could be happened in the lastest mainline code,
> 
>   CPU: 7 PID: 3248 Comm: mpi Kdump: loaded Tainted: G           OE     5.10.0 #1
>   pc : copy_page+0xc/0xbc
>   lr : copy_user_highpage+0x50/0x9c
>   Call trace:
>     copy_page+0xc/0xbc
>     do_cow_fault+0x118/0x2bc
>     do_fault+0x40/0x1a4
>     handle_pte_fault+0x154/0x230
>     __handle_mm_fault+0x1a8/0x38c
>     handle_mm_fault+0xf0/0x250
>     do_page_fault+0x184/0x454
>     do_translation_fault+0xac/0xd4
>     do_mem_abort+0x44/0xbc
> 
> Fix it by using copy_mc_user_highpage() to handle this case and return
> VM_FAULT_HWPOISON for cow fault.
> 
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
>  mm/memory.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/memory.c b/mm/memory.c
> index 42674c0748cb..d310c073a1b3 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -5089,7 +5089,10 @@ static vm_fault_t do_cow_fault(struct vm_fault *vmf)
>  	if (ret & VM_FAULT_DONE_COW)
>  		return ret;
>  
> -	copy_user_highpage(vmf->cow_page, vmf->page, vmf->address, vma);
> +	if (copy_mc_user_highpage(vmf->cow_page, vmf->page, vmf->address, vma)) {
> +		ret = VM_FAULT_HWPOISON;
> +		goto uncharge_out;
> +	}

When copy_mc_user_highpage fails, we should have vmf->page locked and hold the extra refcnt
of vmf->page. So we should call unlock_page(vmf->page) and put_page(vmf->page) before goto
uncharge_out?

Thanks.
.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] mm: support poison recovery from do_cow_fault()
  2024-09-10  1:58   ` Miaohe Lin
@ 2024-09-10  2:13     ` Kefeng Wang
  0 siblings, 0 replies; 12+ messages in thread
From: Kefeng Wang @ 2024-09-10  2:13 UTC (permalink / raw)
  To: Miaohe Lin, Andrew Morton
  Cc: linux-mm, Jane Chu, Naoya Horiguchi, Tony Luck, Jiaqi Yan,
	David Hildenbrand



On 2024/9/10 9:58, Miaohe Lin wrote:
> On 2024/9/6 10:42, Kefeng Wang wrote:
>> Like commit a873dfe1032a ("mm, hwpoison: try to recover from copy-on
>> write faults"), there is another path which could crash because it does
>> not have recovery code where poison is consumed by the kernel in
>> do_cow_fault(), a crash calltrace shown below on old kernel, but it
>> could be happened in the lastest mainline code,
>>
>>    CPU: 7 PID: 3248 Comm: mpi Kdump: loaded Tainted: G           OE     5.10.0 #1
>>    pc : copy_page+0xc/0xbc
>>    lr : copy_user_highpage+0x50/0x9c
>>    Call trace:
>>      copy_page+0xc/0xbc
>>      do_cow_fault+0x118/0x2bc
>>      do_fault+0x40/0x1a4
>>      handle_pte_fault+0x154/0x230
>>      __handle_mm_fault+0x1a8/0x38c
>>      handle_mm_fault+0xf0/0x250
>>      do_page_fault+0x184/0x454
>>      do_translation_fault+0xac/0xd4
>>      do_mem_abort+0x44/0xbc
>>
>> Fix it by using copy_mc_user_highpage() to handle this case and return
>> VM_FAULT_HWPOISON for cow fault.
>>
>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>> ---
>>   mm/memory.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/mm/memory.c b/mm/memory.c
>> index 42674c0748cb..d310c073a1b3 100644
>> --- a/mm/memory.c
>> +++ b/mm/memory.c
>> @@ -5089,7 +5089,10 @@ static vm_fault_t do_cow_fault(struct vm_fault *vmf)
>>   	if (ret & VM_FAULT_DONE_COW)
>>   		return ret;
>>   
>> -	copy_user_highpage(vmf->cow_page, vmf->page, vmf->address, vma);
>> +	if (copy_mc_user_highpage(vmf->cow_page, vmf->page, vmf->address, vma)) {
>> +		ret = VM_FAULT_HWPOISON;
>> +		goto uncharge_out;
>> +	}
> 
> When copy_mc_user_highpage fails, we should have vmf->page locked and hold the extra refcnt
> of vmf->page. So we should call unlock_page(vmf->page) and put_page(vmf->page) before goto
> uncharge_out?
> 

Right, for upstream, we need to more handling for vmf->page, will fix, 
thanks.

> Thanks.
> .


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] mm: support poison recovery from do_cow_fault() fix
  2024-09-06  2:42 ` [PATCH 1/2] mm: support poison recovery from do_cow_fault() Kefeng Wang
  2024-09-06 22:17   ` jane.chu
  2024-09-10  1:58   ` Miaohe Lin
@ 2024-09-10  2:15   ` Kefeng Wang
  2024-09-12  2:03     ` Miaohe Lin
  2 siblings, 1 reply; 12+ messages in thread
From: Kefeng Wang @ 2024-09-10  2:15 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, Jane Chu, Miaohe Lin, Naoya Horiguchi, Tony Luck,
	Jiaqi Yan, David Hildenbrand, Kefeng Wang

unlock/put vmf->page, per Miaohe

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 mm/memory.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mm/memory.c b/mm/memory.c
index b84443e689a8..47c0202136c9 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -5096,11 +5096,12 @@ static vm_fault_t do_cow_fault(struct vm_fault *vmf)
 
 	if (copy_mc_user_highpage(vmf->cow_page, vmf->page, vmf->address, vma)) {
 		ret = VM_FAULT_HWPOISON;
-		goto uncharge_out;
+		goto unlock;
 	}
 	__folio_mark_uptodate(folio);
 
 	ret |= finish_fault(vmf);
+unlock:
 	unlock_page(vmf->page);
 	put_page(vmf->page);
 	if (unlikely(ret & (VM_FAULT_ERROR | VM_FAULT_NOPAGE | VM_FAULT_RETRY)))
-- 
2.27.0



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] mm: support poison recovery from copy_present_page()
  2024-09-06  2:42 ` [PATCH 2/2] mm: support poison recovery from copy_present_page() Kefeng Wang
  2024-09-06 23:14   ` jane.chu
@ 2024-09-10  2:19   ` Miaohe Lin
  2024-09-10  6:35     ` Kefeng Wang
  1 sibling, 1 reply; 12+ messages in thread
From: Miaohe Lin @ 2024-09-10  2:19 UTC (permalink / raw)
  To: Kefeng Wang, Andrew Morton
  Cc: linux-mm, Jane Chu, Naoya Horiguchi, Tony Luck, Jiaqi Yan,
	David Hildenbrand

On 2024/9/6 10:42, Kefeng Wang wrote:
> Similar to other poison recovery, use copy_mc_user_highpage() to
> avoid potentially kernel panic during copy page in copy_present_page()
> from fork, once copy failed due to hwpoison in source page, we need
> to break out of copy in copy_pte_range() and release prealloc folio,
> so copy_mc_user_highpage() is moved ahead before set *prealloc to NULL.
> 
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
>  mm/memory.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/memory.c b/mm/memory.c
> index d310c073a1b3..6e7b78e49d1a 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -926,8 +926,11 @@ copy_present_page(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma
>  	 * We have a prealloc page, all good!  Take it
>  	 * over and copy the page & arm it.
>  	 */
> +
> +	if (copy_mc_user_highpage(&new_folio->page, page, addr, src_vma))
> +		return -EHWPOISON;
> +
>  	*prealloc = NULL;
> -	copy_user_highpage(&new_folio->page, page, addr, src_vma);
>  	__folio_mark_uptodate(new_folio);
>  	folio_add_new_anon_rmap(new_folio, dst_vma, addr, RMAP_EXCLUSIVE);
>  	folio_add_lru_vma(new_folio, dst_vma);
> @@ -1166,8 +1169,9 @@ copy_pte_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,
>  		/*
>  		 * If we need a pre-allocated page for this pte, drop the
>  		 * locks, allocate, and try again.
> +		 * If copy failed due to hwpoison in source page, break out.
>  		 */
> -		if (unlikely(ret == -EAGAIN))
> +		if (unlikely(ret == -EAGAIN || ret == -EHWPOISON))

Will it be better to put checking ret against -EHWPOISON in a new line? -EAGAIN case will enter the
loop again but -EHWPOISON case never does.

>  			break;
>  		if (unlikely(prealloc)) {
>  			/*
> @@ -1197,7 +1201,7 @@ copy_pte_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,
>  			goto out;
>  		}
>  		entry.val = 0;
> -	} else if (ret == -EBUSY) {
> +	} else if (ret == -EBUSY || unlikely(ret == -EHWPOISON)) {

The caller of copy_pte_range() always set errno to -ENOMEM. So fork will failed with ENOMEM even if the real
cause is failed to copy due to hwpoison in source page. It's a pity.

Thanks.
.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] mm: support poison recovery from copy_present_page()
  2024-09-10  2:19   ` Miaohe Lin
@ 2024-09-10  6:35     ` Kefeng Wang
  2024-09-12  2:06       ` Miaohe Lin
  0 siblings, 1 reply; 12+ messages in thread
From: Kefeng Wang @ 2024-09-10  6:35 UTC (permalink / raw)
  To: Miaohe Lin, Andrew Morton
  Cc: linux-mm, Jane Chu, Naoya Horiguchi, Tony Luck, Jiaqi Yan,
	David Hildenbrand



On 2024/9/10 10:19, Miaohe Lin wrote:
> On 2024/9/6 10:42, Kefeng Wang wrote:
>> Similar to other poison recovery, use copy_mc_user_highpage() to
>> avoid potentially kernel panic during copy page in copy_present_page()
>> from fork, once copy failed due to hwpoison in source page, we need
>> to break out of copy in copy_pte_range() and release prealloc folio,
>> so copy_mc_user_highpage() is moved ahead before set *prealloc to NULL.
>>
>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>> ---
>>   mm/memory.c | 10 +++++++---
>>   1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/mm/memory.c b/mm/memory.c
>> index d310c073a1b3..6e7b78e49d1a 100644
>> --- a/mm/memory.c
>> +++ b/mm/memory.c
>> @@ -926,8 +926,11 @@ copy_present_page(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma
>>   	 * We have a prealloc page, all good!  Take it
>>   	 * over and copy the page & arm it.
>>   	 */
>> +
>> +	if (copy_mc_user_highpage(&new_folio->page, page, addr, src_vma))
>> +		return -EHWPOISON;
>> +
>>   	*prealloc = NULL;
>> -	copy_user_highpage(&new_folio->page, page, addr, src_vma);
>>   	__folio_mark_uptodate(new_folio);
>>   	folio_add_new_anon_rmap(new_folio, dst_vma, addr, RMAP_EXCLUSIVE);
>>   	folio_add_lru_vma(new_folio, dst_vma);
>> @@ -1166,8 +1169,9 @@ copy_pte_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,
>>   		/*
>>   		 * If we need a pre-allocated page for this pte, drop the
>>   		 * locks, allocate, and try again.
>> +		 * If copy failed due to hwpoison in source page, break out.
>>   		 */
>> -		if (unlikely(ret == -EAGAIN))
>> +		if (unlikely(ret == -EAGAIN || ret == -EHWPOISON))
> 
> Will it be better to put checking ret against -EHWPOISON in a new line? -EAGAIN case will enter the
> loop again but -EHWPOISON case never does.

Maybe not a newline since we will recheck the ret in the below and 
return directly from copy_present_page().

> 
>>   			break;
>>   		if (unlikely(prealloc)) {
>>   			/*
>> @@ -1197,7 +1201,7 @@ copy_pte_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,
>>   			goto out;
>>   		}
>>   		entry.val = 0;
>> -	} else if (ret == -EBUSY) {
>> +	} else if (ret == -EBUSY || unlikely(ret == -EHWPOISON)) {
> 
> The caller of copy_pte_range() always set errno to -ENOMEM. So fork will failed with ENOMEM even if the real
> cause is failed to copy due to hwpoison in source page. It's a pity.

Yes, it's not just the new -EHWPOISON, all other errnos(ENOENT/EIO) will 
be ignored and return -ENOMEM instead.

> 
> Thanks.
> .


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] mm: support poison recovery from do_cow_fault() fix
  2024-09-10  2:15   ` [PATCH] mm: support poison recovery from do_cow_fault() fix Kefeng Wang
@ 2024-09-12  2:03     ` Miaohe Lin
  0 siblings, 0 replies; 12+ messages in thread
From: Miaohe Lin @ 2024-09-12  2:03 UTC (permalink / raw)
  To: Kefeng Wang, Andrew Morton
  Cc: linux-mm, Jane Chu, Naoya Horiguchi, Tony Luck, Jiaqi Yan,
	David Hildenbrand

On 2024/9/10 10:15, Kefeng Wang wrote:
> unlock/put vmf->page, per Miaohe
> 
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>

With this, this patch looks good to me.

Reviewed-by: Miaohe Lin <linmiaohe@huawei.com>

Thanks.
.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] mm: support poison recovery from copy_present_page()
  2024-09-10  6:35     ` Kefeng Wang
@ 2024-09-12  2:06       ` Miaohe Lin
  0 siblings, 0 replies; 12+ messages in thread
From: Miaohe Lin @ 2024-09-12  2:06 UTC (permalink / raw)
  To: Kefeng Wang, Andrew Morton
  Cc: linux-mm, Jane Chu, Naoya Horiguchi, Tony Luck, Jiaqi Yan,
	David Hildenbrand

On 2024/9/10 14:35, Kefeng Wang wrote:
> 
> 
> On 2024/9/10 10:19, Miaohe Lin wrote:
>> On 2024/9/6 10:42, Kefeng Wang wrote:
>>> Similar to other poison recovery, use copy_mc_user_highpage() to
>>> avoid potentially kernel panic during copy page in copy_present_page()
>>> from fork, once copy failed due to hwpoison in source page, we need
>>> to break out of copy in copy_pte_range() and release prealloc folio,
>>> so copy_mc_user_highpage() is moved ahead before set *prealloc to NULL.
>>>
>>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>>> ---
>>>   mm/memory.c | 10 +++++++---
>>>   1 file changed, 7 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/mm/memory.c b/mm/memory.c
>>> index d310c073a1b3..6e7b78e49d1a 100644
>>> --- a/mm/memory.c
>>> +++ b/mm/memory.c
>>> @@ -926,8 +926,11 @@ copy_present_page(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma
>>>        * We have a prealloc page, all good!  Take it
>>>        * over and copy the page & arm it.
>>>        */
>>> +
>>> +    if (copy_mc_user_highpage(&new_folio->page, page, addr, src_vma))
>>> +        return -EHWPOISON;
>>> +
>>>       *prealloc = NULL;
>>> -    copy_user_highpage(&new_folio->page, page, addr, src_vma);
>>>       __folio_mark_uptodate(new_folio);
>>>       folio_add_new_anon_rmap(new_folio, dst_vma, addr, RMAP_EXCLUSIVE);
>>>       folio_add_lru_vma(new_folio, dst_vma);
>>> @@ -1166,8 +1169,9 @@ copy_pte_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,
>>>           /*
>>>            * If we need a pre-allocated page for this pte, drop the
>>>            * locks, allocate, and try again.
>>> +         * If copy failed due to hwpoison in source page, break out.
>>>            */
>>> -        if (unlikely(ret == -EAGAIN))
>>> +        if (unlikely(ret == -EAGAIN || ret == -EHWPOISON))
>>
>> Will it be better to put checking ret against -EHWPOISON in a new line? -EAGAIN case will enter the
>> loop again but -EHWPOISON case never does.
> 
> Maybe not a newline since we will recheck the ret in the below and return directly from copy_present_page().

Anyway, this patch looks good to me.

Reviewed-by: Miaohe Lin <linmiaohe@huawei.com>

Thanks.
.


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2024-09-12  2:06 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-09-06  2:41 [PATCH 0/2] mm: hwpoison: two more poison recovery Kefeng Wang
2024-09-06  2:42 ` [PATCH 1/2] mm: support poison recovery from do_cow_fault() Kefeng Wang
2024-09-06 22:17   ` jane.chu
2024-09-10  1:58   ` Miaohe Lin
2024-09-10  2:13     ` Kefeng Wang
2024-09-10  2:15   ` [PATCH] mm: support poison recovery from do_cow_fault() fix Kefeng Wang
2024-09-12  2:03     ` Miaohe Lin
2024-09-06  2:42 ` [PATCH 2/2] mm: support poison recovery from copy_present_page() Kefeng Wang
2024-09-06 23:14   ` jane.chu
2024-09-10  2:19   ` Miaohe Lin
2024-09-10  6:35     ` Kefeng Wang
2024-09-12  2:06       ` Miaohe Lin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox