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

[David Laight]

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

I'm glad you are actually going through these patches.

> On Tue, Sep 15, 2026 at 03:13:15PM +0200, Peter Zijlstra wrote:
> > On Mon, Sep 07, 2026 at 09:41:27AM +0100, David Laight wrote:
> >   
> > > +	/*
> > > +	 * If 'prev' tries to remove itself from the list before we write
> > > +	 * a new value to prev->next it will spin in osq_wait_next().
> > > +	 */  
> > 
> > This comment pertains to what? 

IIRC, perhaps it needs:
	Having written prev->next = NULL ...

> >   
> > > +	/* Invalidate prev_cpu matching osq_unlock() */
> > > +	node->prev = 0;  
> > 
> > I'm confused, osq_unlock() does no such thing.  

osq_unlock() sets it to zero on the node that it passes the lock to
(since that node is now at the head of the list).
The comment probably need a few more words.

> The next patch adds a comment stating that this store is not strictly
> needed. And only serves to ensure node->prev ends up being 0 in all
> paths.

I need to look again (the system with the patches is off) to see why
it might be non-zero after this patch.
The original unlock code just set 'next->locked' and left 'next->prev' pointing
into the abyss.
(One of the AI bots reported something about locked and prev getting
out of sync.)

> That comment *should* obviously have been here. Also, I would *much*
> rather have this store at the top, next to node->next = NULL; That is
> much saner.


Part of the original reason I started these patches was to remove
the writes to the current cpu's 'node' in the fast path acquire.
That completely saves reading the cache line.
It also means the initial xchg need only have acquire semantics.

David