Messages in this thread | | | Date | Tue, 12 Jul 2022 10:50:28 +1000 | From | Nicholas Piggin <> | Subject | Re: [PATCH 12/13] locking/qspinlock: separate pv_wait_node from the non-paravirt path |
| |
Excerpts from Peter Zijlstra's message of July 6, 2022 3:34 am: > On Tue, Jul 05, 2022 at 12:38:19AM +1000, Nicholas Piggin wrote: >> pv_wait_node waits until node->locked is non-zero, no need for the >> pv case to wait again by also executing the !pv code path. >> >> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> >> --- >> kernel/locking/qspinlock.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/kernel/locking/qspinlock.c b/kernel/locking/qspinlock.c >> index 9db168753124..19e2f286be0a 100644 >> --- a/kernel/locking/qspinlock.c >> +++ b/kernel/locking/qspinlock.c >> @@ -862,10 +862,11 @@ static inline void queued_spin_lock_mcs_queue(struct qspinlock *lock, bool parav >> /* Link @node into the waitqueue. */ >> WRITE_ONCE(prev->next, node); >> >> + /* Wait for mcs node lock to be released */ >> if (paravirt) >> pv_wait_node(node, prev); >> - /* Wait for mcs node lock to be released */ >> - smp_cond_load_acquire(&node->locked, VAL); >> + else >> + smp_cond_load_acquire(&node->locked, VAL); >> > > (from patch #6): > > +static void pv_wait_node(struct qnode *node, struct qnode *prev) > +{ > + int loop; > + bool wait_early; > + > ... > + > + /* > + * By now our node->locked should be 1 and our caller will not actually > + * spin-wait for it. We do however rely on our caller to do a > + * load-acquire for us. > + */ > +} > >
Oh good catch, thanks so much that's a dumb bug. I'll add a smp_load_acquire at the end of pv_wait_node where that comment is.
Thanks, Nick
| |