[PATCH] e1000 poll behavior

[PATCH] e1000 poll behavior

[Robert Olsson]

Hello!

Seems e1000 never gets into poll mode when tx_cleaned is false. Compare
irq's on RX interfaces eth0, eth2 in the forwarding test below.

Vanilla:
-------
Iface   MTU Met  RX-OK RX-ERR RX-DRP RX-OVR  TX-OK TX-ERR TX-DRP TX-OVR Flags
eth0   1500   0 3983334 8374749 8374749 6016848    111      0      0      0 BRU
eth1   1500   0      1      0      0      0 3982841      0      0      0 BRU
eth2   1500   0 4002156 8507930 8507930 5997844      5      0      0      0 BRU
eth3   1500   0      1      0      0      0 4001653      0      0      0 BRU

           CPU0       
 26:      66366   IO-APIC-level  eth0
 27:      75200   IO-APIC-level  eth1
 28:      66705   IO-APIC-level  eth2
 29:      75132   IO-APIC-level  eth3

--- drivers/net/e1000/e1000_main.c.orig	2004-12-09 17:49:56.000000000 +0100
+++ drivers/net/e1000/e1000_main.c	2004-12-09 19:05:07.000000000 +0100
@@ -2179,8 +2179,8 @@
 	*budget -= work_done;
 	netdev->quota -= work_done;
 	
-	/* if no Rx and Tx cleanup work was done, exit the polling mode */
-	if(!tx_cleaned || (work_done < work_to_do) || 
+	/* if no Tx and not enough Rx work done, exit the polling mode */
+	if((!tx_cleaned && (work_done < work_to_do)) || 
 				!netif_running(netdev)) {
 		netif_rx_complete(netdev);
 		e1000_irq_enable(adapter);

Patched: 
-------
Iface   MTU Met  RX-OK RX-ERR RX-DRP RX-OVR  TX-OK TX-ERR TX-DRP TX-OVR Flags
eth0   1500   0 4193457 8283968 8283968 5806693     99      0      0      0 BRU
eth1   1500   0      1      0      0      0 4192308      0      0      0 BRU
eth2   1500   0 4190698 8441200 8441200 5809302      5      0      0      0 BRU
eth3   1500   0      1      0      0      0 4190171      0      0      0 BRU

           CPU0       
 26:        336   IO-APIC-level  eth0
 27:      58159   IO-APIC-level  eth1
 28:         64   IO-APIC-level  eth2
 29:      58228   IO-APIC-level  eth3

						--ro

Re: [PATCH] e1000 poll behavior

[Martin Josefsson]

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

On Thu, 2004-12-09 at 17:07, Robert Olsson wrote:
> Hello!
> 
> Seems e1000 never gets into poll mode when tx_cleaned is false. Compare
> irq's on RX interfaces eth0, eth2 in the forwarding test below.

Yes, we discussed this in private some time ago, unfortunately I havn't
had any proper hardware to test my changes on :(

> --- drivers/net/e1000/e1000_main.c.orig	2004-12-09 17:49:56.000000000 +0100
> +++ drivers/net/e1000/e1000_main.c	2004-12-09 19:05:07.000000000 +0100
> @@ -2179,8 +2179,8 @@
>  	*budget -= work_done;
>  	netdev->quota -= work_done;
>  	
> -	/* if no Rx and Tx cleanup work was done, exit the polling mode */
> -	if(!tx_cleaned || (work_done < work_to_do) || 
> +	/* if no Tx and not enough Rx work done, exit the polling mode */
> +	if((!tx_cleaned && (work_done < work_to_do)) || 
>  				!netif_running(netdev)) {
>  		netif_rx_complete(netdev);
>  		e1000_irq_enable(adapter);

This patch is broken.
What about the case where tx_cleaned is true but (work_done <
work_to_do) is true. Then the statement is false and we continue, later
we return (work_done >= work_to_do) which equals 0 (not seen in patch).
We are not allowed to return 0 when still on poll_list. That will sort
of continue polling in a degraded way (no increase in quota) but will
screw up device refcounting badly.

That final return must be changed into "return 1;"

-- 
/Martin

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] e1000 poll behavior

[Robert Olsson]

Martin Josefsson writes:

 > What about the case where tx_cleaned is true but (work_done <
 > work_to_do) is true. Then the statement is false and we continue, later
 > we return (work_done >= work_to_do) which equals 0 (not seen in patch).
 > We are not allowed to return 0 when still on poll_list. That will sort
 > of continue polling in a degraded way (no increase in quota) but will
 > screw up device refcounting badly.
 > 
 > That final return must be changed into "return 1;"

 Ohoh thanks yes...

--- drivers/net/e1000/e1000_main.c.orig	2004-12-09 17:49:56.000000000 +0100
+++ drivers/net/e1000/e1000_main.c	2004-12-10 20:13:57.000000000 +0100
@@ -2179,15 +2179,15 @@
 	*budget -= work_done;
 	netdev->quota -= work_done;
 	
-	/* if no Rx and Tx cleanup work was done, exit the polling mode */
-	if(!tx_cleaned || (work_done < work_to_do) || 
+	/* if no Tx and not enough Rx work done, exit the polling mode */
+	if((!tx_cleaned && (work_done < work_to_do)) || 
 				!netif_running(netdev)) {
 		netif_rx_complete(netdev);
 		e1000_irq_enable(adapter);
 		return 0;
 	}
 
-	return (work_done >= work_to_do);
+	return 1;
 }
 
 #endif

Same experiment again.

Iface   MTU Met  RX-OK RX-ERR RX-DRP RX-OVR  TX-OK TX-ERR TX-DRP TX-OVR Flags
eth0   1500   0 4351378 8252365 8252365 5648874    155      0      0      0 BRU
eth1   1500   0      1      0      0      0 4350771      0      0      0 BRU
eth2   1500   0 4352289 8383841 8383841 5647711      5      0      0      0 BRU
eth3   1500   0      1      0      0      0 4352126      0      0      0 BRU

           CPU0       
 26:        434   IO-APIC-level  eth0
 27:        109   IO-APIC-level  eth1
 28:        112   IO-APIC-level  eth2
 29:        112   IO-APIC-level  eth3

						--ro