netdev
[Top] [All Lists]

Re: LLTX and netif_stop_queue

To: "David S. Miller" <davem@xxxxxxxxxxxxx>
Subject: Re: LLTX and netif_stop_queue
From: Eric Lemoine <eric.lemoine@xxxxxxxxx>
Date: Thu, 23 Dec 2004 10:10:34 +0100
Cc: hadi@xxxxxxxxxx, roland@xxxxxxxxxxx, netdev@xxxxxxxxxxx, openib-general@xxxxxxxxxx
Domainkey-signature: a=rsa-sha1; q=dns; c=nofws; s=beta; d=gmail.com; h=received:message-id:date:from:reply-to:to:subject:cc:in-reply-to:mime-version:content-type:content-transfer-encoding:references; b=TiuJhu3uBjraWexE5SSCUybZeCBWz20krqmWpR2n58MvCw8Q2xTempN4sWtq6emUN5YFtc0RMqsAX9P/L09tmSELJCELh1Szo5AdOmGBzpNNGG+vtBZ7e8rf89r3SYdKSsctKLyPKGRxA6m57yna/erXe1zXD28T9WYGQ1/GI4w=
In-reply-to: <20041222202919.057b8331.davem@davemloft.net>
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>
Reply-to: Eric Lemoine <eric.lemoine@xxxxxxxxx>
Sender: netdev-bounce@xxxxxxxxxxx
On Wed, 22 Dec 2004 20:29:19 -0800, David S. Miller <davem@xxxxxxxxxxxxx> wrote:
> On Wed, 22 Dec 2004 19:49:48 +0100
> Eric Lemoine <eric.lemoine@xxxxxxxxx> wrote:
> 
> > Instead, I would suggest to have LLTX drivers check whether queue is
> > stopped after they grab their private tx lock and before they check tx
> > ring fullness. That way we close the race window but keep the driver
> > bug check around.
> >
> > See attached sungem patch.
> 
> That sounds about right.  Nice idea.  It solves the race, and retains
> the error state check.
> 
> I'll apply Eric's patch, and do something similar in the other LLTX
> drivers (except loopback which has not "queue" per se so doesn't need
> this stuff).

Dave,

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

Thanks, 

-- 
Eric

<Prev in Thread] Current Thread [Next in Thread>