Changes in v4:
- collected R-B from Oscar Salvador
- collected Acked-by from Miaohe Lin
- fixed comment on MF_DELAYED, and comments for better coding. - Miaohe Lin
Changes in v3:
- rebased to mainline as of 5/20/2024
- added an acked-by from Miaohe Lin
- picked up a R-B from Oscar Salvador
- fixed/clarified comments about MF_IGNORED/MF_FAILED definition and
usage. - Oscar Salvador
- invoke hwpoison_filter slightly earlier to avoid unnecessary THP split,
and with refcount held. - Miaohe Lin
- added comments to try_to_split_thp_page() on when not to release page
refcount. - Oscar Salvador
- added action_result() in a couple cases, but take care not to overwrite
the intended returns. - Oscar Salvador
Changes in v2:
- rebased to mm-stable as of 5/8/2024
- added RB by Oscar Salvador
- comments from Oscar on patch 1-of-3: clarify changelog
- comments from Miahe Lin on patch 3-of-3: remove unnecessary user page
checking and remove incorrect put_page() in kill_procs_now().
Invoke kill_procs_now() regardless MF_ACTIN_REQUIRED is set or not,
moved hwpoison_filter() higher up.
- added two patches 3-of-5 and 4-of-5
This series aim at the following enhancement -
- Let one hwpoison injector, that is, madvise(MADV_HWPOISON) to behave
more like as if a real UE occurred. Because the other two injectors
such as hwpoison-inject and the 'einj' on x86 can't, and it seems to
me we need a better simulation to real UE scenario.
- For years, if the kernel is unable to unmap a hwpoisoned page, it send
a SIGKILL instead of SIGBUS to prevent user process from potentially
accessing the page again. But in doing so, the user process also lose
important information: vaddr, for recovery. Fortunately, the kernel
already has code to kill process re-accessing a hwpoisoned page, so
remove the '!unmap_success' check.
- Right now, if a thp page under GUP longterm pin is hwpoisoned, and
kernel cannot split the thp page, memory-failure simply ignores
the UE and returns. That's not ideal, it could deliver a SIGBUS with
useful information for userspace recovery.
Jane Chu (5):
mm/memory-failure: try to send SIGBUS even if unmap failed
mm/madvise: Add MF_ACTION_REQUIRED to madvise(MADV_HWPOISON)
mm/memory-failure: improve memory failure action_result messages
mm/memory-failure: move hwpoison_filter() higher up
mm/memory-failure: send SIGBUS in the event of thp split fail
include/linux/mm.h | 2 +
include/ras/ras_event.h | 2 +
mm/madvise.c | 2 +-
mm/memory-failure.c | 106 +++++++++++++++++++++++++++++-----------
4 files changed, 82 insertions(+), 30 deletions(-)
--
2.39.3
For years when it comes down to kill a process due to hwpoison,
a SIGBUS is delivered only if unmap has been successful.
Otherwise, a SIGKILL is delivered. And the reason for that is
to prevent the involved process from accessing the hwpoisoned
page again.
Since then a lot has changed, a hwpoisoned page is marked and
upon being re-accessed, the memory-failure handler invokes
kill_accessing_process() to kill the process immediately.
So let's take out the '!unmap_success' factor and try to deliver
SIGBUS if possible.
Signed-off-by: Jane Chu <[email protected]>
Reviewed-by: Oscar Salvador <[email protected]>
Acked-by: Miaohe Lin <[email protected]>
---
mm/memory-failure.c | 15 ++++-----------
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 16ada4fb02b7..739311e121af 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -514,22 +514,15 @@ void add_to_kill_ksm(struct task_struct *tsk, struct page *p,
*
* Only do anything when FORCEKILL is set, otherwise just free the
* list (this is used for clean pages which do not need killing)
- * Also when FAIL is set do a force kill because something went
- * wrong earlier.
*/
-static void kill_procs(struct list_head *to_kill, int forcekill, bool fail,
+static void kill_procs(struct list_head *to_kill, int forcekill,
unsigned long pfn, int flags)
{
struct to_kill *tk, *next;
list_for_each_entry_safe(tk, next, to_kill, nd) {
if (forcekill) {
- /*
- * In case something went wrong with munmapping
- * make sure the process doesn't catch the
- * signal and then access the memory. Just kill it.
- */
- if (fail || tk->addr == -EFAULT) {
+ if (tk->addr == -EFAULT) {
pr_err("%#lx: forcibly killing %s:%d because of failure to unmap corrupted page\n",
pfn, tk->tsk->comm, tk->tsk->pid);
do_send_sig_info(SIGKILL, SEND_SIG_PRIV,
@@ -1660,7 +1653,7 @@ static bool hwpoison_user_mappings(struct folio *folio, struct page *p,
*/
forcekill = folio_test_dirty(folio) || (flags & MF_MUST_KILL) ||
!unmap_success;
- kill_procs(&tokill, forcekill, !unmap_success, pfn, flags);
+ kill_procs(&tokill, forcekill, pfn, flags);
return unmap_success;
}
@@ -1724,7 +1717,7 @@ static void unmap_and_kill(struct list_head *to_kill, unsigned long pfn,
unmap_mapping_range(mapping, start, size, 0);
}
- kill_procs(to_kill, flags & MF_MUST_KILL, false, pfn, flags);
+ kill_procs(to_kill, flags & MF_MUST_KILL, pfn, flags);
}
/*
--
2.39.3
The soft hwpoison injector via madvise(MADV_HWPOISON) operates in
a synchrous way in a sense, the injector is also a process under
test, and should it have the poisoned page mapped in its address
space, it should get killed as much as in a real UE situation.
Doing so align with what the madvise(2) man page says: "
"This operation may result in the calling process receiving a SIGBUS
and the page being unmapped."
Signed-off-by: Jane Chu <[email protected]>
Reviewed-by: Oscar Salvador <[email protected]>
Acked-by: Miaohe Lin <[email protected]>
---
mm/madvise.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/madvise.c b/mm/madvise.c
index c8ba3f3eb54d..d8a01d7b2860 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -1147,7 +1147,7 @@ static int madvise_inject_error(int behavior,
} else {
pr_info("Injecting memory failure for pfn %#lx at process virtual address %#lx\n",
pfn, start);
- ret = memory_failure(pfn, MF_COUNT_INCREASED | MF_SW_SIMULATED);
+ ret = memory_failure(pfn, MF_ACTION_REQUIRED | MF_COUNT_INCREASED | MF_SW_SIMULATED);
if (ret == -EOPNOTSUPP)
ret = 0;
}
--
2.39.3
Added two explicit MF_MSG messages describing failure in get_hwpoison_page.
Attemped to document the definition of various action names, and made a few
adjustment to the action_result() calls.
Signed-off-by: Jane Chu <[email protected]>
Reviewed-by: Oscar Salvador <[email protected]>
Acked-by: Miaohe Lin <[email protected]>
---
include/linux/mm.h | 2 ++
include/ras/ras_event.h | 2 ++
mm/memory-failure.c | 37 ++++++++++++++++++++++++++++++++-----
3 files changed, 36 insertions(+), 5 deletions(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 9849dfda44d4..b4598c6a393a 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -4111,6 +4111,7 @@ enum mf_action_page_type {
MF_MSG_DIFFERENT_COMPOUND,
MF_MSG_HUGE,
MF_MSG_FREE_HUGE,
+ MF_MSG_GET_HWPOISON,
MF_MSG_UNMAP_FAILED,
MF_MSG_DIRTY_SWAPCACHE,
MF_MSG_CLEAN_SWAPCACHE,
@@ -4124,6 +4125,7 @@ enum mf_action_page_type {
MF_MSG_BUDDY,
MF_MSG_DAX,
MF_MSG_UNSPLIT_THP,
+ MF_MSG_ALREADY_POISONED,
MF_MSG_UNKNOWN,
};
diff --git a/include/ras/ras_event.h b/include/ras/ras_event.h
index c011ea236e9b..b3f6832a94fe 100644
--- a/include/ras/ras_event.h
+++ b/include/ras/ras_event.h
@@ -360,6 +360,7 @@ TRACE_EVENT(aer_event,
EM ( MF_MSG_DIFFERENT_COMPOUND, "different compound page after locking" ) \
EM ( MF_MSG_HUGE, "huge page" ) \
EM ( MF_MSG_FREE_HUGE, "free huge page" ) \
+ EM ( MF_MSG_GET_HWPOISON, "get hwpoison page" ) \
EM ( MF_MSG_UNMAP_FAILED, "unmapping failed page" ) \
EM ( MF_MSG_DIRTY_SWAPCACHE, "dirty swapcache page" ) \
EM ( MF_MSG_CLEAN_SWAPCACHE, "clean swapcache page" ) \
@@ -373,6 +374,7 @@ TRACE_EVENT(aer_event,
EM ( MF_MSG_BUDDY, "free buddy page" ) \
EM ( MF_MSG_DAX, "dax page" ) \
EM ( MF_MSG_UNSPLIT_THP, "unsplit thp" ) \
+ EM ( MF_MSG_ALREADY_POISONED, "already poisoned" ) \
EMe ( MF_MSG_UNKNOWN, "unknown page" )
/*
diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 739311e121af..d1fb1d6f6b11 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -879,6 +879,28 @@ static int kill_accessing_process(struct task_struct *p, unsigned long pfn,
return ret > 0 ? -EHWPOISON : -EFAULT;
}
+/*
+ * MF_IGNORED - The m-f() handler marks the page as PG_hwpoisoned'ed.
+ * But it could not do more to isolate the page from being accessed again,
+ * nor does it kill the process. This is extremely rare and one of the
+ * potential causes is that the page state has been changed due to
+ * underlying race condition. This is the most severe outcomes.
+ *
+ * MF_FAILED - The m-f() handler marks the page as PG_hwpoisoned'ed.
+ * It should have killed the process, but it can't isolate the page,
+ * due to conditions such as extra pin, unmap failure, etc. Accessing
+ * the page again may trigger another MCE and the process will be killed
+ * by the m-f() handler immediately.
+ *
+ * MF_DELAYED - The m-f() handler marks the page as PG_hwpoisoned'ed.
+ * The page is unmapped, and is removed from the LRU or file mapping.
+ * An attempt to access the page again will trigger page fault and the
+ * PF handler will kill the process.
+ *
+ * MF_RECOVERED - The m-f() handler marks the page as PG_hwpoisoned'ed.
+ * The page has been completely isolated, that is, unmapped, taken out of
+ * the buddy system, or hole-punnched out of the file mapping.
+ */
static const char *action_name[] = {
[MF_IGNORED] = "Ignored",
[MF_FAILED] = "Failed",
@@ -893,6 +915,7 @@ static const char * const action_page_types[] = {
[MF_MSG_DIFFERENT_COMPOUND] = "different compound page after locking",
[MF_MSG_HUGE] = "huge page",
[MF_MSG_FREE_HUGE] = "free huge page",
+ [MF_MSG_GET_HWPOISON] = "get hwpoison page",
[MF_MSG_UNMAP_FAILED] = "unmapping failed page",
[MF_MSG_DIRTY_SWAPCACHE] = "dirty swapcache page",
[MF_MSG_CLEAN_SWAPCACHE] = "clean swapcache page",
@@ -906,6 +929,7 @@ static const char * const action_page_types[] = {
[MF_MSG_BUDDY] = "free buddy page",
[MF_MSG_DAX] = "dax page",
[MF_MSG_UNSPLIT_THP] = "unsplit thp",
+ [MF_MSG_ALREADY_POISONED] = "already poisoned",
[MF_MSG_UNKNOWN] = "unknown page",
};
@@ -1013,12 +1037,13 @@ static int me_kernel(struct page_state *ps, struct page *p)
/*
* Page in unknown state. Do nothing.
+ * This is a catch-all in case we fail to make sense of the page state.
*/
static int me_unknown(struct page_state *ps, struct page *p)
{
pr_err("%#lx: Unknown page state\n", page_to_pfn(p));
unlock_page(p);
- return MF_FAILED;
+ return MF_IGNORED;
}
/*
@@ -2055,6 +2080,7 @@ static int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb
if (flags & MF_ACTION_REQUIRED) {
folio = page_folio(p);
res = kill_accessing_process(current, folio_pfn(folio), flags);
+ action_result(pfn, MF_MSG_ALREADY_POISONED, MF_FAILED);
}
return res;
} else if (res == -EBUSY) {
@@ -2062,7 +2088,7 @@ static int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb
flags |= MF_NO_RETRY;
goto retry;
}
- return action_result(pfn, MF_MSG_UNKNOWN, MF_IGNORED);
+ return action_result(pfn, MF_MSG_GET_HWPOISON, MF_IGNORED);
}
folio = page_folio(p);
@@ -2097,7 +2123,7 @@ static int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb
if (!hwpoison_user_mappings(folio, p, pfn, flags)) {
folio_unlock(folio);
- return action_result(pfn, MF_MSG_UNMAP_FAILED, MF_IGNORED);
+ return action_result(pfn, MF_MSG_UNMAP_FAILED, MF_FAILED);
}
return identify_page_state(pfn, p, page_flags);
@@ -2231,6 +2257,7 @@ int memory_failure(unsigned long pfn, int flags)
res = kill_accessing_process(current, pfn, flags);
if (flags & MF_COUNT_INCREASED)
put_page(p);
+ action_result(pfn, MF_MSG_ALREADY_POISONED, MF_FAILED);
goto unlock_mutex;
}
@@ -2267,7 +2294,7 @@ int memory_failure(unsigned long pfn, int flags)
}
goto unlock_mutex;
} else if (res < 0) {
- res = action_result(pfn, MF_MSG_UNKNOWN, MF_IGNORED);
+ res = action_result(pfn, MF_MSG_GET_HWPOISON, MF_IGNORED);
goto unlock_mutex;
}
}
@@ -2363,7 +2390,7 @@ int memory_failure(unsigned long pfn, int flags)
* Abort on fail: __filemap_remove_folio() assumes unmapped page.
*/
if (!hwpoison_user_mappings(folio, p, pfn, flags)) {
- res = action_result(pfn, MF_MSG_UNMAP_FAILED, MF_IGNORED);
+ res = action_result(pfn, MF_MSG_UNMAP_FAILED, MF_FAILED);
goto unlock_page;
}
--
2.39.3
Move hwpoison_filter() higher up as there is no need to spend a lot
cycles only to find out later that the page is supposed to be skipped
from hwpoison handling.
Signed-off-by: Jane Chu <[email protected]>
Reviewed-by: Oscar Salvador <[email protected]>
Acked-by: Miaohe Lin <[email protected]>
---
mm/memory-failure.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index d1fb1d6f6b11..85659dd0ea32 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -2300,6 +2300,18 @@ int memory_failure(unsigned long pfn, int flags)
}
folio = page_folio(p);
+
+ /* filter pages that are protected from hwpoison test by users */
+ folio_lock(folio);
+ if (hwpoison_filter(p)) {
+ ClearPageHWPoison(p);
+ folio_unlock(folio);
+ folio_put(folio);
+ res = -EOPNOTSUPP;
+ goto unlock_mutex;
+ }
+ folio_unlock(folio);
+
if (folio_test_large(folio)) {
/*
* The flag must be set after the refcount is bumped
@@ -2363,14 +2375,6 @@ int memory_failure(unsigned long pfn, int flags)
*/
page_flags = folio->flags;
- if (hwpoison_filter(p)) {
- ClearPageHWPoison(p);
- folio_unlock(folio);
- folio_put(folio);
- res = -EOPNOTSUPP;
- goto unlock_mutex;
- }
-
/*
* __munlock_folio() may clear a writeback folio's LRU flag without
* the folio lock. We need to wait for writeback completion for this
--
2.39.3
While handling hwpoison in a THP page, it is possible that
try_to_split_thp_page() fails. For example, when the THP page has
been RDMA pinned. At this point, the kernel cannot isolate the
poisoned THP page, all it could do is to send a SIGBUS to the user
process with meaningful payload to give user-level recovery a chance.
Signed-off-by: Jane Chu <[email protected]>
Reviewed-by: Oscar Salvador <[email protected]>
Acked-by: Miaohe Lin <[email protected]>
---
mm/memory-failure.c | 34 +++++++++++++++++++++++++++++-----
1 file changed, 29 insertions(+), 5 deletions(-)
diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 85659dd0ea32..dcca7297a94c 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -1706,7 +1706,12 @@ static int identify_page_state(unsigned long pfn, struct page *p,
return page_action(ps, p, pfn);
}
-static int try_to_split_thp_page(struct page *page)
+/*
+ * When 'release' is 'false', it means that if thp split has failed,
+ * there is still more to do, hence the page refcount we took earlier
+ * is still needed.
+ */
+static int try_to_split_thp_page(struct page *page, bool release)
{
int ret;
@@ -1714,7 +1719,7 @@ static int try_to_split_thp_page(struct page *page)
ret = split_huge_page(page);
unlock_page(page);
- if (unlikely(ret))
+ if (ret && release)
put_page(page);
return ret;
@@ -2186,6 +2191,22 @@ static int memory_failure_dev_pagemap(unsigned long pfn, int flags,
return rc;
}
+/*
+ * The calling condition is as such: thp split failed, page might have
+ * been RDMA pinned, not much can be done for recovery.
+ * But a SIGBUS should be delivered with vaddr provided so that the user
+ * application has a chance to recover. Also, application processes'
+ * election for MCE early killed will be honored.
+ */
+static void kill_procs_now(struct page *p, unsigned long pfn, int flags,
+ struct folio *folio)
+{
+ LIST_HEAD(tokill);
+
+ collect_procs(folio, p, &tokill, flags & MF_ACTION_REQUIRED);
+ kill_procs(&tokill, true, pfn, flags);
+}
+
/**
* memory_failure - Handle memory failure of a page.
* @pfn: Page Number of the corrupted page
@@ -2327,8 +2348,11 @@ int memory_failure(unsigned long pfn, int flags)
* page is a valid handlable page.
*/
folio_set_has_hwpoisoned(folio);
- if (try_to_split_thp_page(p) < 0) {
- res = action_result(pfn, MF_MSG_UNSPLIT_THP, MF_IGNORED);
+ if (try_to_split_thp_page(p, false) < 0) {
+ res = -EHWPOISON;
+ kill_procs_now(p, pfn, flags, folio);
+ put_page(p);
+ action_result(pfn, MF_MSG_UNSPLIT_THP, MF_FAILED);
goto unlock_mutex;
}
VM_BUG_ON_PAGE(!page_count(p), p);
@@ -2702,7 +2726,7 @@ static int soft_offline_in_use_page(struct page *page)
};
if (!huge && folio_test_large(folio)) {
- if (try_to_split_thp_page(page)) {
+ if (try_to_split_thp_page(page, true)) {
pr_info("soft offline: %#lx: thp split failed\n", pfn);
return -EBUSY;
}
--
2.39.3