From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f42.google.com (mail-wr1-f42.google.com [209.85.221.42]) (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 E28381B4138 for ; Wed, 11 Mar 2026 21:50:48 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.42 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773265850; cv=none; b=cuyOlGu05WJK0XjGfKKX3a9NpT92wg9fO1xrxLk3iQJPDR4CSvORjnoD249zPxGVKMuzDwM0srI+iyX/T4UeDTkBN5Dn4NGEEc+XYmXXxM+gu4jzTBCkt5jOTrD1ukCHkgbUTx2vdsfwWJcP2qXf+CaABE2UXtOhOrqeX5PZYng= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773265850; c=relaxed/simple; bh=/buOpicRs78lOzl5lZieqi8MT57d+TnZSXVKbfCZMbc=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=D8eWOLr23cLgKKniSV7xkbaXA7jQDlq89ano1bCr2+Y/m0S08pkBIeURvxqkSe0RV6PJkxZBdcQX49E4WVzxR4wPqkXjfDV/gwe+KmrnBk0Fy0RwJ2chgiWBNeltjS4/7ch95300GX0GHj6M8Oo0RCMtyiGVGRZ7zc51Jzy8c+s= 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=TusVC5XF; arc=none smtp.client-ip=209.85.221.42 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="TusVC5XF" Received: by mail-wr1-f42.google.com with SMTP id ffacd0b85a97d-439b611274bso228404f8f.3 for ; Wed, 11 Mar 2026 14:50:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1773265847; x=1773870647; 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=dxPAk8vvFUdJqp47ibshZK62V2gk8jv+YsUteCjeDaY=; b=TusVC5XF7HbbSMRUk6rBp8Gn6D5GaV32NPlxFp4kVMVlPNJUEZjAifQ2YNevOSHPoa UesIB1mi+6eu+nVi/QksfrJ7V8yclJUONjohtc5Ia0lvth+wHYrt68c/8cdt/RT1FLYe ERLguMgvlpJMNjL8mBjuhwuVaaV5D+LNIDmzuidNjBo6zLsP1PHnDD/ZBWdNMDTRhlOf FS5gFwNyyJuR3Kq+JyHww4YALHmddAlCl3DphPpPtha80NpneVPoQy0BYnRDCG44/1cQ lk0PMV5JvoP30MGxWBa+Li7XSLLTwjOC+5YlCBK1Pt0ZPFhaaqVzPsr0Bysgt/ICU/TV JxRA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1773265847; x=1773870647; 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=dxPAk8vvFUdJqp47ibshZK62V2gk8jv+YsUteCjeDaY=; b=RaHBgsDNzS2N9lSCmuNXJfV+cDoHVo2rv1sp1IXDIHjBvwAfAME7VpdMjKgDaZk6x5 SBvtBHB30X3/t1X1WYFshgW6/WeVy8mt/tkKp7PFakI9a21NrdMdorxdTuNgoarwNvo0 x+MJB7krZvcq8E5IgwxaGc4o3cEoNkN7KZyo6tOWjChW/5RbPiVp1s6alHaTV0cLcwEo XSLYlTriAnJfEyFdpl7BU1Byc+NhJSqK/SLexIJ63LtHFWtV1oJA1W7nhXEpnlkVUUES RjNkY7jhrMJ3hhvD+hqtWF7vwl7BX57czYyL3qfxTW7O/dq3XOLSrjpXRMO/sPllp1Z+ /XzQ== X-Forwarded-Encrypted: i=1; AJvYcCX9pmtnjUEzJNPoiwaw8Z6d+9B0+HOOaYSIwE2o2gP75i/hOQC8UjYZlPWI2bQMgM/pbK0hkwLtFBDFf8E=@vger.kernel.org X-Gm-Message-State: AOJu0YzdzC1aMF9j7gO/8Dw6+Q4WPRkkHLJRQ9sqKGmjKArOGItUKJRH 6lQ83Yyyvwk2QPP6I4qrQhAIJyI3rgca/Hehm8MUpM8DWwOKBrSUvXGQ X-Gm-Gg: ATEYQzyOm/mmt3AxocCg8o5FopNwN1NPITqilct5rM7bsYtPvHIDP0Lvy3vexJcJN+f awjzLBuZ9eUVK9xhG2Fec9GRYsvnLcueof/hkX5aK0BUgMfYabn08U4JOhx4c4TuDCO5XkJdOuY SKg60PVIcug+wKPRNmcwT4ZEyFH6LRHhJLm1D+zYnRaAqpBmrJNJo34VqHGtjcavxD9OfxKkVq/ HeY/EJNpHc8Cg7deRWd+/T1PnCFaUohX2eQNzHqDll2Xc5oaZvbXFD3eiMuxOUh8NLhzlSptnBL B0id5pqR40LU3MyxHkyBo0EN8BvhYRNP8z2ZbSzfq60ioxZn4YeevH7GgF6ZDk4d890+OgM+jRt C4m0w5MfhMWIRceyTZUXfY1dFy4KEclp+EZFUxxhXMtoKHCBxgD3dUpMldCgtbotKa9uoEUDr+8 VGxmv0WRgxkluPF8zGJpz3k8E877fSLdchLgb4KOSdX/TNUBQqpWlzVcrxJT3qttaF X-Received: by 2002:a05:6000:2406:b0:439:c008:d81 with SMTP id ffacd0b85a97d-439f81e7d41mr7749378f8f.15.1773265846886; Wed, 11 Mar 2026 14:50:46 -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 ffacd0b85a97d-439fe20b544sm2579876f8f.20.2026.03.11.14.50.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 11 Mar 2026 14:50:46 -0700 (PDT) Date: Wed, 11 Mar 2026 21:50:45 +0000 From: David Laight To: Waiman Long Cc: Peter Zijlstra , Ingo Molnar , Will Deacon , Boqun Feng , linux-kernel@vger.kernel.org, Linus Torvalds , Steven Rostedt , Yafang Shao Subject: Re: [PATCH v3 next 5/5] Avoid writing to node->next in the osq_lock() fast path Message-ID: <20260311215045.5b6a2c65@pumpkin> In-Reply-To: <3afd7e58-8e31-4dba-860c-2055ba372423@redhat.com> References: <20260306225150.93178-1-david.laight.linux@gmail.com> <20260306225150.93178-6-david.laight.linux@gmail.com> <3afd7e58-8e31-4dba-860c-2055ba372423@redhat.com> 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 Wed, 11 Mar 2026 15:27:08 -0400 Waiman Long wrote: > On 3/6/26 5:51 PM, david.laight.linux@gmail.com wrote: > > From: David Laight > > > > When osq_lock() returns false or osq_unlock() returns static > > analysis shows that node->next should always be NULL. > > This means that it isn't necessary to explicitly set it to NULL > > prior to atomic_xchg(&lock->tail, curr) on entry to osq_lock(). > > > > Defer determining the address of the CPU's 'node' until after the > > atomic_exchange() so that it isn't done in the uncontented path. > > > > Signed-off-by: David Laight > > --- > > kernel/locking/osq_lock.c | 6 ++---- > > 1 file changed, 2 insertions(+), 4 deletions(-) > > > > diff --git a/kernel/locking/osq_lock.c b/kernel/locking/osq_lock.c > > index 0619691e2756..3f0cfdf1cd0f 100644 > > --- a/kernel/locking/osq_lock.c > > +++ b/kernel/locking/osq_lock.c > > @@ -92,13 +92,10 @@ osq_wait_next(struct optimistic_spin_queue *lock, > > > > bool osq_lock(struct optimistic_spin_queue *lock) > > { > > - struct optimistic_spin_node *node = this_cpu_ptr(&osq_node); > > - struct optimistic_spin_node *prev, *next; > > + struct optimistic_spin_node *node, *prev, *next; > > unsigned int curr = encode_cpu(smp_processor_id()); > > unsigned int prev_cpu; > > > > - node->next = NULL; > > Although it does look like node->next should always be NULL when > entering osq_lock(), any future change may invalidate this assumption. I > know you want to not touch the osq_node cacheline on fast path, but we > will need a big comment here to explicitly spell out this assumption to > make sure that we won't break it in the future. I've been looking at the code some more. The first new patch adds a load of comments. Unfortunately they are rather wrong (or in the wrong place) by the time I finished. So I need to go around again at least once. The main extra changes: - Set node->prev to zero instead of node->locked = 1. This makes the loop after the smp_cond_load_relaxed() better. And the write to next->prev can move into osq_wait_next(). - Just call osq_wait_next() from osq_unlock() - it is the same code. Removes the unconditional cmpxchg on lock->tail (which needs to be _release in both places). - Change node->next to be the cpu number - it is only used as a pointer once (I think Linus suggested that once). - Stop 'node' being cache-line aligned - it only contains two cpu numbers. Just 8 byte align so it is all in one cache line. The data for the 'wrong' cpu is hardly ever accessed. > > BTW, how much performance gain have you measured with this change? Can > we just leave it there to be safe. You think I've been brave enough to run my changes :-) Actually I've written a little test harness that lets me compile and run the actual kernel source in usespace and 'prod' it with requests. That let me add delays at particular points to check the unusual paths (but not the effects of acquire/release). I will run the next changes, hopefully there won't be anything nasty in the rc kernel to catch me out. But I've no real idea of how to exercise the code. David > > > - > > /* > > * We need both ACQUIRE (pairs with corresponding RELEASE in > > * unlock() uncontended, or fastpath) and RELEASE (to publish > > @@ -109,6 +106,7 @@ bool osq_lock(struct optimistic_spin_queue *lock) > > if (prev_cpu == OSQ_UNLOCKED_VAL) > > return true; > > > > + node = this_cpu_ptr(&osq_node); > > WRITE_ONCE(node->prev_cpu, prev_cpu); > > prev = decode_cpu(prev_cpu); > > node->locked = 0; > > I am fine with moving the initialization here. The other patches also > look good to me. > > Cheers, > Longman >