Re: [PATCH] iphase fix.

Re: [PATCH] iphase fix.

[Jeff Garzik]

Linux Kernel Mailing List wrote:
> ChangeSet 1.1127, 2003/05/14 20:44:02-07:00, davej@codemonkey.org.uk
> 
> 	[PATCH] iphase fix.
> 	
> 	This went into 2.4 nearly a year back with the wonderfully
> 	descriptive  "Fix from maintainer" comment.

> diff -Nru a/drivers/net/fc/iph5526.c b/drivers/net/fc/iph5526.c
> --- a/drivers/net/fc/iph5526.c	Wed May 14 21:17:37 2003
> +++ b/drivers/net/fc/iph5526.c	Wed May 14 21:17:37 2003
> @@ -2984,8 +2984,7 @@
>  	 */
>  	if ((type == ETH_P_ARP) || (status == 0))
>  		dev_kfree_skb(skb);
> -	else
> -		netif_wake_queue(dev);
> +	netif_wake_queue(dev);
>  	LEAVE("iph5526_send_packet");


This appears to revert a fix.

You only want to wake the queue if you have room to queue another skb.

	Jeff

Re: [PATCH] iphase fix.

[Jeff Garzik]

Jeff Garzik wrote:
>>          dev_kfree_skb(skb);
>> -    else
>> -        netif_wake_queue(dev);
>> +    netif_wake_queue(dev);
>>      LEAVE("iph5526_send_packet");
> 
> 
> 
> This appears to revert a fix.
> 
> You only want to wake the queue if you have room to queue another skb.


Actually, I'm wrong.

But it could still use some looking-at.  You don't want to stop_queue at 
the beginning of send_packet and wake_queue at the end.  Instead, the 
queue should be awakened in the Tx completion routine, and the 
stop_queue should be moved from the beginning to the end of the function.

	Jeff

Re: [PATCH] iphase fix.

[Dave Jones]

On Thu, May 15, 2003 at 02:43:10AM -0400, Jeff Garzik wrote:
 > Jeff Garzik wrote:
 > >>         dev_kfree_skb(skb);
 > >>-    else
 > >>-        netif_wake_queue(dev);
 > >>+    netif_wake_queue(dev);
 > >>     LEAVE("iph5526_send_packet");
 > >
 > >This appears to revert a fix.
 > >You only want to wake the queue if you have room to queue another skb.
 > 
 > Actually, I'm wrong.
 > 
 > But it could still use some looking-at.  You don't want to stop_queue at 
 > the beginning of send_packet and wake_queue at the end.  Instead, the 
 > queue should be awakened in the Tx completion routine, and the 
 > stop_queue should be moved from the beginning to the end of the function.

Bring it up with whoever merged it into 2.4..

		Dave