netdev: intel: Eliminate duplicate barriers on weakly-ordered archs

PATCH Superseded Sinan Kaya 2 participants 13 messages Mar 23, 2018

Subsystems (4)

Networking Network drivers Arm Qualcomm mailing list Intel Ethernet

lore

Share & tools

lore

Sinan Kaya

author

2018-03-23 18:21 UTC #0 lore raw

v6 cover netdev: intel: Eliminate duplicate barriers on weakly-ordered archs

Code includes wmb() followed by writel() in multiple places. writel() already has a barrier on some architectures like arm64.

This ends up CPU observing two barriers back to back before executing the register write.

Since code already has an explicit barrier call, changing writel() to writel_relaxed().

I did a regex search for wmb() followed by writel() in each drivers directory.
I scrubbed the ones I care about in this series.

I considered "ease of change", "popular usage" and "performance critical path" as the determining criteria for my filtering.

We used relaxed API heavily on ARM for a long time but
it did not exist on other architectures. For this reason, relaxed architectures have been paying double penalty in order to use the common drivers.

Now that relaxed API is present on all architectures, we can go and scrub all drivers to see what needs to change and what can remain.

We start with mostly used ones and hope to increase the coverage over time. It will take a while to cover all drivers.

Feel free to apply patches individually.

Changes since v5:
add mmiowb to missing places in order not to break PPC

Changes since v4:
posted ixgbevf: keep writel() closer to wmb() to jkircher posted ixgbevf: eliminate duplicate barriers on weakly-ordered archs to

Sinan Kaya (7):
i40e/i40evf: Eliminate duplicate barriers on weakly-ordered archs
ixgbe: eliminate duplicate barriers on weakly-ordered archs
igbvf: eliminate duplicate barriers on weakly-ordered archs
igb: eliminate duplicate barriers on weakly-ordered archs
fm10k: Eliminate duplicate barriers on weakly-ordered archs
ixgbevf: keep writel() closer to wmb()
ixgbevf: eliminate duplicate barriers on weakly-ordered archs

 drivers/net/ethernet/intel/fm10k/fm10k_main.c     |  9 +++++++--
 drivers/net/ethernet/intel/i40e/i40e_txrx.c       | 24 +++++++++++++++++++----
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c     |  9 +++++++--
 drivers/net/ethernet/intel/igb/igb_main.c         |  9 +++++++--
 drivers/net/ethernet/intel/igbvf/netdev.c         |  9 +++++++--
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c     | 23 ++++++++++++++++++----
 drivers/net/ethernet/intel/ixgbevf/ixgbevf.h      |  5 -----
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 21 +++++++++++++++++---
 8 files changed, 85 insertions(+), 24 deletions(-)

Signature · 1 line

2.7.4

Sinan Kaya

author

2018-03-23 18:21 UTC #1 lore raw

PATCH v6 1/7 i40e/i40evf: Eliminate duplicate barriers on weakly-ordered archs

Code includes wmb() followed by writel(). writel() already has a barrier on some architectures like arm64.

This ends up CPU observing two barriers back to back before executing the register write.

Since code already has an explicit barrier call, changing writel() to writel_relaxed().

Signed-off-by Sinan Kaya <[email protected]>

---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   | 24 ++++++++++++++++++++----
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c |  9 +++++++--
 2 files changed, 27 insertions(+), 6 deletions(-)

drivers/net/ethernet/intel/i40e/i40e_txrx.c +20 -4

index c6972bd..fc10cc0 100644
@@ -186,7 +186,13 @@ static int i40e_program_fdir_filter(struct i40e_fdir_filter *fdir_data,
 	/* Mark the data descriptor to be watched */
 	first->next_to_watch = tx_desc;
 
-	writel(tx_ring->next_to_use, tx_ring->tail);
+	writel_relaxed(tx_ring->next_to_use, tx_ring->tail);
+
+	/* We need this if more than one processor can write to our tail
+	 * at a time, it synchronizes IO on IA64/Altix systems
+	 */
+	mmiowb();
+
 	return 0;
 
 dma_fail:

@@ -1529,7 +1535,12 @@ static inline void i40e_release_rx_desc(struct i40e_ring *rx_ring, u32 val)
 	 * such as IA-64).
 	 */
 	wmb();
-	writel(val, rx_ring->tail);
+	writel_relaxed(val, rx_ring->tail);
+
+	/* We need this if more than one processor can write to our tail
+	 * at a time, it synchronizes IO on IA64/Altix systems
+	 */
+	mmiowb();
 }
 
 /**

@@ -2412,7 +2423,12 @@ static int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget)
 		 */
 		wmb();
 
-		writel(xdp_ring->next_to_use, xdp_ring->tail);
+		writel_relaxed(xdp_ring->next_to_use, xdp_ring->tail);
+
+		/* We need this if more than one processor can write to our tail
+		 * at a time, it synchronizes IO on IA64/Altix systems
+		 */
+		mmiowb();
 	}
 
 	rx_ring->skb = skb;

@@ -3437,7 +3453,7 @@ static inline int i40e_tx_map(struct i40e_ring *tx_ring, struct sk_buff *skb,
 
 	/* notify HW of packet */
 	if (netif_xmit_stopped(txring_txq(tx_ring)) || !skb->xmit_more) {
-		writel(i, tx_ring->tail);
+		writel_relaxed(i, tx_ring->tail);
 
 		/* we need this if more than one processor can write to our tail
 		 * at a time, it synchronizes IO on IA64/Altix systems

drivers/net/ethernet/intel/i40evf/i40e_txrx.c +7 -2

index 1ae112f..ca02762 100644
@@ -810,7 +810,12 @@ static inline void i40e_release_rx_desc(struct i40e_ring *rx_ring, u32 val)
 	 * such as IA-64).
 	 */
 	wmb();
-	writel(val, rx_ring->tail);
+	writel_relaxed(val, rx_ring->tail);
+
+	/* We need this if more than one processor can write to our tail
+	 * at a time, it synchronizes IO on IA64/Altix systems
+	 */
+	mmiowb();
 }
 
 /**

@@ -2379,7 +2384,7 @@ static inline void i40evf_tx_map(struct i40e_ring *tx_ring, struct sk_buff *skb,
 
 	/* notify HW of packet */
 	if (netif_xmit_stopped(txring_txq(tx_ring)) || !skb->xmit_more) {
-		writel(i, tx_ring->tail);
+		writel_relaxed(i, tx_ring->tail);
 
 		/* we need this if more than one processor can write to our tail
 		 * at a time, it synchronizes IO on IA64/Altix systems

Signature · 1 line

2.7.4

Alexander Duyck

2018-03-23 18:30 UTC #2 lore raw

Re: [Intel-wired-lan] [PATCH v6 1/7] i40e/i40evf: Eliminate duplicate barriers on weakly-ordered archs

Quoted 23 lines · Sinan Kaya

Code includes wmb() followed by writel(). writel() already has a barrier on some architectures like arm64.
This ends up CPU observing two barriers back to back before executing the register write.
Since code already has an explicit barrier call, changing writel() to writel_relaxed().
Signed-off-by Sinan Kaya <[email protected]>
---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   | 24 ++++++++++++++++++++----
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c |  9 +++++++--
 2 files changed, 27 insertions(+), 6 deletions(-)
drivers/net/ethernet/intel/i40e/i40e_txrx.c +0 -0
index c6972bd..fc10cc0 100644
@@ -186,7 +186,13 @@ static int i40e_program_fdir_filter(struct i40e_fdir_filter *fdir_data,
        /* Mark the data descriptor to be watched */
        first->next_to_watch = tx_desc;
 

drivers/net/ethernet/intel/i40e/i40e_txrx.c static int i40e_program_fdir_filter(struct i40e_fdir_filter *fdir_data, · line 197

Quoted 10 lines

drivers/net/ethernet/intel/i40e/i40e_txrx.c +7 -1

-       writel(tx_ring->next_to_use, tx_ring->tail);
+       writel_relaxed(tx_ring->next_to_use, tx_ring->tail);
+
+       /* We need this if more than one processor can write to our tail
+        * at a time, it synchronizes IO on IA64/Altix systems
+        */
+       mmiowb();
+
        return 0;

The addition of mmiowb here is valid. All of the others in this patch are invalid.

Quoted 72 lines

drivers/net/ethernet/intel/i40e/i40e_txrx.c +13 -3

 dma_fail:

@@ -1529,7 +1535,12 @@ static inline void i40e_release_rx_desc(struct i40e_ring *rx_ring, u32 val)
         * such as IA-64).
         */
        wmb();
-       writel(val, rx_ring->tail);
+       writel_relaxed(val, rx_ring->tail);
+
+       /* We need this if more than one processor can write to our tail
+        * at a time, it synchronizes IO on IA64/Altix systems
+        */
+       mmiowb();
 }
 
 /**

@@ -2412,7 +2423,12 @@ static int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget)
                 */
                wmb();
 
-               writel(xdp_ring->next_to_use, xdp_ring->tail);
+               writel_relaxed(xdp_ring->next_to_use, xdp_ring->tail);
+
+               /* We need this if more than one processor can write to our tail
+                * at a time, it synchronizes IO on IA64/Altix systems
+                */
+               mmiowb();
        }
 
        rx_ring->skb = skb;

@@ -3437,7 +3453,7 @@ static inline int i40e_tx_map(struct i40e_ring *tx_ring, struct sk_buff *skb,
 
        /* notify HW of packet */
        if (netif_xmit_stopped(txring_txq(tx_ring)) || !skb->xmit_more) {
-               writel(i, tx_ring->tail);
+               writel_relaxed(i, tx_ring->tail);
 
                /* we need this if more than one processor can write to our tail
                 * at a time, it synchronizes IO on IA64/Altix systems

drivers/net/ethernet/intel/i40evf/i40e_txrx.c +7 -3

index 1ae112f..ca02762 100644
@@ -810,7 +810,12 @@ static inline void i40e_release_rx_desc(struct i40e_ring *rx_ring, u32 val)
         * such as IA-64).
         */
        wmb();
-       writel(val, rx_ring->tail);
+       writel_relaxed(val, rx_ring->tail);
+
+       /* We need this if more than one processor can write to our tail
+        * at a time, it synchronizes IO on IA64/Altix systems
+        */
+       mmiowb();
 }
 
 /**

@@ -2379,7 +2384,7 @@ static inline void i40evf_tx_map(struct i40e_ring *tx_ring, struct sk_buff *skb,
 
        /* notify HW of packet */
        if (netif_xmit_stopped(txring_txq(tx_ring)) || !skb->xmit_more) {
-               writel(i, tx_ring->tail);
+               writel_relaxed(i, tx_ring->tail);
 
                /* we need this if more than one processor can write to our tail
                 * at a time, it synchronizes IO on IA64/Altix systems
--

2.7.4

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan at osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Sinan Kaya

author

2018-03-23 18:21 UTC #3 lore raw

PATCH v6 2/7 ixgbe: eliminate duplicate barriers on weakly-ordered archs

In reply to Sinan Kaya

Code includes wmb() followed by writel() in multiple places. writel() already has a barrier on some architectures like arm64.

This ends up CPU observing two barriers back to back before executing the register write.

Since code already has an explicit barrier call, changing writel() to writel_relaxed().

Signed-off-by Sinan Kaya <[email protected]>

---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +19 -4

index e3b32ea..1ecc2f5 100644
@@ -1701,7 +1701,12 @@ void ixgbe_alloc_rx_buffers(struct ixgbe_ring *rx_ring, u16 cleaned_count)
 		 * such as IA-64).
 		 */
 		wmb();
-		writel(i, rx_ring->tail);
+		writel_relaxed(i, rx_ring->tail);
+
+		/* We need this if more than one processor can write to our tail
+		 * at a time, it synchronizes IO on IA64/Altix systems
+		 */
+		mmiowb();
 	}
 }

@@ -2470,7 +2475,12 @@ static int ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
 		 * know there are new descriptors to fetch.
 		 */
 		wmb();
-		writel(ring->next_to_use, ring->tail);
+		writel_relaxed(ring->next_to_use, ring->tail);
+
+		/* We need this if more than one processor can write to our tail
+		 * at a time, it synchronizes IO on IA64/Altix systems
+		 */
+		mmiowb();
 
 		xdp_do_flush_map();
 	}

@@ -8101,7 +8111,7 @@ static int ixgbe_tx_map(struct ixgbe_ring *tx_ring,
 	ixgbe_maybe_stop_tx(tx_ring, DESC_NEEDED);
 
 	if (netif_xmit_stopped(txring_txq(tx_ring)) || !skb->xmit_more) {
-		writel(i, tx_ring->tail);
+		writel_relaxed(i, tx_ring->tail);
 
 		/* we need this if more than one processor can write to our tail
 		 * at a time, it synchronizes IO on IA64/Altix systems

@@ -10038,7 +10048,12 @@ static void ixgbe_xdp_flush(struct net_device *dev)
 	 * are new descriptors to fetch.
 	 */
 	wmb();
-	writel(ring->next_to_use, ring->tail);
+	writel_relaxed(ring->next_to_use, ring->tail);
+
+	/* We need this if more than one processor can write to our tail
+	 * at a time, it synchronizes IO on IA64/Altix systems
+	 */
+	mmiowb();
 
 	return;
 }

Signature · 1 line

2.7.4

Sinan Kaya

author

2018-03-23 18:21 UTC #4 lore raw

PATCH v6 3/7 igbvf: eliminate duplicate barriers on weakly-ordered archs

In reply to Sinan Kaya

Code includes wmb() followed by writel(). writel() already has a barrier on some architectures like arm64.

This ends up CPU observing two barriers back to back before executing the register write.

Since code already has an explicit barrier call, changing writel() to writel_relaxed().

Signed-off-by Sinan Kaya <[email protected]>

---
 drivers/net/ethernet/intel/igbvf/netdev.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

drivers/net/ethernet/intel/igbvf/netdev.c +7 -2

index fa07876..6dfd3dc 100644
@@ -252,7 +252,12 @@ static void igbvf_alloc_rx_buffers(struct igbvf_ring *rx_ring,
 		 * such as IA-64).
 		*/
 		wmb();
-		writel(i, adapter->hw.hw_addr + rx_ring->tail);
+		writel_relaxed(i, adapter->hw.hw_addr + rx_ring->tail);
+
+		/* We need this if more than one processor can write to our tail
+		 * at a time, it synchronizes IO on IA64/Altix systems
+		 */
+		mmiowb();
 	}
 }

@@ -2298,7 +2303,7 @@ static inline void igbvf_tx_queue_adv(struct igbvf_adapter *adapter,
 
 	tx_ring->buffer_info[first].next_to_watch = tx_desc;
 	tx_ring->next_to_use = i;
-	writel(i, adapter->hw.hw_addr + tx_ring->tail);
+	writel_relaxed(i, adapter->hw.hw_addr + tx_ring->tail);
 	/* we need this if more than one processor can write to our tail
 	 * at a time, it synchronizes IO on IA64/Altix systems
 	 */

Signature · 1 line

2.7.4

Sinan Kaya

author

2018-03-23 18:21 UTC #5 lore raw

PATCH v6 4/7 igb: eliminate duplicate barriers on weakly-ordered archs

In reply to Sinan Kaya

Code includes wmb() followed by writel(). writel() already has a barrier on some architectures like arm64.

This ends up CPU observing two barriers back to back before executing the register write.

Since code already has an explicit barrier call, changing writel() to writel_relaxed().

Signed-off-by Sinan Kaya <[email protected]>

---
 drivers/net/ethernet/intel/igb/igb_main.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

drivers/net/ethernet/intel/igb/igb_main.c +7 -2

index 3d4ff3c..570af25 100644
@@ -5674,7 +5674,7 @@ static int igb_tx_map(struct igb_ring *tx_ring,
 	igb_maybe_stop_tx(tx_ring, DESC_NEEDED);
 
 	if (netif_xmit_stopped(txring_txq(tx_ring)) || !skb->xmit_more) {
-		writel(i, tx_ring->tail);
+		writel_relaxed(i, tx_ring->tail);
 
 		/* we need this if more than one processor can write to our tail
 		 * at a time, it synchronizes IO on IA64/Altix systems

@@ -8079,7 +8079,12 @@ void igb_alloc_rx_buffers(struct igb_ring *rx_ring, u16 cleaned_count)
 		 * such as IA-64).
 		 */
 		wmb();
-		writel(i, rx_ring->tail);
+		writel_relaxed(i, rx_ring->tail);
+
+		/* We need this if more than one processor can write to our tail
+		 * at a time, it synchronizes IO on IA64/Altix systems
+		 */
+		mmiowb();
 	}
 }

Signature · 1 line

2.7.4

Sinan Kaya

author

2018-03-23 18:21 UTC #6 lore raw

PATCH v6 5/7 fm10k: Eliminate duplicate barriers on weakly-ordered archs

In reply to Sinan Kaya

Code includes wmb() followed by writel(). writel() already has a barrier on some architectures like arm64.

This ends up CPU observing two barriers back to back before executing the register write.

Since code already has an explicit barrier call, changing writel() to writel_relaxed().

Signed-off-by Sinan Kaya <[email protected]>

---
 drivers/net/ethernet/intel/fm10k/fm10k_main.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

drivers/net/ethernet/intel/fm10k/fm10k_main.c +7 -2

index 409554d..360ff9b 100644
@@ -180,7 +180,12 @@ void fm10k_alloc_rx_buffers(struct fm10k_ring *rx_ring, u16 cleaned_count)
 		wmb();
 
 		/* notify hardware of new descriptors */
-		writel(i, rx_ring->tail);
+		writel_relaxed(i, rx_ring->tail);
+
+		/* We need this if more than one processor can write to our tail
+		 * at a time, it synchronizes IO on IA64/Altix systems
+		 */
+		mmiowb();
 	}
 }

@@ -1055,7 +1060,7 @@ static void fm10k_tx_map(struct fm10k_ring *tx_ring,
 
 	/* notify HW of packet */
 	if (netif_xmit_stopped(txring_txq(tx_ring)) || !skb->xmit_more) {
-		writel(i, tx_ring->tail);
+		writel_relaxed(i, tx_ring->tail);
 
 		/* we need this if more than one processor can write to our tail
 		 * at a time, it synchronizes IO on IA64/Altix systems

Signature · 1 line

2.7.4

Sinan Kaya

author

2018-03-23 18:21 UTC #7 lore raw

PATCH v6 6/7 ixgbevf: keep writel() closer to wmb()

In reply to Sinan Kaya

Remove ixgbevf_write_tail() in favor of moving writel() close to wmb().

Signed-off-by Sinan Kaya <[email protected]>

---
 drivers/net/ethernet/intel/ixgbevf/ixgbevf.h      | 5 -----
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 6 +++---
 2 files changed, 3 insertions(+), 8 deletions(-)

drivers/net/ethernet/intel/ixgbevf/ixgbevf.h +0 -5

index f712646..f97091d 100644
@@ -312,11 +312,6 @@ static inline u16 ixgbevf_desc_unused(struct ixgbevf_ring *ring)
 	return ((ntc > ntu) ? 0 : ring->count) + ntc - ntu - 1;
 }
 
-static inline void ixgbevf_write_tail(struct ixgbevf_ring *ring, u32 value)
-{
-	writel(value, ring->tail);
-}
-
 #define IXGBEVF_RX_DESC(R, i)	\
 	(&(((union ixgbe_adv_rx_desc *)((R)->desc))[i]))
 #define IXGBEVF_TX_DESC(R, i)	\

drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c +3 -3

index 3d9033f..815cb1a 100644
@@ -725,7 +725,7 @@ static void ixgbevf_alloc_rx_buffers(struct ixgbevf_ring *rx_ring,
 		 * such as IA-64).
 		 */
 		wmb();
-		ixgbevf_write_tail(rx_ring, i);
+		writel(i, rx_ring->tail);
 	}
 }

@@ -1232,7 +1232,7 @@ static int ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector,
 		 * know there are new descriptors to fetch.
 		 */
 		wmb();
-		ixgbevf_write_tail(xdp_ring, xdp_ring->next_to_use);
+		writel(xdp_ring->next_to_use, xdp_ring->tail);
 	}
 
 	u64_stats_update_begin(&rx_ring->syncp);

@@ -4004,7 +4004,7 @@ static void ixgbevf_tx_map(struct ixgbevf_ring *tx_ring,
 	tx_ring->next_to_use = i;
 
 	/* notify HW of packet */
-	ixgbevf_write_tail(tx_ring, i);
+	writel(i, tx_ring->tail);
 
 	return;
 dma_error:

Signature · 1 line

2.7.4

Sinan Kaya

author

2018-03-23 18:21 UTC #8 lore raw

PATCH v6 7/7 ixgbevf: eliminate duplicate barriers on weakly-ordered archs

In reply to Sinan Kaya

Code includes wmb() followed by writel() in multiple places. writel() already has a barrier on some architectures like arm64.

This ends up CPU observing two barriers back to back before executing the register write.

Since code already has an explicit barrier call, changing writel() to writel_relaxed().

Signed-off-by Sinan Kaya <[email protected]>

---
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c +18 -3

index 815cb1a..9e684b1 100644
@@ -725,7 +725,12 @@ static void ixgbevf_alloc_rx_buffers(struct ixgbevf_ring *rx_ring,
 		 * such as IA-64).
 		 */
 		wmb();
-		writel(i, rx_ring->tail);
+		writel_relaxed(i, rx_ring->tail);
+
+		/* We need this if more than one processor can write to our tail
+		 * at a time, it synchronizes IO on IA64/Altix systems
+		 */
+		mmiowb();
 	}
 }

@@ -1232,7 +1237,12 @@ static int ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector,
 		 * know there are new descriptors to fetch.
 		 */
 		wmb();
-		writel(xdp_ring->next_to_use, xdp_ring->tail);
+		writel_relaxed(xdp_ring->next_to_use, xdp_ring->tail);
+
+		/* We need this if more than one processor can write to our tail
+		 * at a time, it synchronizes IO on IA64/Altix systems
+		 */
+		mmiowb();
 	}
 
 	u64_stats_update_begin(&rx_ring->syncp);

@@ -4004,7 +4014,12 @@ static void ixgbevf_tx_map(struct ixgbevf_ring *tx_ring,
 	tx_ring->next_to_use = i;
 
 	/* notify HW of packet */
-	writel(i, tx_ring->tail);
+	writel_relaxed(i, tx_ring->tail);
+
+	/* We need this if more than one processor can write to our tail
+	 * at a time, it synchronizes IO on IA64/Altix systems
+	 */
+	mmiowb();
 
 	return;
 dma_error:

Signature · 1 line

2.7.4

Alexander Duyck

2018-03-23 18:25 UTC #9 lore raw

Re: [Intel-wired-lan] [PATCH v6 7/7] ixgbevf: eliminate duplicate barriers on weakly-ordered archs

Quoted 20 lines · Sinan Kaya

Code includes wmb() followed by writel() in multiple places. writel() already has a barrier on some architectures like arm64.
This ends up CPU observing two barriers back to back before executing the register write.
Since code already has an explicit barrier call, changing writel() to writel_relaxed().
Signed-off-by Sinan Kaya <[email protected]>
---
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c +0 -0
index 815cb1a..9e684b1 100644
@@ -725,7 +725,12 @@ static void ixgbevf_alloc_rx_buffers(struct ixgbevf_ring *rx_ring,
                 * such as IA-64).

drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c static void ixgbevf_alloc_rx_buffers(struct ixgbevf_ring *rx_ring, · line 734

Quoted 10 lines

drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c +6 -1

                 */
                wmb();
-               writel(i, rx_ring->tail);
+               writel_relaxed(i, rx_ring->tail);
+
+               /* We need this if more than one processor can write to our tail
+                * at a time, it synchronizes IO on IA64/Altix systems
+                */
+               mmiowb();
        }

The mmiowb shouldn't be needed for Rx. Only one CPU will be running NAPI for the queue and we will synchronize this with a full writel anyway when we re-enable the interrupts.

Quoted 37 lines

drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c +12 -3

@@ -1232,7 +1237,12 @@ static int ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector,
                 * know there are new descriptors to fetch.
                 */
                wmb();
-               writel(xdp_ring->next_to_use, xdp_ring->tail);
+               writel_relaxed(xdp_ring->next_to_use, xdp_ring->tail);
+
+               /* We need this if more than one processor can write to our tail
+                * at a time, it synchronizes IO on IA64/Altix systems
+                */
+               mmiowb();
        }
 
        u64_stats_update_begin(&rx_ring->syncp);

@@ -4004,7 +4014,12 @@ static void ixgbevf_tx_map(struct ixgbevf_ring *tx_ring,
        tx_ring->next_to_use = i;
 
        /* notify HW of packet */
-       writel(i, tx_ring->tail);
+       writel_relaxed(i, tx_ring->tail);
+
+       /* We need this if more than one processor can write to our tail
+        * at a time, it synchronizes IO on IA64/Altix systems
+        */
+       mmiowb();
 
        return;
 dma_error:
--

2.7.4

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan at osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Sinan Kaya

author

2018-03-23 18:27 UTC #10 lore raw

Re: [Intel-wired-lan] [PATCH v6 7/7] ixgbevf: eliminate duplicate barriers on weakly-ordered archs

Quoted 9 lines · Alexander Duyck

drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c static void ixgbevf_alloc_rx_buffers(struct ixgbevf_ring *rx_ring, · line 734
Quoted 5 lines
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c +4 -0
+               /* We need this if more than one processor can write to our tail
+                * at a time, it synchronizes IO on IA64/Altix systems
+                */
+               mmiowb();
        }
The mmiowb shouldn't be needed for Rx. Only one CPU will be running NAPI for the queue and we will synchronize this with a full writel anyway when we re-enable the interrupts.

OK. I can fix this on the next version. I did a blanket search and replace for my writel_relaxed() changes as I don't know the code well enough.

Please point me to the redundant ones.

Signature · 3 lines

Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

Alexander Duyck

2018-03-23 18:31 UTC #11 lore raw

Re: [Intel-wired-lan] [PATCH v6 7/7] ixgbevf: eliminate duplicate barriers on weakly-ordered archs

Quoted 11 lines · Sinan Kaya

Quoted 9 lines · Alexander Duyck
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c static void ixgbevf_alloc_rx_buffers(struct ixgbevf_ring *rx_ring, · line 734
Quoted 5 lines
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c +4 -0
+               /* We need this if more than one processor can write to our tail
+                * at a time, it synchronizes IO on IA64/Altix systems
+                */
+               mmiowb();
        }
The mmiowb shouldn't be needed for Rx. Only one CPU will be running NAPI for the queue and we will synchronize this with a full writel anyway when we re-enable the interrupts.

Quoted 4 lines

OK. I can fix this on the next version. I did a blanket search and replace for my writel_relaxed() changes as I don't know the code well enough.
Please point me to the redundant ones.

So from what I can tell only this file and i40e needed any additional mmiowb calls added. The rest are not needed.

- Alex

Sinan Kaya

author

2018-03-23 18:45 UTC #12 lore raw

Re: [Intel-wired-lan] [PATCH v6 7/7] ixgbevf: eliminate duplicate barriers on weakly-ordered archs

Quoted 3 lines · Alexander Duyck

Quoted 1 line
Please point me to the redundant ones.
So from what I can tell only this file and i40e needed any additional mmiowb calls added. The rest are not needed.

Thanks, I'll clean up between 2..6 and then make your suggested changes on 1 and 7.

Signature · 3 lines

Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.