| Message ID | 1541746035-13408-12-git-send-email-n-horiguchi@ah.jp.nec.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hwpoison improvement part 1 | expand |
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.
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
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.
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
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(-)