[PATCH v2 2/5] mm/hwpoison: fix PageHWPoison test/set race

[PATCH v2 2/5] mm/hwpoison: fix PageHWPoison test/set race

[Wanpeng Li]

There is a race between madvise_hwpoison path and memory_failure:

 CPU0					CPU1

madvise_hwpoison
get_user_pages_fast
PageHWPoison check (false)
					memory_failure
					TestSetPageHWPoison
soft_offline_page
PageHWPoison check (true)
return -EBUSY (without put_page)

Suggested-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
 mm/memory-failure.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 56b8a71..e0eb7ab 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -1704,6 +1704,8 @@ int soft_offline_page(struct page *page, int flags)
 
 	if (PageHWPoison(page)) {
 		pr_info("soft offline: %#lx page already poisoned\n", pfn);
+		if (flags & MF_COUNT_INCREASED)
+			put_page(page);
 		return -EBUSY;
 	}
 	if (!PageHuge(page) && PageTransHuge(hpage)) {
-- 
1.7.1

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

[PATCH v2 3/5] mm/hwpoison: introduce put_hwpoison_page to put refcount for memory error handling

[Wanpeng Li]

Introduce put_hwpoison_page to put refcount for memory 
error handling. 

Suggested-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
 include/linux/mm.h  |    1 +
 mm/memory-failure.c |   21 +++++++++++++++++++++
 2 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 554b0f0..c0a0b9f 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2103,6 +2103,7 @@ extern int memory_failure(unsigned long pfn, int trapno, int flags);
 extern void memory_failure_queue(unsigned long pfn, int trapno, int flags);
 extern int unpoison_memory(unsigned long pfn);
 extern int get_hwpoison_page(struct page *page);
+extern void put_hwpoison_page(struct page *page);
 extern int sysctl_memory_failure_early_kill;
 extern int sysctl_memory_failure_recovery;
 extern void shake_page(struct page *p, int access);
diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index e0eb7ab..fa9aa21 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -922,6 +922,27 @@ int get_hwpoison_page(struct page *page)
 }
 EXPORT_SYMBOL_GPL(get_hwpoison_page);
 
+/**
+ * put_hwpoison_page() - Put refcount for memory error handling:
+ * @page:	raw error page (hit by memory error)
+ */
+void put_hwpoison_page(struct page *page)
+{
+	struct page *head = compound_head(page);
+
+	if (PageHuge(head)) {
+		put_page(head);
+		return;
+	}
+
+	if (PageTransHuge(head))
+		if (page != head)
+			put_page(head);
+
+	put_page(page);
+}
+EXPORT_SYMBOL_GPL(put_hwpoison_page);
+
 /*
  * Do all that is necessary to remove user space mappings. Unmap
  * the pages and send SIGBUS to the processes if the data was dirty.
-- 
1.7.1

--
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 v2 3/5] mm/hwpoison: introduce put_hwpoison_page to put refcount for memory error handling

[Andrew Morton]

On Mon, 10 Aug 2015 19:28:21 +0800 Wanpeng Li <wanpeng.li@hotmail.com> wrote:

> Introduce put_hwpoison_page to put refcount for memory 
> error handling. 
> 
> ...
>
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -922,6 +922,27 @@ int get_hwpoison_page(struct page *page)
>  }
>  EXPORT_SYMBOL_GPL(get_hwpoison_page);
>  
> +/**
> + * put_hwpoison_page() - Put refcount for memory error handling:
> + * @page:	raw error page (hit by memory error)
> + */
> +void put_hwpoison_page(struct page *page)
> +{
> +	struct page *head = compound_head(page);
> +
> +	if (PageHuge(head)) {
> +		put_page(head);
> +		return;
> +	}
> +
> +	if (PageTransHuge(head))
> +		if (page != head)
> +			put_page(head);
> +
> +	put_page(page);
> +}
> +EXPORT_SYMBOL_GPL(put_hwpoison_page);

I don't believe the export is needed?

--
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 v2 3/5] mm/hwpoison: introduce put_hwpoison_page to put refcount for memory error handling

[Wanpeng Li]

On 8/12/15 7:24 AM, Andrew Morton wrote:
> On Mon, 10 Aug 2015 19:28:21 +0800 Wanpeng Li <wanpeng.li@hotmail.com> wrote:
>
>> Introduce put_hwpoison_page to put refcount for memory
>> error handling.
>>
>> ...
>>
>> --- a/mm/memory-failure.c
>> +++ b/mm/memory-failure.c
>> @@ -922,6 +922,27 @@ int get_hwpoison_page(struct page *page)
>>   }
>>   EXPORT_SYMBOL_GPL(get_hwpoison_page);
>>   
>> +/**
>> + * put_hwpoison_page() - Put refcount for memory error handling:
>> + * @page:	raw error page (hit by memory error)
>> + */
>> +void put_hwpoison_page(struct page *page)
>> +{
>> +	struct page *head = compound_head(page);
>> +
>> +	if (PageHuge(head)) {
>> +		put_page(head);
>> +		return;
>> +	}
>> +
>> +	if (PageTransHuge(head))
>> +		if (page != head)
>> +			put_page(head);
>> +
>> +	put_page(page);
>> +}
>> +EXPORT_SYMBOL_GPL(put_hwpoison_page);
> I don't believe the export is needed?

Indeed, thanks for your fix. :-)

Regards,
Wanpeng Li

--
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 v2 3/5] mm/hwpoison: introduce put_hwpoison_page to put refcount for memory error handling

[Wanpeng Li]

On 8/12/15 7:24 AM, Andrew Morton wrote:
> On Mon, 10 Aug 2015 19:28:21 +0800 Wanpeng Li <wanpeng.li@hotmail.com> wrote:
>
>> Introduce put_hwpoison_page to put refcount for memory
>> error handling.
>>
>> ...
>>
>> --- a/mm/memory-failure.c
>> +++ b/mm/memory-failure.c
>> @@ -922,6 +922,27 @@ int get_hwpoison_page(struct page *page)
>>   }
>>   EXPORT_SYMBOL_GPL(get_hwpoison_page);
>>   
>> +/**
>> + * put_hwpoison_page() - Put refcount for memory error handling:
>> + * @page:	raw error page (hit by memory error)
>> + */
>> +void put_hwpoison_page(struct page *page)
>> +{
>> +	struct page *head = compound_head(page);
>> +
>> +	if (PageHuge(head)) {
>> +		put_page(head);
>> +		return;
>> +	}
>> +
>> +	if (PageTransHuge(head))
>> +		if (page != head)
>> +			put_page(head);
>> +
>> +	put_page(page);
>> +}
>> +EXPORT_SYMBOL_GPL(put_hwpoison_page);
> I don't believe the export is needed?

ERROR: "put_hwpoison_page" [mm/hwpoison-inject.ko] undefined!
So I'm afraid it should be needed.

Regards,
Wanpeng Li

--
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 v2 3/5] mm/hwpoison: introduce put_hwpoison_page to put refcount for memory error handling

[Naoya Horiguchi]

On Mon, Aug 10, 2015 at 07:28:21PM +0800, Wanpeng Li wrote:
> Introduce put_hwpoison_page to put refcount for memory 
> error handling. 
> 
> Suggested-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>

Thanks!

Acked-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>

> ---
>  include/linux/mm.h  |    1 +
>  mm/memory-failure.c |   21 +++++++++++++++++++++
>  2 files changed, 22 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 554b0f0..c0a0b9f 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2103,6 +2103,7 @@ extern int memory_failure(unsigned long pfn, int trapno, int flags);
>  extern void memory_failure_queue(unsigned long pfn, int trapno, int flags);
>  extern int unpoison_memory(unsigned long pfn);
>  extern int get_hwpoison_page(struct page *page);
> +extern void put_hwpoison_page(struct page *page);
>  extern int sysctl_memory_failure_early_kill;
>  extern int sysctl_memory_failure_recovery;
>  extern void shake_page(struct page *p, int access);
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index e0eb7ab..fa9aa21 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -922,6 +922,27 @@ int get_hwpoison_page(struct page *page)
>  }
>  EXPORT_SYMBOL_GPL(get_hwpoison_page);
>  
> +/**
> + * put_hwpoison_page() - Put refcount for memory error handling:
> + * @page:	raw error page (hit by memory error)
> + */
> +void put_hwpoison_page(struct page *page)
> +{
> +	struct page *head = compound_head(page);
> +
> +	if (PageHuge(head)) {
> +		put_page(head);
> +		return;
> +	}
> +
> +	if (PageTransHuge(head))
> +		if (page != head)
> +			put_page(head);
> +
> +	put_page(page);
> +}
> +EXPORT_SYMBOL_GPL(put_hwpoison_page);
> +
>  /*
>   * Do all that is necessary to remove user space mappings. Unmap
>   * the pages and send SIGBUS to the processes if the data was dirty.
> -- 
> 1.7.1
> 
--
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>

[PATCH v2 4/5] mm/hwpoison: fix refcount of THP head page in no-injection case

[Wanpeng Li]

Hwpoison injection takes a refcount of target page and another refcount
of head page of THP if the target page is the tail page of a THP. However,
current code doesn't release the refcount of head page if the THP is not 
supported to be injected wrt hwpoison filter. 

Fix it by reducing the refcount of head page if the target page is the tail 
page of a THP and it is not supported to be injected.

Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
 mm/hwpoison-inject.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/mm/hwpoison-inject.c b/mm/hwpoison-inject.c
index 5015679..9d26fd9 100644
--- a/mm/hwpoison-inject.c
+++ b/mm/hwpoison-inject.c
@@ -55,7 +55,7 @@ inject:
 	pr_info("Injecting memory failure at pfn %#lx\n", pfn);
 	return memory_failure(pfn, 18, MF_COUNT_INCREASED);
 put_out:
-	put_page(p);
+	put_hwpoison_page(p);
 	return 0;
 }
 
-- 
1.7.1

--
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 v2 4/5] mm/hwpoison: fix refcount of THP head page in no-injection case

[Naoya Horiguchi]

On Mon, Aug 10, 2015 at 07:28:22PM +0800, Wanpeng Li wrote:
> Hwpoison injection takes a refcount of target page and another refcount
> of head page of THP if the target page is the tail page of a THP. However,
> current code doesn't release the refcount of head page if the THP is not 
> supported to be injected wrt hwpoison filter. 
> 
> Fix it by reducing the refcount of head page if the target page is the tail 
> page of a THP and it is not supported to be injected.
> 
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>

Acked-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>

> ---
>  mm/hwpoison-inject.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/mm/hwpoison-inject.c b/mm/hwpoison-inject.c
> index 5015679..9d26fd9 100644
> --- a/mm/hwpoison-inject.c
> +++ b/mm/hwpoison-inject.c
> @@ -55,7 +55,7 @@ inject:
>  	pr_info("Injecting memory failure at pfn %#lx\n", pfn);
>  	return memory_failure(pfn, 18, MF_COUNT_INCREASED);
>  put_out:
> -	put_page(p);
> +	put_hwpoison_page(p);
>  	return 0;
>  }
>  
> -- 
> 1.7.1
> 
--
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>

[PATCH v2 5/5] mm/hwpoison: replace most of put_page in memory error handling by put_hwpoison_page

[Wanpeng Li]

Replace most of put_page in memory error handling by put_hwpoison_page,
except the ones at the front of soft_offline_page since the page maybe 
THP page and the get refcount in madvise_hwpoison is against the single 
4KB page instead of the logic in get_hwpoison_page. 

Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
 mm/memory-failure.c |   28 +++++++++++++---------------
 1 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index fa9aa21..6179fc1 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -1159,9 +1159,7 @@ int memory_failure(unsigned long pfn, int trapno, int flags)
 			pr_err("MCE: %#lx: thp split failed\n", pfn);
 			if (TestClearPageHWPoison(p))
 				atomic_long_sub(nr_pages, &num_poisoned_pages);
-			put_page(p);
-			if (p != hpage)
-				put_page(hpage);
+			put_hwpoison_page(p);
 			return -EBUSY;
 		}
 		VM_BUG_ON_PAGE(!page_count(p), p);
@@ -1222,14 +1220,14 @@ int memory_failure(unsigned long pfn, int trapno, int flags)
 		printk(KERN_ERR "MCE %#lx: just unpoisoned\n", pfn);
 		atomic_long_sub(nr_pages, &num_poisoned_pages);
 		unlock_page(hpage);
-		put_page(hpage);
+		put_hwpoison_page(hpage);
 		return 0;
 	}
 	if (hwpoison_filter(p)) {
 		if (TestClearPageHWPoison(p))
 			atomic_long_sub(nr_pages, &num_poisoned_pages);
 		unlock_page(hpage);
-		put_page(hpage);
+		put_hwpoison_page(hpage);
 		return 0;
 	}
 
@@ -1243,7 +1241,7 @@ int memory_failure(unsigned long pfn, int trapno, int flags)
 	if (PageHuge(p) && PageTail(p) && TestSetPageHWPoison(hpage)) {
 		action_result(pfn, MF_MSG_POISONED_HUGE, MF_IGNORED);
 		unlock_page(hpage);
-		put_page(hpage);
+		put_hwpoison_page(hpage);
 		return 0;
 	}
 	/*
@@ -1477,9 +1475,9 @@ int unpoison_memory(unsigned long pfn)
 	}
 	unlock_page(page);
 
-	put_page(page);
+	put_hwpoison_page(page);
 	if (freeit && !(pfn == my_zero_pfn(0) && page_count(p) == 1))
-		put_page(page);
+		put_hwpoison_page(page);
 
 	return 0;
 }
@@ -1539,7 +1537,7 @@ static int get_any_page(struct page *page, unsigned long pfn, int flags)
 		/*
 		 * Try to free it.
 		 */
-		put_page(page);
+		put_hwpoison_page(page);
 		shake_page(page, 1);
 
 		/*
@@ -1548,7 +1546,7 @@ static int get_any_page(struct page *page, unsigned long pfn, int flags)
 		ret = __get_any_page(page, pfn, 0);
 		if (!PageLRU(page)) {
 			/* Drop page reference which is from __get_any_page() */
-			put_page(page);
+			put_hwpoison_page(page);
 			pr_info("soft_offline: %#lx: unknown non LRU page type %lx\n",
 				pfn, page->flags);
 			return -EIO;
@@ -1571,7 +1569,7 @@ static int soft_offline_huge_page(struct page *page, int flags)
 	lock_page(hpage);
 	if (PageHWPoison(hpage)) {
 		unlock_page(hpage);
-		put_page(hpage);
+		put_hwpoison_page(hpage);
 		pr_info("soft offline: %#lx hugepage already poisoned\n", pfn);
 		return -EBUSY;
 	}
@@ -1582,7 +1580,7 @@ static int soft_offline_huge_page(struct page *page, int flags)
 	 * get_any_page() and isolate_huge_page() takes a refcount each,
 	 * so need to drop one here. 
 	 */
-	put_page(hpage);
+	put_hwpoison_page(hpage);
 	if (!ret) {
 		pr_info("soft offline: %#lx hugepage failed to isolate\n", pfn);
 		return -EBUSY;
@@ -1631,7 +1629,7 @@ static int __soft_offline_page(struct page *page, int flags)
 	wait_on_page_writeback(page);
 	if (PageHWPoison(page)) {
 		unlock_page(page);
-		put_page(page);
+		put_hwpoison_page(page);
 		pr_info("soft offline: %#lx page already poisoned\n", pfn);
 		return -EBUSY;
 	}
@@ -1646,7 +1644,7 @@ static int __soft_offline_page(struct page *page, int flags)
 	 * would need to fix isolation locking first.
 	 */
 	if (ret == 1) {
-		put_page(page);
+		put_hwpoison_page(page);
 		pr_info("soft_offline: %#lx: invalidated\n", pfn);
 		SetPageHWPoison(page);
 		atomic_long_inc(&num_poisoned_pages);
@@ -1663,7 +1661,7 @@ static int __soft_offline_page(struct page *page, int flags)
 	 * Drop page reference which is came from get_any_page()
 	 * successful isolate_lru_page() already took another one.
 	 */
-	put_page(page);
+	put_hwpoison_page(page);
 	if (!ret) {
 		LIST_HEAD(pagelist);
 		inc_zone_page_state(page, NR_ISOLATED_ANON +
-- 
1.7.1

--
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 v2 5/5] mm/hwpoison: replace most of put_page in memory error handling by put_hwpoison_page

[Naoya Horiguchi]

On Mon, Aug 10, 2015 at 07:28:23PM +0800, Wanpeng Li wrote:
> Replace most of put_page in memory error handling by put_hwpoison_page,
> except the ones at the front of soft_offline_page since the page maybe
> THP page and the get refcount in madvise_hwpoison is against the single
> 4KB page instead of the logic in get_hwpoison_page.
>
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>

# Sorry for my late response.

If I read correctly, get_user_pages_fast() (called by madvise_hwpoison)
for a THP tail page takes a refcount from each of head and tail page.
gup_huge_pmd() does this in the fast path, and get_page_foll() does this
in the slow path (maybe via the following code path)

  get_user_pages_unlocked
    __get_user_pages_unlocked
      __get_user_pages_locked
        __get_user_pages
          follow_page_mask
            follow_trans_huge_pmd (with FOLL_GET set)
              get_page_foll

So this should be equivalent to what get_hwpoison_page() does for thp pages
with regard to refcounting.

And I'm expecting that a refcount taken by get_hwpoison_page() is released
by put_hwpoison_page() even if the page's status is changed during error
handling (the typical (or only?) case is successful thp split.)

So I think you can apply put_hwpoison_page() for 3 more callsites in
mm/memory-failure.c.
 - MF_MSG_POISONED_HUGE case
 - "soft offline: %#lx page already poisoned" case (you mentioned above)
 - "soft offline: %#lx: failed to split THP" case (you mentioned above)

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>

Re: [PATCH v2 5/5] mm/hwpoison: replace most of put_page in memory error handling by put_hwpoison_page

[Wanpeng Li]

On 8/12/15 4:55 PM, Naoya Horiguchi wrote:
> On Mon, Aug 10, 2015 at 07:28:23PM +0800, Wanpeng Li wrote:
>> Replace most of put_page in memory error handling by put_hwpoison_page,
>> except the ones at the front of soft_offline_page since the page maybe
>> THP page and the get refcount in madvise_hwpoison is against the single
>> 4KB page instead of the logic in get_hwpoison_page.
>>
>> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> # Sorry for my late response.
>
> If I read correctly, get_user_pages_fast() (called by madvise_hwpoison)
> for a THP tail page takes a refcount from each of head and tail page.
> gup_huge_pmd() does this in the fast path, and get_page_foll() does this
> in the slow path (maybe via the following code path)
>
>   get_user_pages_unlocked
>     __get_user_pages_unlocked
>       __get_user_pages_locked
>         __get_user_pages
>           follow_page_mask
>             follow_trans_huge_pmd (with FOLL_GET set)
>               get_page_foll
>
> So this should be equivalent to what get_hwpoison_page() does for thp pages
> with regard to refcounting.
>
> And I'm expecting that a refcount taken by get_hwpoison_page() is released
> by put_hwpoison_page() even if the page's status is changed during error
> handling (the typical (or only?) case is successful thp split.)

Indeed. :-)

>
> So I think you can apply put_hwpoison_page() for 3 more callsites in
> mm/memory-failure.c.
>  - MF_MSG_POISONED_HUGE case

I have already done this in my patch.

>  - "soft offline: %#lx page already poisoned" case (you mentioned above)
>  - "soft offline: %#lx: failed to split THP" case (you mentioned above)

You are right, I will send a patch rebased on this one since they are
merged.

Regards,
Wanpeng Li

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

Re: [PATCH v2 5/5] mm/hwpoison: replace most of put_page in memory error handling by put_hwpoison_page

[Wanpeng Li]

[-- Attachment #1: Type: text/plain, Size: 1862 bytes --]

On 8/12/15 5:13 PM, Wanpeng Li wrote:
> On 8/12/15 4:55 PM, Naoya Horiguchi wrote:
>> On Mon, Aug 10, 2015 at 07:28:23PM +0800, Wanpeng Li wrote:
>>> Replace most of put_page in memory error handling by put_hwpoison_page,
>>> except the ones at the front of soft_offline_page since the page maybe
>>> THP page and the get refcount in madvise_hwpoison is against the single
>>> 4KB page instead of the logic in get_hwpoison_page.
>>>
>>> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
>> # Sorry for my late response.
>>
>> If I read correctly, get_user_pages_fast() (called by madvise_hwpoison)
>> for a THP tail page takes a refcount from each of head and tail page.
>> gup_huge_pmd() does this in the fast path, and get_page_foll() does this
>> in the slow path (maybe via the following code path)
>>
>>   get_user_pages_unlocked
>>     __get_user_pages_unlocked
>>       __get_user_pages_locked
>>         __get_user_pages
>>           follow_page_mask
>>             follow_trans_huge_pmd (with FOLL_GET set)
>>               get_page_foll
>>
>> So this should be equivalent to what get_hwpoison_page() does for thp pages
>> with regard to refcounting.
>>
>> And I'm expecting that a refcount taken by get_hwpoison_page() is released
>> by put_hwpoison_page() even if the page's status is changed during error
>> handling (the typical (or only?) case is successful thp split.)
> Indeed. :-)
>
>> So I think you can apply put_hwpoison_page() for 3 more callsites in
>> mm/memory-failure.c.
>>  - MF_MSG_POISONED_HUGE case
> I have already done this in my patch.
>
>>  - "soft offline: %#lx page already poisoned" case (you mentioned above)
>>  - "soft offline: %#lx: failed to split THP" case (you mentioned above)
> You are right, I will send a patch rebased on this one since they are
> merged.

The fix patch is in attachment. :)

Regards,
Wanpeng Li


[-- Attachment #2: 0001-mm-hwpoison-mm-hwpoison-replace-most-of-put_page-in-.patch --]
[-- Type: text/plain, Size: 1355 bytes --]

From 3d83c40cc3f6d40883aabf9f6033cd9d78d117b5 Mon Sep 17 00:00:00 2001
From: Wanpeng Li <wanpeng.li@hotmail.com>
Date: Wed, 12 Aug 2015 17:31:42 +0800
Subject: [PATCH] mm/hwpoison: replace most of put_page in memory error handling by put_hwpoison_page fix

Note: The patch description of original patch "mm/hwpoison: replace most of 
put_page in memory error handling by put_hwpoison_page" should be replaced by
"Replace most of put_page() in memory error handling by put_hwpoison_page()".

Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
 mm/memory-failure.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 0acafee..d378188 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -1727,7 +1727,7 @@ int soft_offline_page(struct page *page, int flags)
 	if (PageHWPoison(page)) {
 		pr_info("soft offline: %#lx page already poisoned\n", pfn);
 		if (flags & MF_COUNT_INCREASED)
-			put_page(page);
+			put_hwpoison_page(page);
 		return -EBUSY;
 	}
 	if (!PageHuge(page) && PageTransHuge(hpage)) {
@@ -1735,7 +1735,7 @@ int soft_offline_page(struct page *page, int flags)
 			pr_info("soft offline: %#lx: failed to split THP\n",
 				pfn);
 			if (flags & MF_COUNT_INCREASED)
-				put_page(page);
+				put_hwpoison_page(page);
 			return -EBUSY;
 		}
 	}
-- 
1.7.1