From: Ingo Molnar
> Sent: 30 December 2023 20:38
> 
> * David Laight <david.lai...@aculab.com> wrote:
> 
> >  bool osq_lock(struct optimistic_spin_queue *lock)
> >  {
> > -   struct optimistic_spin_node *node = this_cpu_ptr(&osq_node);
> > +   struct optimistic_spin_node *node = raw_cpu_read(osq_node.self);
> >     struct optimistic_spin_node *prev, *next;
> >     int old;
> >
> > -   if (unlikely(node->cpu == OSQ_UNLOCKED_VAL))
> > -           node->cpu = encode_cpu(smp_processor_id());
> > +   if (unlikely(!node)) {
> > +           int cpu = encode_cpu(smp_processor_id());
> > +           node = decode_cpu(cpu);
> > +           node->self = node;
> > +           node->cpu = cpu;
> 
> This whole initialization sequence is suboptimal and needs to be
> cleaned up first: the node->cpu field is constant once initialized, so
> it should be initialized from appropriate init methods, not runtime in
> osq_lock(), right?

I thought that as well, but there would need to be a list of 'init'
functions for the per-cpu data. I didn't spot one.

        David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)


Reply via email to