Re: [PATCH] locking/osq_lock: Fix false sharing of optimistic_spin_node in osq_lock

[Waiman Long]

On 6/14/23 21:58, Waiman Long wrote:
> > diff --git a/kernel/locking/osq_lock.c b/kernel/locking/osq_lock.c
> > index d5610ad52b92..bdcd216b73c4 100644
> > --- a/kernel/locking/osq_lock.c
> > +++ b/kernel/locking/osq_lock.c
> > @@ -24,7 +24,7 @@ static inline int encode_cpu(int cpu_nr)
> >     static inline int node_cpu(struct optimistic_spin_node *node)
> >   {
> > -    return node->cpu - 1;
> > +    return node->prev_cpu - 1;
> >   }
>
> From the first glance, the patch looks OK. The only issue that I have 
> so far is the "node_cpu" name which is misleading in this case. You 
> should have renamed it to node_prev_cpu() instead as there is only one 
> caller.

One more thing. Unlike node->cpu, node->prev_cpu is subjected to change. 
So you should have use READ_ONCE() to read it to match the WRITE_ONCE() 
that modifies it.

Thanks,
Longman