Re: [PATCH] documentation: explain memory barriers

[Nick Piggin <nickpiggin@yahoo.com.au>]

On Thursday 09 October 2008 12:50, Valdis.Kletnieks@vt.edu wrote:
> On Wed, 08 Oct 2008 18:12:23 PDT, Randy Dunlap said:
> > +
> > +24: All memory barriers {e.g., barrier(), rmb(), wmb()} need a comment
> > in the +    source code that explains the logic of what they are doing
> > and why.
>
> "what they are doing" will almost always be "flush value to RAM" or
> similar.

Memory barriers don't flush anything anywhere. It's simple: you must
explain which operations you are ordering against which.


> How about this instead: 
>
> + 24: All memory barriers ({e.g., barrier(), rmb(), wmb()} need a comment
> in the +    source code that explains the race condition being prevented,
> and states +    the location of the other code or hardware feature that
> races with this. +
> +    An example comment:
> +
> + 	/*
> + 	 * If we don't do a wmb() here, the RBFROBNIZ register on the XU293
> + 	 * card will get confused and wedge the hardware...
> + 	 */
> +	wmb();

This comment is no good, because it doesn't tell you what the memory barrier
does. It tells you what might happen if you omit it.

 /*
  * If we don't do a wmb() here, the store to the RBFROBNIZ,
  * above, might reach the device before the store X, below.
  * 
  * If that happens, then the XU293  card will get confused
  * and wedge the hardware...
  */
  wmb();

If you don't comment like that, then how does the reader know that the wmb
is not *also* supposed to order the store with any other of the limitless
subsequent stores until the next memory ordering operation? Or any of the
previous stores since the last one?