From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 483E64BCABE for ; Wed, 22 Jul 2026 09:56:30 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784714192; cv=none; b=QUn9qm0RqNZKm+uXqBlxD+W0PEH4sm6gKo5/s+fn0mhObfXX2Ssn7PTT3KnuPRmG9W0jVLw8kmYuShHAJ18OE0BzKHpaR98PeaUvlJLFjJgVfenaNrKXsnPbTv8eZ/h0SSB7n40f4V7v+sibmV6MZ9CKUdtYIPmM9JB/eCQNkEM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784714192; c=relaxed/simple; bh=s6fvltpOGEGmjJXh8wtw9szgyMF+oillURfNIHn7Feo=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=DgK7v3apnZqk/ZigHCRspunoG0LIU0fj8Ri5iuzeLqZC6XxyQvFCaxRnokDV+8TBNwDmO5xfyW7bvuUwtYIgpr1EOw8sWgJAYpjOIN7aj7TNeZ7CoJCwOYqIB9wNthynK+384SNPGPWlBU9IyTH0cQwaD/GVO//foJ0rL+pCtGo= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=Thaeh4vD; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="Thaeh4vD" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 504EE1F000E9; Wed, 22 Jul 2026 09:56:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1784714189; bh=tEkmbsohu70wSpeu9kL7X6w5e0YxqXmWzPLQ1TJA370=; h=Date:From:To:Cc:Subject:References:In-Reply-To; b=Thaeh4vDN4ML6kySImSIFQXqo0elsCjKfQgz9DWNjlMi+hJ5c+8WEWhOt7n3J+C21 yM2sQxBRdZIzu0JozK8N2jlcG8kHE7+yDz2S3H/ME+95NaHe4Ygaw/y7gS6fISgr2I xDbRa+vb//u1nFVzXI4Q+goD08fCNNdIoh9zL5IWUP5mIRBQ2aZ1y2S8Z1ucYleL3I 9gi/hGiaE6KQyeTlx4Svc+ZrBs+olFub5pHk+Q2D95yDK2hSKyt4CmiwRNgND39t1d tkB4CUFoIilk6uo5VO+gscHioDKA6DxmgmWy3lw3b2irOPgMZfB/wzfJtG4V7lj5gg f5WrjDdsXi2tw== Date: Wed, 22 Jul 2026 10:56:13 +0100 From: "Lorenzo Stoakes (ARM)" To: Jiale Yao Cc: Andrew Morton , David Hildenbrand , "Liam R. Howlett" , Vlastimil Babka , Mike Rapoport , Suren Baghdasaryan , Michal Hocko , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] mm/page_idle: call folio_test_lru() after folio_get() Message-ID: References: <20260722092642.1123347-1-yaojiale02@163.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260722092642.1123347-1-yaojiale02@163.com> On Wed, Jul 22, 2026 at 05:26:42PM +0800, Jiale Yao wrote: > page_idle_get_folio() speculatively calls folio_test_lru() before > folio_try_get(). The folio can get freed and reallocated to a tail page > in the meantime. In that case, VM_BUG_ON_PGFLAGS() in > const_folio_flags() can be triggered. Remove the speculative call. > > Also mark the folio_test_lru() check right after folio_try_get() success > as no more unlikely. Slightly strange wording but not sure why you're doing that? It is generally unlikely a given folio will be !LRU right? > > This is a sibling-path bug: damon_get_folio() was copied from this > function with the same flawed pattern. Commit d6b8b02a27b3 > ("mm/damon/ops-common: call folio_test_lru() after folio_get()") fixed > damon_get_folio(), but page_idle_get_folio() was left unfixed. KCSAN > (strict mode) confirms the data race on the folio flags: > > BUG: KCSAN: data-race in ... / percpu_counter_add_batch > page_idle_get_folio+0x7a/0x2d0 > page_idle_bitmap_read+0xc9/0x220 > > Signed-off-by: Jiale Yao Yeah generally this seems obviously correct (TM), if we can't be sure a folio is kept around any other way to the extent we're doing folio_try_get() we should gate any actual interactions with the folio on succeeding the get first...! With the unlikely thing changed, LGTM so: Reviewed-by: Lorenzo Stoakes (ARM) > --- > mm/page_idle.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/mm/page_idle.c b/mm/page_idle.c > index 9c67cbac2965..29ee18f0e8ce 100644 > --- a/mm/page_idle.c > +++ b/mm/page_idle.c > @@ -40,9 +40,9 @@ static struct folio *page_idle_get_folio(unsigned long pfn) > return NULL; > > folio = page_folio(page); > - if (!folio_test_lru(folio) || !folio_try_get(folio)) I guess this was meant as a racey check... > + if (!folio_try_get(folio)) > return NULL; > - if (unlikely(page_folio(page) != folio || !folio_test_lru(folio))) { > + if (unlikely(page_folio(page) != folio) || !folio_test_lru(folio)) { As above, not sure why you're changing this? Seems unrelated, I'd just keep it as it was. > folio_put(folio); > folio = NULL; > } > -- > 2.34.1 > Cheers, Lorenzo