netdev
[Top] [All Lists]

Re: 2.6.13-rc6-rt6

To: Steven Rostedt <rostedt@xxxxxxxxxxx>
Subject: Re: 2.6.13-rc6-rt6
From: "Paul E. McKenney" <paulmck@xxxxxxxxxx>
Date: Fri, 19 Aug 2005 16:12:23 -0700
Cc: Ingo Molnar <mingo@xxxxxxx>, netdev@xxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx
In-reply-to: <1124492562.18408.35.camel@xxxxxxxxxxxxxxxxxxxxx>
References: <1124252419.5764.83.camel@xxxxxxxxxxxxxxxxxxxxx> <1124257580.5764.105.camel@xxxxxxxxxxxxxxxxxxxxx> <20050817064750.GA8395@xxxxxxx> <1124287505.5764.141.camel@xxxxxxxxxxxxxxxxxxxxx> <1124288677.5764.154.camel@xxxxxxxxxxxxxxxxxxxxx> <1124295214.5764.163.camel@xxxxxxxxxxxxxxxxxxxxx> <20050817162324.GA24495@xxxxxxx> <1124486548.18408.18.camel@xxxxxxxxxxxxxxxxxxxxx> <20050819224758.GJ1298@xxxxxxxxxx> <1124492562.18408.35.camel@xxxxxxxxxxxxxxxxxxxxx>
Reply-to: paulmck@xxxxxxxxxx
Sender: netdev-bounce@xxxxxxxxxxx
User-agent: Mutt/1.4.1i
On Fri, Aug 19, 2005 at 07:02:42PM -0400, Steven Rostedt wrote:
> 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) {" :-)

Right you are!  ;-)

> >                     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.

Understood!

> > 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.

Hmmm....  Guess it is time for me to stop procrastinating on better
understanding git...

                                                Thanx, Paul

<Prev in Thread] Current Thread [Next in Thread>