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 8EB34C021B5 for ; Fri, 21 Feb 2025 18:59:04 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 0EDE128000B; Fri, 21 Feb 2025 13:59:04 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 09D8028000A; Fri, 21 Feb 2025 13:59:04 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E578928000B; Fri, 21 Feb 2025 13:59:03 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id C188A28000A for ; Fri, 21 Feb 2025 13:59:03 -0500 (EST) Received: from smtpin01.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 6B6501A1E7D for ; Fri, 21 Feb 2025 18:59:03 +0000 (UTC) X-FDA: 83144864166.01.460C301 Received: from tor.source.kernel.org (tor.source.kernel.org [172.105.4.254]) by imf06.hostedemail.com (Postfix) with ESMTP id C8CFF180002 for ; Fri, 21 Feb 2025 18:59:01 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=pNuPejVA; spf=pass (imf06.hostedemail.com: domain of mcgrof@kernel.org designates 172.105.4.254 as permitted sender) smtp.mailfrom=mcgrof@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1740164341; 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=UI9oSWASdKUnewQ87AhYS8XCksBBt0rA0o+n7Y4r83k=; b=ZcUP270LGA4RNoGoUxy/uJ9JWYtPHyvo2NkTNPBeYHgi7MDpy88nzOEg+1uVbNVo97u2Xy v43Z6kty6MUa1HDJxxyjI8/kenYvWYhUhavpurzwqq8qMj9bgwMKKJP2XDvzQaFUbOzNME zuHy4Btp2EAetvVOx8rTrBzkYZN9CCI= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=pNuPejVA; spf=pass (imf06.hostedemail.com: domain of mcgrof@kernel.org designates 172.105.4.254 as permitted sender) smtp.mailfrom=mcgrof@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1740164341; a=rsa-sha256; cv=none; b=0DSLJS0ebJY88QuNU71rA0gJchbAFNJRcoEHMWrY7oRxZrwAX5C66cdcrDuHzLAgVA5C3p t15ybe2630XVwt1i28bV6ULxUVPLWSAUjI+hJJ7e/Q7VtdaidjUob1Q+Us19AUh+73upaT evPT5Kuy+q4Wq3eiVeHRiVSNuCxBZ6A= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by tor.source.kernel.org (Postfix) with ESMTP id 88AB2615DC; Fri, 21 Feb 2025 18:58:58 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 52E0CC4CED6; Fri, 21 Feb 2025 18:59:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1740164340; bh=D1gndCuNHXnXWR+5KjZLuhUrgN/6gjce6B2PXKEtvJc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=pNuPejVA8VhXg7mw02ryLSNiBIcvpQV48LtkfBDqGeg9IP/S8FDcIKl29JlXQPvvI 3i4e3mveISKcofvf4+fqwGBKOTwXPw11yGMfNkkFwGO4w0EANJPg+LocjTt76vAkJF qtbWYFpBFNXI4oNjBalh0O/OcmHrCdXnjDz/tjz1VxHUV4ZUP4ORNH3t4e2QQVwYxD 70HC1j/43KbjRK3DBh4xCcvkpN6JL817QUmba+A2MnskvoVrwUuoXqnSgisJ/9a9kl nPAOgnPPC2W6arLJAs1ly2MSuBTv/qY7/274FMQFx8qutbw1w+rNzZL5eb5k5a4JFa 2F1wx1fBfZI+w== Date: Fri, 21 Feb 2025 10:58:58 -0800 From: Luis Chamberlain To: Hannes Reinecke Cc: Matthew Wilcox , dave@stgolabs.net, david@fromorbit.com, djwong@kernel.org, kbusch@kernel.org, john.g.garry@oracle.com, hch@lst.de, ritesh.list@gmail.com, linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org, linux-mm@kvack.org, linux-block@vger.kernel.org, gost.dev@samsung.com, p.raghav@samsung.com, da.gomez@samsung.com, kernel@pankajraghav.com Subject: Re: [PATCH v2 4/8] fs/mpage: use blocks_per_folio instead of blocks_per_page Message-ID: References: <20250204231209.429356-1-mcgrof@kernel.org> <20250204231209.429356-5-mcgrof@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: C8CFF180002 X-Stat-Signature: meqwzgjc4jcu39mz3w44msz7b778if48 X-Rspam-User: X-HE-Tag: 1740164341-421703 X-HE-Meta: U2FsdGVkX18VnLbg9KJ6Ly7sSjncfu+g5Q7M+iPyQsv1t6AwJZ0cTDWhogfKyuA8Z5ue+xX8MGRJZ/N+id+dheY2gXqAEtORioJ9+W1QBrLdLZ1MOjFeSxA602CIu1OcuYQk9ejoqZRTzpUHPA9P76t87B5bRI9XooxjTJR21QgdT/qx3jGf1BXu2oPsT8QZXurBJfEv8z7Gs22zqBfO3TAQquKFuTeN6wNQwhfJKEdqh8VCz4hNU8weBN47WRFVBgba6tH4C6JEPI6eZ3W2v81vFhAEySqhDf4bPL/wjyq/oiwgfoX+vn4Uhki39v/aNJKTpm9d3L5cAEfIbzJBFm4C3Qf+fhFwNBf3EXyb/SCem2oLz0FhKzNRL2NGyvIa7Wm4TIhdHvMjB6Z/JMsblrnE8+CjpNfZs4y2ssNMsBMsXSdaLL6MM3+zxkof8wUk7ABMsd+WJRaUfH71tXRkTkkP0gH6+rJEO9NWtBFvU2lFMmE08sTDab9J35EniVNCTyBFG3je+Ry7Rtw1n6QEkG4iPSfBu+WG6XA7lTOi8NDWtZcmekfYqEv/FlxbQlBzp2ncc8VmSEL/EJ8VHM+WWfiO0h5ckVfACR17j16vaVYBV8FxKGRra+pO4W4V9+PGQeXvzmqQ5AeKDA+YzgqGk45XjencjrB4uk/06PpeWEZ59boEoEOVMXg60FT8Py40pqpbxe9lyR8mR6JD9KsO3KT6yLqpnfhA9Xg/jYizvLqKJJmKxUuv9NbC58VYWlpvIoxy4IWJn/MJQBvhEPQ4aSLCczS0/UsnOoqil1oyNNJFJW6ODCMiwvVNN0igF3U1m1Rgy70ORPQ3/GgqIfBLgWSYGrWDS2xLokHeBa/KD1mPGqDadl0WYx1/AKqUJ/OQDfW+VkYUrt2avxY0zjz86PLD8yPcGJM6PXKm148M5GnNXdkkenzZxV+dSPMp8uIweo6GsjgOq1JpLa0YBO2 amKeKQ/O MjPg1VMP/cvZ0qwRzcwFI3Ph8i4iJNB/y51mHvHDFp4HujEnr4fPjHmBU8SeR1KSu9yaMQ+Yo0kXbol8seiqyldmaXec7tWc05UzHD/2axAo+rfua3Z0UxUYIAEGCGtd8UrrCZ9YoMHaq9jSf5jVIx6lmiJEH5fzbs2osnO9QWERQXlSDjzczoQIJuLl7GoDjpwOiW5FFOTMI41UZqzm3mwYQ/SWBXN4amjf45P5Fok/j6ao5UYzGQZOg31Z/O1ZoXKpLjRLdYNjCpN/HXsNCe4cLXqM9Ok1baCBlPkxMJ2nAbV6EK43DUstpq3P+G3Z5K8v6wO+QaAKNylE= 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: List-Subscribe: List-Unsubscribe: On Tue, Feb 18, 2025 at 04:02:43PM +0100, Hannes Reinecke wrote: > On 2/17/25 22:58, Matthew Wilcox wrote: > > On Tue, Feb 04, 2025 at 03:12:05PM -0800, Luis Chamberlain wrote: > > > @@ -182,7 +182,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args) > > > goto confused; > > > block_in_file = folio_pos(folio) >> blkbits; > > > - last_block = block_in_file + args->nr_pages * blocks_per_page; > > > + last_block = block_in_file + args->nr_pages * blocks_per_folio; > > > > In mpage_readahead(), we set args->nr_pages to the nunber of pages (not > > folios) being requested. In mpage_read_folio() we currently set it to > > 1. So this is going to read too far ahead for readahead if using large > > folios. > > > > I think we need to make nr_pages continue to mean nr_pages. Or we pass > > in nr_bytes or nr_blocks. > > > I had been pondering this, too, while developing the patch. > The idea I had here was to change counting by pages over to counting by > folios, as then the logic is essentially unchanged. > > Not a big fan of 'nr_pages', as then the question really is how much > data we should read at the end of the day. So I'd rather go with 'nr_blocks' > to avoid any confusion. I think the easier answer is to adjust nr_pages in terms of min-order requirements and fix last_block computation so we don't lie for large folios as follows. While at it, I noticed a folio_zero_segment() was missing folio_size(). diff --git a/fs/mpage.c b/fs/mpage.c index c17d7a724e4b..624bf30f0b2e 100644 --- a/fs/mpage.c +++ b/fs/mpage.c @@ -152,6 +152,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args) { struct folio *folio = args->folio; struct inode *inode = folio->mapping->host; + const unsigned min_nrpages = mapping_min_folio_nrpages(folio->mapping); const unsigned blkbits = inode->i_blkbits; const unsigned blocks_per_folio = folio_size(folio) >> blkbits; const unsigned blocksize = 1 << blkbits; @@ -172,6 +173,8 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args) /* MAX_BUF_PER_PAGE, for example */ VM_BUG_ON_FOLIO(folio_test_large(folio), folio); + VM_BUG_ON_FOLIO(args->nr_pages < min_nrpages, folio); + VM_BUG_ON_FOLIO(!IS_ALIGNED(args->nr_pages, min_nrpages), folio); if (args->is_readahead) { opf |= REQ_RAHEAD; @@ -182,7 +185,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args) goto confused; block_in_file = folio_pos(folio) >> blkbits; - last_block = block_in_file + args->nr_pages * blocks_per_folio; + last_block = block_in_file + ((args->nr_pages * PAGE_SIZE) >> blkbits); last_block_in_file = (i_size_read(inode) + blocksize - 1) >> blkbits; if (last_block > last_block_in_file) last_block = last_block_in_file; @@ -269,7 +272,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args) } if (first_hole != blocks_per_folio) { - folio_zero_segment(folio, first_hole << blkbits, PAGE_SIZE); + folio_zero_segment(folio, first_hole << blkbits, folio_size(folio)); if (first_hole == 0) { folio_mark_uptodate(folio); folio_unlock(folio); @@ -385,7 +388,7 @@ int mpage_read_folio(struct folio *folio, get_block_t get_block) { struct mpage_readpage_args args = { .folio = folio, - .nr_pages = 1, + .nr_pages = mapping_min_folio_nrpages(folio->mapping), .get_block = get_block, };