Messages in this thread | | | From | Björn Töpel <> | Date | Tue, 2 Mar 2021 21:51:29 +0100 | Subject | Re: XDP socket rings, and LKMM litmus tests |
| |
On Tue, 2 Mar 2021 at 21:41, Paul E. McKenney <paulmck@kernel.org> wrote: > > On Tue, Mar 02, 2021 at 09:24:04PM +0100, Björn Töpel wrote: > > On Tue, 2 Mar 2021 at 20:57, Paul E. McKenney <paulmck@kernel.org> wrote: > > > > > > On Tue, Mar 02, 2021 at 07:46:27PM +0100, Björn Töpel wrote: > > > > [...] > > > > > > > > Before digging in too deeply, does the following simplification > > > still capture your intent? > > > > > > > Thanks for having a look, Paul! > > > > > P0(int *prod, int *cons, int *data) > > > { > > > int p; > > > int cond = 0; > > > > > > p = READ_ONCE(*prod); > > > if (p == READ_ONCE(*cons)) > > > cond = 1; > > > > With this, yes! > > > > > if (cond) { > > > smp_mb(); > > > WRITE_ONCE(*data, 1); > > > smp_wmb(); > > > WRITE_ONCE(*prod, p ^ 1); > > > } > > > } > > > > > > P1(int *prod, int *cons, int *data) > > > { > > > int c; > > > int d = -1; > > > int cond = 0; > > > > > > c = READ_ONCE(*cons); > > > if (READ_ONCE(*prod) == c) > > > cond = 1; > > > > Hmm, this would not be the correct state transition. > > > > c==1 && p==1 would set cond to 1, right? > > > > I would agree with: > > c = READ_ONCE(*cons); > > if (READ_ONCE(*prod) != c) > > Right you are! > > With that, it looks to me like LKMM is OK with removing the smp_mb(). > My guess is that the issue is that LKMM confines the effect of control > dependencies to a single "if" statement, hence my reworking of your > original. >
Interesting!
I tried the acquire/release version:
P0(int *prod, int *cons, int *data) { int p; int cond = 0;
p = READ_ONCE(*prod); if (p == READ_ONCE(*cons)) { WRITE_ONCE(*data, 1); smp_store_release(prod, p ^ 1); } }
P1(int *prod, int *cons, int *data) { int c; int d = -1;
c = READ_ONCE(*cons); if (smp_load_acquire(prod) != c) { d = READ_ONCE(*data); smp_store_release(cons, c ^ 1); } }
and as with the previous example, restructuring the if-statement makes "if (p == READ_ONCE(*cons)) {" sufficient, instead of "if (p == smp_load_acquire(cons)) {".
Yay!
Björn
> Thanx, Paul > > > > > > > if (cond == 1) { > > > smp_rmb(); > > > d = READ_ONCE(*data); > > > smp_mb(); > > > WRITE_ONCE(*cons, c ^ 1); > > > } > > > } > > > > > > Thanx, Paul > > > > > > > [...] > > > > Björn
| |