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 646FAC433EF for ; Tue, 1 Feb 2022 17:47:33 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C30EE6B0254; Tue, 1 Feb 2022 12:47:32 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id BDF3F6B0255; Tue, 1 Feb 2022 12:47:32 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id ACDBC6B0256; Tue, 1 Feb 2022 12:47:32 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0038.hostedemail.com [216.40.44.38]) by kanga.kvack.org (Postfix) with ESMTP id 9B8606B0254 for ; Tue, 1 Feb 2022 12:47:32 -0500 (EST) Received: from smtpin06.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 49BED182751A2 for ; Tue, 1 Feb 2022 17:47:32 +0000 (UTC) X-FDA: 79094943144.06.A7992F5 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf15.hostedemail.com (Postfix) with ESMTP id 17EB2A0003 for ; Tue, 1 Feb 2022 17:47:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=FIJD2o4yQZlq3gVuICmPW8oWwoPVQauDL+h4yMAdXqI=; b=XxBpUg6E66ImHa7iBb8/adgNOt E8K+fklKMun7RuEISINCxOrQ9hsxh5Emb4eVprAuGHaxRHTdkP4emK15UtTjDVtdl8EZ3cjJTF62D dIcOqFQZkVMuhKZ5+mkLcxKDbx3bAQZmqmPQ/88QaEBaN9QZ83/YA28vWExOz2D9flUa9wqWHQbmb rRFgC88BGa7Rnc1MOmifmf9fgG2otVBkbJMvvU1EoNPYIp/FdHsgyJc5JVzBAezAx9rCRclk9Monq AsHzVuO17CqWLRKdJzOqmdhU9e2EXAjzFX0o/6cek9F5nUhu0A11ggVBdZGB9tePA/6VLD0HYbYli 1aBXrDYw==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1nExFQ-00CmXz-1o; Tue, 01 Feb 2022 17:47:20 +0000 Date: Tue, 1 Feb 2022 17:47:20 +0000 From: Matthew Wilcox To: Joao Martins Cc: Jane Chu , Naoya Horiguchi , linux-mm@kvack.org, Andrew Morton , Dan Williams Subject: Re: [RFC] Missing compound_head() in memory-failure Message-ID: References: <30a327fd-ae49-7412-ef77-9ec19480626e@oracle.com> <74d4bd51-6f16-542d-9710-3c609346cfe7@oracle.com> <54f9f4bc-48ca-5c18-d923-a779c2c80b86@oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <54f9f4bc-48ca-5c18-d923-a779c2c80b86@oracle.com> X-Rspamd-Queue-Id: 17EB2A0003 X-Rspam-User: nil Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=XxBpUg6E; dmarc=none; spf=none (imf15.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org X-Stat-Signature: 5ez4kp4h5rdq9a1c57fmyi559jxn9e5i X-Rspamd-Server: rspam08 X-HE-Tag: 1643737650-900815 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 Tue, Feb 01, 2022 at 05:40:57PM +0000, Joao Martins wrote: > On 2/1/22 16:01, Joao Martins wrote: > > On 2/1/22 15:46, Matthew Wilcox wrote: > >> On Mon, Jan 31, 2022 at 08:54:39PM +0000, Joao Martins wrote: > >>> On 1/31/22 20:29, Matthew Wilcox wrote: > >>>> Unless I am mistaken, you have to pass the compound head of the page > >>>> which has the error to collect_procs(). Am I mistaken? > >>>> > >>> -rc2 already has a fix for it: > >>> > >>> https://lore.kernel.org/linux-mm/20220129021420.PgBIZm-q9%25akpm@linux-foundation.org/ > >>> > >>> Earlier in that function there's a: > >>> > >>> page = compound_head(page); > >>> > >>> So the @page passed to collect_procs() already is a head page. > >> > >> It's wrong though ;-( You set the HWPoison bit on the page after > >> calling compound_head(), so you set the bit on the head page instead > >> of the precise page that had the poison. > >> > > Considering that on device-dax we would unmap the whole 2M page regardless > > of the poisoned subpage isn't that actually representative still? > > > > To say this another way. We do set the HWPoison on the head page, > and not the subpage as you say, but we end up propagating the resultant > MCE action on the superset of pages in the whole PMD or PUD. > > What I was trying to say in perhaps a convoluted way is that device-dax > case isn't different than HugeTLB that only wants to poison head. If there's > a head page, there's likely a PMD or PUD populated (depending what the device > was onlined with) and thus that's what gets unmapped. There's no idea of > subpages being treated any differently, at least as far as device-dax is > concerned -- unless I miss auditing some other code path. Using HugeTLB as a model is not a good idea. THP is the model to aim for; one can choose to map pages askew (ie not aligned with a PMD), and I don't think you'll find all the mappings with the current code (eg if someone has mapped a single page of the hugepage). > fsdax IIUC seems to rely more on the subpage bit being flagged but no > functional change here for fsdax as there's only base pages there (no heads). > > >> I'm fixing this up as part of the folio patches, but you may wish to > >> fix it earlier than that. > (+Dan in case I misrepresented or missed something) > > Should we deem it a problem, I'll fix for the next -rc. > Just in case, here's the diff stashed: > > diff --git a/mm/memory-failure.c b/mm/memory-failure.c > index 2e2f740c63dc..661c23df8115 100644 > --- a/mm/memory-failure.c > +++ b/mm/memory-failure.c > @@ -1577,7 +1577,7 @@ static int memory_failure_hugetlb(unsigned long pfn, int flags) > static int memory_failure_dev_pagemap(unsigned long pfn, int flags, > struct dev_pagemap *pgmap) > { > - struct page *page = pfn_to_page(pfn); > + struct page *page = pfn_to_page(pfn), *subpage = page; > unsigned long size = 0; > struct to_kill *tk; > LIST_HEAD(tokill); > @@ -1631,7 +1631,7 @@ static int memory_failure_dev_pagemap(unsigned long pfn, int flags, > * Use this flag as an indication that the dax page has been > * remapped UC to prevent speculative consumption of poison. > */ > - SetPageHWPoison(page); > + SetPageHWPoison(subpage); > > /* > * Unlike System-RAM there is no possibility to swap in a