Re: [PATCH 2/3] r8169: Use load_acquire() and store_release() to reduce memory barrier overhead
From: Francois Romieu <romieu@fr.zoreil.com>
Date: 2014-11-13 21:33:31
Also in:
linux-arch, lkml
Alexander Duyck [off-list ref] : [...]
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:
quoted hunk ↗ jump to hunk
@@ -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