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 0EB10C6FD1F for ; Tue, 26 Mar 2024 13:42:01 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 6CD226B0089; Tue, 26 Mar 2024 09:42:01 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 67CD16B008A; Tue, 26 Mar 2024 09:42:01 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 545446B0092; Tue, 26 Mar 2024 09:42:01 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 446A36B0089 for ; Tue, 26 Mar 2024 09:42:01 -0400 (EDT) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id E21381C0BA6 for ; Tue, 26 Mar 2024 13:42:00 +0000 (UTC) X-FDA: 81939303600.29.60E280E Received: from mout-p-101.mailbox.org (mout-p-101.mailbox.org [80.241.56.151]) by imf18.hostedemail.com (Postfix) with ESMTP id E47741C000B for ; Tue, 26 Mar 2024 13:41:58 +0000 (UTC) Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=pankajraghav.com header.s=MBO0001 header.b=CNJVI1FR; spf=pass (imf18.hostedemail.com: domain of kernel@pankajraghav.com designates 80.241.56.151 as permitted sender) smtp.mailfrom=kernel@pankajraghav.com; dmarc=pass (policy=quarantine) header.from=pankajraghav.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1711460519; 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=mkLVdxNBsIuMF+BkrOfqIhXREY5OUiqOSXmyiAReOck=; b=xUCKW9ff/8TGoSo9tBf0pMy94UU12D+45mRDNWMTurKdZUZTGLP6sefZ8bBQGc3utQnxf/ rKjJ7lp5+vGN6pcQvIWQLog+ugl9XK6bUSf1OquGtUVVDxeyQ7Bt3V/gamiNk/Erj2pPJx lAKCNnHRNuLLJbD2+hTAvnGb19J70Sc= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1711460519; a=rsa-sha256; cv=none; b=FQ4MvAMBcg9cBeo4iH2Fktbo1n9aiurwjPLiSKmnGy3jC7W8aBC5oxXaHoqwx/mKiCUBKO DScAfYYsQc1thjurkYOWhqkXF6UzZ/qVllZTVzq0RD7legpNOqSnvPAxYGBWKiLJEAZQ/o PC+Xm+kTiz2Z4fR04sU4JnfHe0Si4Jo= ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=pass header.d=pankajraghav.com header.s=MBO0001 header.b=CNJVI1FR; spf=pass (imf18.hostedemail.com: domain of kernel@pankajraghav.com designates 80.241.56.151 as permitted sender) smtp.mailfrom=kernel@pankajraghav.com; dmarc=pass (policy=quarantine) header.from=pankajraghav.com Received: from smtp2.mailbox.org (smtp2.mailbox.org [IPv6:2001:67c:2050:b231:465::2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-p-101.mailbox.org (Postfix) with ESMTPS id 4V3rZ94mGwz9sVy; Tue, 26 Mar 2024 14:41:53 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pankajraghav.com; s=MBO0001; t=1711460513; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=mkLVdxNBsIuMF+BkrOfqIhXREY5OUiqOSXmyiAReOck=; b=CNJVI1FROQGHF4ufS62L/Zp20lMd2A0dHsT6RA1hi03L8KSj0jsbBPjPhF14tboZdw3y2e QQPfe650Uz4vUzE8fL/DiQE5j8nbuNrF1o/a9vQE1Nxghm7X0nJq40hS/t2DguaK+5Hv32 XPlZoxo1OLN8b9gPkVeJ9EW6PyZrmxepC3FfUVJISPnWB1M6aBSsvSHnJexcV4256oWFCg PfTp6NMk6rPcGhq9eTzJIg8uZZh9txPOv6wIfDJhZmQmjqQ2uz/bR2K6+WURPz4Rm0mHoS KwLJS5VCWKVqomvwSrNGXwu8t5pB8Z38aDQncFyX+mU0ubf/O1gHE78ZYZv4Jg== Date: Tue, 26 Mar 2024 14:41:49 +0100 From: "Pankaj Raghav (Samsung)" To: Hannes Reinecke Cc: Matthew Wilcox , linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org, gost.dev@samsung.com, chandan.babu@oracle.com, mcgrof@kernel.org, djwong@kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, david@fromorbit.com, akpm@linux-foundation.org, Pankaj Raghav Subject: Re: [PATCH v3 04/11] readahead: rework loop in page_cache_ra_unbounded() Message-ID: References: <20240313170253.2324812-1-kernel@pankajraghav.com> <20240313170253.2324812-5-kernel@pankajraghav.com> <7217df4e-470b-46ab-a4fc-1d4681256885@suse.de> <5e5523b1-0766-43b2-abb1-f18ea63906d6@pankajraghav.com> <3aa8bdf1-24f6-4e1f-a5c4-8dc2d11ca292@suse.de> <1a4a6ad3-6b88-47ea-a6c4-144a1485f614@pankajraghav.com> <2b1a2ded-d26f-4c9e-bd48-2384b5a7c2c9@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2b1a2ded-d26f-4c9e-bd48-2384b5a7c2c9@suse.de> X-Rspamd-Queue-Id: E47741C000B X-Rspam-User: X-Stat-Signature: utucpf9yeu5stq9sbgjg9cesdriqdpsf X-Rspamd-Server: rspam03 X-HE-Tag: 1711460518-494184 X-HE-Meta: U2FsdGVkX1+JO7AVVZfhhUns40WgHCKs22OZP8ZYuNT/R+VMZETy9PaE2UifHD1gP6P7iOsMF59iEuGmxtjVOm6qkvKGu8j6Fsb4yNnFV6ZRwfg9PuoRtMjcO1NpP1BCfaNWCx30gMP7NJT5Jo2r2v3XgTjwCx511TwLMA/ZYbClic6GkQ/OLSLYz232qkwhi5xDYcR3kk+2IX/f6AehUblh6VKdslr2tPlX7f/tJY96WC19UBsVrbiFvRuJLk8k4nUn/7zIyj2oy2igbFgF65ioPiUwIENY3I+zCNEhNtw+tuDNgJVcP4b8PkTRFjYSw5iMeACRU6sRMqmaySOdx0L6ge0wKNFTeJV8ZB/j0/Z7fGJeyAJJyX5x0/nxBdaQZFatsCiuw8R+YFedct7x4o4w3pjJA9ZXHMmIgezfR7VEggYlv1D2BnXQyAqUf5RT5jJjWz3uUgQ0GXoZyxBHkem5Du3CufFRorwIAvxdAGnZKXuO32oCaH0IcwKfuh7gLvqje7IlK4t67zPo9ygNE2jmsPc4RXKuW+gE7CZiFCJEkDFzTP2WgFajK+gYTaMiKrSgtM+TbK/XodN2doW9BG3XY6WP5pWYhgVhTEoUj/e5ApAK7d0RpiKysEqKSg1gjjRjC0XjOxghrDEM/YMurmd+Bq1QfRcdL2uexIhP0RveKeLI9LLmfPiAR1qeGoyiivIpeBHDbUj8GHhuzAFbJnLB7S25IFNQVl7eS5BhPaN0PoZpJdR1yCJA3dkzH1z2SfbkysY2LeOkcd5K3PB+l4ntCeMzFnw2adsjXYNWXieL6LoJAlUl86Lx2UATNpBBTals+6VW6zud9GerRgkQhpX15Ph+DrC6HT6kemr1IOEVZvOhmx3pD10FLrIhIJOkXfuIJfgdJPzu/TvmYOFEzE6QgPCgg3qhKGVsg5ONCGLti5HeIs5OILJqH3wc5gxAgCnodlEgzpGTic8jnWf xfqRbjX+ eFh2vvbgNvgazAnwK5XI7vBPZQuPE/8hD8w/CYmZC9DNpNc1TiMdrimUbZmSGo2nFEZkLJXJylIWKSH832/DhrjkgFTeQzq74nFf+WiUSWf9LEPBcUdAxm1WI03PQmXMNHfW3NvMmXOQ3DexH24KsMwzpNfam2p9kQIqBiNE5CCSyDJJBduBk71Oc48X+qQS2zRLa9mpdkLAvB0rCRtS/gfX15pRuUXREELGTd2+KycTq3qDCre2coVGOqM5oMGV8aD4zXvE5ZlzxanpxJr58CSTKRHbU6hTCetLO9H02qqZsPCU= 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, Mar 26, 2024 at 11:55:06AM +0100, Hannes Reinecke wrote: > > > Bah. That really is overly complicated. When we attempt a conversion that conversion should be > > > stand-alone, not rely on some other patch modifications later on. > > > We definitely need to work on that to make it easier to review, even > > > without having to read the mail thread. > > > > > > > I don't know understand what you mean by overly complicated. This conversion is standalone and it is > > wrong to use folio_nr_pages after we `put` the folio. This patch just reworks the loop and in the > > next patch I add min order support to readahead. > > > > This patch doesn't depend on the next patch. > > > > Let me rephrase: what does 'ractl->_index' signify? > From my understanding it should be the index of the > first folio/page in ractl, right? > > If so I find it hard to understand how we _could_ increase it by one; _index > should _always_ in units of the minimal pagemap size. I still have not introduced the minimal pagemap size concept here. That comes in the next patch. This patch only reworks the loop and should not have any functional changes. So the minimal pagemap size unit here is 1. And to your next question how could we increase it only by one here: // We come here if we didn't find any folio at index + i ... folio = filemap_alloc_folio(gfp_mask, 0); // order 0 => 1 page if (!folio) break; if (filemap_add_folio(mapping, folio, index + i, gfp_mask) < 0) { folio_put(folio); read_pages(ractl); ractl->_index++; ... If we failed to add a folio of order 0 at (index + i), we put the folio and start a read_pages() on whatever pages we added so far (ractl->index to ractl->index + ractl->nr_pages). read_pages() updates the ractl->index to ractl->index + ractl->nr_pages. ractl->index after read_pages() should point to (index + i). As we had issue adding a folio of order 0, we skip that index by incrementing the ractl->index by 1. Does this clarify? In your original patch, you used folio_nr_pages() here. As I said before, we already know the size of the folio we tried to add was 1, so we could just increment by 1, and we should not use the folio to deduce the size after folio_put() as it is use after free. > And if we don't have it here (as you suggested in the mailthread) > I'd rather move this patch _after_ the minimal pagesize is introduced > to ensure that _index is always incremented by the right amount. > I intended to have it as two atomic changes where there is non-functional change that helps with the functional change that comes later. If it is confusing, I could also combine this with the next patch? Or, I could have it as the first patch before I start adding the concept of folio_min_order. Then it makes it clear that it is intended to be a non-function change? -- Pankaj