From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (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 A8848331230 for ; Wed, 11 Mar 2026 19:27:19 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.133.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773257241; cv=none; b=qkCJFkUVTOJuu16yMSGfBaKHZ7doiqE4DKUNeYDh1FVoDYB9rJia7/aW0rGcw5RDbrs5baCaehutaEEPk/IW+X5/6YqLf2UZpzgohdnx8IpT6xLwdr95rd1pr9UmB2k0JE3GScH8TvJg3xS6/M4c0KWh9hhiNAMn3h1ZHPxdPfY= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773257241; c=relaxed/simple; bh=HbhOALS0xcH6u6D2utscJkaug4r9/QjP9Y7emqgovC4=; h=Message-ID:Date:MIME-Version:Subject:To:References:From: In-Reply-To:Content-Type; b=j69HUlTltt461mS395zIN+2p6v7/vCJcDOf7oiqppW6mDfvooOfBHa119mkX0NGCxUu6/i4ekNwMEXzQgLvY8xPWFMMu709jEHOsNeM+hl1E/0R5jLpqLOFlFLtNOeZl6U4B7s5F9UMwdOkN7vmE8LO0UtBgYIVf6fKrnYCCZHk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=PGXipLKh; arc=none smtp.client-ip=170.10.133.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="PGXipLKh" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1773257238; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=E1WWpzSwilqbxG40E/Wm5ChT5wKX8t830mAv5j6Ybz0=; b=PGXipLKhW/RiDEimhZl8Z/sr7xVe4yZnBio/sdqPGAmqmVN80ugdUuBDpKdt0GMkot8Yvk 7aopIZujBvxySLbwTM6O5MPEvyIrRxEpztrQXjMQ0sMi0lC2zGdDJev+NTO51xIWoAYG91 UYS0PbfBGxHr/XmCASDimql5M/n00+Y= Received: from mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-288-Z7Tu1A-pPd2IsSDyxDU74g-1; Wed, 11 Mar 2026 15:27:13 -0400 X-MC-Unique: Z7Tu1A-pPd2IsSDyxDU74g-1 X-Mimecast-MFC-AGG-ID: Z7Tu1A-pPd2IsSDyxDU74g_1773257232 Received: from mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.17]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id E24021956095; Wed, 11 Mar 2026 19:27:11 +0000 (UTC) Received: from [10.22.90.45] (unknown [10.22.90.45]) by mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id EE1BB1956095; Wed, 11 Mar 2026 19:27:09 +0000 (UTC) Message-ID: <3afd7e58-8e31-4dba-860c-2055ba372423@redhat.com> Date: Wed, 11 Mar 2026 15:27:08 -0400 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 next 5/5] Avoid writing to node->next in the osq_lock() fast path To: david.laight.linux@gmail.com, Peter Zijlstra , Ingo Molnar , Will Deacon , Boqun Feng , linux-kernel@vger.kernel.org, Linus Torvalds , Steven Rostedt , Yafang Shao References: <20260306225150.93178-1-david.laight.linux@gmail.com> <20260306225150.93178-6-david.laight.linux@gmail.com> Content-Language: en-US From: Waiman Long In-Reply-To: <20260306225150.93178-6-david.laight.linux@gmail.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 3.0 on 10.30.177.17 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. BTW, how much performance gain have you measured with this change? Can we just leave it there to be safe. > - > /* > * 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