Messages in this thread | | | Date | Thu, 13 Nov 2014 22:30:49 +0100 | From | Francois Romieu <> | Subject | Re: [PATCH 2/3] r8169: Use load_acquire() and store_release() to reduce memory barrier overhead |
| |
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.
However I disagree with the change below:
> @@ -7284,11 +7280,11 @@ static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp, u32 budget > struct RxDesc *desc = tp->RxDescArray + entry; > u32 status; > > - rmb(); > - status = le32_to_cpu(desc->opts1) & tp->opts1_mask; > - > + status = cpu_to_le32(load_acquire(&desc->opts1)); > if (status & DescOwn) > break; > + > + status &= tp->opts1_mask;
-> tp->opts1_mask is not __le32 tainted.
Btw, should I consider the sketch above as a skeleton in my r8169 closet ?
NIC CPU0 CPU1 | CPU | NIC | CPU | CPU |
| CPU | NIC | CPU | CPU | ^ tx_dirty
[start_xmit...
| CPU | CPU | CPU | CPU | (NIC did it's job) [rtl_tx... | ... | ... | NIC | NIC | (ring update) (tx_dirty increases)
| CPU | CPU | ??? | ??? | tx_dirty ? reaping about-to-be-sent buffers on some platforms ? ...start_xmit]
-- Ueimor
| |