mm/memory-failure.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
From: Shengming Hu <shengminghu512@qq.com>
Date: Tue, 23 Sep 2025 20:56:28 +0800
Subject: [PATCH V2] mm/memory-failure: Ensure collect_procs is retried when
unmap fails
In the memory_failure process, if collect_procs is not executed with the
flag set, the to_kill list may be empty. Even if there are pages that fail
to be unmapped, SIGKILL or SIGBUS cannot be sent to the process via
collect_procs.
This patch fixes the issue by re-executing collect_procs when the to_kill
list is empty and unmap fails. This collects processes with unmap failures
into the to_kill list, allowing SIGBUS or SIGKILL to terminate them in
subsequent code.
V2:
- Resent as plain text (previous version was HTML).
- No functional changes.
Signed-off-by: Shengming Hu <hu.shengming@zte.com.cn>
---
mm/memory-failure.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index a24806bb8e82..8157823c7fb7 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -1600,9 +1600,12 @@ static bool hwpoison_user_mappings(struct folio *folio, struct page *p,
collect_procs(folio, p, &tokill, flags & MF_ACTION_REQUIRED);
unmap_success = !unmap_poisoned_folio(folio, pfn, flags & MF_MUST_KILL);
- if (!unmap_success)
+ if (!unmap_success) {
pr_err("%#lx: failed to unmap page (folio mapcount=%d)\n",
pfn, folio_mapcount(folio));
+ if (list_empty(&tokill))
+ collect_procs(folio, p, &tokill, 1);
+ }
/*
* try_to_unmap() might put mlocked page in lru cache, so call
--
2.25.1On Tue, Sep 23, 2025 at 7:57 AM shengminghu512 <shengminghu512@qq.com> wrote:
>
> From: Shengming Hu <shengminghu512@qq.com>
> Date: Tue, 23 Sep 2025 20:56:28 +0800
> Subject: [PATCH V2] mm/memory-failure: Ensure collect_procs is retried when
> unmap fails
>
> In the memory_failure process, if collect_procs is not executed with the
> flag set, the to_kill list may be empty. Even if there are pages that fail
Hi Shengming,
I am trying to figure out what your code is for.
If we get into hwpoison_user_mappings with MF_ACTION_REQUIRED *not*
set in flags, force_early should not be set and it is totally valid
that collect_procs add nothing to to kill...
> to be unmapped, SIGKILL or SIGBUS cannot be sent to the process via
unless some process opt-ed in MCE_KILL_EARLY and collect_procs will
find that process if it mapped the poisoned page, regardless if
force_early is 1 or 0.
IOW I don't think there is any reason (no bug to fix and nothing to
improve) for what you are trying to do here.
> collect_procs.
>
> This patch fixes the issue by re-executing collect_procs when the to_kill
> list is empty and unmap fails. This collects processes with unmap failures
> into the to_kill list, allowing SIGBUS or SIGKILL to terminate them in
> subsequent code.
>
> V2:
> - Resent as plain text (previous version was HTML).
> - No functional changes.
>
> Signed-off-by: Shengming Hu <hu.shengming@zte.com.cn>
> ---
> mm/memory-failure.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index a24806bb8e82..8157823c7fb7 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -1600,9 +1600,12 @@ static bool hwpoison_user_mappings(struct folio *folio, struct page *p,
> collect_procs(folio, p, &tokill, flags & MF_ACTION_REQUIRED);
>
> unmap_success = !unmap_poisoned_folio(folio, pfn, flags & MF_MUST_KILL);
> - if (!unmap_success)
> + if (!unmap_success) {
> pr_err("%#lx: failed to unmap page (folio mapcount=%d)\n",
> pfn, folio_mapcount(folio));
> + if (list_empty(&tokill))
> + collect_procs(folio, p, &tokill, 1);
> + }
>
> /*
> * try_to_unmap() might put mlocked page in lru cache, so call
> --
> 2.25.1
Jiaqi Yan <jiaqiyan@google.com> writes:
> On Tue, Sep 23, 2025 at 7:57 AM shengminghu512 <shengminghu512@qq.com> wrote:
> >
> > From: Shengming Hu <shengminghu512@qq.com>
> > Date: Tue, 23 Sep 2025 20:56:28 +0800
> > Subject: [PATCH V2] mm/memory-failure: Ensure collect_procs is retried when
> > unmap fails
> >
> > In the memory_failure process, if collect_procs is not executed with the
> > flag set, the to_kill list may be empty. Even if there are pages that fail
>
> Hi Shengming,
>
> I am trying to figure out what your code is for.
>
> If we get into hwpoison_user_mappings with MF_ACTION_REQUIRED *not*
> set in flags, force_early should not be set and it is totally valid
> that collect_procs add nothing to to kill...
>
Hi Jiaqi
Thank you for the review and for the explanation. I’d like to double-check
my understanding and ask for your guidance.
> > to be unmapped, SIGKILL or SIGBUS cannot be sent to the process via
>
> unless some process opt-ed in MCE_KILL_EARLY and collect_procs will
> find that process if it mapped the poisoned page, regardless if
> force_early is 1 or 0.
>
> IOW I don't think there is any reason (no bug to fix and nothing to
> improve) for what you are trying to do here.
>
Your explanation of the collect_procs collection flow was extremely detailed
and helpful. After diving deeper into the code myself, I’d like to discuss
with you a few scenarios I’m particularly worried about where things might
go wrong.
From reading the code, my understanding of the flow is:
- hwpoison_user_mappings handles poisoned pages in two ways:
(1) mark PTEs with hwpoison so that later accesses trigger SIGBUS, or
(2) proactively send SIGBUS/SIGKILL to terminate processes.
- The sequence is:
collect_procs -> unmap_poisoned_folio -> kill_procs
- For kill_procs to send signals, three conditions must be satisfied:
forcekill = folio_test_dirty(folio) || (flags & MF_MUST_KILL) || !unmap_success
and `tokill` must not be empty.
My concern is the following corner case:
* If unmap_poisoned_folio() fails on a poisoned page, it may not install
a hwpoison PTE entry.
* As you note If collect_procs() earlier ran without MF_ACTION_REQUIRED (and
some processes did not opt into MCE_KILL_EARLY), `tokill` can remain empty.
* In this situation, kill_procs() will not deliver any signal, and a
process can still run while using the poisoned page.
My patch retries collect_procs() (with force_early = 1) when
unmap_poisoned_folio() fails and `tokill` is empty, ensuring processes that
still hold the mapping are collected and can receive SIGBUS/SIGKILL. That
is the motivation for the change.
My question is:
Is there already a guarantee in the current design that either (a) a
hwpoison PTE entry will always be installed, or (b) a process will always
be collected into `tokill` in this unmap failure case?
If such a guarantee exists, I may have misunderstood the intended flow —
could you help clarify where that happens in the code? If not, does my
approach of retrying collect_procs make sense?
Thanks a lot for the feedback and guidance — I’d like to align with the
intended semantics and update the patch accordingly.
> > collect_procs.
> >
> > This patch fixes the issue by re-executing collect_procs when the to_kill
> > list is empty and unmap fails. This collects processes with unmap failures
> > into the to_kill list, allowing SIGBUS or SIGKILL to terminate them in
> > subsequent code.
> >
> > V2:
> > - Resent as plain text (previous version was HTML).
> > - No functional changes.
> >
> > Signed-off-by: Shengming Hu <hu.shengming@zte.com.cn>
> > ---
> > mm/memory-failure.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> > index a24806bb8e82..8157823c7fb7 100644
> > --- a/mm/memory-failure.c
> > +++ b/mm/memory-failure.c
> > @@ -1600,9 +1600,12 @@ static bool hwpoison_user_mappings(struct folio *folio, struct page *p,
> > collect_procs(folio, p, &tokill, flags & MF_ACTION_REQUIRED);
> >
> > unmap_success = !unmap_poisoned_folio(folio, pfn, flags & MF_MUST_KILL);
> > - if (!unmap_success)
> > + if (!unmap_success) {
> > pr_err("%#lx: failed to unmap page (folio mapcount=%d)\n",
> > pfn, folio_mapcount(folio));
> > + if (list_empty(&tokill))
> > + collect_procs(folio, p, &tokill, 1);
> > + }
> >
> > /*
> > * try_to_unmap() might put mlocked page in lru cache, so call
> > --
> > 2.25.1
Best regards,
Shengming Hu
Jiaqi Yan <jiaqiyan@google.com> writes:
> On Tue, Sep 23, 2025 at 7:57 AM shengminghu512 <shengminghu512@qq.com> wrote:
> >
> > From: Shengming Hu <shengminghu512@qq.com>
> > Date: Tue, 23 Sep 2025 20:56:28 +0800
> > Subject: [PATCH V2] mm/memory-failure: Ensure collect_procs is retried when
> > unmap fails
> >
> > In the memory_failure process, if collect_procs is not executed with the
> > flag set, the to_kill list may be empty. Even if there are pages that fail
>
> Hi Shengming,
>
> I am trying to figure out what your code is for.
>
> If we get into hwpoison_user_mappings with MF_ACTION_REQUIRED *not*
> set in flags, force_early should not be set and it is totally valid
> that collect_procs add nothing to to kill...
>
Hi Jiaqi
Thank you for the review and for the explanation. I’d like to double-check
my understanding and ask for your guidance.
> > to be unmapped, SIGKILL or SIGBUS cannot be sent to the process via
>
> unless some process opt-ed in MCE_KILL_EARLY and collect_procs will
> find that process if it mapped the poisoned page, regardless if
> force_early is 1 or 0.
>
> IOW I don't think there is any reason (no bug to fix and nothing to
> improve) for what you are trying to do here.
>
Your explanation of the collect_procs collection flow was extremely detailed
and helpful. After diving deeper into the code myself, I’d like to discuss
with you a few scenarios I’m particularly worried about where things might
go wrong.
From reading the code, my understanding of the flow is:
- hwpoison_user_mappings handles poisoned pages in two ways:
(1) mark PTEs with hwpoison so that later accesses trigger SIGBUS, or
(2) proactively send SIGBUS/SIGKILL to terminate processes.
- The sequence is:
collect_procs -> unmap_poisoned_folio -> kill_procs
- For kill_procs to send signals, three conditions must be satisfied:
forcekill = folio_test_dirty(folio) || (flags & MF_MUST_KILL) || !unmap_success
and `tokill` must not be empty.
My concern is the following corner case:
* If unmap_poisoned_folio() fails on a poisoned page, it may not install
a hwpoison PTE entry.
* As you note If collect_procs() earlier ran without MF_ACTION_REQUIRED (and
some processes did not opt into MCE_KILL_EARLY), `tokill` can remain empty.
* In this situation, kill_procs() will not deliver any signal, and a
process can still run while using the poisoned page.
My patch retries collect_procs() (with force_early = 1) when
unmap_poisoned_folio() fails and `tokill` is empty, ensuring processes that
still hold the mapping are collected and can receive SIGBUS/SIGKILL. That
is the motivation for the change.
My question is:
Is there already a guarantee in the current design that either (a) a
hwpoison PTE entry will always be installed, or (b) a process will always
be collected into `tokill` in this unmap failure case?
If such a guarantee exists, I may have misunderstood the intended flow —
could you help clarify where that happens in the code? If not, does my
approach of retrying collect_procs make sense?
Thanks a lot for the feedback and guidance — I’d like to align with the
intended semantics and update the patch accordingly.
> > collect_procs.
> >
> > This patch fixes the issue by re-executing collect_procs when the to_kill
> > list is empty and unmap fails. This collects processes with unmap failures
> > into the to_kill list, allowing SIGBUS or SIGKILL to terminate them in
> > subsequent code.
> >
> > V2:
> > - Resent as plain text (previous version was HTML).
> > - No functional changes.
> >
> > Signed-off-by: Shengming Hu <hu.shengming@zte.com.cn>
> > ---
> > mm/memory-failure.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> > index a24806bb8e82..8157823c7fb7 100644
> > --- a/mm/memory-failure.c
> > +++ b/mm/memory-failure.c
> > @@ -1600,9 +1600,12 @@ static bool hwpoison_user_mappings(struct folio *folio, struct page *p,
> > collect_procs(folio, p, &tokill, flags & MF_ACTION_REQUIRED);
> >
> > unmap_success = !unmap_poisoned_folio(folio, pfn, flags & MF_MUST_KILL);
> > - if (!unmap_success)
> > + if (!unmap_success) {
> > pr_err("%#lx: failed to unmap page (folio mapcount=%d)\n",
> > pfn, folio_mapcount(folio));
> > + if (list_empty(&tokill))
> > + collect_procs(folio, p, &tokill, 1);
> > + }
> >
> > /*
> > * try_to_unmap() might put mlocked page in lru cache, so call
> > --
> > 2.25.1
Best regards,
Shengming Hu</hu.shengming@zte.com.cn></shengminghu512@qq.com></shengminghu512@qq.com></jiaqiyan@google.com>
© 2016 - 2025 Red Hat, Inc.