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 D957535AC17 for ; Mon, 29 Jun 2026 16:45:33 +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=1782751534; cv=none; b=TyKmB9UQLwv/kD5drw42DiQONorgqvIaybenOfuas1GflufxkfAXYRpqR5Ht8Rx8MaJWyVPS3QKwozF/xpWcIYYChnR1VNWb+/oGLuiwU863Fze7jZ3myVKzxxogmJZGKXprfOsEz6FFIkJG7qRSRpFkAyJOocSbBwlhdgeS2A4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782751534; c=relaxed/simple; bh=qINY8pLzDoBybTuaVZvJq3rv48LrGjfJltJdQiQf8+g=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=fxXeACu4gWesmBe1+u54ukvf0JJDHtSFu9ZXRFlQIZyr7WTQi/czBI+9Y+3ck0gtHmDO4mNT9uKP9G3kkzC2uiKLnXVAaYVfvsItCFCmBMt+Pntgbd+Gerwds5hHOeCcSIa8wQASM2m9XAz9XBXy0adIjD31Pa7ZELWKRRMM1NA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=Lmn4qK3g; 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="Lmn4qK3g" Received: by smtp.kernel.org (Postfix) with ESMTPSA id BE4B31F000E9; Mon, 29 Jun 2026 16:45:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1782751533; bh=xcjVTGjAJE+I7o68X7PqqKi8r93qpsaE24acBtIWCzU=; h=Date:From:To:Cc:Subject:References:In-Reply-To; b=Lmn4qK3gZL8FI01KVbpmGZnH3S6ngM9jaoe2ZOPWkRG6jz1A3D1LukNUy8uVkZeSy nQ1IYmQORVGHOUlXofB0DEWa/yweWnETjgOhQlNAk7XsDdUtgv6z53oviBDaK+xsMK XI8vbUn4d+d8dDjYr9gMnG/qPZe51jDqnToAo2fJ0hA8mi8gz5T9B0JlVVKVAqrK/8 2poqo2uGHw17WyZ/IX6qLYaugoG7SrFMZffvru3LrYRp+QiXhToC5SvGC8LNABuw6m ihDwdWEDLO5dNH8cA3yj0g2lGnkFh3W4v1xXGR67UZrrHLGMyxTl8muAMoudW7LfIx XQREl8mVYlgtQ== Date: Mon, 29 Jun 2026 17:45:22 +0100 From: Lorenzo Stoakes To: Andrew Morton Cc: "David Hildenbrand (Arm)" , Xuewen Wang , liam@infradead.org, vbabka@kernel.org, jannh@google.com, pfalcato@suse.de, 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, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v3] mm: annotate data-race in cpu_needs_drain() Message-ID: References: <20260626053700.2036899-1-wangxuewen@kylinos.cn> <04ea3e05-5b83-4c2d-8759-67c544a7b484@kernel.org> <20260629093402.a4d14d4f95ccde7d4c804115@linux-foundation.org> 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: <20260629093402.a4d14d4f95ccde7d4c804115@linux-foundation.org> On Mon, Jun 29, 2026 at 09:34:02AM -0700, Andrew Morton wrote: > On Mon, 29 Jun 2026 11:23:26 +0100 Lorenzo Stoakes wrote: > > > > > folio_batch_count(&fbatches->lru_lazyfree) || > > > > folio_batch_count(&fbatches->lru_activate) || > > > > - need_mlock_drain(cpu) || > > > > + need_mlock_drain(cpu)) || > > > > > > The indentation is a bit suboptimal now. > > > > > > Would read nicer as > > > > > > diff --git a/mm/swap.c b/mm/swap.c > > > index 588f50d8f1a8c..5958e6fdd3593 100644 > > > --- a/mm/swap.c > > > +++ b/mm/swap.c > > > @@ -828,13 +828,13 @@ 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) || > > > - need_mlock_drain(cpu) || > > > + 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); > > > > Yeah that works for me. > > > > Andrew - maybe easier if you fix that up? :) > > > > Sure. > > --- a/mm/swap.c~mm-annotate-data-race-in-cpu_needs_drain-fix > +++ a/mm/swap.c > @@ -832,12 +832,12 @@ static bool cpu_needs_drain(unsigned int > > /* 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)) || > + 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); > } > > > The removal of data_race() in need_mlock_drain() is a little worrisome > - perhaps any future callers would have needed it? > > need_mlock_drain() has only a single caller. How about I remove it and > open-code it within cpu_needs_drain()? That references a static per-CPU variable (mlock_fbatch) in mlock.c's compilation unit so I think it has to stay as it us unfortunately. And it's better I think to only use the data_race() here where we definitely know we need it (and as the only instance of that). Cheers, Lorenzo