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 4478A37DADD for ; Thu, 25 Jun 2026 14:27:17 +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=1782397641; cv=none; b=JoJPB1bOkbkh95KJLMkawbQwfwQeTgjEcu/HirtHGyKVsOTN0FZhvfjXwuxBpTCTQsrin5NBEXpafGvw2cfqXAfH/pjwDcjJX7f/tMDFtsx2bW2cyGyLLqC6X1nF6smwZyf2zeK3n96oNxtUsAe/ADo9ZHCCbbI8wtonA2pyKgM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782397641; c=relaxed/simple; bh=im6B5JGoIjOrEA6DdYpXz30qtvoHU5OGJDNWmeufBIg=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=onrXuvQzUW9eosEn8330od8PoD5qUe2uxqTMqOTrEKJR1fq/76VBsBngu1jztEn/TAF5KIkkm45V9ewobCppZV3/xNUqmf0olBB3PKwLPWgmcgFImnF5oyUrpGbLQ2f44Wap0JdF/tgovVvW/o8zjZR2gdaP/YdMYkuirwl40gg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=J+NsGq+Z; 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="J+NsGq+Z" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 261221F000E9; Thu, 25 Jun 2026 14:27:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1782397637; bh=KI3dWK73xGnVU+pIVknxWpsIpvRbOvudq2UA4+TXP/I=; h=Date:From:To:Cc:Subject:References:In-Reply-To; b=J+NsGq+ZI4fvGX8kJDFAraUgVvTUUuuF6XQJNX2NQwhy5XXwsMunrEFmB3SZwQStM Yp0xm/UhGXkbmEYqK0Mj3VWNvH7GKdd+ARKqlXWGXg6nDLxPma2Kyf880DYWi2uyzc wYx+SdoVKjaOnjOsCjBVuLdN4kTykgnXJSOtB1h3nAmUE4e8FaZ9xEvxKKKND+at4r KSJmaZl9V6D6CB/ze3oodqLhRr9tcNVHf5bhi0ZJU9qkvz5EeIgeyrf07H3mxlhRrS v9qDoNBL4YhBlI0lgVVhfp4lusZifLAU619MiR51y0Z8+OCYgPjfMyxhQsGDTQEphP R2EUH6JPYIaUA== Date: Thu, 25 Jun 2026 15:27:08 +0100 From: Lorenzo Stoakes To: Pedro Falcato Cc: Xuewen Wang , akpm@linux-foundation.org, liam@infradead.org, vbabka@kernel.org, jannh@google.com, chrisl@kernel.org, kasong@tencent.com, shikemeng@huaweicloud.com, nphamcs@gmail.com, baoquan.he@linux.dev, baohua@kernel.org, youngjun.park@lge.com, qi.zheng@linux.dev, shakeel.butt@linux.dev, axelrasmussen@google.com, yuanchu@google.com, weixugc@google.com, david@kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] mm: annotate data-race in cpu_needs_drain() and need_mlock_drain() Message-ID: References: <20260625065153.1581419-1-wangxuewen@kylinos.cn> 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: On Thu, Jun 25, 2026 at 10:31:24AM +0100, Pedro Falcato wrote: > On Thu, Jun 25, 2026 at 02:51:53PM +0800, Xuewen Wang wrote: > > KCSAN reports a data-race when cpu_needs_drain() reads another CPU's > > per-cpu folio_batch->nr without locking, while the owning CPU writes > > to it via folio_batch_add(). The same race exists in need_mlock_drain() > > which is called from cpu_needs_drain(). > > > > Reading a slightly stale value is harmless -- cpu_needs_drain() only > > decides whether to schedule a drain, and the next iteration of > > __lru_add_drain_all() will re-check. > > > > All other callers of folio_batch_count() either use stack variables or > > access their own CPU's per-cpu data where no race exists, so > > data_race() is added at the call sites rather than in > > folio_batch_count() itself to avoid suppressing KCSAN warnings for > > future callers that may have real bugs. > > > > Signed-off-by: Xuewen Wang > > --- > > Changes in v2: > > - Use data_race() instead of READ_ONCE() in folio_batch_count(), as > > suggested by Lorenzo. READ_ONCE() is unnecessary for a single-byte > > read and imposes overhead on all callers, most of which have no race. > > - Move the annotation from folio_batch_count() to the actual call sites > > (cpu_needs_drain() and need_mlock_drain()) where the cross-CPU race > > occurs, rather than affecting all callers. > > - Add need_mlock_drain() which has the same cross-CPU race. > > - Add comments explaining why the data race is safe. > > v1: > > https://lore.kernel.org/all/20260624092606.1083449-1-wangxuewen@kylinos.cn/ > > --- > > mm/mlock.c | 2 +- > > mm/swap.c | 12 ++++++------ > > 2 files changed, 7 insertions(+), 7 deletions(-) > > > > diff --git a/mm/mlock.c b/mm/mlock.c > > index 8c227fefa2df..fbdb5018e2c3 100644 > > --- a/mm/mlock.c > > +++ b/mm/mlock.c > > @@ -232,7 +232,7 @@ void mlock_drain_remote(int cpu) > > > > bool need_mlock_drain(int cpu) > > { > > - return folio_batch_count(&per_cpu(mlock_fbatch.fbatch, cpu)); > > + return data_race(folio_batch_count(&per_cpu(mlock_fbatch.fbatch, cpu))); > > } > > > > /** > > diff --git a/mm/swap.c b/mm/swap.c > > index 588f50d8f1a8..d046428caed6 100644 > > --- a/mm/swap.c > > +++ b/mm/swap.c > > @@ -828,12 +828,12 @@ static bool cpu_needs_drain(unsigned int cpu) > > struct cpu_fbatches *fbatches = &per_cpu(cpu_fbatches, cpu); > > > > /* Check these in order of likelihood that they're not zero */ > > - return folio_batch_count(&fbatches->lru_add) || > > - folio_batch_count(&fbatches->lru_move_tail) || > > - folio_batch_count(&fbatches->lru_deactivate_file) || > > - folio_batch_count(&fbatches->lru_deactivate) || > > - folio_batch_count(&fbatches->lru_lazyfree) || > > - folio_batch_count(&fbatches->lru_activate) || > > + return data_race(folio_batch_count(&fbatches->lru_add)) || > > + data_race(folio_batch_count(&fbatches->lru_move_tail)) || > > + data_race(folio_batch_count(&fbatches->lru_deactivate_file)) || > > + data_race(folio_batch_count(&fbatches->lru_deactivate)) || > > + data_race(folio_batch_count(&fbatches->lru_lazyfree)) || > > + data_race(folio_batch_count(&fbatches->lru_activate)) || > > need_mlock_drain(cpu) || > > has_bh_in_lru(cpu, NULL); > > } > > eww. > > How about: > > static bool cpu_needs_drain(unsigned int cpu) > { > struct cpu_fbatches *fbatches = &per_cpu(cpu_fbatches, cpu); > > /* Check these in order of likelihood that they're not zero */ > return data_race( > folio_batch_count(&fbatches->lru_add) || > folio_batch_count(&fbatches->lru_move_tail) || > folio_batch_count(&fbatches->lru_deactivate_file) || > folio_batch_count(&fbatches->lru_deactivate) || > folio_batch_count(&fbatches->lru_lazyfree) || > folio_batch_count(&fbatches->lru_activate) || > need_mlock_drain(cpu)) || > has_bh_in_lru(cpu, NULL); > } > > this should work equally well, while being far more aesthetically pleasing :) Yes...! > > > -- > > 2.25.1 > > > > -- > Pedro Cheers, Lorenzo