Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1207326

Re: [PATCH] mm/hwpoison: fix race between soft_offline_page and unpoison_memory

From Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Newsgroups linux.kernel
Subject Re: [PATCH] mm/hwpoison: fix race between soft_offline_page and unpoison_memory
Date 2015-08-14 06:30 +0200
Message-ID <pXeMW-81o-1@gated-at.bofh.it> (permalink)
References <pWVhf-53B-23@gated-at.bofh.it> <pWWwF-6LA-3@gated-at.bofh.it> <pWWQ3-7nz-21@gated-at.bofh.it> <pWXCu-7m-115@gated-at.bofh.it> <pWXVN-uO-51@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Thu, Aug 13, 2015 at 06:27:40PM +0800, Wanpeng Li wrote:
> On 8/13/15 6:04 PM, Naoya Horiguchi wrote:
> > On Thu, Aug 13, 2015 at 05:18:56PM +0800, Wanpeng Li wrote:
> >> On 8/13/15 4:53 PM, Naoya Horiguchi wrote:
> > ...
> >>> I think that unpoison is used only in testing so this race never affects
> >>> our end-users/customers, so going back to this migratetype change stuff
> >>> looks unworthy to me.
> >> Migratetype stuff is just removed by you two months ago, then this bug
> >> occurs recently since the more and more patches which you fix some races.
> > Yes, this race (which existed before my recent changes) became more visible
> 
> IIUC, no. The page will be freed before PageHWPoison is set. So the race
> doesn't exist.

OK ...

> > with that changes. But I don't think that simply reverting them is a right solution.
> >
> >>> If I read correctly, the old migratetype approach has a few problems:
> >>>   1) it doesn't fix the problem completely, because
> >>>      set_migratetype_isolate() can fail to set MIGRATE_ISOLATE to the
> >>>      target page if the pageblock of the page contains one or more
> >>>      unmovable pages (i.e. has_unmovable_pages() returns true).
> >>>   2) the original code changes migratetype to MIGRATE_ISOLATE forcibly,
> >>>      and sets it to MIGRATE_MOVABLE forcibly after soft offline, regardless
> >>>      of the original migratetype state, which could impact other subsystems
> >>>      like memory hotplug or compaction.
> >> Maybe we can add a "FIXME" comment on the Migratetype stuff, since the
> >> current linus tree calltrace and it should be fixed immediately, and I
> >> don't see obvious bugs appear on migratetype stuffs at least currently,
> >> so "FIXME" is enough. :-)
> > Sorry if confusing, but my intention in saying about "FIXME" comment was
> > that we can find another solution for this race rather than just reverting,
> > so adding comment about the reported bug in current code (keeping code from
> > 4491f712606) is OK for very short term.
> > I understand that leaving a race window of BUG_ON is not the best thing, but
> > as I said, this race shouldn't affect end-users, so this is not an urgent bug.
> > # It's enough if testers know this.
> 
> The 4.2 is coming, this patch can be applied as a temporal solution in
> order to fix the broken linus tree, and the any final solution can be
> figured out later.

I didn't reproduce this problem yet in my environment, but from code reading
I guess that checking PageHWPoison flag in unmap_and_move() like below could
avoid the problem. Could you testing with this, please?

Thanks,
Naoya Horiguchi
---
From: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Date: Fri, 14 Aug 2015 08:04:03 +0900
Subject: [PATCH] mm: hwpoison: migrate: fix race b/w soft-offline and unpoison

[description to be written ...]

Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
---
 mm/migrate.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/mm/migrate.c b/mm/migrate.c
index eb4267107d1f..24f5b9acc26a 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -953,7 +953,13 @@ static ICE_noinline int unmap_and_move(new_page_t get_new_page,
 				page_is_file_cache(page));
 		/* Soft-offlined page shouldn't go through lru cache list */
 		if (reason == MR_MEMORY_FAILURE)
-			put_page(page);
+			/*
+			 * Check race condition with unpoison, where the source
+			 * page is handled by unpoison handler which decrements
+			 * the refcount, so no need to call put_page() here.
+			 */
+			if (PageHWPoison(page))
+				put_page(page);
 		else
 			putback_lru_page(page);
 	}
-- 
2.4.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH] mm/hwpoison: fix race between soft_offline_page and unpoison_memory Wanpeng Li <wanpeng.li@hotmail.com> - 2015-08-13 09:40 +0200
  Re: [PATCH] mm/hwpoison: fix race between soft_offline_page and  unpoison_memory Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2015-08-13 11:00 +0200
    Re: [PATCH] mm/hwpoison: fix race between soft_offline_page and  unpoison_memory Wanpeng Li <wanpeng.li@hotmail.com> - 2015-08-13 11:20 +0200
      Re: [PATCH] mm/hwpoison: fix race between soft_offline_page and  unpoison_memory Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2015-08-13 12:10 +0200
        Re: [PATCH] mm/hwpoison: fix race between soft_offline_page and  unpoison_memory Wanpeng Li <wanpeng.li@hotmail.com> - 2015-08-13 12:30 +0200
          Re: [PATCH] mm/hwpoison: fix race between soft_offline_page and  unpoison_memory Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2015-08-14 06:30 +0200
            Re: [PATCH] mm/hwpoison: fix race between soft_offline_page and  unpoison_memory Wanpeng Li <wanpeng.li@hotmail.com> - 2015-08-14 07:10 +0200
              Re: [PATCH] mm/hwpoison: fix race between soft_offline_page and  unpoison_memory Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2015-08-14 09:30 +0200
                Re: [PATCH] mm/hwpoison: fix race between soft_offline_page and  unpoison_memory Wanpeng Li <wanpeng.li@hotmail.com> - 2015-08-14 10:00 +0200
                Re: [PATCH] mm/hwpoison: fix race between soft_offline_page and  unpoison_memory Wanpeng Li <wanpeng.li@hotmail.com> - 2015-08-14 10:00 +0200
                Re: [PATCH] mm/hwpoison: fix race between soft_offline_page and  unpoison_memory Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2015-08-14 10:40 +0200
                Re: [PATCH] mm/hwpoison: fix race between soft_offline_page and  unpoison_memory Wanpeng Li <wanpeng.li@hotmail.com> - 2015-08-14 11:10 +0200
                Re: [PATCH] mm/hwpoison: fix race between soft_offline_page and  unpoison_memory Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2015-08-17 06:40 +0200
                [PATCH v2 2/3] mm/hwpoison: fix race between soft_offline_page and  unpoison_memory Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2015-08-17 06:40 +0200
                [PATCH v2 3/3] mm/hwpoison: don't try to unpoison  containment-failed pages Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2015-08-17 06:40 +0200
                [PATCH v2 1/3] mm/hwpoison: introduce num_poisoned_pages wrappers Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2015-08-17 06:40 +0200
                Re: [PATCH] mm/hwpoison: fix race between soft_offline_page and  unpoison_memory Wanpeng Li <wanpeng.li@hotmail.com> - 2015-08-17 07:30 +0200
                Re: [PATCH] mm/hwpoison: fix race between soft_offline_page and  unpoison_memory Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2015-08-14 10:20 +0200

csiph-web