Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753347AbbHNEUY (ORCPT ); Fri, 14 Aug 2015 00:20:24 -0400 Received: from TYO202.gate.nec.co.jp ([210.143.35.52]:35784 "EHLO tyo202.gate.nec.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751027AbbHNEUW convert rfc822-to-8bit (ORCPT ); Fri, 14 Aug 2015 00:20:22 -0400 From: Naoya Horiguchi To: Wanpeng Li CC: Andrew Morton , "linux-mm@kvack.org" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH] mm/hwpoison: fix race between soft_offline_page and unpoison_memory Thread-Topic: [PATCH] mm/hwpoison: fix race between soft_offline_page and unpoison_memory Thread-Index: AQHQ1Zq57OyJ15BfMkCMbnAvybI4V54JCPoAgAAHGQCAAAyggIAABpQAgAErgoA= Date: Fri, 14 Aug 2015 04:19:40 +0000 Message-ID: <20150814041939.GA9951@hori1.linux.bs1.fc.nec.co.jp> References: <20150813085332.GA30163@hori1.linux.bs1.fc.nec.co.jp> <20150813100407.GA2993@hori1.linux.bs1.fc.nec.co.jp> In-Reply-To: Accept-Language: ja-JP, en-US Content-Language: ja-JP X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.128.101.24] Content-Type: text/plain; charset="iso-2022-jp" Content-ID: Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3869 Lines: 88 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 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 --- 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/