From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.remote-ip=100.103.45.18 ARC-Seal: i=1; d=subspace.kernel.org; s=arc20261015; a=rsa-sha256; cv=none; t=1793206514; b=fmSQn5a95V/vA7hrw6x2c0DGMjk3iwU6epWRU6oiF9Ny1gshOlODubqT71DKnwRHFQG6 xWW1KIdNX8zyhtZvH2O2J+Gh8dlr81JTsGsXEYDuxDt6fFSgXOSzX+iuRs/qbUrno9qix oB5GntgIkS244jSSpMelQ3/Rm2BVFP15PMmOC1o7YitykEzPYXWxLU1BjN//u9PXgmT3O NrN0i7jdSW655vb4SncyhJCNq1hvDpCCrHZcPKOqYIwrOjVBCqh+E4xUoXt8NwpTDJEup B/JpJJf3/nZENwrQfd7XdCIJ1K44IGtWEsvEHxo2LzEhwHpdWW50X6SgNm14UUtplzA== ARC-Message-Signature: i=1; d=subspace.kernel.org; s=arc20261015; a=rsa-sha256; c=relaxed/relaxed; t=1793206514; h=Received:DKIM-Signature:Received:Received:X-ME-Sender:X-ME-Received: X-ME-Proxy-Cause:X-ME-Proxy:Feedback-ID:Received:Date:From:To:Cc: Subject:Message-ID:References:MIME-Version:Content-Type: Content-Disposition:In-Reply-To; bh=Z0ZhwvuKB4zpYCGY+1GjLRRMsHPTa5Lvp91s/JwcgmI=; b=iadeoavAhf+7OPCVCzzH8Fak2B/el4RNkRiuAARBaHb3ThMctsfB4jQsGIPn+The6dCi ejuijsTGlMF2fOgINYeglpdzd5V51vfrKSE9UG2D3ZqBv2NTKj57kzhTHS/h/rpP3UtAV KZhhGN/pICJLYwvd79HYV0G7xGH3Nt/nMj8dt0PkB7/wtvZCSYDFf97OFLF+MSC2FBdIR wMBqbnKTaZpScVYR5T2cuHVAQqtXcsAPv8xnpEyn9Yul1Va9UBD884X2CNXMV/UQ8bmWh aT6Jrg5oLXxR8THqNYPeuX/iVW+aBNUD5iNWkPnGtBOKuDjWdG3PL1QCcg/eFqYPGAw== ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256 header.s=k20260515 header.b=fI1hKuaG; arc=none smtp.remote-ip=100.103.45.18 Received: from smtp.kernel.org (quasi.space.kernel.org [100.103.45.18]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6BA01606A32E for ; Wed, 28 Oct 2026 16:55:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key, unprotected) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256 header.s=k20260515 header.b=fI1hKuaG Received: by smtp.kernel.org (Postfix) with ESMTPSA id 2C95B1F01380; Wed, 28 Oct 2026 16:55:11 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1793206514; bh=Z0ZhwvuKB4zpYCGY+1GjLRRMsHPTa5Lvp91s/JwcgmI=; h=Date:From:To:Cc:Subject:References:In-Reply-To; b=fI1hKuaGJ5PA8UZE14Z+7bai/BBuHt67u/X/VnMhTWuBatdGig2wHSiJGL2TqApfI mTODLCFihyDqj7eK9JShB0q5QQoKi+K8cFc/SlaDzwBV72mEmD9uElkTkJNj0kxskh RzBNa3jbANSogWNZNBoIm6YA2znaLvSfSw6MGJlDvhCIYiinyMB8YC6dR4Kw4VY6sm LP1KKG/aRMM1Ts0tsHOjoj53HcdyIh4MV+MqdFibVXIF7SIp5HVc7uf8ooRz1UDaE+ 7uOsfvIdHpPeqEcCU/S/pMr0Uh5EMwKza/pFZWaxyCAl0RaP29Y+Oa1HVYO1VV8cQ+ Muuzs+ehVUB0g== Received: from phl-compute-01.internal (phl-compute-01.internal [10.202.2.41]) by mailfauth.ams.internal (Postfix) with ESMTP id 781C0198004A; Wed, 28 Oct 2026 12:55:00 -0400 (EDT) Received: from phl-frontend-03 ([10.202.2.162]) by phl-compute-01.internal (MEProxy); Wed, 28 Oct 2026 12:55:10 -0400 X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: dmFkZTEH/JXmOVrY1taDCoPaJq3IVEd1uOJXsXXF/9Hj9n77Aoo8DF/Z2+y6VwUMjdyxL/ vNcNxlWPOrzJjnTTnhU/i8mG2RaecFzcvs1d/58U+81UT2rrSqZcuXVcmJPvN+85Vxmst6 M+xIX1By265vICLgVpWMTiDzp4SikoR94mt5KoeDHe0EMSHOdb4HjYnJ2gFn6Jl/jXpTbW 1cj1cjuHzE0QOe4Vj87hxPzVA7/e5WKQO7i7HlZnpcOqFBH4nZCcHIT12UXxL8WeWRxmIl 5Im68ywTuP3WqKspjS2QQUDWVpnJdjOhauD/wAZQybXH1dmkgbh8FRIT9xxu5hpwJaaomC mvUYquwQCKDqfI6kfvDJMybmqBz7q/9PFZ6wxnj0QM2XOrQem1LihDgZxlgfmoxJzYIsWu xbnrF8t8WIJ5hEFFFVA/MpFaq7kGfFEDtPj7c/5lR+6uEB6MT88r5l0hakRGsn+Sd9HUTl b3RB3DDyj5tujdg60fA6EJQdbrWiA2uBzz55umormQ0ogMeP6OGN8c56LXAD6GGQ/PaZmQ IQISuPc1mDylIDH5VLMmYpcJ31tdiRrFvBKWYGWu+h5ifZZ2RLfDnGQTfxhGSfZR7jcFMW MFdExhcE3jnb1xzwCCOxlhc/jz01DYKk67mV4yzeBryfzQ34jWvPrzhEgeUA X-ME-Proxy: Feedback-ID: i10464835:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 28 Oct 2026 12:54:58 -0400 (EDT) Date: Wed, 28 Oct 2026 16:54:57 +0000 From: Kiryl Shutsemau To: "David Hildenbrand (Arm)" Cc: Andrew Morton , Johannes Weiner , Michal Hocko , Qi Zheng , Shakeel Butt , Lorenzo Stoakes , Kairui Song , Barry Song , Axel Rasmussen , Yuanchu Xie , Wei Xu , Baoquan He , Baolin Wang , "Liam R. Howlett" , Vlastimil Babka , Mike Rapoport , Suren Baghdasaryan , Michal Hocko , Kees Cook , Peter Zijlstra , Jann Horn , Pedro Falcato , Hugh Dickins , Pasha Tatashin , Pratyush Yadav , "Matthew Wilcox (Oracle)" , Jan Kara , Zi Yan , Lance Yang , Rik van Riel , Harry Yoo , Alistair Popple , Nico Pache , Ryan Roberts , Dev Jain , Usama Arif , Matthew Brost , John Hubbard , Peter Xu , Will Deacon , "Aneesh Kumar K.V" , Nick Piggin , Oscar Salvador , Minchan Kim , Muchun Song , Xu Xin , Chengming Zhou , Chris Li , Kemeng Shi , Nhat Pham , Youngjun Park , Gregory Price , Joshua Hahn , Rakie Kim , Ying Huang , Jason Gunthorpe , linux-mm@kvack.org, linux-kernel@vger.kernel.org, kexec@lists.infradead.org, linux-fsdevel@vger.kernel.org, linux-arch@vger.kernel.org, linux-cxl@vger.kernel.org, stable@vger.kernel.org, Josef Bacik Subject: Re: [PATCH 05/46] mm/truncate: look again if a folio left the mapping Message-ID: References: <20261027-fixes_core-v1-0-6f74bec9a5bb@kernel.org> <20261027-fixes_core-v1-5-6f74bec9a5bb@kernel.org> X-Mailing-List: linux-cxl@vger.kernel.org List-Id: List-Post: List-Help: List-Subscribe: List-Unsubscribe: List-Owner: Precedence: list MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20261027-fixes_core-v1-5-6f74bec9a5bb@kernel.org> On Tue, Oct 27, 2026 at 12:17:25AM +0100, David Hildenbrand (Arm) wrote: > invalidate_inode_pages2_range() skips a folio which no longer belongs to > the mapping by the time it has got the folio lock, on the assumption > that somebody else has removed it from the page cache. But the folio may > have been replaced, and then there is a new folio with the same data at > that index. > > For most cases, like folio migration, that is impossible. However, > collapse_file() only checks the refcount of the old folios, under the > i_pages lock, and relies on their folio locks from there on. That works for > users which repeat the lookup when they find folio->mapping changed. > The following scenario is, however, possible: > > CPU0 pwrite(O_DIRECT) CPU1 collapse_file() > kiocb_invalidate_pages() > read the folio (old data), lock it > refcount check > the write completes copy to new_folio > kiocb_invalidate_post_direct_write() > find_get_entries() -> old folio > folio_lock() ... xas_store(new_folio) > folio->mapping = NULL > folio_unlock(folio) > folio->mapping != mapping > -> continue The diagram reads as if the folio lock should have closed the race. It might be clearer with CPU0's folio_lock() after CPU1's folio_unlock(), since the reference taken after the refcount check is what matters: CPU0 pwrite(O_DIRECT) CPU1 collapse_file() kiocb_invalidate_pages() read the folio (old data), lock it refcount check the write completes copy to new_folio kiocb_invalidate_post_direct_write() find_get_entries() -> old folio xas_store(new_folio) folio->mapping = NULL folio_unlock(folio) folio_lock() folio->mapping != mapping -> continue > In that scenario, the collapsed folio with the data from before the write > stays in the page cache after the write has returned. > > This used to be impossible, as khugepaged only collapsed files that nobody > had open for writing. > > Restart the lookup at that index, like shmem_undo_range() does. The same > problem seems to exist in truncate_inode_pages_range() as well when > truncate_inode_folio() fails, so handle it there as well. > > Fixes: 1b6444331beb ("mm/khugepaged: enable clean pagecache folio collapse for writable files") > Cc: stable@vger.kernel.org > Reported-by: Josef Bacik > Signed-off-by: David Hildenbrand (Arm) > --- > mm/truncate.c | 17 +++++++++++++++-- > 1 file changed, 15 insertions(+), 2 deletions(-) > > diff --git a/mm/truncate.c b/mm/truncate.c > index 733d2713a875a..6d8c0ec4623fe 100644 > --- a/mm/truncate.c > +++ b/mm/truncate.c > @@ -499,7 +499,15 @@ void truncate_inode_pages_range(struct address_space *mapping, > folio_lock(folio); > VM_BUG_ON_FOLIO(!folio_contains(folio, indices[i]), folio); > folio_wait_writeback(folio); > - truncate_inode_folio(mapping, folio); > + if (truncate_inode_folio(mapping, folio)) { > + folio_unlock(folio); > + /* > + * Folio might have been replaced: look it up > + * again. > + */ > + index = indices[i]; > + break; > + } > folio_unlock(folio); > } > truncate_folio_batch_exceptionals(mapping, &fbatch, indices); This hunk looks unneeded to me: its second loop restarts from start until find_get_entries() finds nothing, so the new folio gets truncated on a later round. Or am I missing a case? > @@ -760,7 +768,12 @@ int invalidate_inode_pages2_range(struct address_space *mapping, > folio_lock(folio); > if (unlikely(folio->mapping != mapping)) { > folio_unlock(folio); > - continue; > + /* > + * Folio might have been replaced: look up > + * again. > + */ > + index = indices[i]; > + break; > } > VM_BUG_ON_FOLIO(!folio_contains(folio, indices[i]), folio); > folio_wait_writeback(folio); > > -- > 2.43.0 > -- Kiryl Shutsemau / Kirill A. Shutemov