From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id AABA7C433F5 for ; Mon, 20 Dec 2021 08:49:03 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D19716B0071; Mon, 20 Dec 2021 03:49:02 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id CC83C6B0073; Mon, 20 Dec 2021 03:49:02 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BB6DC6B0074; Mon, 20 Dec 2021 03:49:02 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0128.hostedemail.com [216.40.44.128]) by kanga.kvack.org (Postfix) with ESMTP id A8D8F6B0071 for ; Mon, 20 Dec 2021 03:49:02 -0500 (EST) Received: from smtpin08.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 4FB52181AC9CC for ; Mon, 20 Dec 2021 08:49:02 +0000 (UTC) X-FDA: 78937547724.08.58F0597 Received: from out2.migadu.com (out2.migadu.com [188.165.223.204]) by imf26.hostedemail.com (Postfix) with ESMTP id 7B61714003F for ; Mon, 20 Dec 2021 08:49:01 +0000 (UTC) Date: Mon, 20 Dec 2021 17:48:51 +0900 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1639990139; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=f7ymSe3JNfGnfp68YJJDL+vOmYRtr+smRMVOZWaLcTE=; b=c9uD0xpYsmmXMR1fvEILCUGXsbc00cyAskBGoAfziYV4uuq12OqqNpheCyNK3/mlODYtu2 06vAcNneyR6VG1D9kq1jkzrFehbebz3B9tWGjsvIPCRTdj64LXuMCwPSewA7NziOCv5S37 n67BUOMORZtiv5mprc0ILSzCwzTovPM= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Naoya Horiguchi To: Andrew Morton Cc: linux-mm@kvack.org, luofei@unicloud.com, Naoya Horiguchi , Mike Kravetz , linux-kernel@vger.kernel.org Subject: Re: [PATCH v1] mm, hwpoison: fix condition in free hugetlb page path Message-ID: <20211220084851.GA1460264@u2004> References: <20211210110208.879740-1-naoya.horiguchi@linux.dev> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20211210110208.879740-1-naoya.horiguchi@linux.dev> X-Migadu-Flow: FLOW_OUT X-Migadu-Auth-User: linux.dev X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 7B61714003F X-Stat-Signature: z1eyhrqesdn3yuj4i7h67xsrq6owditu Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=c9uD0xpY; spf=pass (imf26.hostedemail.com: domain of naoya.horiguchi@linux.dev designates 188.165.223.204 as permitted sender) smtp.mailfrom=naoya.horiguchi@linux.dev; dmarc=pass (policy=none) header.from=linux.dev X-HE-Tag: 1639990141-284340 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Fri, Dec 10, 2021 at 08:02:08PM +0900, Naoya Horiguchi wrote: > From: Naoya Horiguchi > > When a memory error hits a tail page of a free hugepage, > __page_handle_poison() is expected to be called to isolate the error in > 4kB unit, but it's not called due to the outdated if-condition in > memory_failure_hugetlb(). This loses the chance to isolate the error in > the finer unit, so it's not optimal. Drop the condition. > > This "(p != head && TestSetPageHWPoison(head)" condition is based on the > old semantics of PageHWPoison on hugepage (where PG_hwpoison flag was > set on the subpage), so it's not necessray any more. By getting to set > PG_hwpoison on head page for hugepages, concurrent error events on > different subpages in a single hugepage can be prevented by > TestSetPageHWPoison(head) at the beginning of memory_failure_hugetlb(). > So dropping the condition should not reopen the race window originally > mentioned in commit b985194c8c0a ("hwpoison, hugetlb: > lock_page/unlock_page does not match for handling a free hugepage") > > Reported-by: Fei Luo > Signed-off-by: Naoya Horiguchi > Cc: # v5.14+ > --- > I set v5.14+ for stable trees because the base code was greatly changed > by commit 0ed950d1f281 ("mm,hwpoison: make get_hwpoison_page() call > get_any_page()"), and this patch is not cleanly applicable, although the > original issue was introduced more previously. > --- > mm/memory-failure.c | 21 +++++++-------------- > 1 file changed, 7 insertions(+), 14 deletions(-) > > diff --git a/mm/memory-failure.c b/mm/memory-failure.c > index 8f0ee5b08696..68d9a35f8908 100644 > --- a/mm/memory-failure.c > +++ b/mm/memory-failure.c > @@ -1521,24 +1521,17 @@ static int memory_failure_hugetlb(unsigned long pfn, int flags) > if (!(flags & MF_COUNT_INCREASED)) { > res = get_hwpoison_page(p, flags); > if (!res) { > - /* > - * Check "filter hit" and "race with other subpage." > - */ > lock_page(head); > - if (PageHWPoison(head)) { > - if ((hwpoison_filter(p) && TestClearPageHWPoison(p)) > - || (p != head && TestSetPageHWPoison(head))) { > + if (hwpoison_filter(p)) { > + if (TestClearPageHWPoison(head)) > num_poisoned_pages_dec(); > - unlock_page(head); > - return 0; > - } > + unlock_page(head); > + return 0; > } > unlock_page(head); > - res = MF_FAILED; > - if (__page_handle_poison(p)) { > - page_ref_inc(p); > - res = MF_RECOVERED; > - } > + res = MF_RECOVERED; > + if (!page_handle_poison(p, true, false)) > + res = MF_FAILED; Sorry, I just found that this change broke "HardwareCorrupted" counter because page_handle_poison() calls num_poisoned_pages_inc() so the counter was incremented twice in total by a single error event. page_handle_poison() is supposed to be called only in soft-offline context, and __page_handle_poison() should be called in hard-offline context. Andrew, could you fold the following diff on mm-hwpoison-fix-condition-in-free-hugetlb-page-path.patch in your tree? (Or if desirable, I'll send a updated full patch.) diff --git a/mm/memory-failure.c b/mm/memory-failure.c index 68d9a35f8908..ee51d6410f9a 100644 --- a/mm/memory-failure.c +++ b/mm/memory-failure.c @@ -1529,9 +1529,11 @@ static int memory_failure_hugetlb(unsigned long pfn, int flags) return 0; } unlock_page(head); - res = MF_RECOVERED; - if (!page_handle_poison(p, true, false)) - res = MF_FAILED; + res = MF_FAILED; + if (__page_handle_poison(p)) { + page_ref_inc(p); + res = MF_RECOVERED; + } action_result(pfn, MF_MSG_FREE_HUGE, res); return res == MF_RECOVERED ? 0 : -EBUSY; } else if (res < 0) { Thanks, Naoya Horiguchi