Re: [patch,rfc] allow registration of multiple netpolls per interface

[Matt Mackall <mpm@selenic.com>]

On Wed, Jun 22, 2005 at 05:05:15PM -0400, Jeff Moyer wrote:
> mpm> It might be simpler to have a single lock here..?
> >> 
> >> Maybe.  You can't really have netpoll code running on multiple cpus at the
> >> same time, right?  This is the rx path, remember, so the other cpu should
> >> be spinning on the poll_lock.
> >> 
> >> Keeping separate locks would allow you to unregister a struct netpoll
> >> associated with another net device without causing lock contention.  This
> >> is a very minor win, obviously.
> >> 
> >> I still feel like this npinfo struct is the right place for this, though.
> >> If you're strongly opposed to that, I'll change it.
> 
> mpm> No, certainly having it in npinfo makes sense. I just was wondering if
> mpm> we really need two locks in there.
> 
> Oh, I misunderstood.  Well, one protects recursing into the driver's poll
> routine, the other protects access to the np_rx pointer, which may later
> become a list.  I don't think we can lump these two together, do you?

I don't see why we couldn't, but let's worry about it later.
 
> >> >> +	spin_lock_irqsave(&npinfo->rx_lock, flags);
> >> >> +	if (npinfo->rx_np->dev == skb->dev)
> >> >> +		np = npinfo->rx_np;
> >> >> +	spin_unlock_irqrestore(&npinfo->rx_lock, flags);
> >> 
> mpm> And I think that means we don't need the lock here either.  
> >> 
> >> Sure we do.  We need to protect against rmmod's.
> 
> mpm> How can we have an rmmmod when we're trapped?
> 
> Looking over the code, I don't see what would prevent this.  Could you
> point me the code which prevents this?

I forgot we overloaded trapped for dealing with NAPI. Formerly
trapping meant "I'm stopping the box, drop every packet that's not
addressed to me" which also implied no one should be pulling the rug
out from under us.

> (Interdiff first)

Looks fine.

-- 
Mathematics is the supreme nostalgia of our time.