Re: [NET]: Add netif_tx_lock

2006-06-09 Thread David Miller
From: Herbert Xu [EMAIL PROTECTED] Date: Fri, 9 Jun 2006 15:48:16 +1000 On Thu, Jun 01, 2006 at 09:15:03PM +1000, herbert wrote: OK, here is a patch which does this. [NET]: Add netif_tx_lock Just noticed that I showed dyslexia in winbond.c :) Here is the corrected version. [NET

Re: [NET]: Add netif_tx_lock

2006-06-08 Thread Herbert Xu
On Thu, Jun 01, 2006 at 09:15:03PM +1000, herbert wrote: OK, here is a patch which does this. [NET]: Add netif_tx_lock Just noticed that I showed dyslexia in winbond.c :) Here is the corrected version. [NET]: Add netif_tx_lock Various drivers use xmit_lock internally to synchronise

Re: [NET]: Add netif_tx_lock

2006-06-06 Thread Roland Dreier
David As long as you never take priv-lock while -xmit_lock is David held your patch should be OK. Duh ... unfortunately priv-lock is taken from interrupt context so that patch isn't safe. A correct fix would be the following, which leads to a trivial conversion to using netif_tx_lock().

Re: [NET]: Add netif_tx_lock

2006-06-06 Thread Herbert Xu
On Mon, Jun 05, 2006 at 09:32:50PM -0700, David Miller wrote: IPOIB is going to BUG() with this change. Because now, in their multicast code, you're going to local_bh_disable() via netif_tx_unlock() with hw IRQs disabled which is illegal. It shows a bug here in the locking of the IPOIB

Re: [NET]: Add netif_tx_lock

2006-06-05 Thread David Miller
for those drivers that already make use of xmit_lock without setting the owner. So I suppose something like net_xmit_lock to obtain xmit_lock is called for. OK, here is a patch which does this. [NET]: Add netif_tx_lock IPOIB is going to BUG() with this change. Because now

Re: [NET]: Add netif_tx_lock

2006-06-05 Thread Roland Dreier
IPOIB is going to BUG() with this change. Because now, in their multicast code, you're going to local_bh_disable() via netif_tx_unlock() with hw IRQs disabled which is illegal. It shows a bug here in the locking of the IPOIB driver. Sorry, I haven't followed this thread closely. Can

Re: [NET]: Add netif_tx_lock

2006-06-05 Thread Roland Dreier
Roland Sorry, I haven't followed this thread closely. Can you Roland expand on what the bug in ipoib's multicast locking is? You know, looking at the ipoib code, I can't even recreate why xmit_lock is taken in the set_multicast_list method anyway, or how it works at all -- it seems

Re: [NET]: Add netif_tx_lock

2006-06-05 Thread Roland Dreier
Roland You know, looking at the ipoib code, I can't even recreate Roland why xmit_lock is taken in the set_multicast_list method Roland anyway, or how it works at all -- it seems Roland set_multicast_list will always be called with xmit_lock Roland already held. What the heck

Re: [NET]: Add netif_tx_lock

2006-06-05 Thread David Miller
From: Roland Dreier [EMAIL PROTECTED] Date: Mon, 05 Jun 2006 21:44:01 -0700 IPOIB is going to BUG() with this change. Because now, in their multicast code, you're going to local_bh_disable() via netif_tx_unlock() with hw IRQs disabled which is illegal. It shows a bug here in the

Re: [NET]: Add netif_tx_lock

2006-06-05 Thread Roland Dreier
David Isn't the IPOIB driver LLTX and therefore the upper layers David are not taking the xmit_lock? Yeah, but I think that should be OK. If I'm remembering the intention of the code correctly, the reason xmit_lock is being taken there is just to protect dev-mc_list -- and this is needed

Re: [NET]: Add netif_tx_lock

2006-06-05 Thread David Miller
From: Roland Dreier [EMAIL PROTECTED] Date: Mon, 05 Jun 2006 22:04:34 -0700 Am I right in thinking that dev-xmit_lock still serializes mc_list operations, even for LLTX drivers? Correct. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL

Re: [NET]: Add netif_tx_lock

2006-06-05 Thread David Miller
From: Roland Dreier [EMAIL PROTECTED] Date: Mon, 05 Jun 2006 21:57:51 -0700 Roland You know, looking at the ipoib code, I can't even recreate Roland why xmit_lock is taken in the set_multicast_list method Roland anyway, or how it works at all -- it seems Roland

[NET]: Add netif_tx_lock

2006-06-01 Thread Herbert Xu
. So I suppose something like net_xmit_lock to obtain xmit_lock is called for. OK, here is a patch which does this. [NET]: Add netif_tx_lock Various drivers use xmit_lock internally to synchronise with their transmission routines. They do so without setting xmit_lock_owner. This is fine