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 3275EC433EF for ; Wed, 24 Nov 2021 00:58:31 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 858406B0075; Tue, 23 Nov 2021 19:58:15 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 807226B0078; Tue, 23 Nov 2021 19:58:15 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6A63A6B007B; Tue, 23 Nov 2021 19:58:15 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0191.hostedemail.com [216.40.44.191]) by kanga.kvack.org (Postfix) with ESMTP id 5B9B46B0075 for ; Tue, 23 Nov 2021 19:58:15 -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 21AE718135C54 for ; Wed, 24 Nov 2021 00:58:05 +0000 (UTC) X-FDA: 78842012130.06.DB26D5A Received: from mail-ed1-f50.google.com (mail-ed1-f50.google.com [209.85.208.50]) by imf18.hostedemail.com (Postfix) with ESMTP id 65A86400209B for ; Wed, 24 Nov 2021 00:58:02 +0000 (UTC) Received: by mail-ed1-f50.google.com with SMTP id o20so2492768eds.10 for ; Tue, 23 Nov 2021 16:58: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=jz4MF7nnznnQm8pZqjbMLHggsWMHGqMDHoC9+F9iqNI=; b=WAOvspr1bGt7DQUK555UUsWtMDX3OBGqMARHkZ2wCCH6Kz3brwWLwfZjrv8bsTmA2Y J55xC5aJLClA2v+AscNbikcNrSmpQXykLAvhTjdkyVcuYSD7/aYGjWUsGIn42k2BVZMM 7BT/FnZa5zqMvgKW72meABK2Fd9tcghhKEq0/VZv3G/El5t6JWTz7pN8GrAFdAz8HPBC DggzCHG30vrSipw8uVybtia0CXelOEB2ChdH7qeI6h8avDsWV5tkRvD5TP+8tx/ebQjH ktDZqdJ/jGNmAGq9JT1nofLeMO6bX2ViPvspcjKLLT5xIIXj3wyGoX9lDHH5X+PuotPt mWxg== 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=jz4MF7nnznnQm8pZqjbMLHggsWMHGqMDHoC9+F9iqNI=; b=P8pEp+HbViab+QCq8tmoYZA/qxdUzF+tmKuWG8UIE4IpU7BLxqztK1HaYfnEQeIwns oqDcZu/5ChIyTHdBgFwdztKmmQXF9WbMw2sTI7twMHAzK226ICZK6ldzZxyZq0wUpR8+ TE7boPlhvX/Gd28YaCQFV9lYt6VoY1kl1kuxtoPcUYSJe5sHLEv+0kWvvu/QE1k7fD+n pD1sMs7PErxTSmxkSbFFuAJ+hmr1rOIbQ8EHu2gSoF2EiHRlzPgP1OXC/S3y9iQ13dwR NLKLp2lueLmzRdiMsXbv96vdytdcQmYdG0Px6roY4nDfhLhGiOfFiR87Ec8KyOFrONwB wJOw== X-Gm-Message-State: AOAM533f+Qw9FXHPG7pFXisgdVauj88E8GBSoWrtoGc3OhRu8BTeTXwN Tbvm4v8mFtVbzcnBbIGTnqZCY+pqLUendo5p8b8= X-Google-Smtp-Source: ABdhPJzqTcBdyWq/aPEOGW3ChiJWyvWlqFiHiHWQz2MB4xmh8j1No1rCPg2Pr7Y7H9YMnLtS/mAEgBiGhHozsw9CJWg= X-Received: by 2002:a50:fb09:: with SMTP id d9mr17126644edq.283.1637715483479; Tue, 23 Nov 2021 16:58:03 -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: From: Yang Shi Date: Tue, 23 Nov 2021 16:57:51 -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-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 65A86400209B X-Stat-Signature: xhkxtrffrgph651iqs6rgpo8srbcpjas Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=WAOvspr1; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf18.hostedemail.com: domain of shy828301@gmail.com designates 209.85.208.50 as permitted sender) smtp.mailfrom=shy828301@gmail.com X-HE-Tag: 1637715482-704441 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:32 PM Yang Shi wrote: > > 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 individuall= y > > > > 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. O= r > > > 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 *v= mf, > > > 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 *v= mf, > > > do_set_pte(vmf, page, addr); > > > /* no need to invalidate: a not-present page won't be= cached */ > > > 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. Err... I think I misread the code. It does iterate every subpages to map each of them by PTE and just skip the hwpoisoned subpages. Sorry for the confusion. > > 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= page *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 > > > >