Re: [PATCH v4 next 2/9] locking/osq_lock: Save the cpu number for 'prev' not the node address

[Waiman Long]

On 9/15/26 6:26 AM, Peter Zijlstra wrote:
> On Tue, Sep 15, 2026 at 10:33:40AM +0200, Peter Zijlstra wrote:
> > On Mon, Sep 07, 2026 at 09:41:26AM +0100, David Laight wrote:
> > > The cpu number of node->prev is needed for both the vcpu_is_preempted()
> > > test and to update lock->tail.
> > > This saves reading the cache line for the other cpu's per-cpu data.
> > >
> > > The cpu member of optimistic_spin_node is no longer needed.
> > >
> > > Merges patches 2 and 3 from v3.
> > >
> > > Signed-off-by: David Laight <david.laight.linux@xxxxxxxxx>
> > > ---
> > >   kernel/locking/osq_lock.c | 33 ++++++++++++++-------------------
> > >   1 file changed, 14 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/kernel/locking/osq_lock.c b/kernel/locking/osq_lock.c
> > > index b17aa704c449..01988d00c480 100644
> > > --- a/kernel/locking/osq_lock.c
> > > +++ b/kernel/locking/osq_lock.c
> > > @@ -34,9 +34,9 @@
> > >    */
> > >   
> > >   struct optimistic_spin_node {
> > > -	struct optimistic_spin_node *next, *prev;
> > > +	struct optimistic_spin_node *next;
> > >   	int locked; /* 1 if lock acquired */
> > > -	int cpu; /* encoded CPU # + 1 value */
> > > +	int prev; /* CPU number offset by 1 */
> > >   };
> > >   
> > >   static DEFINE_PER_CPU_SHARED_ALIGNED(struct optimistic_spin_node, osq_node);
> > > @@ -114,13 +109,12 @@ 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 *prev_ptr, *next;
> > >   	int curr = encode_cpu(smp_processor_id());
> > > -	int old;
> > > +	int prev;
> > I'm not a fan in the asymmetry of the naming, that is very confusing at
> > best.
> Maybe something like so?
>
> ---
> --- a/kernel/locking/osq_lock.c
> +++ b/kernel/locking/osq_lock.c
> @@ -34,9 +34,9 @@
>    */
>   
>   struct optimistic_spin_node {
> -	struct optimistic_spin_node *next, *prev;
> +	struct optimistic_spin_node *next;
>   	int locked; /* 1 if lock acquired */
> -	int cpu; /* encoded CPU # + 1 value */
> +	int prev_cpu; /* CPU number offset by 1 */
>   };

Yes, I like the prev_cpu name better than prev. It makes the code easier 
to understand.

Cheers,
Longman