On Thu Nov 10, 2022 at 10:43 AM AEST, Jordan Niethe wrote:
> On Thu, 2022-07-28 at 16:31 +1000, Nicholas Piggin wrote:
> [resend as utf-8, not utf-7]
> > Allow for a reduction in the number of times a CPU from a different
> > node than the owner can attempt to steal the lock before queueing.
> > This could bias the transfer behaviour of the lock across the
> > machine and reduce NUMA crossings.
> > ---
> > arch/powerpc/lib/qspinlock.c | 34 +++++++++++++++++++++++++++++++---
> > 1 file changed, 31 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/powerpc/lib/qspinlock.c b/arch/powerpc/lib/qspinlock.c
> > index d4594c701f7d..24f68bd71e2b 100644
> > --- a/arch/powerpc/lib/qspinlock.c
> > +++ b/arch/powerpc/lib/qspinlock.c
> > @@ -4,6 +4,7 @@
> > #include <linux/export.h>
> > #include <linux/percpu.h>
> > #include <linux/smp.h>
> > +#include <linux/topology.h>
> > #include <asm/qspinlock.h>
> > #include <asm/paravirt.h>
> >
> > @@ -24,6 +25,7 @@ struct qnodes {
> >
> > /* Tuning parameters */
> > static int STEAL_SPINS __read_mostly = (1<<5);
> > +static int REMOTE_STEAL_SPINS __read_mostly = (1<<2);
> > #if _Q_SPIN_TRY_LOCK_STEAL == 1
> > static const bool MAYBE_STEALERS = true;
> > #else
> > @@ -39,9 +41,13 @@ static bool pv_prod_head __read_mostly = false;
> >
> > static DEFINE_PER_CPU_ALIGNED(struct qnodes, qnodes);
> >
> > -static __always_inline int get_steal_spins(bool paravirt)
> > +static __always_inline int get_steal_spins(bool paravirt, bool remote)
> > {
> > - return STEAL_SPINS;
> > + if (remote) {
> > + return REMOTE_STEAL_SPINS;
> > + } else {
> > + return STEAL_SPINS;
> > + }
> > }
> >
> > static __always_inline int get_head_spins(bool paravirt)
> > @@ -380,8 +386,13 @@ static __always_inline bool try_to_steal_lock(struct
> > qspinlock *lock, bool parav
> >
> > iters++;
> >
> > - if (iters >= get_steal_spins(paravirt))
> > + if (iters >= get_steal_spins(paravirt, false))
> > break;
> > + if (iters >= get_steal_spins(paravirt, true)) {
>
> There's no indication of what true and false mean here which is hard to read.
> To me it feels like two separate functions would be more clear.
Good point. I reworked this a bit.
>
>
> > + int cpu = get_owner_cpu(val);
> > + if (numa_node_id() != cpu_to_node(cpu))
>
> What about using node_distance() instead?
We don't really need the distance, just whether or not it's the same or
not. I think distance not only has to look up the nodes, but then has to
look up a matrix to get a number.
Thanks,
Nick
