On 05/31/2016 05:41 AM, Peter Zijlstra wrote:
Since all asm/barrier.h should/must include asm-generic/barrier.h the latter is a good place for generic infrastructure like this. This also allows archs to override the new smp_acquire__after_ctrl_dep(). Signed-off-by: Peter Zijlstra (Intel)<peterz@xxxxxxxxxxxxx> --- a/include/asm-generic/barrier.h +++ b/include/asm-generic/barrier.h @@ -194,7 +194,7 @@ do { \ }) #endif -#endif +#endif /* CONFIG_SMP */ /* Barriers for virtual machine guests when talking to an SMP host */ #define virt_mb() __smp_mb() @@ -207,5 +207,61 @@ do { \ #define virt_store_release(p, v) __smp_store_release(p, v) #define virt_load_acquire(p) __smp_load_acquire(p) +/** + * smp_acquire__after_ctrl_dep() - Provide ACQUIRE ordering after a control dependency + * + * A control dependency provides a LOAD->STORE order, the additional RMB + * provides LOAD->LOAD order, together they provide LOAD->{LOAD,STORE} order, + * aka. (load)-ACQUIRE. + * + * Architectures that do not do load speculation can have this be barrier(). + */ +#ifndef smp_acquire__after_ctrl_dep +#define smp_acquire__after_ctrl_dep() smp_rmb() +#endif + +/** + * cmpwait - compare and wait for a variable to change + * @ptr: pointer to the variable to wait on + * @val: the value it should change from + * + * A simple constuct that waits for a variable to change from a known + * value; some architectures can do this in hardware. + */ +#ifndef cmpwait +#define cmpwait(ptr, val) do { \ + typeof (ptr) __ptr = (ptr); \ + typeof (val) __val = (val); \ + while (READ_ONCE(*__ptr) == __val) \ + cpu_relax(); \ +} while (0) +#endif + +/** + * smp_cond_load_acquire() - (Spin) wait for cond with ACQUIRE ordering + * @ptr: pointer to the variable to wait on + * @cond: boolean expression to wait for + * + * Equivalent to using smp_load_acquire() on the condition variable but employs + * the control dependency of the wait to reduce the barrier on many platforms. + * + * Due to C lacking lambda expressions we load the value of *ptr into a + * pre-named variable @VAL to be used in @cond. + */ +#ifndef smp_cond_load_acquire +#define smp_cond_load_acquire(ptr, cond_expr) ({ \ + typeof(ptr) __PTR = (ptr); \ + typeof(*ptr) VAL; \ + for (;;) { \ + VAL = READ_ONCE(*__PTR); \ + if (cond_expr) \ + break; \ + cmpwait(__PTR, VAL); \ + } \ + smp_acquire__after_ctrl_dep(); \ + VAL; \ +}) +#endif
You are doing two READ_ONCE's in the smp_cond_load_acquire loop. Can we change it to do just one READ_ONCE, like
--- a/include/asm-generic/barrier.h +++ b/include/asm-generic/barrier.h @@ -229,12 +229,18 @@ do { * value; some architectures can do this in hardware. */ #ifndef cmpwait -#define cmpwait(ptr, val) do { \ +#define cmpwait(ptr, val) ({ \ typeof (ptr) __ptr = (ptr); \ - typeof (val) __val = (val); \ - while (READ_ONCE(*__ptr) == __val) \ + typeof (val) __old = (val); \ + typeof (val) __new; \ + for (;;) { \ + __new = READ_ONCE(*__ptr); \ + if (__new != __old) \ + break; \ cpu_relax(); \ -} while (0) + } \ + __new; \ +}) #endif /** @@ -251,12 +257,11 @@ do { #ifndef smp_cond_load_acquire #define smp_cond_load_acquire(ptr, cond_expr) ({ \ typeof(ptr) __PTR = (ptr); \ - typeof(*ptr) VAL; \ + typeof(*ptr) VAL = READ_ONCE(*__PTR); \ for (;;) { \ - VAL = READ_ONCE(*__PTR); \ if (cond_expr) \ break; \ - cmpwait(__PTR, VAL); \ + VAL = cmpwait(__PTR, VAL); \ } \ smp_acquire__after_ctrl_dep(); \ VAL; \ Cheers, Longman -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html