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 AE843C433EF for ; Wed, 24 Nov 2021 00:33:31 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E8E5D6B0075; Tue, 23 Nov 2021 19:33:15 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id E3D206B0078; Tue, 23 Nov 2021 19:33:15 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D2BDB6B007B; Tue, 23 Nov 2021 19:33:15 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0201.hostedemail.com [216.40.44.201]) by kanga.kvack.org (Postfix) with ESMTP id C470F6B0075 for ; Tue, 23 Nov 2021 19:33:15 -0500 (EST) Received: from smtpin20.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 8FA848D6FF for ; Wed, 24 Nov 2021 00:33:05 +0000 (UTC) X-FDA: 78841949130.20.4D8E5C7 Received: from mail-ed1-f45.google.com (mail-ed1-f45.google.com [209.85.208.45]) by imf29.hostedemail.com (Postfix) with ESMTP id 9ABD19000ECE for ; Wed, 24 Nov 2021 00:33:02 +0000 (UTC) Received: by mail-ed1-f45.google.com with SMTP id z5so2442944edd.3 for ; Tue, 23 Nov 2021 16:33:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=SadW2vcFUt1XrgyHo7rNQAbxEHua7lFEAF+FKRSfsTo=; b=ALefVNOPiNZEsWpvCHqq+d1b0Eh/PvAF8fvSJkcsTMrIha+W2PoJ5y0HhvGvLDHxe3 XDnPsqLOg4Vj070AM1KgqAWG/TCWCkpInTTK0+DwPWgpc0hd4A96DY5nttdTfCI7nRKK scgZW0feWuPZQsLElgdptP1AeqxK7D/awfLYaZDFD3UBgenDf3MrJWEEo2O50iuO/UXq ovBJATl3Oa/2ePWez2tVE5ar87Fp5y8kNkoYV749O3ZrD4lJL4sQCG19ugr+rr14fzcw PmP3Ozy1+knSgeGvzmZPYO2S/JtwdR6CSi4yeCsvIuZOSrY9qR2XnCq6g+20+gt+REkR UyQQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=SadW2vcFUt1XrgyHo7rNQAbxEHua7lFEAF+FKRSfsTo=; b=Dh89VOv+tO062NlaQ8NtmLkKqkjCNolODahLEvBysJSqvEwGB4zJiNeK4U3SMfkS8y csUJShWFBrbSVktD6708jvmTf9uymZLTswlP5xxgY3Uvvv0U5WU317jbGIPnX0srBgI6 /wYiT4d6qH3hdMwMQzDaLPiqrHQnaaJrsqs5F3QyUaoK/PTInyeInPr9Ld39U35ZIO+p aZvwNa3oDfLHXazAFxZfFnmsigGA/YXK6M/jBYVnek5damfgBTIPXCGfQMjq9mmM4dNS C6xJIOE9+2nuymA2W6PtjCHJBBsVptZ4DH18bN4QKO7K7AvGHamWr9n6UswpiK0dLB9A u80A== X-Gm-Message-State: AOAM530QwdL9enbABUEUQNuMfH5hYPBkXPv78O+2teZuBZwcAa00PA5g F5MgjcUZISgNOYdSCmYRbH8Q5HNbbU33kpUhF6I= X-Google-Smtp-Source: ABdhPJxi8xeeG66UmgEi9+8kDUgoe4mg1u4323cus3yTewE2awuWtME9RRRKwaj8gjfnPhf7OzR3Y5NjUbXkMKIH2sc= X-Received: by 2002:a05:6402:1a4d:: with SMTP id bf13mr17025487edb.101.1637713984011; Tue, 23 Nov 2021 16:33:04 -0800 (PST) MIME-Version: 1.0 References: <20211120174429.2596303-1-willy@infradead.org> <20211124001113.GA2122045@hori.linux.bs1.fc.nec.co.jp> In-Reply-To: <20211124001113.GA2122045@hori.linux.bs1.fc.nec.co.jp> From: Yang Shi Date: Tue, 23 Nov 2021 16:32:52 -0800 Message-ID: Subject: Re: [PATCH] filemap: Remove PageHWPoison check from next_uptodate_page() To: =?UTF-8?B?SE9SSUdVQ0hJIE5BT1lBKOWggOWPoyDnm7TkuZ8p?= Cc: "Matthew Wilcox (Oracle)" , "Kirill A . Shutemov" , Andrew Morton , Linux MM , Hugh Dickins Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: ge74iebz6z5zukbb5i5r84t7ssmpbd5b X-Rspamd-Queue-Id: 9ABD19000ECE X-Rspamd-Server: rspam07 Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=ALefVNOP; spf=pass (imf29.hostedemail.com: domain of shy828301@gmail.com designates 209.85.208.45 as permitted sender) smtp.mailfrom=shy828301@gmail.com; dmarc=pass (policy=none) header.from=gmail.com X-HE-Tag: 1637713982-924791 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, Nov 23, 2021 at 4:11 PM HORIGUCHI NAOYA(=E5=A0=80=E5=8F=A3=E3=80=80= =E7=9B=B4=E4=B9=9F) wrote: > > On Mon, Nov 22, 2021 at 11:28:10AM -0800, Yang Shi wrote: > > On Sat, Nov 20, 2021 at 9:44 AM Matthew Wilcox (Oracle) > > wrote: > > > > > > Pages are individually marked as suffering from hardware poisoning. > > > Checking that the head page is not hardware poisoned doesn't make > > > sense; we might be after a subpage. We check each page individually > > > before we use it, so this was an optimisation gone wrong. > > > > Yeah, it doesn't make too much sense to check the head page. And it > > seems the non-poisoned subpages could be PTE mapped instead of > > skipping the whole THP. > > > > Not sure if this is by design, it seems the hwpoisoned check in > > filemap_map_pages() does skip the subpages after the poisoned page. Or > > we should just skip the poisoned page itself? If so the below change > > may be needed: > > > > diff --git a/mm/filemap.c b/mm/filemap.c > > index daa0e23a6ee6..f1f0cb263b4a 100644 > > --- a/mm/filemap.c > > +++ b/mm/filemap.c > > @@ -3318,7 +3318,7 @@ vm_fault_t filemap_map_pages(struct vm_fault *vmf= , > > do { > > page =3D find_subpage(head, xas.xa_index); > > if (PageHWPoison(page)) > > - goto unlock; > > + goto skip; > > > > if (mmap_miss > 0) > > mmap_miss--; > > @@ -3337,6 +3337,7 @@ vm_fault_t filemap_map_pages(struct vm_fault *vmf= , > > do_set_pte(vmf, page, addr); > > /* no need to invalidate: a not-present page won't be c= ached */ > > update_mmu_cache(vma, addr, vmf->pte); > > +skip: > > unlock_page(head); > > continue; > > unlock: > > first_map_page() or next_map_page() returns a page (if found) with > holding the refcount, and the new 'goto skip' path skips releasing it. > So this looks to me lead to the mismatch of refcount. > Could you explain the intention a little more (maybe related to your > recent patch about keeping hwpoison page in pagecache?) ? No, not related to my patches. The current code maps the subpages by PTEs *before* the poisoned page, but skips the subpages *after* the poisoned page IIUC. It seems not right, I thought the code was intended to map all subpages by PTEs except the poisoned pages. So the suggested code is trying to fix the misbehavior. That code is just a quick and untested illustration to the above hypothesis. The corrected version: diff --git a/mm/filemap.c b/mm/filemap.c index daa0e23a6ee6..1a76e3edc878 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -3317,8 +3317,11 @@ vm_fault_t filemap_map_pages(struct vm_fault *vmf, vmf->pte =3D pte_offset_map_lock(vma->vm_mm, vmf->pmd, addr, &vmf->= ptl); do { page =3D find_subpage(head, xas.xa_index); - if (PageHWPoison(page)) - goto unlock; + if (PageHWPoison(page)) { + unlock_page(page); + put_page(page); + continue; + } if (mmap_miss > 0) mmap_miss--; > > Thanks, > Naoya Horiguchi > > > > > > > > > > > Signed-off-by: Matthew Wilcox (Oracle) > > > --- > > > mm/filemap.c | 2 -- > > > 1 file changed, 2 deletions(-) > > > > > > diff --git a/mm/filemap.c b/mm/filemap.c > > > index 0b6f996108b4..65973204112d 100644 > > > --- a/mm/filemap.c > > > +++ b/mm/filemap.c > > > @@ -3239,8 +3239,6 @@ static struct page *next_uptodate_page(struct p= age *page, > > > goto skip; > > > if (!PageUptodate(page) || PageReadahead(page)) > > > goto skip; > > > - if (PageHWPoison(page)) > > > - goto skip; > > > if (!trylock_page(page)) > > > goto skip; > > > if (page->mapping !=3D mapping) > > > -- > > > 2.33.0 > > >