[PATCH] HWPOISON: Fix the handling path of the victimized page frame that belong to non-LUR

[PATCH] HWPOISON: Fix the handling path of the victimized page frame that belong to non-LUR

[Chen Yucong]

Until now, the kernel has the same policy to handle victimized page frames that
belong to kernel-space(reserved/slab-subsystem) or non-LRU(unknown page state).
In other word, the result of handling either of these victimized page frames is
(IGNORED | FAILED), and the return value of memory_failure() is -EBUSY.

This patch is to avoid that memory_failure() returns very soon due to the "true"
value of (!PageLRU(p)), and it also ensures that action_result() can report more
precise information("reserved kernel",  "kernel slab", and "unknown page state")
instead of "non LRU", especially for memory errors which are detected by memory-scrubbing.

Signed-off-by: Chen Yucong <slaoub@gmail.com>
---
 mm/memory-failure.c |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index e3154d9..39daadc 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -862,7 +862,7 @@ static int hwpoison_user_mappings(struct page *p, unsigned long pfn,
 	struct page *hpage = *hpagep;
 	struct page *ppage;
 
-	if (PageReserved(p) || PageSlab(p))
+	if (PageReserved(p) || PageSlab(p) || !PageLRU(p))
 		return SWAP_SUCCESS;
 
 	/*
@@ -1126,9 +1126,6 @@ int memory_failure(unsigned long pfn, int trapno, int flags)
 					action_result(pfn, "free buddy, 2nd try", DELAYED);
 				return 0;
 			}
-			action_result(pfn, "non LRU", IGNORED);
-			put_page(p);
-			return -EBUSY;
 		}
 	}
 
-- 
1.7.10.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] HWPOISON: Fix the handling path of the victimized page frame that belong to non-LUR

[Naoya Horiguchi]

On Tue, Jun 03, 2014 at 03:29:42PM +0800, Chen Yucong wrote:
> Until now, the kernel has the same policy to handle victimized page frames that
> belong to kernel-space(reserved/slab-subsystem) or non-LRU(unknown page state).
> In other word, the result of handling either of these victimized page frames is
> (IGNORED | FAILED), and the return value of memory_failure() is -EBUSY.
> 
> This patch is to avoid that memory_failure() returns very soon due to the "true"
> value of (!PageLRU(p)), and it also ensures that action_result() can report more
> precise information("reserved kernel",  "kernel slab", and "unknown page state")
> instead of "non LRU", especially for memory errors which are detected by memory-scrubbing.

Yes, this fixes the poor messaging.

> Signed-off-by: Chen Yucong <slaoub@gmail.com>
> ---
>  mm/memory-failure.c |    5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index e3154d9..39daadc 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -862,7 +862,7 @@ static int hwpoison_user_mappings(struct page *p, unsigned long pfn,
>  	struct page *hpage = *hpagep;
>  	struct page *ppage;
>  
> -	if (PageReserved(p) || PageSlab(p))
> +	if (PageReserved(p) || PageSlab(p) || !PageLRU(p))
>  		return SWAP_SUCCESS;

I don't think that this check never becomes true in the original code, because
we return immediately before coming to this point for !PageLRU as you said.

But I think this is still useful because error handling code is permitted to
fail error containment and in such case it's supposed to leave error pages.
So keeping this false positive is better than stopping here with VM_BUG_ON()
or running unmapping code for kernel page and triggering some unexpected bug.
It makes sure that this function works as intended even for accidental input.

So in summary I agree with this change although it's not run (with my suggestion
below.)

>  	/*
> @@ -1126,9 +1126,6 @@ int memory_failure(unsigned long pfn, int trapno, int flags)
>  					action_result(pfn, "free buddy, 2nd try", DELAYED);
>  				return 0;
>  			}
> -			action_result(pfn, "non LRU", IGNORED);
> -			put_page(p);
> -			return -EBUSY;

Not only removing these lines, you had better also call goto just after
if (hwpoison_filter(p)) block, and jump directly to just before the code
determining the page_state.

Thanks,
Naoya Horiguchi

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>