[PATCH v2 net 0/2] r8169: revert two commits due to a regression

[PATCH v2 net 0/2] r8169: revert two commits due to a regression

[Heiner Kallweit]

Sander reported a regression (kernel panic, see[1]), therefore let's
revert these commits. Removal of the barriers doesn't seem to
contribute to the issue, the patch just overlaps with the problematic
one and only reverting both patches was tested.

[1] https://marc.info/?t=154965066400001&r=1&w=2

v2:
- improve commit message

Heiner Kallweit (2):
  Revert "r8169: remove unneeded mmiowb barriers"
  Revert "r8169: make use of xmit_more and __netdev_sent_queue"

 drivers/net/ethernet/realtek/r8169.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

-- 
2.20.1

[PATCH v2 net 1/2] Revert "r8169: remove unneeded mmiowb barriers"

[Heiner Kallweit]

This reverts commit bd7153bd83b806bfcc2e79b7a6f43aa653d06ef3.

There doesn't seem to be anything wrong with this patch,
it's just reverted to get a stable baseline again.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index abb94c543aa2..bba806ce57d3 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -1286,11 +1286,13 @@ static u16 rtl_get_events(struct rtl8169_private *tp)
 static void rtl_ack_events(struct rtl8169_private *tp, u16 bits)
 {
 	RTL_W16(tp, IntrStatus, bits);
+	mmiowb();
 }
 
 static void rtl_irq_disable(struct rtl8169_private *tp)
 {
 	RTL_W16(tp, IntrMask, 0);
+	mmiowb();
 }
 
 #define RTL_EVENT_NAPI_RX	(RxOK | RxErr)
@@ -6130,8 +6132,10 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
 	if (unlikely(stop_queue))
 		netif_stop_queue(dev);
 
-	if (__netdev_sent_queue(dev, skb->len, skb->xmit_more))
+	if (__netdev_sent_queue(dev, skb->len, skb->xmit_more)) {
 		RTL_W8(tp, TxPoll, NPQ);
+		mmiowb();
+	}
 
 	if (unlikely(stop_queue)) {
 		/* Sync with rtl_tx:
@@ -6483,7 +6487,9 @@ static int rtl8169_poll(struct napi_struct *napi, int budget)
 
 	if (work_done < budget) {
 		napi_complete_done(napi, work_done);
+
 		rtl_irq_enable(tp);
+		mmiowb();
 	}
 
 	return work_done;
-- 
2.20.1

[PATCH v2 net 2/2] Revert "r8169: make use of xmit_more and __netdev_sent_queue"

[Heiner Kallweit]

This reverts commit 2e6eedb4813e34d8d84ac0eb3afb668966f3f356.

Sander reported a regression causing a kernel panic[1],
therefore let's revert this commit.

[1] https://marc.info/?t=154965066400001&r=1&w=2

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index bba806ce57d3..6e36b88ca7c9 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -6074,7 +6074,6 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
 	struct device *d = tp_to_dev(tp);
 	dma_addr_t mapping;
 	u32 opts[2], len;
-	bool stop_queue;
 	int frags;
 
 	if (unlikely(!rtl_tx_slots_avail(tp, skb_shinfo(skb)->nr_frags))) {
@@ -6116,6 +6115,8 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
 
 	txd->opts2 = cpu_to_le32(opts[1]);
 
+	netdev_sent_queue(dev, skb->len);
+
 	skb_tx_timestamp(skb);
 
 	/* Force memory writes to complete before releasing descriptor */
@@ -6128,16 +6129,16 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
 
 	tp->cur_tx += frags + 1;
 
-	stop_queue = !rtl_tx_slots_avail(tp, MAX_SKB_FRAGS);
-	if (unlikely(stop_queue))
-		netif_stop_queue(dev);
+	RTL_W8(tp, TxPoll, NPQ);
 
-	if (__netdev_sent_queue(dev, skb->len, skb->xmit_more)) {
-		RTL_W8(tp, TxPoll, NPQ);
-		mmiowb();
-	}
+	mmiowb();
 
-	if (unlikely(stop_queue)) {
+	if (!rtl_tx_slots_avail(tp, MAX_SKB_FRAGS)) {
+		/* Avoid wrongly optimistic queue wake-up: rtl_tx thread must
+		 * not miss a ring update when it notices a stopped queue.
+		 */
+		smp_wmb();
+		netif_stop_queue(dev);
 		/* Sync with rtl_tx:
 		 * - publish queue status and cur_tx ring index (write barrier)
 		 * - refresh dirty_tx ring index (read barrier).
-- 
2.20.1

Re: [PATCH v2 net 0/2] r8169: revert two commits due to a regression

[David Miller]

From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Sun, 10 Feb 2019 15:24:37 +0100

> Sander reported a regression (kernel panic, see[1]), therefore let's
> revert these commits. Removal of the barriers doesn't seem to
> contribute to the issue, the patch just overlaps with the problematic
> one and only reverting both patches was tested.
> 
> [1] https://marc.info/?t=154965066400001&r=1&w=2
> 
> v2:
> - improve commit message

Series applied, thanks.