Re: [PATCH v4 next 3/9] locking/osq_lock: Set prev_cpu=0 instead of locked=1

[David Laight]

On Tue, 15 Sep 2026 10:50:13 +0200
Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:

> On Mon, Sep 07, 2026 at 09:41:27AM +0100, David Laight wrote:
> > There is no need for separate prev_cpu and locked members of
> > struct optimistic_spin_node.
> > Using a single field simplifies the code slightly.
> > It also removes any possibility of the two values being out of sync.
> > 
> > When cancelling a lock request explicitly set prev_cpu to zero.
> > Nothing actually looks at the field, but it means that it will be zero
> > after a subsequent 'fast path' osq_lock() call making things consistent.
> > The cache line is likely to be dirty (or be dirtied) so there shouldn't
> > be a performance hit.
> > 
> > Signed-off-by: David Laight <david.laight.linux@xxxxxxxxx>
> > ---  
> 
> 
> --- a/kernel/locking/osq_lock.c
> +++ b/kernel/locking/osq_lock.c
> @@ -170,10 +170,11 @@ bool osq_lock(struct optimistic_spin_que
>  	 * is per-cpu data the memory can always be read.
>  	 */
>  
> -	for (;; prev = READ_ONCE(node->prev)) {
> -		if (!prev)
> -			/* Lock acquired */
> +	for (;;) {
> +		if (!prev) {
> +			smp_acquire__after_ctrl_dep();

Do you think the equivalent might fix the existing code?
I think it is only going to matter if there isn't another read barrier
before the osq_unlock() - which is unlikely except for the test program.

>  			return true;
> +		}
>  
>  		prev_ptr = decode_cpu(prev);
>  
> @@ -185,8 +186,8 @@ bool osq_lock(struct optimistic_spin_que
>  		 * 'prev' must have unlinked (or be in the process of unlinking)
>  		 * itself from the list.
>  		 */
> -
>  		cpu_relax();
> +		prev = READ_ONCE(node->prev);

A matter of taste, I prefer it at the top.

David

>  	}
>  
>  	/*