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 dyslexi
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
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 IPO
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
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> set_mu
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
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 need
From: Roland Dreier <[EMAIL PROTECTED]>
Date: Mon, 05 Jun 2006 21:50:11 -0700
> 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
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 h
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 h
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 set_
> 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.
t;
> > You're right. In fact this can deadlock today 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.
>
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
Various drivers use xmit_lock internally to synchronise with their
transmission routines. They do so without
14 matches
Mail list logo
