Currently memory_failure() calls shake_page() to sweep pages out from pcplists
only when the victim page is 4kB LRU page or thp head page. But we should do
this for a thp tail page too.
Consider that a memory error hits a thp tail page whose head page is on a
pcplist when memory_failure() runs. Then, the current kernel skips shake_pages()
part, so hwpoison_user_mappings() returns without calling split_huge_page() nor
try_to_unmap() because PageLRU of the thp head is still cleared due to the skip
of shake_page().
As a result, me_huge_page() runs for the thp, which is a broken behavior.
This patch fixes this problem by calling shake_page() for thp tail case.
Fixes: 385de35722c9 ("thp: allow a hwpoisoned head page to be put back to LRU")
Signed-off-by: Naoya Horiguchi <[email protected]>
Cc: [email protected] # v3.4+
---
mm/memory-failure.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git v4.0.orig/mm/memory-failure.c v4.0/mm/memory-failure.c
index d487f8dc6d39..2cc1d578144b 100644
--- v4.0.orig/mm/memory-failure.c
+++ v4.0/mm/memory-failure.c
@@ -1141,10 +1141,10 @@ int memory_failure(unsigned long pfn, int trapno, int flags)
* The check (unnecessarily) ignores LRU pages being isolated and
* walked by the page reclaim code, however that's not a big loss.
*/
- if (!PageHuge(p) && !PageTransTail(p)) {
- if (!PageLRU(p))
- shake_page(p, 0);
- if (!PageLRU(p)) {
+ if (!PageHuge(p)) {
+ if (!PageLRU(hpage))
+ shake_page(hpage, 0);
+ if (!PageLRU(hpage)) {
/*
* shake_page could have turned it free.
*/
--
2.1.0
On Wed, Apr 15, 2015 at 07:25:46AM +0000, Naoya Horiguchi wrote:
> Currently memory_failure() calls shake_page() to sweep pages out from pcplists
> only when the victim page is 4kB LRU page or thp head page. But we should do
> this for a thp tail page too.
> Consider that a memory error hits a thp tail page whose head page is on a
> pcplist when memory_failure() runs. Then, the current kernel skips shake_pages()
> part, so hwpoison_user_mappings() returns without calling split_huge_page() nor
> try_to_unmap() because PageLRU of the thp head is still cleared due to the skip
> of shake_page().
> As a result, me_huge_page() runs for the thp, which is a broken behavior.
>
> This patch fixes this problem by calling shake_page() for thp tail case.
>
> Fixes: 385de35722c9 ("thp: allow a hwpoisoned head page to be put back to LRU")
> Signed-off-by: Naoya Horiguchi <[email protected]>
> Cc: [email protected] # v3.4+
Looks good to me.
Reviewed-by: Andi Kleen <[email protected]>
-Andi
> ---
> mm/memory-failure.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git v4.0.orig/mm/memory-failure.c v4.0/mm/memory-failure.c
> index d487f8dc6d39..2cc1d578144b 100644
> --- v4.0.orig/mm/memory-failure.c
> +++ v4.0/mm/memory-failure.c
> @@ -1141,10 +1141,10 @@ int memory_failure(unsigned long pfn, int trapno, int flags)
> * The check (unnecessarily) ignores LRU pages being isolated and
> * walked by the page reclaim code, however that's not a big loss.
> */
> - if (!PageHuge(p) && !PageTransTail(p)) {
> - if (!PageLRU(p))
> - shake_page(p, 0);
> - if (!PageLRU(p)) {
> + if (!PageHuge(p)) {
> + if (!PageLRU(hpage))
> + shake_page(hpage, 0);
> + if (!PageLRU(hpage)) {
> /*
> * shake_page could have turned it free.
> */
> --
> 2.1.0
>
--
[email protected] -- Speaking for myself only.
On 04/15/2015 02:25 AM, Naoya Horiguchi wrote:
> Currently memory_failure() calls shake_page() to sweep pages out from pcplists
> only when the victim page is 4kB LRU page or thp head page. But we should do
> this for a thp tail page too.
> Consider that a memory error hits a thp tail page whose head page is on a
> pcplist when memory_failure() runs. Then, the current kernel skips shake_pages()
> part, so hwpoison_user_mappings() returns without calling split_huge_page() nor
> try_to_unmap() because PageLRU of the thp head is still cleared due to the skip
> of shake_page().
> As a result, me_huge_page() runs for the thp, which is a broken behavior.
>
> This patch fixes this problem by calling shake_page() for thp tail case.
>
> Fixes: 385de35722c9 ("thp: allow a hwpoisoned head page to be put back to LRU")
> Signed-off-by: Naoya Horiguchi <[email protected]>
This looks correct to me. Thanks!
Acked-by: Dean Nelson <[email protected]>
> Cc: [email protected] # v3.4+
> ---
> mm/memory-failure.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git v4.0.orig/mm/memory-failure.c v4.0/mm/memory-failure.c
> index d487f8dc6d39..2cc1d578144b 100644
> --- v4.0.orig/mm/memory-failure.c
> +++ v4.0/mm/memory-failure.c
> @@ -1141,10 +1141,10 @@ int memory_failure(unsigned long pfn, int trapno, int flags)
> * The check (unnecessarily) ignores LRU pages being isolated and
> * walked by the page reclaim code, however that's not a big loss.
> */
> - if (!PageHuge(p) && !PageTransTail(p)) {
> - if (!PageLRU(p))
> - shake_page(p, 0);
> - if (!PageLRU(p)) {
> + if (!PageHuge(p)) {
> + if (!PageLRU(hpage))
> + shake_page(hpage, 0);
> + if (!PageLRU(hpage)) {
> /*
> * shake_page could have turned it free.
> */
>
On Wed, 15 Apr 2015 07:25:46 +0000 Naoya Horiguchi <[email protected]> wrote:
> Currently memory_failure() calls shake_page() to sweep pages out from pcplists
> only when the victim page is 4kB LRU page or thp head page. But we should do
> this for a thp tail page too.
> Consider that a memory error hits a thp tail page whose head page is on a
> pcplist when memory_failure() runs. Then, the current kernel skips shake_pages()
> part, so hwpoison_user_mappings() returns without calling split_huge_page() nor
> try_to_unmap() because PageLRU of the thp head is still cleared due to the skip
> of shake_page().
> As a result, me_huge_page() runs for the thp, which is a broken behavior.
>
> This patch fixes this problem by calling shake_page() for thp tail case.
>
> Fixes: 385de35722c9 ("thp: allow a hwpoisoned head page to be put back to LRU")
> Signed-off-by: Naoya Horiguchi <[email protected]>
> Cc: [email protected] # v3.4+
What are the userspace-visible effects of the bug? This info is needed
for backporting into -stable and other kernels, please.
On Mon, Apr 20, 2015 at 02:30:14PM -0700, Andrew Morton wrote:
> On Wed, 15 Apr 2015 07:25:46 +0000 Naoya Horiguchi <[email protected]> wrote:
>
> > Currently memory_failure() calls shake_page() to sweep pages out from pcplists
> > only when the victim page is 4kB LRU page or thp head page. But we should do
> > this for a thp tail page too.
> > Consider that a memory error hits a thp tail page whose head page is on a
> > pcplist when memory_failure() runs. Then, the current kernel skips shake_pages()
> > part, so hwpoison_user_mappings() returns without calling split_huge_page() nor
> > try_to_unmap() because PageLRU of the thp head is still cleared due to the skip
> > of shake_page().
> > As a result, me_huge_page() runs for the thp, which is a broken behavior.
> >
> > This patch fixes this problem by calling shake_page() for thp tail case.
> >
> > Fixes: 385de35722c9 ("thp: allow a hwpoisoned head page to be put back to LRU")
> > Signed-off-by: Naoya Horiguchi <[email protected]>
> > Cc: [email protected] # v3.4+
>
> What are the userspace-visible effects of the bug? This info is needed
> for backporting into -stable and other kernels, please.
One effect is memory leak of the thp. And another is to fail to isolate
the memory error, so later access to the error address causes another MCE,
which kills the processes which used the thp.
Thanks,
Naoya Horiguchi-