netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alexander Duyck <alexander.h.duyck@redhat.com>
To: linux-arch@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
Cc: mikey@neuling.org, tony.luck@intel.com,
	mathieu.desnoyers@polymtl.ca, donald.c.skidmore@intel.com,
	peterz@infradead.org, benh@kernel.crashing.org,
	heiko.carstens@de.ibm.com, oleg@redhat.com, will.deacon@arm.com,
	davem@davemloft.net, michael@ellerman.id.au,
	matthew.vick@intel.com, nic_swsd@realtek.com,
	geert@linux-m68k.org, jeffrey.t.kirsher@intel.com,
	fweisbec@gmail.com, schwidefsky@de.ibm.com,
	linux@arm.linux.org.uk, paulmck@linux.vnet.ibm.com,
	torvalds@linux-foundation.org, mingo@kernel.org
Subject: [PATCH 2/3] r8169: Use load_acquire() and store_release() to reduce memory barrier overhead
Date: Thu, 13 Nov 2014 11:27:35 -0800	[thread overview]
Message-ID: <20141113192735.12579.22892.stgit@ahduyck-server> (raw)
In-Reply-To: <20141113191250.12579.19694.stgit@ahduyck-server>

The r8169 use a pair of wmb() calls when setting up the descriptor rings.
The first is to synchronize the descriptor data with the descriptor status,
and the second is to synchronize the descriptor status with the use of the
MMIO doorbell to notify the device that descriptors are ready.  This can come
at a heavy price on some systems, and is not really necessary on systems such
as x86 as a simple barrier() would suffice to order store/store accesses.

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.  As such I believe this is actually an acquire access pattern so
I have updated the code and removed the barrier using a load_acquire() call.

Cc: Realtek linux nic maintainers <nic_swsd@realtek.com>
Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
 drivers/net/ethernet/realtek/r8169.c |   23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index cf154f7..42b4645 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -6601,14 +6601,13 @@ static inline void rtl8169_mark_to_asic(struct RxDesc *desc, u32 rx_buf_sz)
 {
 	u32 eor = le32_to_cpu(desc->opts1) & RingEnd;
 
-	desc->opts1 = cpu_to_le32(DescOwn | eor | rx_buf_sz);
+	store_release(&desc->opts1, cpu_to_le32(DescOwn | eor | rx_buf_sz));
 }
 
 static inline void rtl8169_map_to_asic(struct RxDesc *desc, dma_addr_t mapping,
 				       u32 rx_buf_sz)
 {
 	desc->addr = cpu_to_le64(mapping);
-	wmb();
 	rtl8169_mark_to_asic(desc, rx_buf_sz);
 }
 
@@ -7077,11 +7076,9 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
 
 	skb_tx_timestamp(skb);
 
-	wmb();
-
 	/* Anti gcc 2.95.3 bugware (sic) */
 	status = opts[0] | len | (RingEnd * !((entry + 1) % NUM_TX_DESC));
-	txd->opts1 = cpu_to_le32(status);
+	store_release(&txd->opts1, cpu_to_le32(status));
 
 	tp->cur_tx += frags + 1;
 
@@ -7183,16 +7180,15 @@ static void rtl_tx(struct net_device *dev, struct rtl8169_private *tp)
 	while (tx_left > 0) {
 		unsigned int entry = dirty_tx % NUM_TX_DESC;
 		struct ring_info *tx_skb = tp->tx_skb + entry;
-		u32 status;
+		__le32 status;
 
-		rmb();
-		status = le32_to_cpu(tp->TxDescArray[entry].opts1);
-		if (status & DescOwn)
+		status = load_acquire(&tp->TxDescArray[entry].opts1);
+		if (status & cpu_to_le32(DescOwn))
 			break;
 
 		rtl8169_unmap_tx_skb(&tp->pci_dev->dev, tx_skb,
 				     tp->TxDescArray + entry);
-		if (status & LastFrag) {
+		if (status & cpu_to_le32(LastFrag)) {
 			pkts_compl++;
 			bytes_compl += tx_skb->skb->len;
 			dev_kfree_skb_any(tx_skb->skb);
@@ -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;
 		if (unlikely(status & RxRES)) {
 			netif_info(tp, rx_err, dev, "Rx ERROR. status = %08x\n",
 				   status);
@@ -7350,7 +7346,6 @@ process_pkt:
 		}
 release_descriptor:
 		desc->opts2 = 0;
-		wmb();
 		rtl8169_mark_to_asic(desc, rx_buf_sz);
 	}
 

  parent reply	other threads:[~2014-11-13 19:28 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-13 19:27 [PATCH 0/3] Introduce load_acquire() and store_release() Alexander Duyck
2014-11-13 19:27 ` [PATCH 1/3] arch: " Alexander Duyck
2014-11-14 10:19   ` Will Deacon
2014-11-14 16:00     ` Alexander Duyck
2014-11-14 10:45   ` David Laight
2014-11-14 16:58     ` Alexander Duyck
2014-11-13 19:27 ` Alexander Duyck [this message]
2014-11-13 21:30   ` [PATCH 2/3] r8169: Use load_acquire() and store_release() to reduce memory barrier overhead Francois Romieu
2014-11-13 23:11     ` Alexander Duyck
2014-11-15 21:13       ` Francois Romieu
2014-11-13 19:27 ` [PATCH 3/3] fm10k/igb/ixgbe: Use load_acquire on Rx descriptor Alexander Duyck
2014-11-14 17:25   ` Jeff Kirsher

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141113192735.12579.22892.stgit@ahduyck-server \
    --to=alexander.h.duyck@redhat.com \
    --cc=benh@kernel.crashing.org \
    --cc=davem@davemloft.net \
    --cc=donald.c.skidmore@intel.com \
    --cc=fweisbec@gmail.com \
    --cc=geert@linux-m68k.org \
    --cc=heiko.carstens@de.ibm.com \
    --cc=jeffrey.t.kirsher@intel.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=mathieu.desnoyers@polymtl.ca \
    --cc=matthew.vick@intel.com \
    --cc=michael@ellerman.id.au \
    --cc=mikey@neuling.org \
    --cc=mingo@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=nic_swsd@realtek.com \
    --cc=oleg@redhat.com \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=schwidefsky@de.ibm.com \
    --cc=tony.luck@intel.com \
    --cc=torvalds@linux-foundation.org \
    --cc=will.deacon@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).