Re: [PATCH] [e1000]: Remove unnecessary tx_lock

[jamal <hadi@cyberus.ca>]

On Mon, 2006-07-08 at 20:47 +0200, Edgar E. Iglesias wrote:

> I think we are out of sync :) 

Imagine that, eh? ;->

> My, fault I haven't been clear enough.
> 

Not just your transmit but also my receive is at fault ;-> (aka, I may
not be listening as well as i should). Now two machines or CPUs you
would think wont have this problem since they dont possess minds;->

> First of all, I don't think the patch with jamal undefined has any problems. I 
> assumed wrongly from the start that you somehow wanted that part to go in 
> aswell, sorry about that. As you say, the flow goes just as before.
> 
> Now, with jamal defined, I only see e1000_prune_tx_ring beeing called if
> fdesc < tx_ring->prunet or fdesc < tx_ring->waket. 

Ok, thats the code that has been commented out, no? i.e there is no
fdesc otherwise.

> In other words, the freing
> of skbs is dependant on external events that might not become true if the
> host is quiet. Skb's could end up sitting on the ring indefinitely.
> 

Yes, this has _always_ been true. In the patch i posted it merely
converted things, example:

-#define E1000_TX_WEIGHT 64
-               /* weight of a sort for tx, to avoid endless transmit
cleanup */
-               if (count++ == E1000_TX_WEIGHT) break;
+               /* avoid endless transmit cleanup */
+               if (count++ == tx_ring->prunet) break;

As you can see E1000_TX_WEIGHT threshold exists today and you are right
if no TX interupts, packet arrivals or scheduled wakes happen the that
descriptor that was not pruned will sit there forever (which is a bad
thing for TCP). Are we in sync?
If yes, what is the likelihood they will sit there forever? I think
perhaps some TX interupts will happen, no?

cheers,
jamal