Hello,

> The bus_space and barriers issue appeared on the list few times,
> some solutions were proposed, but the problem was not fixed.
> It needs to be done sooner or later. I suggest sooner. It confuses
> people new to NetBSD.

Break them of the habit now.  The old-timers are going to have
to learn to adapt to the newer models that come along with
efficiently programming today's high performance systems!

> 
> Currently, bus_space_read/write_n() functions do implicit memory
> barrier. Existing drivers rely on this. According to bus_space(9),
> it is incorrect.
> 
> All other read/write functions are affected as well.

IMO this is _broken_.  Implicit barriers in macros are inefficient
and only contribute to hidden bugs.  I pointed out a bug to 
a fellow engineer here who had no idea that barriers were
things that were needed in a driver.  It was a case of "this should
work like this but the real semantics are different".  Such issues
are not only related to hardware, but to software synchronization
as well.  Better to wean them now then to have them writing
dangerous code later..

> First, we need to decide whether to change current semantics or not.
> some change pros:
> 
>         - C language intuition tells that read or write function
>           should complete read or write before returning.
>         
>         - some programmers don't want to care about barriers
> 
>         - implicit barriers are, in fact, what many drivers need
>           (in some cases hardware design may force to do barrier
>           between _every_ access)

If they don't care now, they WILL CARE later.  Because things like
this may not be caught by the compiler, leading to miscompiled code:

buf = devp->bufs[i];
spinlock(some lock);
devp->buf_state[i] = BUF_USED;
spinunlock(some lock);
...
some_operation_on_buffer(buf);
devp->buf_state[i] = BUF_FREE;  <-- breaks 

See the problem here?  A write memory barrier isn't needed here,
because the buffer won't be used twice at once.  A CPU that
still has the buffer state as BUF_USED in its prefetch buffers
may incorrectly skip over it when it _could_ be used, but that
won't lead to data corruption.  However, suppose that
some_operation_on_buffer(buf) gets interrupted, and the compiler
issued the (apparently unrelated) assignment _before_ the function
call as an optimization.  Or worse yet, that the CPU re-ordered
the instructions because it knew it would have to flush the
pipelines to make the branch.  Either way, the buffer is seen as
free when it's still being accessed.  DATA CORRUPTION.  This is
the bug I found in a device driver here.  The fix is simple--
issue a read memory barrier before the assignment:

some_operation_on_buffer(buf);
rmb();
devp->buf_state[i] = BUF_FREE;

Of course, using a spinlock to access the buf_state[] array
also fixes the problem -- but because it issues an implicit
barrier.  This is less efficient, and from the programmer's
standpoint, unnecessary..  So I think the _right_ way should
be taught, instead.

> 
> Note that at this point no code needs to be changed, only the manual
> page. It won't get worse.
> 
> If we choose to change semantics, we would have to decide how to
> provide no-barrier version.
> 
> Jason Thorpe mentioned __BUS_SPACE_NO_IMPLIED_BARRIERS.
> I have looked at drivers that use bus_space_barrier:

[...]

I suggest setting an ultimatum.  And document it well.
"This will be broken in version xxx.  Until then,
if you want to be safe, use the new function call".

Please don't clobber the code with #ifdef THIS_BROKEN_INTERFACE
etc.  It's messy.  The old Linux way of fixing this was to
make you #define _USE_SUCHANDSUCH_BROKEN_INTERFACE to access
the calls.  Rather, make the common case issue the implicit
barrier, and leave bus_space_barrier() functions in for now.
Then, make the common case more efficient later.

> So - only few drivers use barriers, and even fewer get any benefits
> from doing so.

Yes.

> Implementation on current processors: barrier instruction after
> reads and writes. (alternatively - before)
> (in current implementation, on some platforms, I've found barriers
> before and after)

rmb() -- read
wmb() -- write
mb()  -- read and write.

For example, on PowerPC:

rmb() = asm("eieio");
wmb() = asm("sync");
mb() = asm("sync");

> Few words about performance: actual implementation of bus_space
> functions is much more important than optimizing barriers.
> For example: despite the fact that in bus_space(9)
> bus_space_write_region_N() is allowed to execute writes in
> any order, on many platforms there are barriers between each write.
> I don't think drivers rely on such behavior.  With new semantics,
> one barrier would be needed.

Agreed.  Don't murder your poor 4GHz Pentium VI or Alpha 21464 10GHz
CPUs. Be good to them instead. :)

--
Eric Lowe
Software Engineer, Systran Corporation
elowe@systran.com