Hi Matias,
I think you may be right. I created some test benches and could
produce the error I was expecting. My reasoning was based on the
following:
"This will not have the desired effect because there is no actual data
dependency, but rather a control dependency that the CPU may short-circuit
by attempting to predict the outcome in advance, so that other CPUs see
the load from b as having happened before the load from a. In such a
case what's actually required is:
q = READ_ONCE(a);
if (q) {
<read barrier>
p = READ_ONCE(b);
}"
As I understand above, the software gives no guarantee that above
works without the read_barrier. The section somewhat contradicts
Matias's reference. Reading more on x86-TSO it seems as if the
hardware does not allow reordering of read(a) and read(b) so we should
be safe, however, ARM hardware would definitely allow reordering of
the two reads. Summary: The proposed smp_rmb() is not required for x86
but it may be for ARM.
On Thu, Jun 28, 2018 at 10:15 AM Javier Gonzalez <javier@xxxxxxxxxxxx> wrote:
>
> > On 28 Jun 2018, at 09.59, Matias Bjørling <mb@xxxxxxxxxxx> wrote:
> >
> > On 06/28/2018 01:31 AM, Heiner Litz wrote:
> >> There is a control dependency between two disjoint variables (only read data if flags == WRITTEN). Because x86-TSO allows re-ordering of loads the control dependency can be violated.
> >
> > I'm sorry, I do not see it :)
> >
> > Here is my understanding:
> >
> > entry->w_ctx.flags is used as a flagging mechanism to wait for data to become available when "flags" has PBLK_WRITTEN_DATA.
> >
> > The later access to entry->data is independent of this. It is assumed that entry->w_ctx.flags is the guarding barrier.
>
> This is correct. The motivation is to allow several producers to fill the
> buffer simultaneously.
>
> >
> > Here is the titbit from the control dependency section that makes me
> > say that the entry->data loads is not possible to be done before
> > entry->w_ctx.flags:
> >
> > " (*) Control dependencies apply only to the then-clause and else-clause
> > of the if-statement containing the control dependency, including
> > any functions that these two clauses call. Control dependencies
> > do -not- apply to code following the if-statement containing the
> > control dependency."
> >
> > The read of entry->data must come after the READ_ONCE of entry->w_ctx.flags.
> >
>
> I also understood it this way when implementing the barrier, but after
> an offline discussion with Heiner, he convinced me that reordering could
> occur.
>
> >> BTW: ARM has an even more relaxed memory model and also allows to
> >> re-order writes. If we want to support ARM correctly, I think we also
> >> need to insert a memory barrier between writing data and writing
> >> flags (and there might be a couple other places where we need to
> >> check).
>
> Heiner: Can you develop on this?
>
> Javier
