From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 0F9733ECBE9 for ; Mon, 18 May 2026 10:29:58 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779100199; cv=none; b=oJ4FE9TKWbwlKDQ2C9l3Zretr92rCWwEOuzd3RkanRlh8D18Z4XKlg9L4srX/dWXILYcMjf3M/OeHby0jLM2krg2MDZxdooQOw81UMjks+Of2BHMcP7Azi/W3ZigdBmSptKm7Tiagyp5IsDvUU1V02OBpYh3nbGO9vtaMiIUhxo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779100199; c=relaxed/simple; bh=t1NO2uV40ogN8ivCy3yiprvPKQcKO84tlE3x3DlS56g=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Vi3rBOn1OkZ0/S+2A2VfAhOuCRuaLJ+raweO8ruH3K+aCRZGn8U/PMNiclLvCXocnuQgFB00lRFLCOXRfTDQZExdWzh1g2VdOwKS27a89DJ+20wQ/qX0cASVms90SurRq++p+138YdWnAZfkJeUl8PAfZDLTXrrIcpAJcCZ13rs= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=hP8xns+W; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="hP8xns+W" Received: by smtp.kernel.org (Postfix) with ESMTPSA id BF65DC2BCB7; Mon, 18 May 2026 10:29:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1779100198; bh=t1NO2uV40ogN8ivCy3yiprvPKQcKO84tlE3x3DlS56g=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=hP8xns+WuSJ8LlKS7JA2qI2ZZlTL53C8HRWq+kJMHadHSn9BF/3E7uUAiXqKCfQzg 31JhyPcD4Fyfw06EHJZjZyvwCtHEXooENY6t4yhlF+0dNGlLsL1rwyxetSOupYYT2v zpQiJxhJD8z+x6DhwwpSu8rF3IZtNqOWh4PNSK0H4qMq8P89YMGEJX5tK4Se8NigRQ mBvUeQdDXhJdgRq+8xyXKnMeyTJlE2eSfxOFXtyi8Mm3dhcrqbacbtPsNjqtaL3O/K Q6XaS5KSmx++UqwtldTmWf0V5pI8GJalGsCLdovTaUeiLzj8G1SKVZbR9TABz3GmwN DeZtgYmd+8XkQ== Date: Mon, 18 May 2026 11:29:53 +0100 From: Will Deacon To: Yu Peng Cc: Peter Zijlstra , Ingo Molnar , Boqun Feng , Waiman Long , linux-kernel@vger.kernel.org Subject: Re: [PATCH] locking/osq_lock: Use READ_ONCE() for node->prev Message-ID: References: <20260330013255.25937-1-pengyu@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: <20260330013255.25937-1-pengyu@kylinos.cn> On Mon, Mar 30, 2026 at 09:32:55AM +0800, Yu Peng wrote: > osq_lock() consults node->prev in the vcpu_is_preempted() heuristic while > a concurrent predecessor may update it via WRITE_ONCE(next->prev, prev) > during unqueue. > > This read only affects the decision to abort optimistic spinning; stale > values do not affect queue linkage or lock correctness. Use READ_ONCE() > to mark the shared read and match the concurrent WRITE_ONCE() update. > > No functional change intended. > > Signed-off-by: Yu Peng > --- > kernel/locking/osq_lock.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/locking/osq_lock.c b/kernel/locking/osq_lock.c > index b4233dc2c2b04..db4545e7bb72c 100644 > --- a/kernel/locking/osq_lock.c > +++ b/kernel/locking/osq_lock.c > @@ -144,7 +144,7 @@ bool osq_lock(struct optimistic_spin_queue *lock) > * polling, be careful. > */ > if (smp_cond_load_relaxed(&node->locked, VAL || need_resched() || > - vcpu_is_preempted(node_cpu(node->prev)))) > + vcpu_is_preempted(node_cpu(READ_ONCE(node->prev))))) > return true; Hmm, I wonder whether this is actually sufficient... Architectures with relaxed memory models won't order plain reads to different addresses, so the read of 'node->locked' is unordered wrt the read of 'node->prev' in this condition. Given that we're using smp_cond_load_relaxed(), can we end up using a value of 'node->prev' that was loaded in a previous iteration of the loop? I'd be much more comfortable if this was smp_cond_load_acquire(), in addition to the READ_ONCE() that you are proposing. Will