r8169: 2.6.22rc6+patches works, but 2.6.21.5 and 2.6.22rc6 without patches->network transfer blocking

Hello.

The hardware involved:

Motherboard: Asus P5B
lspci:
02:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL8111/8168B PCI Express Gigabit Ethernet
controller (rev 01)

First the non-working scenario (2.6.21.5, 2.6.22rc6 unpatched):
During the use of network connections, we experience "network transfer stops" during which a transfer seems to stall completely for many seconds, after which the transfer runs as if nothing happened.

This is reproducable everytime w.
svn co http://svnserver/svn/tree
(hangs VERY LONG)

Now with all patches under
http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.22-rc3/r8169-20070527/

and the single patch
http://www.fr.zoreil.com/people/francois/misc/20070619-2.6.22-rc5-r8169-test.patch

the problem is gone. Without the single patch, the problem persists.

Is there any possibility to get this fix working in 2.6.21.5? Would it be possible to apply the single patch to 2.6.21.5 and get a working driver? Or would it be possible to create the fixing patch for the driver in 2.6.21.5?

Greets,
Jens

[email protected]
			23.....56.......drifting
By caffeine alone I set my mind in motion, By the beans of Java
do thoughts acquire speed,  hands acquire shaking,  the shaking
becomes a warning, By caffeine alone do I set my mind in motion

Jens Stroebel <[email protected]> :
[...]

Mantra: mainline first, stable later.

In the next future, most of this patchset will hopefully go into 2.6.23-rc1. Some people will then complain that 2.6.23-rc1 breaks their 816x. After that the patchkit will be amended and the r8169 regressions in 2.6.23-rcX fixed.

Then it will be time to feed some r8169 bits in 2.6.2x.y

It may help and/or accelerate things if you can narrow the fix(es) in the current r8169 serie.

Ueimor

hm .. OK.

By this time, probably the hosts which should run w. the driver will be standing all around the world, which makes applying a patch slightly more interesting... ;-)

But see below:

As I mentioned in the initial message, I had experimented with 2.6.22rc6; unfortunately

a) we have some stuff depending on the kernel where

    2.6.21.5<->2.6.22rc6 makes a difference (does not work)

b) I experienced a hard lockup w. kernel 2.6.22rc6 this morning while not really doing something special (file IO, extracting a tar. As this was reproducably happening, we retreated from experimenting with it now.

Instead, I built 2.6.21.5+[a patch I snatched from a mail communication you had on 2007-06-20
(Msg-ID: <[email protected]> )]. I will attach it with this mail, as it is really small and you don't have to dig around to know what I'm talking about.

As I am no kernel-developer, I am not sure there are no undesired side effects by applying this patch to this kernel; if you think there are (could/should be), it would be nice if you could state that, so I'd refrain from testing with this combination.

greets,
jens

[email protected]
			23.....56.......drifting
By caffeine alone I set my mind in motion, By the beans of Java
do thoughts acquire speed,  hands acquire shaking,  the shaking
becomes a warning, By caffeine alone do I set my mind in motion

@@ -2514,7 +2514,7 @@ static inline u32 rtl8169_tso_csum(struct sk_buff *skb, struct net_device *dev)
 static int rtl8169_start_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct rtl8169_private *tp = netdev_priv(dev);
-	unsigned int frags, entry = tp->cur_tx % NUM_TX_DESC;
+	unsigned int frags, cur_tx, entry = tp->cur_tx % NUM_TX_DESC;
 	struct TxDesc *txd = tp->TxDescArray + entry;
 	void __iomem *ioaddr = tp->mmio_addr;
 	dma_addr_t mapping;

@@ -2567,13 +2567,17 @@ static int rtl8169_start_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	dev->trans_start = jiffies;
 
+	cur_tx = tp->cur_tx;
+
 	tp->cur_tx += frags + 1;
 
-	smp_wmb();
+	mmiowb();
 
-	RTL_W8(TxPoll, NPQ);	/* set polling bit */
+	smp_mb();
 
-	if (TX_BUFFS_AVAIL(tp) < MAX_SKB_FRAGS) {
+	if (cur_tx == tp->dirty_tx)
+		RTL_W8(TxPoll, NPQ);
+	else if (TX_BUFFS_AVAIL(tp) < MAX_SKB_FRAGS) {
 		netif_stop_queue(dev);
 		smp_rmb();
 		if (TX_BUFFS_AVAIL(tp) >= MAX_SKB_FRAGS)

@@ -2677,10 +2681,18 @@ static void rtl8169_tx_interrupt(struct net_device *dev,
 
 	if (tp->dirty_tx != dirty_tx) {
 		tp->dirty_tx = dirty_tx;
-		smp_wmb();
-		if (netif_queue_stopped(dev) &&
-		    (TX_BUFFS_AVAIL(tp) >= MAX_SKB_FRAGS)) {
-			netif_wake_queue(dev);
+		smp_mb();
+		if (unlikely(netif_queue_stopped(dev))) {
+			netif_tx_lock(dev);
+		    	if (TX_BUFFS_AVAIL(tp) >= MAX_SKB_FRAGS)
+				netif_wake_queue(dev);
+			if (dirty_tx != tp->cur_tx)
+				RTL_W8(TxPoll, NPQ);
+			netif_tx_unlock(dev);
+		} else if (dirty_tx != tp->cur_tx) {
+			netif_tx_lock(dev);
+			RTL_W8(TxPoll, NPQ);
+			netif_tx_unlock(dev);
 		}
 	}
 }

Following up to myself:

It seems like the patch is able to change things in a way which makes the machine lock up hard. Pity that, looked so promising at first...

greets,
jens

[email protected]
			23.....56.......drifting
By caffeine alone I set my mind in motion, By the beans of Java
do thoughts acquire speed,  hands acquire shaking,  the shaking
becomes a warning, By caffeine alone do I set my mind in motion

Jens Stroebel <[email protected]> :
[...]

As a 8168 user, you should really apply the serie I sent yesterday before anything else. Then you can try again this patch.

Can you publish your .config somewhere ?

Ueimor