Re: [PATCH 2/3] r8169: Use load_acquire() and store_release() to reduce memory barrier overhead

[Francois Romieu <romieu@fr.zoreil.com>]

Alexander Duyck <alexander.duyck@gmail.com> :
> On 11/13/2014 01:30 PM, Francois Romieu wrote:
> > Alexander Duyck <alexander.h.duyck@redhat.com> :
> > [...]
> >> In addition the r8169 uses a rmb() however I believe it is placed incorrectly
> >> as I assume it supposed to be ordering descriptor reads after the check for
> >> ownership.
> > Not exactly. It's a barrier against compiler optimization from 2004.
> > It should not matter.
> 
> Okay.  Do you recall the kind of problem it was you were seeing ?

Mildly, I had to grep the local archives.

The relevant code used to be included in the irq handler at that time
(napi support for this driver took place in may 2004). One did not want
a runaway loop in the Tx reaper.

Compiler optimization was suggested by Manfred Spraul in the thread below:
http://marc.info/?l=linux-kernel&m=108096868119004

> The origin of the rmb() for the Intel drivers was a PowerPC issue in
> which it was fetching the length of a buffer before it checked the DD
> bit (equivalent of DescOwn).  I'm wondering if the issue you were seeing
> was something similar where it had reordered reads in the descriptor to
> cause that type of result.

The problem was only reported on Intel 32 bit + slackware + gcc 3.2.3.

Adam Nielsen - Cc: added - did not return for this bug.

-- 
Ueimor