Re: 2.6.13-rc6-rt6

[Steven Rostedt <rostedt@goodmis.org>]

On Fri, 2005-08-19 at 15:47 -0700, Paul E. McKenney wrote:

> Good catch -- but a few changes needed to be perfectly safe:
> 
> 	static inline void *netpoll_poll_lock(struct net_device *dev)
> 	{
> 
> 		struct netpoll_info *npi;
> 
> 		rcu_read_lock();
> 		npi = rcu_dereference(dev)->npinfo;
> 		if (have) {

Here I'm sure you mean "if (npi) {" :-)

> 			spin_lock(&npi->poll_lock);
> 			npi->poll_owner = smp_processor_id();
> 			return npi;
> 		}
> 		return NULL;
> 	}
> 
> The earlier version could get in trouble if dev->npinfo was set
> to NULL while this was executing.

Truth be told,  I was just fixing the race with getting the npinfo
pointer set between netpoll_poll_lock and netpoll_poll_unlock.  I wrote
a patch that fixed that but nothing with the rcu_locks.  Then I looked
at the current git tree and saw that they already had my changes, but
also included the rcu locks.  So I just (blindly) added them.


> 
> Again, I do not fully understand this code, so a grain of salt might
> come in handy.  But there definitely need to be some rcu_dereference()
> and rcu_assign_pointer() primitives in there somewhere.  ;-)
> 
> The following changes look good to me, but, as I said earlier, I do
> not claim to fully understand this code.

netpoll has changed quite a bit in the last few releases. I've seen lots
of fixup code sent in (which usually means there's lots of new broken
code ;-)

Anyway, I don't quite fully understand RCU. I read a few of the
documents on your web site, but I haven't had time to really digest it.
Have you taken a look at the latest git tree?  The rcu_locks are used
for net poll quite a bit more there.

-- Steve