From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f41.google.com (mail-wm1-f41.google.com [209.85.128.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 276A9395D8C for ; Mon, 18 May 2026 21:46:09 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.41 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779140771; cv=none; b=tij+wUI1mcyHiS1sTqtXPERkxHG00PtcbCqzbAO54G7z76OBdUz+RylsWmcJGqgSzJSz/N3i7JzOY48Fy5Iyrxqg4sPZCBFVPMBp0yfUJrDAZdtD4uFZY88CKteBlg5b0wxgoM3VZ7qCDHmtFS56jUYr551IIFvJdiw3+HyGjOI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779140771; c=relaxed/simple; bh=HPEFS83TLBDvZNefyNKoeiiNMbIA3WBgJ6n441Wm6Jg=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=Rp7or1vDjc+uJyFw3DD3cKs0U2usZ+FkCaeF4lls+7S966HnC5ylfaT9XGXTpsVcdaB0ZmzXUYGkU8JCatFFaonq+Qe38ToilgsUS1HTVlGTwMl/C03MgnvPONNM1cTCnfUMFxHcde9oJBr9QCaF1qsrrWy34T5BnPdQInV0GUw= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=ZdctUeZ4; arc=none smtp.client-ip=209.85.128.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="ZdctUeZ4" Received: by mail-wm1-f41.google.com with SMTP id 5b1f17b1804b1-488b8bc6bc9so16253255e9.3 for ; Mon, 18 May 2026 14:46:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1779140768; x=1779745568; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=ca/GAo9J7O59kJzlmVcDRivI4o8pW5NCQo0yI5mTv4A=; b=ZdctUeZ4PzbfdDcnuH86xsm/f+HLDcJICzttRMvjVKTJttm8ROLA3REYyu2TmNIru4 2KamG5IZqTXi8jdTTmDJlvRZ5MuMKcIQ6tyTwZWGPHT2ZFg+zouMY8G2aSC0CMM2OKMT qch6NRs1v+V+PtHLYz+tOVRTJjnCx9dRagLEMZdQi61BmIWQR3JCgztGvEeTabtV/GZl YLgz7d3G2oNpoXTenIZhsiIS3zhApNnG4pUk3TVGEU1hQlKun4Y8iSd+hTEdjungwITR tiazwL4sfnBAmmRf67rNMVEnno2hLvqzCgPxj0YaDZvouJqOJEWLlrCNQzj41N9XWRFm BcuA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1779140768; x=1779745568; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=ca/GAo9J7O59kJzlmVcDRivI4o8pW5NCQo0yI5mTv4A=; b=F2YaU/kLXzXqD9P8Cj6MQA1DH81E9GplIOvwTXwQPzZsypaFygh9/ojywVNQdt1ueU hSElYR2wc/vDSMQMa1nGn8XNM0F+NVlgqYH3Q8LR6NT9R3TFG4gmC8whfq4mYo4n918Y /UYmdtuD54EEbXrpsecIH1vLWEDxryMqTsQaGoJ8400dL+Rja4nCIlcB/R5/z8S0JOQi gLZcMFUjzJ5VZq1dphJoQLKDBR24jT3W8KhVk1j0mC3a5ao0gom9sKjOIghuidH2FrYk DGt38plvX280uNPqEacMrPVbCpcfVp5yQS9/F/AC/6Oy5/mnB5s/wtujt1kqKLRi7bFM Q6wA== X-Forwarded-Encrypted: i=1; AFNElJ99MQQXH3n3WEFTePcAVEy/+ChV0TPmzGcNqnmohhVN5Y2UWiuuuRe4th5ooZdWf5fLqtkXkCSjarjBR0w=@vger.kernel.org X-Gm-Message-State: AOJu0YznEDddWZZl8v8fw98/P7zc04x8lz0sYavFSoIhvAmw/CBB5AnY iG/PfHasR9PRx74Pdz4bHarlMb0cK0WcH/RhPFpSDgtys4nkqFTdtL2h X-Gm-Gg: Acq92OGzcV9wPQWF/YFMAqVpfvKu30tLaG55EZK5LJw7UaQAI/ViC6CNgapBm5shgMI BIsPW3CKb9WPfbTGCvV/JY8upcvgeoK+chaiwrALuFQCs5rtPh+nWjRTLoyl62PKppSPvExHo99 JUazu8FeBt+ebX38RI/XBqc21x7/v+gqsgZUacWNVLRegggwu9l8gBlulmYZuq7z35BsYTMuXMt yn3qd7toNOH9LW2lyPyuL3xTXqIquUZXmB9622Zpewy4LIJWvLuVw2+KC4SYGepqL2JNO7Ap7QF pv1T4C1GcRph/EBmxDVnAtb7X/0OtNf+Z87ISVZdraK0I75WK0rHtzioSaCnqLNNUpNgTPlufNZ PAQXetQIDZxyUx3+T4w/Hr+J1eVOIGkZvrbiTfFgsznto1JO7RliIF8tMwWQ13sOEtjq6tLd+UF 4IS0zXei73UwPKBJqFb9mAIrEbyckwDAoSqwWPfh96K1JJ1Z4u8pXvEPQ9ncsl5qViiO2CmnNv7 oY= X-Received: by 2002:a05:600c:8b6e:b0:485:9a50:3370 with SMTP id 5b1f17b1804b1-48fe60ecc24mr275209565e9.8.1779140768311; Mon, 18 May 2026 14:46:08 -0700 (PDT) Received: from pumpkin (82-69-66-36.dsl.in-addr.zen.co.uk. [82.69.66.36]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-48fe4c833fcsm281508165e9.2.2026.05.18.14.46.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 18 May 2026 14:46:08 -0700 (PDT) Date: Mon, 18 May 2026 22:46:05 +0100 From: David Laight To: Will Deacon Cc: Yu Peng , 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: <20260518224605.63169ccc@pumpkin> In-Reply-To: References: <20260330013255.25937-1-pengyu@kylinos.cn> X-Mailer: Claws Mail 4.1.1 (GTK 3.24.38; arm-unknown-linux-gnueabihf) 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-Transfer-Encoding: 7bit On Mon, 18 May 2026 11:29:53 +0100 Will Deacon wrote: > 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've just found my unposted patches (later than the v3 ones posted mid march). This all got sorted. Basically node->prev can be replaced by node->prev_cpu and then node->locked is equivalent to node->prev_cpu == 0. It ends up with vcpu_is_preempted(VAL - 1). The final struct optimistic_spin_node just has two 'int' members for the next and prev cpu numbers. I think I got bogged down trying to fix the comments. Although the vcpu_is_preempted() return value is always stale. So it just can't matter if the code does 'return false' at any time. Otherwise it would all be terribly broken anyway. (I've never looked at the callers of this code.) David > > I'd be much more comfortable if this was smp_cond_load_acquire(), in > addition to the READ_ONCE() that you are proposing. > > Will >