Re: LLTX and netif_stop_queue

[Eric Lemoine <eric.lemoine@xxxxxxxxx>]

On Thu, 23 Dec 2004 17:37:24 +0100, Patrick McHardy <kaber@xxxxxxxxx> wrote:
> Eric Lemoine wrote:
> > I still have one concern with the LLTX code (and it may be that the
> > correct patch is Jamal's) :
> >
> > Without LLTX we do : lock(queue_lock), lock(xmit_lock),
> > release(queue_lock), release(xmit_lock). With LLTX (without Jamal's
> > patch) we do : lock(queue_lock), release(queue_lock), lock(tx_lock),
> > release(tx_lock). LLTX doesn't look correct because it creates a race
> > condition window between the the two lock-protected sections. So you
> > may want to reconsider Jamal's patch or pull out LLTX...
> 
> You're right, it can cause packet reordering if something like this
> happens:
> 
> CPU1                    CPU2
> lock(queue_lock)
> dequeue
> unlock(queue_lock)
>                         lock(queue_lock)
>                         dequeue
>                         unlock(queue_lock)
>                         lock(xmit_lock)
>                         hard_start_xmit
>                         unlock(xmit_lock)
> lock(xmit_lock)
> hard_start_xmit
> unlock(xmit_lock)
> 
> Jamal's patch should fix this.

Yes but requiring drivers to release a lock that they should not even
be aware of doesn't sound good. Another way would be to keep
dev->queue_lock grabbed when entering start_xmit() and let the driver
drop it (and re-acquire it before it returns) only if it wishes so.
Although I don't like this too much either, that's the best way I can
think of up to now...

-- 
Eric