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 5260FE784BF for ; Mon, 2 Oct 2023 16:02:13 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C61DD8D0047; Mon, 2 Oct 2023 12:02:12 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C12088D000E; Mon, 2 Oct 2023 12:02:12 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id ADD048D0047; Mon, 2 Oct 2023 12:02:12 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 9E0B78D000E for ; Mon, 2 Oct 2023 12:02:12 -0400 (EDT) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 45C3D1CA157 for ; Mon, 2 Oct 2023 16:02:11 +0000 (UTC) X-FDA: 81300988062.29.0BDBB93 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf02.hostedemail.com (Postfix) with ESMTP id 16FC280071 for ; Mon, 2 Oct 2023 16:02:03 +0000 (UTC) Authentication-Results: imf02.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=E6lZ4CUI; spf=none (imf02.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1696262524; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=vPWjZhtEz0XZOfaHC6v5q0++fyRvHTSEI8HhpIfDjOA=; b=yJw3bce/VYpS3trq86Y5fC6X/lAjseRBimb2mbrTfN1YsFqsL9XddqilDBXH3KFw+7bC45 cih0WAUI/kySFAn1Lxqg2QNE9V1T5LSHRNUSq7jURxVF7iWTMHca684Oz9XILaOOBmBzoR 22WPfNyyZf+vsDX9nxoecbXU3Kgi/6I= ARC-Authentication-Results: i=1; imf02.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=E6lZ4CUI; spf=none (imf02.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1696262524; a=rsa-sha256; cv=none; b=0KZPV+QXXSObAz7ca1ZUURjDLW0cPJP18kONRAEwvZFxXHGDEEdNkGTe7Oxbysq8WsF92T LTQeO0oLs2SjnejVZAtnocEz94ZIGaIgeWnHMeKRp1Ly5KVVekVGzgkS6S/00OyAkNizmt hbXOTEshDTkOdKT4Gq2iQxpXAL+1dow= 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=vPWjZhtEz0XZOfaHC6v5q0++fyRvHTSEI8HhpIfDjOA=; b=E6lZ4CUI7Wr7e0ZDL5o1kKW2iH 8dZp176IPQmhLWHsCYyq+xODP0IoDOkbBLN2HdkbJw41kE7l1ko/zDp/jpboryWxKxSG5KZHC8pZI VE9MUGh6v0h+FW4wV3opPFMo19kgDo9xm+gWXHqJKAcApZoRkJ6e8MrJ+jN6GhpeLeg67YWNCdRUR zQVR6O/phSWirUIYJ7/SlxxHCO3XguVu12sPxqU5kFhLlB9ihyNpL4gSeHaDXuCziSrGsjTWQ6Rol sksg8yoe7FlsZOWsbkCGn95h6EAdP01ExcE6YEph7I69sGRyEpsHt9iOJmJctkRRmXxcT8lX7H/qc mGMv4mGA==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1qnLMu-009vnO-6t; Mon, 02 Oct 2023 16:02:00 +0000 Date: Mon, 2 Oct 2023 17:02:00 +0100 From: Matthew Wilcox To: Lorenzo Stoakes Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrew Morton , Jan Kara Subject: Re: [PATCH] mm/filemap: clarify filemap_fault() comments for not uptodate case Message-ID: References: <20230930231029.88196-1-lstoakes@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230930231029.88196-1-lstoakes@gmail.com> X-Rspamd-Queue-Id: 16FC280071 X-Rspam-User: X-Stat-Signature: nhfi4edabekrged6dj1yxtkixiuyr1nt X-Rspamd-Server: rspam01 X-HE-Tag: 1696262523-456997 X-HE-Meta: U2FsdGVkX1+isL5gMWZPpKs2OUyhzMnbvUo+56JW1K35Qk0F8m2yBJtIyUDUYpM3AA7ego8o+LFa6gqR0/2AMx267YhcYOAaSU7Pp5JeRwkLc104MqNRi66wbcCtRhxQ9niAbGJERKagvwboxhXPybqwurn5i3tJ3vt2Rom9+kIQPVIlt+j5uPiDDkqfe0/9OL+4a1we8mURtx6DnYR23Q4CGtUdVo2hWgJycc0hTOj9sAfH1qDEQaDsf6OyhMCI4j+W5y4lPReVRHK03DzXLSyV0xx30Q6kKn8VbRR2ZPJkvno+UtKATOSw2SD4gwyyXgf8F5YwvSzbsvyPJf2ULgFqjrthDLKk0eNKnhLKKV3trWbXZozFtngjXz2ZbniRI9zOH9WZYnVF3NDm8HKX4dtnZ5/BrzyZhq6+HKFoqSnpVP3N1K7xCEYdleOeWF4VrAJYXXYJzgQz+D4MQxJadjnw0qjZawBhYkzPpFN0GLEbVZVf84ucf4XzLAcayqf5SZR0STSjONlAKJDOTV32ggCnSqwQjaS7YCXYO+0bW7hye5EsbtyJeKMPJBfVvQxVYuZyDFFUNvJ30W329ACks7wdzAX+9LgibOC4cMAuK0sI0Zn0oR1rpwLUbn5ar/g5AkwlumgzJ+qQXuJ3RbejJ6KAxU9d24iT5uSybW6yAVx3XOwmrGHII+aRUeVb1slo1zDtSiuZvZFBYF/8iDBDRT/CcO2eM+3CRTCU2twISjJFMswud918bEbonEh/iRysgEKd3ZYJyK3Ahn9OHJZ6b91mFo0s6DR1uA/PonvxxBVgpB18lJkJpgQcSP4RYUA4jUv2rsedaccn4Y5kIJJHx25PgImpgx9tiVdnz9nLmzUR8jtJ5JIm54GgfPsVZSOYTjTCzR12cbM2xEPUYtJghRiOGMw+3mKfQdQWKwO2bpMR5J3zCt3yKimmXsYKvZvONq7M2iqnTiKX4qgDEJD 9fu6dQpZ NityI1nMEP6V2ODJ3UIFDG+++8+mFtaoygcV2+6XzWonOQ7IjZ5GnSboAhFDAPKwQ7ojMnN8Sr0UmrHAkqVD5/2OZ0igWY20pHCRRDpQUMpZoJsvfLACaXgCo++mgFOaEpGgWlUQT2SPwnQmYqfFihDQu3OG6CpEEQgQBqx9Zf+F4uX+TdIH41zh+Pw== 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 Sun, Oct 01, 2023 at 12:10:29AM +0100, Lorenzo Stoakes wrote: > The existing comments in filemap_fault() suggest that, after either a minor > fault has occurred and filemap_get_folio() found a folio in the page cache, > or a major fault arose and __filemap_get_folio(FGP_CREATE...) did the job > (having relied on do_sync_mmap_readahead() or filemap_read_folio() to read > in the folio), the only possible reason it could not be uptodate is because > of an error. > > This is not so, as if, for instance, the fault occurred within a VMA which > had the VM_RAND_READ flag set (via madvise() with the MADV_RANDOM flag > specified), this would cause even synchronous readahead to fail to read in > the folio. > > I confirmed this by dropping page caches and faulting in memory madvise()'d > this way, observing that this code path was reached on each occasion. > > Clarify the comments to include this case, and additionally update the > comment recently added around the invalidate lock logic to make it clear > the comment explicitly refers to the minor fault case. I do appreciate the comment being made accurate, but I wonder if we shouldn't change the code to match the comment. We're got two "should"s pointing in different directions -- we "should" not use readahead if readahead is disabled, and we "should" always use readahead first, using read_folio() only if readahead doesn't succeed. The code isn't ideally structured for this, but I'm going to play with it a bit and see what I can create.