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

From: Peter Zijlstra

Date: Mon Sep 14 2026 - 08:59:08 EST


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.