Messages in this thread | | | Date | Fri, 04 Dec 2015 15:51:28 -0500 | From | Waiman Long <> | Subject | Re: [PATCH 3/4] locking: Introduce smp_cond_acquire() |
| |
On 12/03/2015 03:26 PM, Peter Zijlstra wrote: > On Thu, Dec 03, 2015 at 04:37:26PM +0000, Will Deacon wrote: >>> +#define smp_cond_acquire(cond) do { \ >>> + while (!(cond)) \ >>> + cpu_relax(); \ >>> + smp_rmb(); /* ctrl + rmb := acquire */ \ >>> +} while (0) >>> + smp_cond_acquire(!((val = atomic_read(&lock->val))& _Q_LOCKED_PENDING_MASK)); >> I think we spoke about this before, but what would work really well for >> arm64 here is if we could override smp_cond_acquire in such a way that >> the atomic_read could be performed explicitly in the macro. That would >> allow us to use an LDXR to set the exclusive monitor, which in turn >> means we can issue a WFE and get a cheap wakeup when lock->val is >> actually modified. >> >> With the current scheme, there's not enough information expressed in the >> "cond" parameter to perform this optimisation. > Right, but I'm having a hard time constructing something pretty that can > do that. Lambda functions would be lovely, but we don't have those :/ > > While we can easily pass a pointer to an arbitrary type, we need > an expression to evaluate the result of the pointer load to act as our > condition. > > smp_cond_acquire(&lock->val.counter, > [](int val){ return !(val& _Q_LOCKED_PENDING_MASK); }); > > Would be nice, but alas. > > The best we can do is hardcode a variable name; maybe something like: > > #define smp_cond_acquire(ptr, expr) do { \ > typeof(*ptr) val; \ > while ((val = READ_ONCE(*ptr)), expr) \ > cpu_relax(); \ > smp_rmb(); /* ctrl + rmb := acquire */ \ > } while (0) > > Which would let us write: > > smp_cond_acquire(&lock->val.counter, !(val& _Q_LOCKED_PENDING_MASK)); > > > Thoughts? Will the following work?
#define smp_cond_load_acquire(ptr, cond_expr, neg) ({ \ typeof(*ptr) _val; \ while (neg ((_val = READ_ONCE(*ptr)) cond_expr))\ cpu_relax(); \ smp_rmb(); \ _val; \ })
val = smp_cond_load_acquire(&lock->val.counter, & _Q_LOCKED_PENDING_MASK, !);
Cheers, Longman
| |