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

[David Laight]

On Mon, 14 Sep 2026 14:01:47 +0200
Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:

> On Wed, Sep 09, 2026 at 07:52:23PM +0100, David Laight wrote:
> > On Wed, 9 Sep 2026 14:01:59 -0400
> > Waiman Long <longman@xxxxxxxxxx> wrote:
> >   
> > > On 9/7/26 4:41 AM, 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>
> > > > ---
> > > >   kernel/locking/osq_lock.c | 57 +++++++++++++++++++--------------------
> > > >   1 file changed, 28 insertions(+), 29 deletions(-)
> > > >
> > > > diff --git a/kernel/locking/osq_lock.c b/kernel/locking/osq_lock.c
> > > > index 01988d00c480..23f00c670507 100644
> > > > --- a/kernel/locking/osq_lock.c
> > > > +++ b/kernel/locking/osq_lock.c
> > > > @@ -35,7 +35,6 @@
> > > >   
> > > >   struct optimistic_spin_node {
> > > >   	struct optimistic_spin_node *next;
> > > > -	int locked; /* 1 if lock acquired */
> > > >   	int prev; /* CPU number offset by 1 */
> > > >   };
> > > >       
> > > I think we should document the fact that prev=0 can be viewed as a 
> > > marker that the osq lock has been acquired.  
> > 
> > I think that happens a bit later in the series.
> > Trying to keep the comments in step is quite hard work.  
> 
> Still, that's what you gotta do. Now you're asking us to try and reverse
> engineer things while reviewing.

That's what I had to do before changing the code :-)
It'll win a 'zero unless waiting' comment in the next version.

David