Re: NetBSD 5.1 TCP performance issue (lots of ACK)

[Dennis Ferguson <dennis.c.ferguson%gmail.com@localhost>]

On 29 Oct, 2011, at 12:59 , Manuel Bouyer wrote:
> On Fri, Oct 28, 2011 at 06:55:30PM +0100, David Laight wrote:
>> On Fri, Oct 28, 2011 at 04:10:36PM +0200, Manuel Bouyer wrote:
>>> Here is an updated patch. The key point to avoid the receive errors is
>>> to do another BUS_DMASYNC after reading wrx_status, before reading the
>>> other values to avoid reading e.g. len before status gets updated.
>>> The errors were because of 0-len receive descriptors.
>> 
>> I'm not entirely clear where the mis-ordering happens. I presume the
>> fields a volatile so gcc won't re-order them. Which seems to imply
>> that the only problem can be the adapter writing the fields in the
>> wrong order (unless the data is cached and spans cache lines).
>> In that case the BUS_DMASYNC is also acting as a delay.
> 
> AFAIK the CPU is allowed to reorder reads. linux has a rmb() here,
> which is an equivalent of our x86_lfence() I guess.
> But for platforms where BUS_DMASYNC is not a simple barrier,
> 2 BUS_DMASYNC calls are needed.

CPUs in general are allowed to reorder reads, but Intel and AMD
x86 CPUs in particular won't do that.  The linux rmb() expands to
an empty asm() statement, essentially (not quite) a NOP.

There might be another problem, though.  If your code looks like

    volatile int a, b;
    <. . .>
    my_a = a;
    my_b = b;

I don't think it is guaranteed that the compiler will generate code
which reads a before it reads b.  The volatile declarations ensure
that a and b will be (re-)read, but don't indicate to the compiler that
reads from different locations need to be done in the order you wrote
them.  To ensure the latter you need to do something like

    my_a = a;
    something();
    my_b = b;

where something() is either a function call or that empty asm()
statement that linux rmb() expands to.  In essence this means
that you can't ever leave out the calls to the memory barrier
primitives if the code depends on reads being done in order,
even on uniprocessors and even if the CPU hardware doesn't
reorder reads, because you still need something there to tell
the compiler to maintain the order.  On NetBSD you would need a
membar_consumer() in there (though it would be better if the
x86 membar_consumer() turned into an empty asm() statement
the way linux x86 rmb() does).

I've been writing a lot of SMP data structure code recently, and
you sometimes find bugs like this when you change compilers.

Dennis Ferguson