Re: [RFC][PATCH v1 11/11] mm: hwpoison: introduce clear_hwpoison_free_buddy_page()
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
>>
>> 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(>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(>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.
Re: [RFC][PATCH v1 11/11] mm: hwpoison: introduce clear_hwpoison_free_buddy_page()
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
>>
>> 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(>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(>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.
Re: [RFC][PATCH v1 11/11] mm: hwpoison: introduce clear_hwpoison_free_buddy_page()
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
>
> 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(>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(>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
Re: [RFC][PATCH v1 11/11] mm: hwpoison: introduce clear_hwpoison_free_buddy_page()
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
>
> 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(>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(>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
Re: [RFC][PATCH v1 11/11] mm: hwpoison: introduce clear_hwpoison_free_buddy_page()
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
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(>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(>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.
Re: [RFC][PATCH v1 11/11] mm: hwpoison: introduce clear_hwpoison_free_buddy_page()
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
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(>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(>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.
[RFC][PATCH v1 11/11] mm: hwpoison: introduce clear_hwpoison_free_buddy_page()
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
---
include/linux/page-flags.h | 8 +++-
mm/memory-failure.c| 5 +++--
mm/page_alloc.c| 21 +
3 files changed, 31 insertions(+), 3 deletions(-)
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, _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(>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(>lock);
+ return unpoisoned;
+}
#endif
--
2.7.0
[RFC][PATCH v1 11/11] mm: hwpoison: introduce clear_hwpoison_free_buddy_page()
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
---
include/linux/page-flags.h | 8 +++-
mm/memory-failure.c| 5 +++--
mm/page_alloc.c| 21 +
3 files changed, 31 insertions(+), 3 deletions(-)
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, _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(>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(>lock);
+ return unpoisoned;
+}
#endif
--
2.7.0
