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 89FD9C3ABAA for ; Mon, 5 May 2025 13:00:24 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 0978C6B008A; Mon, 5 May 2025 09:00:22 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 01C8B6B008C; Mon, 5 May 2025 09:00:21 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E27526B0092; Mon, 5 May 2025 09:00:21 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id C06E46B008A for ; Mon, 5 May 2025 09:00:21 -0400 (EDT) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 6CE5B1C7E73 for ; Mon, 5 May 2025 13:00:23 +0000 (UTC) X-FDA: 83408862726.02.6A62E54 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf07.hostedemail.com (Postfix) with ESMTP id 9A7FA4001A for ; Mon, 5 May 2025 13:00:21 +0000 (UTC) Authentication-Results: imf07.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf07.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1746450021; 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=4Rl3wKQwgp1nX77vmfc/9c3BPbi2pdFsocWgOtxdfpA=; b=noNXKYTnkrlmMDsf5GZ9AjC4jOacwYCbExdDbzqrfPNxTn233lrFUoe7yt9uVe9CS85xrE SyQPFj8GtGqLVqqeu1ysw5O8TSyb/+eSE5K3EmdFAp6DMqes4af4cA6552fxENuyQMTtHd TLSpItqepgX2CdHzBbRVNoe7dA3bSyA= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1746450021; a=rsa-sha256; cv=none; b=g7POLPHpSZi332O99kH95pmNeW8ksV3nRWVmWAuELxb2+kcDGcf+OkWfvnPSIEZ4G52mGG b/m5r4DZluwFSVOO8V279Hi24Lam7YHSv8ACmLZKt8vwwCFRe1wsEdGD5z0qAAwe3TSoKh HKGgCsIA6UDPBvy4qXZym6SaZK14WWA= ARC-Authentication-Results: i=1; imf07.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf07.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com 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 63F7F1007; Mon, 5 May 2025 06:00:11 -0700 (PDT) Received: from [10.57.93.118] (unknown [10.57.93.118]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 204093F58B; Mon, 5 May 2025 06:00:17 -0700 (PDT) Message-ID: Date: Mon, 5 May 2025 14:00:16 +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: Anshuman Khandual , Andrew Morton , "Matthew Wilcox (Oracle)" , Alexander Viro , Christian Brauner , Jan Kara , David Hildenbrand , Dave Chinner , Catalin Marinas , Will Deacon , Kalesh Singh , Zi Yan Cc: 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> <8e3ca5fc-dadc-4a0a-902e-d2522740cbce@arm.com> From: Ryan Roberts In-Reply-To: <8e3ca5fc-dadc-4a0a-902e-d2522740cbce@arm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 9A7FA4001A X-Rspam-User: X-Stat-Signature: 1sobbd37zrjcz8ortkmozc7n8iib6of8 X-HE-Tag: 1746450021-493912 X-HE-Meta: U2FsdGVkX1/lAYSnKdirCPDAQMHGkWXgsGz4/90u0O8CrN4XZjDQ0xR3ELcXNqkRUebe2zbWRCHKqUSiUUDZ9rMkVbW2gh6c1KrX7e172nDCG8ayv+H9CTgza6DHCAO0kcFFPi9a+kYUoMIDKbSFILSNlewwDMGkMblCQsLg4F+220g7kppT7ZafJIuuTbf3ecex1ovp1DK3GPVkegJgVmFSsj4QMqkgbhRRYRzgK+/IX8aqBPudmpXwcBVUZxvFZmWGZD5TiMjroJiTE7jDRFjciUZjh0Z4SdQEMQSjCwyoRx0qV0oFfWQLuJFioKb/xPbz0zcSkweLKLyO9D9svAUpQbEjRC38J1b71/ZXvDZ8LUOn7AUGcbFs7Ywx2sdDdHVJIOZHBGDUxUiip5Oj06YFyrvymQhTIZxcER8NP1Lr7jV/94FMuCLF7pdIeQMI4wCI92lugco0p7Qs+8JsuIdC1E9FgodX2bvnO9qPcb3phYJbFLaqen9uuEUgbM8SbGuYbHPXzWryqcSGzysmJafX7LMW9EGGn2TnA7dMh59WFV2zMUDmFnxdlIXUMMTcWX+IoLhn8081RICjxoiEUtuWblFJd1r6lBBwxoKhM/EtgqKX5phOBqAdPH6z15dhuUxYoVwITF8A9xQeoZ9hwlePQmwZpBudn3vyzf5cfbf29ZtSGkAdxUl/ljtNGqCJgAqN5GYxYuduUqYBB9rDnNhvdCoAOuE9pg5kaKOGVqnQovBz2faLIKkcrE6gRhkXRZ5aNotQfKoJIqF3vxwwONkOK5gDZtLV2pa3up2xxVG+Kljl6nYoNsb77/yD8TcWfsT5lSlnOY5A9X8aDYoU9J/FNfKjJLuLJ26pMlaMaM6ZG4Hhz/oGuAaYtWWASTM6PYlz9Pb960ZHkroCUAoQRc2VgF0edXdjFOuhTPJmQTsLeCJseoKsDYfAZyvOXus7WLb0RiaExVPCuU0yw75 FkOXGOzP +6IKKR5kID05FpTTSce4MhInUXWlEPWTJQzt5/BNNe999XBKRhxPbtTgpASkZMT9E3Vho++Kv+lM8luCy8irtjvys+JWnbLeT2Rb4NOezuiCp5n8CODuy0pKWFJd7v5r6nkbfpxPQu0gOKURt/hX/rwa9X/ol/2SkscF6FrKR56TiGVK9sF20DyF51jefScpcHPfpLFcWQutiqwzDYSEbIofq5O4C+8x79v9cp6q+5bnjqSOUnqErIkPpQQ== 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 05/05/2025 11:09, Anshuman Khandual wrote: > > > On 4/30/25 20:29, Ryan Roberts wrote: >> page_cache_ra_order() takes a parameter called new_order, which is >> intended to express the preferred order of the folios that will be >> allocated for the readahead operation. Most callers indeed call this >> with their preferred new order. But page_cache_async_ra() calls it with >> the preferred order of the previous readahead request (actually the >> order of the folio that had the readahead marker, which may be smaller >> when alignment comes into play). >> >> And despite the parameter name, page_cache_ra_order() always treats it >> at the old order, adding 2 to it on entry. As a result, a cold readahead >> always starts with order-2 folios. >> >> Let's fix this behaviour by always passing in the *new* order. > > Makes sense. > >> >> Worked example: >> >> Prior to the change, mmaping an 8MB file and touching each page >> sequentially, resulted in the following, where we start with order-2 >> folios for the first 128K then ramp up to order-4 for the next 128K, >> then get clamped to order-5 for the rest of the file because pa_pages is >> limited to 128K: >> >> TYPE STARTOFFS ENDOFFS SIZE STARTPG ENDPG NRPG ORDER >> ----- ---------- ---------- --------- ------- ------- ----- ----- >> FOLIO 0x00000000 0x00004000 16384 0 4 4 2 >> FOLIO 0x00004000 0x00008000 16384 4 8 4 2 >> FOLIO 0x00008000 0x0000c000 16384 8 12 4 2 >> FOLIO 0x0000c000 0x00010000 16384 12 16 4 2 >> FOLIO 0x00010000 0x00014000 16384 16 20 4 2 >> FOLIO 0x00014000 0x00018000 16384 20 24 4 2 >> FOLIO 0x00018000 0x0001c000 16384 24 28 4 2 >> FOLIO 0x0001c000 0x00020000 16384 28 32 4 2 >> FOLIO 0x00020000 0x00030000 65536 32 48 16 4 >> FOLIO 0x00030000 0x00040000 65536 48 64 16 4 >> FOLIO 0x00040000 0x00060000 131072 64 96 32 5 >> FOLIO 0x00060000 0x00080000 131072 96 128 32 5 >> FOLIO 0x00080000 0x000a0000 131072 128 160 32 5 >> FOLIO 0x000a0000 0x000c0000 131072 160 192 32 5 >> ... >> >> After the change, the same operation results in the first 128K being >> order-0, then we start ramping up to order-2, -4, and finally get >> clamped at order-5: >> >> TYPE STARTOFFS ENDOFFS SIZE STARTPG ENDPG NRPG ORDER >> ----- ---------- ---------- --------- ------- ------- ----- ----- >> FOLIO 0x00000000 0x00001000 4096 0 1 1 0 >> FOLIO 0x00001000 0x00002000 4096 1 2 1 0 >> FOLIO 0x00002000 0x00003000 4096 2 3 1 0 >> FOLIO 0x00003000 0x00004000 4096 3 4 1 0 >> FOLIO 0x00004000 0x00005000 4096 4 5 1 0 >> FOLIO 0x00005000 0x00006000 4096 5 6 1 0 >> FOLIO 0x00006000 0x00007000 4096 6 7 1 0 >> FOLIO 0x00007000 0x00008000 4096 7 8 1 0 >> FOLIO 0x00008000 0x00009000 4096 8 9 1 0 >> FOLIO 0x00009000 0x0000a000 4096 9 10 1 0 >> FOLIO 0x0000a000 0x0000b000 4096 10 11 1 0 >> FOLIO 0x0000b000 0x0000c000 4096 11 12 1 0 >> FOLIO 0x0000c000 0x0000d000 4096 12 13 1 0 >> FOLIO 0x0000d000 0x0000e000 4096 13 14 1 0 >> FOLIO 0x0000e000 0x0000f000 4096 14 15 1 0 >> FOLIO 0x0000f000 0x00010000 4096 15 16 1 0 >> FOLIO 0x00010000 0x00011000 4096 16 17 1 0 >> FOLIO 0x00011000 0x00012000 4096 17 18 1 0 >> FOLIO 0x00012000 0x00013000 4096 18 19 1 0 >> FOLIO 0x00013000 0x00014000 4096 19 20 1 0 >> FOLIO 0x00014000 0x00015000 4096 20 21 1 0 >> FOLIO 0x00015000 0x00016000 4096 21 22 1 0 >> FOLIO 0x00016000 0x00017000 4096 22 23 1 0 >> FOLIO 0x00017000 0x00018000 4096 23 24 1 0 >> FOLIO 0x00018000 0x00019000 4096 24 25 1 0 >> FOLIO 0x00019000 0x0001a000 4096 25 26 1 0 >> 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 >> FOLIO 0x00030000 0x00034000 16384 48 52 4 2 >> FOLIO 0x00034000 0x00038000 16384 52 56 4 2 >> FOLIO 0x00038000 0x0003c000 16384 56 60 4 2 >> FOLIO 0x0003c000 0x00040000 16384 60 64 4 2 >> FOLIO 0x00040000 0x00050000 65536 64 80 16 4 >> FOLIO 0x00050000 0x00060000 65536 80 96 16 4 >> FOLIO 0x00060000 0x00080000 131072 96 128 32 5 >> FOLIO 0x00080000 0x000a0000 131072 128 160 32 5 >> FOLIO 0x000a0000 0x000c0000 131072 160 192 32 5 >> FOLIO 0x000c0000 0x000e0000 131072 192 224 32 5 > > I guess performance wise this will be worse than earlier ? Maybe, maybe not. If higer order always gave better performance then we would always surely use the highest order? Order-0 is a bit easier to allocate than order-2. So if the file actually isn't being accessed sequentially, allocating order-0 for the cold cache case might actually be better over all? > Although it > does fix the semantics for page_cache_ra_order() with respect to the > parameter 'new_order'. Yes that's the piece I was keen to sort out; Once you get to patch 5 it's important that new_order really does mean new_order otherwise we would end up allocating higher order than the arch intedended. If we think we really *should* be starting at order-2 instead of order-0, we should pass 2 as new_order instead of 0. > >> ... >> >> 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, >> >> limit = min(limit, index + ra->size - 1); >> >> - if (new_order < mapping_max_folio_order(mapping)) >> - new_order += 2; >> - >> 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: > > Should not the earlier conditional check also be brought here before > incrementing the order ? Just curious. > > if (new_order < mapping_max_folio_order(mapping)) No that's not needed. page_cache_ra_order() will clamp new_order appropriately. The conditional that I removed was unneeded becaude the following lines are clamping the new value explicitly anyway. Thanks, Ryan > >> + order += 2; >> ractl->_index = ra->start; >> page_cache_ra_order(ractl, ra, order); >> }