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 174B0C3ABC3 for ; Fri, 9 May 2025 13:30:33 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 34AC3280038; Fri, 9 May 2025 09:30:31 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 2D34D280031; Fri, 9 May 2025 09:30:31 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 173D6280038; Fri, 9 May 2025 09:30:31 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id EC316280031 for ; Fri, 9 May 2025 09:30:30 -0400 (EDT) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 095EA160150 for ; Fri, 9 May 2025 13:30:32 +0000 (UTC) X-FDA: 83423453904.15.D59CB08 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf01.hostedemail.com (Postfix) with ESMTP id 0AC7840018 for ; Fri, 9 May 2025 13:30:29 +0000 (UTC) Authentication-Results: imf01.hostedemail.com; dkim=none; spf=pass (imf01.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com; dmarc=pass (policy=none) header.from=arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1746797430; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=kd/srjdm1xnvaZel3ttLP5jXGyV2SDPmX2hnkuWxMsU=; b=rWWCGX39klcmM02OovS3OmL9ZX1GDVAFotDwxhfm51inLY2McDulHUG9XBW9OXUzdadeVe NkRK+ewVjfom0QmX/pQS9INlVJuYU+LMfT8E65Knm5rJV0cZvPyC+vtkcaTzEOPNiLasJW 0BXqt35RGzbX/RfkVkECm5nZqiyyX2w= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=none; spf=pass (imf01.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com; dmarc=pass (policy=none) header.from=arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1746797430; a=rsa-sha256; cv=none; b=T6+YIE9it5G+QSPxmgJ08+apoUP1222iEARKFACCIpXs9ZBje2OLRtYw6RDbcQuVjGAMSm mHzpk/lRU8BUhg9Dr/3PjUa8OFVC0Fw4yZY7HGBsdWDgHjITabiWdXi2ky/KFvq8tB8ztL VWyw+dFLxz+oMXocgtpDSRLPA5Fj3KE= Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id C665D175D; Fri, 9 May 2025 06:30:17 -0700 (PDT) Received: from [10.57.90.222] (unknown [10.57.90.222]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 4CA773F58B; Fri, 9 May 2025 06:30:26 -0700 (PDT) Message-ID: <22e4167a-6ed0-4bda-86b8-a11c984f0a71@arm.com> Date: Fri, 9 May 2025 14:30:24 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [RFC PATCH v4 1/5] mm/readahead: Honour new_order in page_cache_ra_order() Content-Language: en-GB To: "Pankaj Raghav (Samsung)" Cc: Andrew Morton , "Matthew Wilcox (Oracle)" , Alexander Viro , Christian Brauner , Jan Kara , David Hildenbrand , Dave Chinner , Catalin Marinas , Will Deacon , Kalesh Singh , Zi Yan , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org References: <20250430145920.3748738-1-ryan.roberts@arm.com> <20250430145920.3748738-2-ryan.roberts@arm.com> From: Ryan Roberts In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Stat-Signature: qypop6gfd3q7mqy4wtwn5y8hzjn57efn X-Rspamd-Server: rspam01 X-Rspamd-Queue-Id: 0AC7840018 X-Rspam-User: X-HE-Tag: 1746797429-299067 X-HE-Meta: U2FsdGVkX19EZZsL/Fqre3EnpeYhQf0gp9pGYv0ByDgnN0briCNhIH2vhZyR9WVDHsyTyvVmQFVuhAMkHdzDpXGIqrreun0eLqd9aW0YN/P8GlMvLxPTFduxWg/PH0OsNHi+6Bv9Ucp0EXGxlm7hAZ7UfvvoKT0SXLL+6mzkLhkJ1tAkRdVX3kFD4OUpAQKlPDZZUxOu42fVp0nmHi9JJICu16d1WVz9dLB6F5qZ++feLz2d1ek8r1qZKBlXuNcBZaoMJt1Hk1jFX1xJlhkrOWemrs/a7RM5eIQeplxh2VqcPo9nOo35zFeexxeXaGbvmjuJQbuHPTzcoEaQmrOUgE2TAaJmMmySGmgRtZneuyRodPIBQk6kteo0JaDPivqeOKZcfJ0bR3ar0tT0NfDGBiA2jjESUfTy+81SK9tjoaw9yTYC1pUZEYOmlE4y3B4doyVAWtLi0t3E9TE4EqbEuWvObpothpoQXhFR9LJlqRWUHluauYCLiFlTjGe95bS4ZC+E0DRbTY2uGvLqIjFwagtMdx/5VGjkP940nB1XNRqN0QcIxPcQOj4Yf1pqLviqvjFmLtohqVPf1XV9l6SnkyNPIx93MiVY/a4yOgMKAawcTlnqEKqYBN1sS7po4wUgRfE1vlT13TwjrqKL2Cofa4J4GVJLll10PMrM8Ue83q5a0nSX+sxCDTQZ//sK1XA8xwn4DdGXnq8ru8wbQJE+2o5s99DwA5Iox/2A3MWCfLR+FrlcmI9d5624isbuY8VogiNjBYAqBh3maZ4kaGfO4LBq9Gy3dKNkRQ2D7ON/+twGwu39r6WnZ3O6WB2gsyiMBOtV2OEEhhI6noCAv0TH+LZEYNkBLdHr0j6yJ5FqFnLSzLRcGknFe3XA70KOxNEllEDiA5JMkamBumcVBEoiKU0LCMROfqjQ4s7qJypVowBtsBxit2D3k8Sp/wALLctXZwB+XhWTZgw0cy98n99 k9iRe2Ft f6f9rHWxcAvZKgSeQYmS3PuKQXSJUJrfL3msmlJBe5ztjyTNL/NLF20B5TNUjO5qoBWIP6ytjBIiQ1Dj2CLfvoMnIykFUNq1i/UYe/l40dL/pdFA9XYZamrbK133Lv8NJOSKwztbSxksctWAwJCZwT7hCqLeNNZsm5r6yzeoANIFDhWZhBeZdkl3E9FiIScurpSfePZIm6oaW471d1Qgm+xh9pOmGmR7/LAD1Vb40PydwOlfXoxODZzPneiHmRa9Jo3S6HUTD7a99QDqNfRmVqEbBSCLtOsYEZn7f 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: Hi Pankaj, Thanks for the review! ... On 08/05/2025 13:55, Pankaj Raghav (Samsung) wrote: > Hey Ryan, > > On Wed, Apr 30, 2025 at 03:59:14PM +0100, Ryan Roberts wrote: >> FOLIO 0x0001a000 0x0001b000 4096 26 27 1 0 >> FOLIO 0x0001b000 0x0001c000 4096 27 28 1 0 >> FOLIO 0x0001c000 0x0001d000 4096 28 29 1 0 >> FOLIO 0x0001d000 0x0001e000 4096 29 30 1 0 >> FOLIO 0x0001e000 0x0001f000 4096 30 31 1 0 >> FOLIO 0x0001f000 0x00020000 4096 31 32 1 0 >> FOLIO 0x00020000 0x00024000 16384 32 36 4 2 >> FOLIO 0x00024000 0x00028000 16384 36 40 4 2 >> FOLIO 0x00028000 0x0002c000 16384 40 44 4 2 >> FOLIO 0x0002c000 0x00030000 16384 44 48 4 2 >> ... >> >> Signed-off-by: Ryan Roberts >> --- >> mm/readahead.c | 4 +--- >> 1 file changed, 1 insertion(+), 3 deletions(-) >> >> diff --git a/mm/readahead.c b/mm/readahead.c >> index 6a4e96b69702..8bb316f5a842 100644 >> --- a/mm/readahead.c >> +++ b/mm/readahead.c >> @@ -479,9 +479,6 @@ void page_cache_ra_order(struct readahead_control *ractl, >> > > So we always had a fallback to do_page_cache_ra() if the size of the > readahead is less than 4 pages (16k). I think this was there because we > were adding `2` to the new_order: If this is the reason for the magic number 4, then it's a bug in itself IMHO. 4 pages is only 16K when the page size is 4K; arm64 supports other page sizes. But additionally, it's not just ra->size that dictates the final order of the folio; it also depends on alignment in the file, EOF, etc. If we remove the fallback condition completely, things will still work out. So unless someone can explain the reason for that condition (Matthew?), my vote would be to remove it entirely. > > unsigned int min_ra_size = max(4, mapping_min_folio_nrpages(mapping)); > > /* > * Fallback when size < min_nrpages as each folio should be > * at least min_nrpages anyway. > */ > if (!mapping_large_folio_support(mapping) || ra->size < min_ra_size) > goto fallback; > >> limit = min(limit, index + ra->size - 1); >> >> - if (new_order < mapping_max_folio_order(mapping)) >> - new_order += 2; > > Now that you have moved this, we could make the lhs of the max to be 2 > (8k) instead of 4(16k). I don't really understand why magic number 2 would now be correct? > > - unsigned int min_ra_size = max(4, mapping_min_folio_nrpages(mapping)); > + unsigned int min_ra_size = max(2, mapping_min_folio_nrpages(mapping)); > > I think if we do that, we might ramp up to 8k sooner rather than jumping > from 4k to 16k directly? In practice I don't think so; This would only give us order-1 where we didn't have it before if new_order >= 1 and ra->size is 3 or 4 pages. But as I said, my vote would be to remove this fallback condition entirely. What do you think? Thanks, Ryan > >> - >> new_order = min(mapping_max_folio_order(mapping), new_order); >> new_order = min_t(unsigned int, new_order, ilog2(ra->size)); >> new_order = max(new_order, min_order); >> @@ -683,6 +680,7 @@ void page_cache_async_ra(struct readahead_control *ractl, >> ra->size = get_next_ra_size(ra, max_pages); >> ra->async_size = ra->size; >> readit: >> + order += 2; >> ractl->_index = ra->start; >> page_cache_ra_order(ractl, ra, order); >> } >> -- >> 2.43.0 >> > > -- > Pankaj