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

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

[Jens Stroebel]

			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
-- 
drifter@bcsoft.de
			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

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

[Francois Romieu]

Jens Stroebel <drifter@bcsoft.de> :
[...]
> 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?

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

2.6.21.5+1small patch: not blocking [was: Re: r8169: 2.6.22rc6+patches works, but 2.6.21.5 and 2.6.22rc6 without patches->network transfer blocking]

[Jens Stroebel]

[-- Attachment #1: Type: text/plain, Size: 2025 bytes --]

Francois Romieu wrote:
> Jens Stroebel <drifter@bcsoft.de> :
> [...]
>> Would it
>> be possible to apply the single patch to 2.6.21.5 and get a working
>> driver?

> Mantra: mainline first, stable later.

hm .. OK.

> 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

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:

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

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: <20070620211530.GA10042@electric-eye.fr.zoreil.com> )].
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
-- 
drifter@bcsoft.de
			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

[-- Attachment #2: 8169_stop_blocking.patch --]
[-- Type: text/plain, Size: 1711 bytes --]

diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -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);
 		}
 	}
 }

Re: 2.6.21.5+1small patch: not blocking

[Jens Stroebel]

Jens Stroebel wrote:
> Francois Romieu wrote:
>> [...]

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

> Instead, I built 2.6.21.5+[a patch I snatched from a mail communication
> you had on 2007-06-20
> (Msg-ID: <20070620211530.GA10042@electric-eye.fr.zoreil.com> )].
> 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.

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
-- 
drifter@bcsoft.de
			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

Re: 2.6.21.5+1small patch: not blocking

[Francois Romieu]

Jens Stroebel <drifter@bcsoft.de> :
[...]
> 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...

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