All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>,
	David Miller <davem@davemloft.net>,
	netdev@vger.kernel.org
Subject: Re: [RFC] NETDEV_UNREGISTER_BATCH seems unused nowaday ?
Date: Fri, 10 Aug 2012 10:55:16 -0700	[thread overview]
Message-ID: <20120810175516.GE2371@linux.vnet.ibm.com> (raw)
In-Reply-To: <87vcgq955v.fsf@xmission.com>

On Fri, Aug 10, 2012 at 07:45:48AM -0700, Eric W. Biederman wrote:
> Eric Dumazet <eric.dumazet@gmail.com> writes:
> 
> > On Fri, 2012-08-10 at 03:42 -0700, David Miller wrote:
> >> From: Eric Dumazet <eric.dumazet@gmail.com>
> >> Date: Fri, 10 Aug 2012 11:27:04 +0200
> >> 
> >> > NETDEV_UNREGISTER_BATCH seems unused we can probably remove it.
> >> 
> >> Indeed, the routing cache was the final real user.
> >> 
> >> > I am tracking a device refcount issue, delaying net device dismantle by
> >> > 1 second in netdev_wait_allrefs()
> >> > 
> >> > I guess we need to add a notifier called _after_ the final
> >> > synchronize_net() in rollback_registered_many()
> >> 
> >> It's essentially caused by DST_GC_INC, right?
> >
> > No, we in fact need a rcu_barrier(), then another call to
> > dst_dev_event().
> >
> > rcu_barrier() is needed so that in-flight call_rcu() of routes (from
> > rt_free()) are completed. Or else we miss these dst in the
> > dst_dev_event().
> 
> > I have a working patch, adding the rcu_barrier() and one additional
> > NETDEV_UNREGISTER_FINAL event.
> 
> Can someone help bring me up to speed.  What has changed in the
> dst ref counting that has invalidated our previous solutions?
> 
> As for the idea of putting an rcu_barrier inside of the rtnl_lock.  I
> really don't like it. You are trading off a 1000ms singled threaded wait
> without locks for extending the hold times of rtnl lock by 12ms or so.
> 
> We already have an rcu_barrier on that path in netdev_run_todo,
> so we can reorganize things to use that barrier I would be much
> happer.  Furthermore I talked to Paul McKenney a while ago
> about creating an rcu_barrier expedited and he really did not
> like the idea.

For whatever it is worth, I do have rcu_barrier_expedited() on my list
of things that at least one person has expressed interest in, but that
I do not yet have a good solution for.  Obstacles include the following:

1.	If a given CPU has lots of callbacks, but is running a real-time
	process, what do you do?  (a) Hammer the real-time process?
	(b) Make rcu_barrier_expedited() wait (current likely choice)?
	(c) Handle via a set priority for callback processing (which
	might be the case for BOOST_RCU builds)?  (d) Force migration
	of the callbacks (mmmaybe...)?  (e) Force migration of the
	real-time process (ouch!)?

2.	Ditto, but huge numbers of callbacks and non-realtime processes.
	Similar solution space.

3.	There will be some real-time disruption from any reasonable
	implementation of rcu_barrier_expedited().  Maybe the RT
	guys choose to map it to rcu_barrier()?

On the other hand, in the same email thread back in May you were also
looking for a kmem_cache_free_rcu().  At the time I didn't have a
good solution for this, but I do believe that I have one now.  ;-)

							Thanx, Paul

> Reading through the code we really should get dst_rcu_free
> out of the header and make it non-line.  dst_rcu_free can't
> possibly be called from a location where it can be inlined.
> 
> Trying to understand your analysis I have stared at the code for
> a while and I am definitely not seeing any rcu callbacks that
> result in calling rt_free.  So one of us is missing something.
> 
> All I am seeing from your trace is one call of rtnetlink_dev_notifier
> the refcount is at 7 and the next call of rtnetlink_dev_notifier the
> refcnt has dropped to 1.
> 
> Eric
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

  parent reply	other threads:[~2012-08-10 17:55 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-10  9:27 [RFC] NETDEV_UNREGISTER_BATCH seems unused nowaday ? Eric Dumazet
2012-08-10 10:42 ` David Miller
2012-08-10 11:06   ` Eric Dumazet
2012-08-10 14:45     ` Eric W. Biederman
2012-08-10 15:01       ` Eric Dumazet
2012-08-10 17:55       ` Paul E. McKenney [this message]
2012-08-10 14:46     ` [PATCH] net: remove delay at device dismantle Eric Dumazet
2012-08-10 15:42       ` [PATCH net-next v2] " Eric Dumazet
2012-08-11  5:54         ` Eric Dumazet
2012-08-11  5:57           ` David Miller
2012-08-23  2:18           ` David Miller
2012-08-23  2:25           ` Gao feng
2012-08-23  2:51             ` David Miller
2012-08-23  2:58               ` Eric Dumazet

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120810175516.GE2371@linux.vnet.ibm.com \
    --to=paulmck@linux.vnet.ibm.com \
    --cc=davem@davemloft.net \
    --cc=ebiederm@xmission.com \
    --cc=eric.dumazet@gmail.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.