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 X-Spam-Level: X-Spam-Status: No, score=-3.5 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0BAA9C433E0 for ; Wed, 3 Mar 2021 21:22:53 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id E4C5B64EF8 for ; Wed, 3 Mar 2021 21:22:50 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E4C5B64EF8 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 2D7036B0007; Wed, 3 Mar 2021 16:22:50 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 2608E6B0008; Wed, 3 Mar 2021 16:22:50 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0DAD66B000A; Wed, 3 Mar 2021 16:22:50 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0061.hostedemail.com [216.40.44.61]) by kanga.kvack.org (Postfix) with ESMTP id E33D76B0007 for ; Wed, 3 Mar 2021 16:22:49 -0500 (EST) Received: from smtpin16.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id A8AEC1836B09B for ; Wed, 3 Mar 2021 21:22:49 +0000 (UTC) X-FDA: 77879837658.16.BA0BD0B Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf16.hostedemail.com (Postfix) with ESMTP id D3AA5801A80D for ; Wed, 3 Mar 2021 21:22:44 +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=6YnCoO0+G2JF2F+BhHRjoqQWT6hCxoMjV93OHsnj2Pg=; b=nh1cxFtXR32Q1Ey2dAz7CqL1N3 2CMb5CwfQLoCfYSwEp4o55HGkgT32RAwoFB51aHrjYX2f8b2wouPlavwKDNBimQOQhk8Zzruwmvws k+wWjIe0v0RFc3Ghh+sbpaT09iPg4g20XZTCNyPelQC1RAcX9QmZzx9YkpT+/sGTh8FqPOG+02Uva AchBiKSDQoJISjse1ynnHR1VCSGv/fthNzwJo9su/fXw/ZM0u2mF0oeqZZbq/ofO8R3pGCO2CzYoK gyj0Jd+oBmG4ddh5jLgeqc7kDhgV5EDYbRr7rYt7itWS3M4jhful0lN2BjynzkhpeSvgdYsbB1OdY BjbZaPpg==; Received: from willy by casper.infradead.org with local (Exim 4.94 #2 (Red Hat Linux)) id 1lHYYq-004Amt-2e; Wed, 03 Mar 2021 20:57:37 +0000 Date: Wed, 3 Mar 2021 20:57:36 +0000 From: Matthew Wilcox To: Andrew Morton Cc: linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, Kent Overstreet , Christoph Hellwig Subject: Re: [PATCH] mm/filemap: Use filemap_read_page in filemap_fault Message-ID: <20210303205736.GG2723601@casper.infradead.org> References: <20210226140011.2883498-1-willy@infradead.org> <20210302173039.4625f403846abd20413f6dad@linux-foundation.org> <20210303013313.GZ2723601@casper.infradead.org> <20210302220735.1f150f28323f676d2955ab49@linux-foundation.org> <20210303132640.GB2723601@casper.infradead.org> <20210303121253.9f44d8129f148b1e2e78cc81@linux-foundation.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210303121253.9f44d8129f148b1e2e78cc81@linux-foundation.org> X-Stat-Signature: 4qtym53dc1hxc359uuip3313u3oa6bey X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: D3AA5801A80D Received-SPF: none (infradead.org>: No applicable sender policy available) receiver=imf16; identity=mailfrom; envelope-from=""; helo=casper.infradead.org; client-ip=90.155.50.34 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1614806564-242544 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 Wed, Mar 03, 2021 at 12:12:53PM -0800, Andrew Morton wrote: > On Wed, 3 Mar 2021 13:26:40 +0000 Matthew Wilcox wrote: > > > But here's the thing ... invalidate_mapping_pages() doesn't > > ClearPageUptodate. The only places where we ClearPageUptodate is on an > > I/O error. > > yup. > > > So ... as far as I can tell, the only way to hit this is: > > > > - Get an I/O error during the wait > > - Have another thread cause the page to be removed from the page cache > > (eg do direct I/O to the file) before this thread is run. > > > > and the consequence to this change is that we have another attempt to > > read the page instead of returning an error immediately. I'm OK with > > that unintentional change, although I think the previous behaviour was > > also perfectly acceptable (after all, there was an I/O error while trying > > to read this page). > > > > Delving into the linux-fullhistory tree, this code was introduced by ... > > > > commit 56f0d5fe6851037214a041a5cb4fc66199256544 > > Author: Andrew Morton > > Date: Fri Jan 7 22:03:01 2005 -0800 > > > > [PATCH] readpage-vs-invalidate fix > > > > A while ago we merged a patch which tried to solve a problem wherein a > > concurrent read() and invalidate_inode_pages() would cause the read() to > > return -EIO because invalidate cleared PageUptodate() at the wrong time. > > > > We no longer clear PageUptodate, so I think this is stale code? Perhaps > > you could check with the original author ... > > Which code do you think might be stale? We need the !PageUptodate > check to catch IO errors and we need the !page->mapping check to catch > invalidates. Am a bit confused. I think the check of !page->mapping here: if (PageUptodate(page)) return 0; if (!page->mapping) /* page truncated */ return AOP_TRUNCATED_PAGE; is no longer needed. If we didn't see an error, the page will be Uptodate, regardless of whether it's been removed from the page cache. If we did see an error, it's OK to return -EIO, even if the page has been removed from the page cache in the interim.