Received-SPF: pass (google.com: domain of linux-kernel+bounces-13605-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
From: David Laight <David.Laight@ACULAB.COM>
To: 'Ingo Molnar' <mingo@kernel.org>
CC: "'linux-kernel@vger.kernel.org'" <linux-kernel@vger.kernel.org>,
	"'peterz@infradead.org'" <peterz@infradead.org>, "'longman@redhat.com'"
	<longman@redhat.com>, "'mingo@redhat.com'" <mingo@redhat.com>,
	"'will@kernel.org'" <will@kernel.org>, "'boqun.feng@gmail.com'"
	<boqun.feng@gmail.com>, 'Linus Torvalds' <torvalds@linux-foundation.org>,
	"'xinhui.pan@linux.vnet.ibm.com'" <xinhui.pan@linux.vnet.ibm.com>,
	"'virtualization@lists.linux-foundation.org'"
	<virtualization@lists.linux-foundation.org>, 'Zeng Heng'
	<zengheng4@huawei.com>
Subject: RE: [PATCH next 4/5] locking/osq_lock: Optimise per-cpu data
 accesses.
Thread-Topic: [PATCH next 4/5] locking/osq_lock: Optimise per-cpu data
 accesses.
Thread-Index: Ado6mZUJWFdx4PkETd+mn/PWVjPd0AAxm5QAAARO0qA=
Date: Sat, 30 Dec 2023 22:47:19 +0000
Message-ID: <f11f674ae0ab4ef0a2ae76235c6352d2@AcuMS.aculab.com>
References: <73a4b31c9c874081baabad9e5f2e5204@AcuMS.aculab.com>
 <bddb6b00434d4492abca4725c10f8d5a@AcuMS.aculab.com>
 <ZZB/jIvKgKQ2sV7M@gmail.com>
In-Reply-To: <ZZB/jIvKgKQ2sV7M@gmail.com>
Accept-Language: en-GB, en-US
Precedence: bulk
MIME-Version: 1.0
Content-Language: en-US
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

From: Ingo Molnar
> Sent: 30 December 2023 20:38
>=20
> * David Laight <David.Laight@ACULAB.COM> wrote:
>=20
> >  bool osq_lock(struct optimistic_spin_queue *lock)
> >  {
> > -=09struct optimistic_spin_node *node =3D this_cpu_ptr(&osq_node);
> > +=09struct optimistic_spin_node *node =3D raw_cpu_read(osq_node.self);
> >  =09struct optimistic_spin_node *prev, *next;
> >  =09int old;
> >
> > -=09if (unlikely(node->cpu =3D=3D OSQ_UNLOCKED_VAL))
> > -=09=09node->cpu =3D encode_cpu(smp_processor_id());
> > +=09if (unlikely(!node)) {
> > +=09=09int cpu =3D encode_cpu(smp_processor_id());
> > +=09=09node =3D decode_cpu(cpu);
> > +=09=09node->self =3D node;
> > +=09=09node->cpu =3D cpu;
>=20
> 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.

=09David

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