On Thu, 23 Dec 2004 17:37:24 +0100, Patrick McHardy <[EMAIL PROTECTED]> 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 _______________________________________________ openib-general mailing list [email protected] http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
