Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753551AbbHNHyp (ORCPT ); Fri, 14 Aug 2015 03:54:45 -0400 Received: from blu004-omc1s11.hotmail.com ([65.55.116.22]:51172 "EHLO BLU004-OMC1S11.hotmail.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751952AbbHNHyo (ORCPT ); Fri, 14 Aug 2015 03:54:44 -0400 X-TMN: [wAyJ5zFu9B4ZnrvDsNypIPa6WTVvFcGK] X-Originating-Email: [wanpeng.li@hotmail.com] Message-ID: Subject: Re: [PATCH] mm/hwpoison: fix race between soft_offline_page and unpoison_memory To: Naoya Horiguchi References: <20150813085332.GA30163@hori1.linux.bs1.fc.nec.co.jp> <20150813100407.GA2993@hori1.linux.bs1.fc.nec.co.jp> <20150814041939.GA9951@hori1.linux.bs1.fc.nec.co.jp> <20150814072649.GA31021@hori1.linux.bs1.fc.nec.co.jp> CC: Andrew Morton , "linux-mm@kvack.org" , "linux-kernel@vger.kernel.org" From: Wanpeng Li Date: Fri, 14 Aug 2015 15:54:36 +0800 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:38.0) Gecko/20100101 Thunderbird/38.0.1 MIME-Version: 1.0 In-Reply-To: <20150814072649.GA31021@hori1.linux.bs1.fc.nec.co.jp> Content-Type: text/plain; charset="iso-2022-jp" Content-Transfer-Encoding: 7bit X-OriginalArrivalTime: 14 Aug 2015 07:54:42.0474 (UTC) FILETIME=[7A8BE0A0:01D0D666] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4334 Lines: 93 On 8/14/15 3:26 PM, Naoya Horiguchi wrote: > On Fri, Aug 14, 2015 at 01:03:53PM +0800, Wanpeng Li wrote: >> On 8/14/15 12:19 PM, Naoya Horiguchi wrote: > ... >>>>>>> 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? >> I have already try to modify unmap_and_move() the same as what you do >> before I post migratetype stuff. It doesn't work and have other calltrace. > OK, then I rethink of handling the race in unpoison_memory(). > > Currently properly contained/hwpoisoned pages should have page refcount 1 > (when the memory error hits LRU pages or hugetlb pages) or refcount 0 > (when the memory error hits the buddy page.) And current unpoison_memory() > implicitly assumes this because otherwise the unpoisoned page has no place > to go and it's just leaked. > So to avoid the kernel panic, adding prechecks of refcount and mapcount > to limit the page to unpoison for only unpoisonable pages looks OK to me. > The page under soft offlining always has refcount >=2 and/or mapcount > 0, > so such pages should be filtered out. > > Here's a patch. In my testing (run soft offline stress testing then repeat > unpoisoning in background,) the reported (or similar) bug doesn't happen. > Can I have your comments? As page_action() prints out page maybe still referenced by some users, however, PageHWPoison has already set. So you will leak many poison pages. Regards, Wanpeng Li > > Thanks, > Naoya Horiguchi > --- > From: Naoya Horiguchi > Subject: [PATCH] mm/hwpoison: don't unpoison for pinned or mapped page > > Signed-off-by: Naoya Horiguchi > --- > mm/memory-failure.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/mm/memory-failure.c b/mm/memory-failure.c > index d1f85f6278ee..c6f14d2cd919 100644 > --- a/mm/memory-failure.c > +++ b/mm/memory-failure.c > @@ -1442,6 +1442,16 @@ int unpoison_memory(unsigned long pfn) > return 0; > } > > + if (page_count(page) > 1) { > + pr_info("MCE: Someone grabs the hwpoison page %#lx\n", pfn); > + return 0; > + } > + > + if (page_mapped(page)) { > + pr_info("MCE: Someone maps the hwpoison page %#lx\n", pfn); > + return 0; > + } > + > /* > * unpoison_memory() can encounter thp only when the thp is being > * worked by memory_failure() and the page lock is not held yet. -- 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/