Received: with ECARTIS (v1.0.0; list netdev); Thu, 23 Dec 2004 08:37:46 -0800 (PST) Received: from kaber.coreworks.de ([62.206.217.67]) by oss.sgi.com (8.13.0/8.13.0) with ESMTP id iBNGbIF3025029 for ; Thu, 23 Dec 2004 08:37:39 -0800 Received: from eru.coreworks.de ([172.16.0.2] helo=trash.net) by kaber.coreworks.de with esmtp (Exim 4.34) id 1ChVyi-0001X8-Kq; Thu, 23 Dec 2004 17:38:16 +0100 Message-ID: <41CAF444.3000305@trash.net> Date: Thu, 23 Dec 2004 17:37:24 +0100 From: Patrick McHardy User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040413 Debian/1.6-5 X-Accept-Language: en MIME-Version: 1.0 To: Eric Lemoine CC: "David S. Miller" , hadi@cyberus.ca, roland@topspin.com, netdev@oss.sgi.com, openib-general@openib.org Subject: Re: LLTX and netif_stop_queue References: <52llbwoaej.fsf@topspin.com> <20041217214432.07b7b21e.davem@davemloft.net> <1103484675.1050.158.camel@jzny.localdomain> <5cac192f04122210491d64d4b6@mail.gmail.com> <20041222202919.057b8331.davem@davemloft.net> <5cac192f0412230110628749e3@mail.gmail.com> In-Reply-To: <5cac192f0412230110628749e3@mail.gmail.com> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit X-Virus-Scanned: ClamAV 0.80/638/Tue Dec 21 14:41:34 2004 clamav-milter version 0.80j on 127.0.0.1 X-Virus-Status: Clean X-archive-position: 13001 X-ecartis-version: Ecartis v1.0.0 Sender: netdev-bounce@oss.sgi.com Errors-to: netdev-bounce@oss.sgi.com X-original-sender: kaber@trash.net Precedence: bulk X-list: netdev 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. Regards Patrick