Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Alistair Popple <apopple@nvidia.com>
To:     akpm@linux-foundation.org
Cc:     linux-mm@kvack.org, linux-kernel@vger.kernel.org,
        willy@infradead.org, Alistair Popple <apopple@nvidia.com>
Subject: [PATCH] mm/filemap.c: Always read one page in do_sync_mmap_readahead()
Date:   Tue,  7 Jun 2022 18:37:14 +1000
Message-Id: <20220607083714.183788-1-apopple@nvidia.com>
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?AjUU0wOcSO54SgPTDMsqLZe5TaDDuwt0CKwRiLV106PzlAgVf5maNUIh2gCa?=
 =?us-ascii?Q?hU5i4GmesnDwGe+9fsyTvaPXKr1Lql2IWJm67uGKRDHACKxJX3PJ+QwGoNTC?=
 =?us-ascii?Q?VaXBzW86MGOrUNn0Rs7WosnGGTQr6cdKlNQ1NdqjXV6Vy65rH/anEZ1NZ+2l?=
 =?us-ascii?Q?bs3u/VIgaVj7/VYyBV0Tlv7pBSYp9tgBUcGLnyE+Joz2Mw/BC+Q75ENY8xWX?=
 =?us-ascii?Q?TyrI+UVtYzH5gkOXFsu3HUwWXpfavJpEDaBi7sP2VWjmevKXMAaxs0s8gtU8?=
 =?us-ascii?Q?rZHgrkdDiy+tmUdS9dh7fwH0snn4f34/2R0D255V058sZV0srQ503jk/i2dN?=
 =?us-ascii?Q?iXrJZ9nNeQjpn+xBpmi21pTb3q2ouRA+dafPdnU15Xzp/ci34YDZHiIFhuDF?=
 =?us-ascii?Q?dZuAQHfmlF/8irBvnVHtfCek3rbFUlIFN2tH0CooF7c29mSg3+UZRekD6rAF?=
 =?us-ascii?Q?jKpBeB1/Ze/5i1UxaFkIVphWSUe5xkHh4ZoYk7nk8wz8pCtAXKyqkikj7YMh?=
 =?us-ascii?Q?vtqlnJdZxKH7D4zF/8FIR06GbCRnl5cyQHzkFoCwJac7fbnboJY8AonngHAt?=
 =?us-ascii?Q?Z2V2FsOQ6K2x2HIgFgwZGiGro7h4Jqy0k8KiNaYHFsnzZBF98Ybt/pZnDSjf?=
 =?us-ascii?Q?ZM045fd5VU203pLqV4PsVKQjIogfm59v4T460w+iQv12tRtBOvSGIpKA+VzP?=
 =?us-ascii?Q?CwqAlzRXsyzRbiGx0phPWAbH++kp8+d3meE9yFssAIkcAPntn8FA/YvMY8RM?=
 =?us-ascii?Q?5El/JzlZOEyFUTYREXtD2sCn6/jDbl8laQ7Pg7sK/SNaa4clCfexDEL9BpYM?=
 =?us-ascii?Q?zUS9kYc3Wax/g9UI5UlesAUdSofeHCFjuTxdQ+lOVchW0+gRYpNCjTwBXSaL?=
 =?us-ascii?Q?aiFlkU9Il3TpHe9X55SrlsHHuRNE8Sycc8mwp+ZQwz6Hfsj6f5GA7x7euttE?=
 =?us-ascii?Q?EJlMsPbkkZKhLwgCbFuNvgVhuBTQHkdlpd2vn0twytBKzcSWueuOG5zO/nUI?=
 =?us-ascii?Q?e5l74t6qRXly+h98DRwbz09438JTx4aQqic6R4xK7tju+xN0X8+QpRKJXKXO?=
 =?us-ascii?Q?Ix9di3g4arCr/muN15rc1FQQ0m3EXGyn71IXv6JOSZwYq7yphhtGuWCZ1SXq?=
 =?us-ascii?Q?Csfk++SknKJNDUS1ufT7MoXGzwDOGp3umfdXYVIPERMmN1ktACgEWAP7ynk5?=
 =?us-ascii?Q?brbzDQ83U9lbCFaZZu3RVHiN1bk1rmKQwvxOAk6+nC2jKRU49dhzzU7Adutz?=
 =?us-ascii?Q?FumSbalVa7dEwF2fB2wuCfu/5pg2L496G9fH8xmNnaOHtXeg4TEnLPpIC4J5?=
 =?us-ascii?Q?EunT5EFgfaBpQ4B8YhTOhKAdT+627i1doZV4lSOiw9E9uLfUHik3lj7CjzPK?=
 =?us-ascii?Q?xWZRb1NgF3HjHj6ZfvnajE+9te5fRwUhBcMoIEjMbXK6p8Vx/Lf1dsyxSeNB?=
 =?us-ascii?Q?UM+2t22Vlt8Os780tb+2EByRkx+sOBxNsWUkyY5MEpmyfmfvSGPZeRYvUuYL?=
 =?us-ascii?Q?bVf42p9YUNwHLW/R/vYU9MKB197XGsMqcbPUiyHMnmILePBsetK4FPREPyLC?=
 =?us-ascii?Q?VfdBjlzzdpCGUib+peXIaIGOtIYLM/pXebNpUtjuknbDhnPC1/Pdz9DE9nqE?=
 =?us-ascii?Q?WIGv/GeuPdZF0U/xexe1C8PE/DBk5D/NTvgp/rxtjZohqD/7WoxMho2DWSSD?=
 =?us-ascii?Q?Gf0/KMmdy0LoMtvziJ4SY1TBUtvK5COY8j+Rx1ywZ2gzwHzaqhuSc/6v5mPs?=
 =?us-ascii?Q?pSzRIi+adQ=3D=3D?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 559140f5-de26-444f-6733-08da4860f6ea
X-MS-Exchange-CrossTenant-AuthSource: BYAPR12MB3176.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Jun 2022 08:37:32.4213
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: iR5hvgM9WsaNdubcUJIWb4mcZcFAjHA9Vo7b3ZUD2AvuS3D8kTcPaYtOMRxjPPQFW3BuInoiJE7WDtzZQS6zpw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR12MB5543
X-Spam-Status: No, score=-2.3 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

filemap_fault() calls do_sync_mmap_readahead() to read pages when no
page is found in the page cache. However do_sync_mmap_readahead() will
not actually read any pages if VM_RAND_READ is specified or if there
have been a lot of page cache misses.

This means filemap_fault() will see a folio that is not up-to-date which
is treated as an IO error. The IO error handling path happens to make
VM_RAND_READ work as expected though because it retries reading the
page.

However it would be cleaner if this was handled in
do_sync_mmap_readahead() to match what is done for VM_HUGEPAGE. Also as
do_sync_mmap_readahead() adds the newly allocated folio to the pagecache
and unlocks it this clean-up also allows the FGP_FOR_MMAP flag to be
removed.

Signed-off-by: Alistair Popple <apopple@nvidia.com>
Suggested-by: Matthew Wilcox <willy@infradead.org>
---
 include/linux/pagemap.h |  7 +++---
 mm/filemap.c            | 47 +++++++++++++----------------------------
 2 files changed, 18 insertions(+), 36 deletions(-)

diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index 993994cd943a..e0e0f5e7d4a0 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -505,10 +505,9 @@ pgoff_t page_cache_prev_miss(struct address_space *mapping,
 #define FGP_WRITE		0x00000008
 #define FGP_NOFS		0x00000010
 #define FGP_NOWAIT		0x00000020
-#define FGP_FOR_MMAP		0x00000040
-#define FGP_HEAD		0x00000080
-#define FGP_ENTRY		0x00000100
-#define FGP_STABLE		0x00000200
+#define FGP_HEAD		0x00000040
+#define FGP_ENTRY		0x00000080
+#define FGP_STABLE		0x00000100
 
 struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index,
 		int fgp_flags, gfp_t gfp);
diff --git a/mm/filemap.c b/mm/filemap.c
index 9a1eef6c5d35..15d7e0a0ad4b 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -1922,9 +1922,6 @@ static void *mapping_get_entry(struct address_space *mapping, pgoff_t index)
  * * %FGP_CREAT - If no page is present then a new page is allocated using
  *   @gfp and added to the page cache and the VM's LRU list.
  *   The page is returned locked and with an increased refcount.
- * * %FGP_FOR_MMAP - The caller wants to do its own locking dance if the
- *   page is already in cache.  If the page was allocated, unlock it before
- *   returning so the caller can do the same dance.
  * * %FGP_WRITE - The page will be written to by the caller.
  * * %FGP_NOFS - __GFP_FS will get cleared in gfp.
  * * %FGP_NOWAIT - Don't get blocked by page lock.
@@ -1993,7 +1990,7 @@ struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index,
 		if (!folio)
 			return NULL;
 
-		if (WARN_ON_ONCE(!(fgp_flags & (FGP_LOCK | FGP_FOR_MMAP))))
+		if (WARN_ON_ONCE(!(fgp_flags & FGP_LOCK)))
 			fgp_flags |= FGP_LOCK;
 
 		/* Init accessed so avoid atomic mark_page_accessed later */
@@ -2007,13 +2004,6 @@ struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index,
 			if (err == -EEXIST)
 				goto repeat;
 		}
-
-		/*
-		 * filemap_add_folio locks the page, and for mmap
-		 * we expect an unlocked page.
-		 */
-		if (folio && (fgp_flags & FGP_FOR_MMAP))
-			folio_unlock(folio);
 	}
 
 	return folio;
@@ -3011,14 +3001,8 @@ static struct file *do_sync_mmap_readahead(struct vm_fault *vmf)
 	}
 #endif
 
-	/* If we don't want any read-ahead, don't bother */
-	if (vmf->vma->vm_flags & VM_RAND_READ)
-		return fpin;
-	if (!ra->ra_pages)
-		return fpin;
-
+	fpin = maybe_unlock_mmap_for_io(vmf, fpin);
 	if (vmf->vma->vm_flags & VM_SEQ_READ) {
-		fpin = maybe_unlock_mmap_for_io(vmf, fpin);
 		page_cache_sync_ra(&ractl, ra->ra_pages);
 		return fpin;
 	}
@@ -3029,19 +3013,20 @@ static struct file *do_sync_mmap_readahead(struct vm_fault *vmf)
 		WRITE_ONCE(ra->mmap_miss, ++mmap_miss);
 
 	/*
-	 * Do we miss much more than hit in this file? If so,
-	 * stop bothering with read-ahead. It will only hurt.
+	 * mmap read-around. If we don't want any read-ahead or if we miss more
+	 * than we hit don't bother with read-ahead and just read a single page.
 	 */
-	if (mmap_miss > MMAP_LOTSAMISS)
-		return fpin;
+	if ((vmf->vma->vm_flags & VM_RAND_READ) ||
+	    !ra->ra_pages || mmap_miss > MMAP_LOTSAMISS) {
+		ra->start = vmf->pgoff;
+		ra->size = 1;
+		ra->async_size = 0;
+	} else {
+		ra->start = max_t(long, 0, vmf->pgoff - ra->ra_pages / 2);
+		ra->size = ra->ra_pages;
+		ra->async_size = ra->ra_pages / 4;
+	}
 
-	/*
-	 * mmap read-around
-	 */
-	fpin = maybe_unlock_mmap_for_io(vmf, fpin);
-	ra->start = max_t(long, 0, vmf->pgoff - ra->ra_pages / 2);
-	ra->size = ra->ra_pages;
-	ra->async_size = ra->ra_pages / 4;
 	ractl._index = ra->start;
 	page_cache_ra_order(&ractl, ra, 0);
 	return fpin;
@@ -3145,9 +3130,7 @@ vm_fault_t filemap_fault(struct vm_fault *vmf)
 			filemap_invalidate_lock_shared(mapping);
 			mapping_locked = true;
 		}
-		folio = __filemap_get_folio(mapping, index,
-					  FGP_CREAT|FGP_FOR_MMAP,
-					  vmf->gfp_mask);
+		folio = filemap_get_folio(mapping, index);
 		if (!folio) {
 			if (fpin)
 				goto out_retry;
-- 
2.35.1