Re: Do we need to correct barriering in circular-buffers.rst?

[Peter Zijlstra]

On Fri, Sep 27, 2019 at 01:43:18PM -0700, Nick Desaulniers wrote:
> On Fri, Sep 27, 2019 at 5:49 AM Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:

> Oh, in that case I'm less sure (I still don't think so, but I would
> love to be proven wrong, preferably with a godbolt link).  I think the
> best would be to share a godbolt.org link to a case that's clearly
> broken, or cite the relevant part of the ISO C standard (which itself
> leaves room for interpretation), otherwise the discussion is too
> hypothetical.  Those two things are single-handedly the best way to
> communicate with compiler folks.

Ah, I'm not sure current compilers will get it wrong -- and I'm trying
to be preemptive here. I'm looking for a guarantee that compilers will
recognise and respect control depenencies.

The C language spec does not recognise the construct at _all_ and I'd be
fine with it being behind some optional compiler knob.

So far we're mostly very careful when using it, recognising that
compilers can screw us over because they have no clue.

> > Using WRITE_ONCE() defeats this because volatile indicates external
> > visibility.
> 
> Could data be declared as a pointer to volatile qualified int?

It's not actually 'int' data, mostly its a void* and we use memcpy().

> > Barring LTO the above works for perf because of inter-translation-unit
> > function calls, which imply a compiler barrier.

Having looked at it again, I think we're good and have sufficient
barrier() in there to not rely on function calls being a sync point.

> > Now, when the compiler inlines, it looses that sync point (and thereby
> > subtlely changes semantics from the non-inline variant). I suspect LTO
> > does the same and can cause subtle breakage through this transformation.
> 
> Do you have a bug report or godbolt link for the above?  I trust that
> you're familiar enough with the issue to be able to quickly reproduce
> it?  These descriptions of problems are difficult for me to picture in
> code or generated code, and when I try to read through
> memory-barriers.txt my eyes start to glaze over (then something else
> catches fire and I have to go put that out).  Having a concise test
> case I think would better illustrate potential issues with LTO that
> we'd then be able to focus on trying to fix/support.
> 
> We definitely have heavy hitting language lawyers and our LTO folks
> are super sharp; I just don't have the necessary compiler experience
> just yet to be as helpful in these discussions as we need but I'm
> happy to bring them cases that don't work for the kernel and drive
> their resolution.

Like said; I've not seen it go wrong -- but it is one of the things I'm
always paranoid about with LTO.

Furthermore, if it were to go wrong, it'd be a very subtle data race and
finding it would be super hard and painful. Which is again why I would
love to get compiler folks on board to actually support control
dependencies in some way.

Like I said before, something like: "disallowing store hoists over control
flow depending on a volatile load" would be sufficient I think.

Yes this is outside of ISO/C, but it is something that is really
important to us because, as said above, getting it wrong would be
*SUPER* painful.

So basically I'm asking for a language extension I suppose; a new
guarantee from the compiler's memory model that does not exist _at_all_
in the spec, one that we're actively using.

And I'm hoping that getting the compilers to (optionally) support this
is easier than waiting another few decades until Paul McKenney has
wrestled the C committee into sanity and only then (maybe) getting it.
Look at the horrible mess vs data dependencies and consume ordering
(another fun thing we're actively using lots that the compilers are
still struggling with).