Netdev Archive on lore.kernel.org
 help / Atom feed
* [PATCH v2 net 0/2] r8169: revert two commits due to a regression
@ 2019-02-10 14:24 Heiner Kallweit
  2019-02-10 14:26 ` [PATCH v2 net 1/2] Revert "r8169: remove unneeded mmiowb barriers" Heiner Kallweit
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Heiner Kallweit @ 2019-02-10 14:24 UTC (permalink / raw)
  To: Realtek linux nic maintainers, David Miller; +Cc: Sander Eikelenboom, netdev

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2 net 1/2] Revert "r8169: remove unneeded mmiowb barriers"
  2019-02-10 14:24 [PATCH v2 net 0/2] r8169: revert two commits due to a regression Heiner Kallweit
@ 2019-02-10 14:26 ` Heiner Kallweit
  2019-02-10 14:28 ` [PATCH v2 net 2/2] Revert "r8169: make use of xmit_more and __netdev_sent_queue" Heiner Kallweit
  2019-02-10 20:57 ` [PATCH v2 net 0/2] r8169: revert two commits due to a regression David Miller
  2 siblings, 0 replies; 4+ messages in thread
From: Heiner Kallweit @ 2019-02-10 14:26 UTC (permalink / raw)
  To: Realtek linux nic maintainers, David Miller; +Cc: Sander Eikelenboom, netdev

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




^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2 net 2/2] Revert "r8169: make use of xmit_more and __netdev_sent_queue"
  2019-02-10 14:24 [PATCH v2 net 0/2] r8169: revert two commits due to a regression Heiner Kallweit
  2019-02-10 14:26 ` [PATCH v2 net 1/2] Revert "r8169: remove unneeded mmiowb barriers" Heiner Kallweit
@ 2019-02-10 14:28 ` Heiner Kallweit
  2019-02-10 20:57 ` [PATCH v2 net 0/2] r8169: revert two commits due to a regression David Miller
  2 siblings, 0 replies; 4+ messages in thread
From: Heiner Kallweit @ 2019-02-10 14:28 UTC (permalink / raw)
  To: Realtek linux nic maintainers, David Miller; +Cc: Sander Eikelenboom, netdev

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




^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 net 0/2] r8169: revert two commits due to a regression
  2019-02-10 14:24 [PATCH v2 net 0/2] r8169: revert two commits due to a regression Heiner Kallweit
  2019-02-10 14:26 ` [PATCH v2 net 1/2] Revert "r8169: remove unneeded mmiowb barriers" Heiner Kallweit
  2019-02-10 14:28 ` [PATCH v2 net 2/2] Revert "r8169: make use of xmit_more and __netdev_sent_queue" Heiner Kallweit
@ 2019-02-10 20:57 ` David Miller
  2 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2019-02-10 20:57 UTC (permalink / raw)
  To: hkallweit1; +Cc: nic_swsd, linux, netdev

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.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-10 14:24 [PATCH v2 net 0/2] r8169: revert two commits due to a regression Heiner Kallweit
2019-02-10 14:26 ` [PATCH v2 net 1/2] Revert "r8169: remove unneeded mmiowb barriers" Heiner Kallweit
2019-02-10 14:28 ` [PATCH v2 net 2/2] Revert "r8169: make use of xmit_more and __netdev_sent_queue" Heiner Kallweit
2019-02-10 20:57 ` [PATCH v2 net 0/2] r8169: revert two commits due to a regression David Miller

Netdev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netdev/0 netdev/git/0.git
	git clone --mirror https://lore.kernel.org/netdev/1 netdev/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netdev netdev/ https://lore.kernel.org/netdev \
		netdev@vger.kernel.org netdev@archiver.kernel.org
	public-inbox-index netdev


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netdev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox