[RFC,v1,11/11] mm: hwpoison: introduce clear_hwpoison_free_buddy_page()
diff mbox series

Message ID 1541746035-13408-12-git-send-email-n-horiguchi@ah.jp.nec.com
State New
Headers show
Series
  • hwpoison improvement part 1
Related show

Commit Message

Naoya Horiguchi Nov. 9, 2018, 6:47 a.m. UTC
The new function is a reverse operation of set_hwpoison_free_buddy_page()
to adjust unpoison_memory() to the new semantics.

Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
---
 include/linux/page-flags.h |  8 +++++++-
 mm/memory-failure.c        |  5 +++--
 mm/page_alloc.c            | 21 +++++++++++++++++++++
 3 files changed, 31 insertions(+), 3 deletions(-)

Comments

Anshuman Khandual Nov. 9, 2018, 11:33 a.m. UTC | #1
On 11/09/2018 12:17 PM, Naoya Horiguchi wrote:
> The new function is a reverse operation of set_hwpoison_free_buddy_page()
> to adjust unpoison_memory() to the new semantics.
> 
> Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>

snip

> +
> +/*
> + * Reverse operation of set_hwpoison_free_buddy_page(), which is expected
> + * to work only on error pages isolated from buddy allocator.
> + */
> +bool clear_hwpoison_free_buddy_page(struct page *page)
> +{
> +	struct zone *zone = page_zone(page);
> +	bool unpoisoned = false;
> +
> +	spin_lock(&zone->lock);
> +	if (TestClearPageHWPoison(page)) {
> +		unsigned long pfn = page_to_pfn(page);
> +		int migratetype = get_pfnblock_migratetype(page, pfn);
> +
> +		__free_one_page(page, pfn, zone, 0, migratetype);
> +		unpoisoned = true;
> +	}
> +	spin_unlock(&zone->lock);
> +	return unpoisoned;
> +}
>  #endif
> 

Though there are multiple page state checks in unpoison_memory() leading
upto clearing HWPoison flag, the page must not be in buddy already if
__free_one_page() would be called on it.
Naoya Horiguchi Nov. 13, 2018, 12:19 a.m. UTC | #2
On Fri, Nov 09, 2018 at 05:03:06PM +0530, Anshuman Khandual wrote:
> 
> 
> On 11/09/2018 12:17 PM, Naoya Horiguchi wrote:
> > The new function is a reverse operation of set_hwpoison_free_buddy_page()
> > to adjust unpoison_memory() to the new semantics.
> > 
> > Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> 
> snip
> 
> > +
> > +/*
> > + * Reverse operation of set_hwpoison_free_buddy_page(), which is expected
> > + * to work only on error pages isolated from buddy allocator.
> > + */
> > +bool clear_hwpoison_free_buddy_page(struct page *page)
> > +{
> > +	struct zone *zone = page_zone(page);
> > +	bool unpoisoned = false;
> > +
> > +	spin_lock(&zone->lock);
> > +	if (TestClearPageHWPoison(page)) {
> > +		unsigned long pfn = page_to_pfn(page);
> > +		int migratetype = get_pfnblock_migratetype(page, pfn);
> > +
> > +		__free_one_page(page, pfn, zone, 0, migratetype);
> > +		unpoisoned = true;
> > +	}
> > +	spin_unlock(&zone->lock);
> > +	return unpoisoned;
> > +}
> >  #endif
> > 
> 
> Though there are multiple page state checks in unpoison_memory() leading
> upto clearing HWPoison flag, the page must not be in buddy already if
> __free_one_page() would be called on it.

Yes, you're right.
clear_hwpoison_free_buddy_page() is intended to cancel the isolation by
set_hwpoison_free_buddy_page() which removes the target page from buddy allocator,
so the page clear_hwpoison_free_buddy_page() tries to handle is not a buddy page
actually (not linked to any freelist).

Thanks,
Naoya Horiguchi
Anshuman Khandual Nov. 14, 2018, 8:23 a.m. UTC | #3
On 11/13/2018 05:49 AM, Naoya Horiguchi wrote:
> On Fri, Nov 09, 2018 at 05:03:06PM +0530, Anshuman Khandual wrote:
>>
>>
>> On 11/09/2018 12:17 PM, Naoya Horiguchi wrote:
>>> The new function is a reverse operation of set_hwpoison_free_buddy_page()
>>> to adjust unpoison_memory() to the new semantics.
>>>
>>> Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
>>
>> snip
>>
>>> +
>>> +/*
>>> + * Reverse operation of set_hwpoison_free_buddy_page(), which is expected
>>> + * to work only on error pages isolated from buddy allocator.
>>> + */
>>> +bool clear_hwpoison_free_buddy_page(struct page *page)
>>> +{
>>> +	struct zone *zone = page_zone(page);
>>> +	bool unpoisoned = false;
>>> +
>>> +	spin_lock(&zone->lock);
>>> +	if (TestClearPageHWPoison(page)) {
>>> +		unsigned long pfn = page_to_pfn(page);
>>> +		int migratetype = get_pfnblock_migratetype(page, pfn);
>>> +
>>> +		__free_one_page(page, pfn, zone, 0, migratetype);
>>> +		unpoisoned = true;
>>> +	}
>>> +	spin_unlock(&zone->lock);
>>> +	return unpoisoned;
>>> +}
>>>  #endif
>>>
>>
>> Though there are multiple page state checks in unpoison_memory() leading
>> upto clearing HWPoison flag, the page must not be in buddy already if
>> __free_one_page() would be called on it.
> 
> Yes, you're right.
> clear_hwpoison_free_buddy_page() is intended to cancel the isolation by
> set_hwpoison_free_buddy_page() which removes the target page from buddy allocator,
> so the page clear_hwpoison_free_buddy_page() tries to handle is not a buddy page
> actually (not linked to any freelist).

Got it. Thanks for the explanation.

Patch
diff mbox series

diff --git v4.19-mmotm-2018-10-30-16-08/include/linux/page-flags.h v4.19-mmotm-2018-10-30-16-08_patched/include/linux/page-flags.h
index 50ce1bd..ab0bde0 100644
--- v4.19-mmotm-2018-10-30-16-08/include/linux/page-flags.h
+++ v4.19-mmotm-2018-10-30-16-08_patched/include/linux/page-flags.h
@@ -382,11 +382,17 @@  PAGEFLAG(HWPoison, hwpoison, PF_ANY)
 TESTSCFLAG(HWPoison, hwpoison, PF_ANY)
 #define __PG_HWPOISON (1UL << PG_hwpoison)
 extern bool set_hwpoison_free_buddy_page(struct page *page);
+extern bool clear_hwpoison_free_buddy_page(struct page *page);
 #else
 PAGEFLAG_FALSE(HWPoison)
 static inline bool set_hwpoison_free_buddy_page(struct page *page)
 {
-	return 0;
+	return false;
+}
+
+static inline bool clear_hwpoison_free_buddy_page(struct page *page)
+{
+	return false;
 }
 #define __PG_HWPOISON 0
 #endif
diff --git v4.19-mmotm-2018-10-30-16-08/mm/memory-failure.c v4.19-mmotm-2018-10-30-16-08_patched/mm/memory-failure.c
index af541141..a0e1cd4 100644
--- v4.19-mmotm-2018-10-30-16-08/mm/memory-failure.c
+++ v4.19-mmotm-2018-10-30-16-08_patched/mm/memory-failure.c
@@ -1590,8 +1590,9 @@  int unpoison_memory(unsigned long pfn)
 	}
 
 	if (!get_hwpoison_page(p)) {
-		if (TestClearPageHWPoison(p))
-			num_poisoned_pages_dec();
+		if (!clear_hwpoison_free_buddy_page(p))
+			return 0;
+		num_poisoned_pages_dec();
 		unpoison_pr_info("Unpoison: Software-unpoisoned free page %#lx\n",
 				 pfn, &unpoison_rs);
 		return 0;
diff --git v4.19-mmotm-2018-10-30-16-08/mm/page_alloc.c v4.19-mmotm-2018-10-30-16-08_patched/mm/page_alloc.c
index 27826b3..9a90f93 100644
--- v4.19-mmotm-2018-10-30-16-08/mm/page_alloc.c
+++ v4.19-mmotm-2018-10-30-16-08_patched/mm/page_alloc.c
@@ -8270,4 +8270,25 @@  bool set_hwpoison_free_buddy_page(struct page *page)
 
 	return hwpoisoned;
 }
+
+/*
+ * Reverse operation of set_hwpoison_free_buddy_page(), which is expected
+ * to work only on error pages isolated from buddy allocator.
+ */
+bool clear_hwpoison_free_buddy_page(struct page *page)
+{
+	struct zone *zone = page_zone(page);
+	bool unpoisoned = false;
+
+	spin_lock(&zone->lock);
+	if (TestClearPageHWPoison(page)) {
+		unsigned long pfn = page_to_pfn(page);
+		int migratetype = get_pfnblock_migratetype(page, pfn);
+
+		__free_one_page(page, pfn, zone, 0, migratetype);
+		unpoisoned = true;
+	}
+	spin_unlock(&zone->lock);
+	return unpoisoned;
+}
 #endif