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

["Edgar E. Iglesias" <edgar.iglesias@axis.com>]

On Mon, Aug 07, 2006 at 03:03:33PM -0400, jamal wrote:
> 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.

Exactly.

> 
> > 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?

Yep :)

> If yes, what is the likelihood they will sit there forever? I think
> perhaps some TX interupts will happen, no?

with jamal undefined, absolutely. With jamal defined, TX interrupts will come
but I couldnt find a way into e1000_prune_tx_ring unless fdesc met the 
conditions. Correct? 

Best regards
-- 
        Programmer
        Edgar E. Iglesias <edgar.iglesias@axis.com> 46.46.272.1946