| To: | hadi@xxxxxxxxxx |
|---|---|
| Subject: | Re: LLTX and netif_stop_queue |
| From: | Roland Dreier <roland@xxxxxxxxxxx> |
| Date: | Sun, 19 Dec 2004 15:16:58 -0800 |
| Cc: | "David S. Miller" <davem@xxxxxxxxxxxxx>, netdev@xxxxxxxxxxx, openib-general@xxxxxxxxxx |
| In-reply-to: | <1103497592.1046.235.camel@xxxxxxxxxxxxxxxx> (jamal's message of "19 Dec 2004 18:06:32 -0500") |
| References: | <52llbwoaej.fsf@xxxxxxxxxxx> <20041217214432.07b7b21e.davem@xxxxxxxxxxxxx> <1103484675.1050.158.camel@xxxxxxxxxxxxxxxx> <52fz21ncgh.fsf@xxxxxxxxxxx> <1103497592.1046.235.camel@xxxxxxxxxxxxxxxx> |
| Sender: | netdev-bounce@xxxxxxxxxxx |
| User-agent: | Gnus/5.1006 (Gnus v5.10.6) XEmacs/21.4 (Security Through Obscurity, linux) |
Roland> This seems a little risky. I can't point to a specific
Roland> deadlock but it doesn't seem right on general principles
Roland> to unlock in a different order than you nested the locks
Roland> when acquiring them -- if I understand correctly, you're
Roland> suggesting lock(queue_lock), lock(tx_lock),
Roland> unlock(queue_lock), unlock(tx_lock).
jamal> There is no deadlock. Thats exactly how things work. Try
jamal> the patches i posted.
Thinking about it more, I guess it's OK. I was just think about the
basic general rule that locks need to be acquired/released in LIFO
order to avoid deadlock (eg lock(A), lock(B), unlock(B), unlock(A)).
However in this case unlocking queue_lock after acquiring the driver's
tx_lock seems to be OK because the driver does a trylock on tx_lock in
the xmit path, so it can't deadlock. At worst the trylock will just
fail to get the lock.
Thanks,
Roland
|
| <Prev in Thread] | Current Thread | [Next in Thread> |
|---|---|---|
| ||
| Previous by Date: | Re: LLTX and netif_stop_queue, jamal |
|---|---|
| Next by Date: | Re: [PATCH] PKT_SCHED: dsmark must take care of shared/cloned skbs, Thomas Graf |
| Previous by Thread: | Re: LLTX and netif_stop_queue, jamal |
| Next by Thread: | Re: LLTX and netif_stop_queue, Eric Lemoine |
| Indexes: | [Date] [Thread] [Top] [All Lists] |