The patch titled
Subject: mm,hwpoison: rework soft offline for in-use pages
has been added to the -mm tree. Its filename is
mmhwpoison-rework-soft-offline-for-in-use-pages.patch
This patch should soon appear at
http://ozlabs.org/~akpm/mmots/broken-out/mmhwpoison-rework-soft-offline-for-in-use-pages.patch
and later at
http://ozlabs.org/~akpm/mmotm/broken-out/mmhwpoison-rework-soft-offline-for-in-use-pages.patch
Before you just go and hit "reply", please:
a) Consider who else should be cc'ed
b) Prefer to cc a suitable mailing list as well
c) Ideally: find the original patch on the mailing list and do a
reply-to-all to that, adding suitable additional cc's
*** Remember to use Documentation/process/submit-checklist.rst when testing your code ***
The -mm tree is included into linux-next and is updated
there every 3-4 working days
------------------------------------------------------
From: Oscar Salvador <osalvador@xxxxxxx>
Subject: mm,hwpoison: rework soft offline for in-use pages
This patch changes the way we set and handle in-use poisoned pages. Until
now, poisoned pages were released to the buddy allocator, trusting that
the checks that take place prior to hand the page would act as a safe net
and would skip that page.
This has proved to be wrong, as we got some pfn walkers out there, like
compaction, that all they care is the page to be PageBuddy and be in a
freelist.
Although this might not be the only user, having poisoned pages in the
buddy allocator seems a bad idea as we should only have free pages that
are ready and meant to be used as such.
Before explaining the taken approach, let us break down the kind
of pages we can soft offline.
- Anonymous THP (after the split, they end up being 4K pages)
- Hugetlb
- Order-0 pages (that can be either migrated or invalited)
* Normal pages (order-0 and anon-THP)
- If they are clean and unmapped page cache pages, we invalidate
then by means of invalidate_inode_page().
- If they are mapped/dirty, we do the isolate-and-migrate dance.
Either way, do not call put_page directly from those paths.
Instead, we keep the page and send it to page_set_poison to perform the
right handling.
page_set_poison sets the HWPoison flag and does the last put_page.
This call to put_page is mainly to be able to call __page_cache_release,
since this function is not exported.
Down the chain, we placed a check for HWPoison page in
free_pages_prepare, that just skips any poisoned page, so those pages
do not end up in any pcplist/freelist.
After that, we set the refcount on the page to 1 and we increment
the poisoned pages counter.
We could do as we do for free pages:
1) wait until the page hits buddy's freelists
2) take it off
3) flag it
The problem is that we could race with an allocation, so by the time we
want to take the page off the buddy, the page is already allocated, so we
cannot soft-offline it.
This is not fatal of course, but if it is better if we can close the race
as does not require a lot of code.
* Hugetlb pages
- We isolate-and-migrate them
After the migration has been successful, we call dissolve_free_huge_page,
and we set HWPoison on the page if we succeed.
Hugetlb has a slightly different handling though.
While for non-hugetlb pages we cared about closing the race with an
allocation, doing so for hugetlb pages requires quite some additional
code (we would need to hook in free_huge_page and some other places).
So I decided to not make the code overly complicated and just fail
normally if the page we allocated in the meantime.
Because of the way we handle now in-use pages, we no longer need the
put-as-isolation-migratetype dance, that was guarding for poisoned pages
to end up in pcplists.
Link: http://lkml.kernel.org/r/20200731122112.11263-13-nao.horiguchi@xxxxxxxxx
Signed-off-by: Oscar Salvador <osalvador@xxxxxxx>
Signed-off-by: Naoya Horiguchi <naoya.horiguchi@xxxxxxx>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@xxxxxxxxxxxxx>
Cc: Aneesh Kumar K.V <aneesh.kumar@xxxxxxxxxxxxxxxxxx>
Cc: Dave Hansen <dave.hansen@xxxxxxxxx>
Cc: David Hildenbrand <david@xxxxxxxxxx>
Cc: Dmitry Yakunin <zeil@xxxxxxxxxxxxxx>
Cc: Michal Hocko <mhocko@xxxxxxxx>
Cc: Mike Kravetz <mike.kravetz@xxxxxxxxxx>
Cc: Naoya Horiguchi <n-horiguchi@xxxxxxxxxxxxx>
Cc: Oscar Salvador <osalvador@xxxxxxxx>
Cc: Qian Cai <cai@xxxxxx>
Cc: Tony Luck <tony.luck@xxxxxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
---
include/linux/page-flags.h | 5 ---
mm/memory-failure.c | 45 ++++++++++++-----------------------
mm/migrate.c | 11 ++------
mm/page_alloc.c | 28 ---------------------
4 files changed, 19 insertions(+), 70 deletions(-)
--- a/include/linux/page-flags.h~mmhwpoison-rework-soft-offline-for-in-use-pages
+++ a/include/linux/page-flags.h
@@ -422,14 +422,9 @@ PAGEFLAG_FALSE(Uncached)
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 take_page_off_buddy(struct page *page);
#else
PAGEFLAG_FALSE(HWPoison)
-static inline bool set_hwpoison_free_buddy_page(struct page *page)
-{
- return 0;
-}
#define __PG_HWPOISON 0
#endif
--- a/mm/memory-failure.c~mmhwpoison-rework-soft-offline-for-in-use-pages
+++ a/mm/memory-failure.c
@@ -65,8 +65,12 @@ int sysctl_memory_failure_recovery __rea
atomic_long_t num_poisoned_pages __read_mostly = ATOMIC_LONG_INIT(0);
-static void page_handle_poison(struct page *page)
+static void page_handle_poison(struct page *page, bool release)
{
+ if (release) {
+ put_page(page);
+ drain_all_pages(page_zone(page));
+ }
SetPageHWPoison(page);
page_ref_inc(page);
num_poisoned_pages_inc();
@@ -1757,19 +1761,13 @@ static int soft_offline_huge_page(struct
ret = -EIO;
} else {
/*
- * We set PG_hwpoison only when the migration source hugepage
- * was successfully dissolved, because otherwise hwpoisoned
- * hugepage remains on free hugepage list, then userspace will
- * find it as SIGBUS by allocation failure. That's not expected
- * in soft-offlining.
+ * We set PG_hwpoison only when we were able to take the page
+ * off the buddy.
*/
- ret = dissolve_free_huge_page(page);
- if (!ret) {
- if (set_hwpoison_free_buddy_page(page))
- num_poisoned_pages_inc();
- else
- ret = -EBUSY;
- }
+ if (!dissolve_free_huge_page(page) && take_page_off_buddy(page))
+ page_handle_poison(page, false);
+ else
+ ret = -EBUSY;
}
return ret;
}
@@ -1804,10 +1802,8 @@ static int __soft_offline_page(struct pa
* would need to fix isolation locking first.
*/
if (ret == 1) {
- put_page(page);
pr_info("soft_offline: %#lx: invalidated\n", pfn);
- SetPageHWPoison(page);
- num_poisoned_pages_inc();
+ page_handle_poison(page, true);
return 0;
}
@@ -1838,7 +1834,9 @@ static int __soft_offline_page(struct pa
list_add(&page->lru, &pagelist);
ret = migrate_pages(&pagelist, new_page, NULL, MPOL_MF_MOVE_ALL,
MIGRATE_SYNC, MR_MEMORY_FAILURE);
- if (ret) {
+ if (!ret) {
+ page_handle_poison(page, true);
+ } else {
if (!list_empty(&pagelist))
putback_movable_pages(&pagelist);
@@ -1857,27 +1855,16 @@ static int __soft_offline_page(struct pa
static int soft_offline_in_use_page(struct page *page)
{
int ret;
- int mt;
struct page *hpage = compound_head(page);
if (!PageHuge(page) && PageTransHuge(hpage))
if (try_to_split_thp_page(page, "soft offline") < 0)
return -EBUSY;
- /*
- * Setting MIGRATE_ISOLATE here ensures that the page will be linked
- * to free list immediately (not via pcplist) when released after
- * successful page migration. Otherwise we can't guarantee that the
- * page is really free after put_page() returns, so
- * set_hwpoison_free_buddy_page() highly likely fails.
- */
- mt = get_pageblock_migratetype(page);
- set_pageblock_migratetype(page, MIGRATE_ISOLATE);
if (PageHuge(page))
ret = soft_offline_huge_page(page);
else
ret = __soft_offline_page(page);
- set_pageblock_migratetype(page, mt);
return ret;
}
@@ -1886,7 +1873,7 @@ static int soft_offline_free_page(struct
int rc = -EBUSY;
if (!dissolve_free_huge_page(page) && take_page_off_buddy(page)) {
- page_handle_poison(page);
+ page_handle_poison(page, false);
rc = 0;
}
--- a/mm/migrate.c~mmhwpoison-rework-soft-offline-for-in-use-pages
+++ a/mm/migrate.c
@@ -1222,16 +1222,11 @@ out:
* we want to retry.
*/
if (rc == MIGRATEPAGE_SUCCESS) {
- put_page(page);
- if (reason == MR_MEMORY_FAILURE) {
+ if (reason != MR_MEMORY_FAILURE)
/*
- * Set PG_HWPoison on just freed page
- * intentionally. Although it's rather weird,
- * it's how HWPoison flag works at the moment.
+ * We release the page in page_handle_poison.
*/
- if (set_hwpoison_free_buddy_page(page))
- num_poisoned_pages_inc();
- }
+ put_page(page);
} else {
if (rc != -EAGAIN) {
if (likely(!__PageMovable(page))) {
--- a/mm/page_alloc.c~mmhwpoison-rework-soft-offline-for-in-use-pages
+++ a/mm/page_alloc.c
@@ -8839,32 +8839,4 @@ bool take_page_off_buddy(struct page *pa
spin_unlock_irqrestore(&zone->lock, flags);
return ret;
}
-
-/*
- * Set PG_hwpoison flag if a given page is confirmed to be a free page. This
- * test is performed under the zone lock to prevent a race against page
- * allocation.
- */
-bool set_hwpoison_free_buddy_page(struct page *page)
-{
- struct zone *zone = page_zone(page);
- unsigned long pfn = page_to_pfn(page);
- unsigned long flags;
- unsigned int order;
- bool hwpoisoned = false;
-
- spin_lock_irqsave(&zone->lock, flags);
- for (order = 0; order < MAX_ORDER; order++) {
- struct page *page_head = page - (pfn & ((1 << order) - 1));
-
- if (PageBuddy(page_head) && page_order(page_head) >= order) {
- if (!TestSetPageHWPoison(page))
- hwpoisoned = true;
- break;
- }
- }
- spin_unlock_irqrestore(&zone->lock, flags);
-
- return hwpoisoned;
-}
#endif
_
Patches currently in -mm which might be from osalvador@xxxxxxx are
mmmadvise-refactor-madvise_inject_error.patch
mmhwpoison-un-export-get_hwpoison_page-and-make-it-static.patch
mmhwpoison-kill-put_hwpoison_page.patch
mmhwpoison-unify-thp-handling-for-hard-and-soft-offline.patch
mmhwpoison-rework-soft-offline-for-free-pages.patch
mmhwpoison-rework-soft-offline-for-in-use-pages.patch
mmhwpoison-refactor-soft_offline_huge_page-and-__soft_offline_page.patch
mmhwpoison-return-0-if-the-page-is-already-poisoned-in-soft-offline.patch
