Re: [PATCH v3 next 5/5] Avoid writing to node->next in the osq_lock() fast path

[Waiman Long <longman@redhat.com>]

On 3/11/26 3:27 PM, Waiman Long wrote:
> On 3/6/26 5:51 PM, david.laight.linux@gmail.com wrote:
>> From: David Laight <david.laight.linux@gmail.com>
>>
>> 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 <david.laight.linux@gmail.com>
>> ---
>>   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.
>
Or you could have something like

         if (IS_ENABLED(CONFIG_PROVE_LOCKING))
                 WARN_ON_ONCE(node->next != NULL);

Cheers,
Longman