Linux network development

Eric Dumazet | 10 Aug 2012 11:27

Picon

[RFC] NETDEV_UNREGISTER_BATCH seems unused nowaday ?

NETDEV_UNREGISTER_BATCH seems unused we can probably remove it.

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()

[   52.022261] calling rtnetlink_dev_notifier msg/val 9 data ffff880610bea000 refcnt 15->15
[   52.022266] calling fib_rules_notifier msg/val 9 data ffff880610bea000 refcnt 15->15
[   52.022270] calling arp_netdev_notifier msg/val 9 data ffff880610bea000 refcnt 15->15
[   52.022274] calling ip_netdev_notifier msg/val 9 data ffff880610bea000 refcnt 15->15
[   52.022277] calling fib_netdev_notifier msg/val 9 data ffff880610bea000 refcnt 15->15
[   52.022280] calling ip_mr_notifier msg/val 9 data ffff880610bea000 refcnt 15->15
[   52.022284] calling ndisc_netdev_notifier msg/val 9 data ffff880610bea000 refcnt 15->15
[   52.022287] calling ip6_route_dev_notifier msg/val 9 data ffff880610bea000 refcnt 15->15
[   52.022290] calling ipv6_dev_notf msg/val 9 data ffff880610bea000 refcnt 15->15
[   52.022293] calling packet_netdev_notifier msg/val 9 data ffff880610bea000 refcnt 15->15
[   52.022297] calling dst_dev_notifier msg/val 9 data ffff880610bea000 refcnt 15->15
[   52.022331] calling rtnetlink_dev_notifier msg/val 2 data ffff880610bea000 refcnt 15->15
[   52.022335] calling fib_rules_notifier msg/val 2 data ffff880610bea000 refcnt 15->15
[   52.022340] calling arp_netdev_notifier msg/val 2 data ffff880610bea000 refcnt 15->15
[   52.022350] calling ip_netdev_notifier msg/val 2 data ffff880610bea000 refcnt 15->15
[   52.022374] arp: arp_ifdown dev ffff880610bea000 refcnt 15
[   52.022382] calling fib_netdev_notifier msg/val 2 data ffff880610bea000 refcnt 15->14
[   52.022931] IPv4: rt_free(ffff8802fee7e180)
[   52.023394] calling ip_mr_notifier msg/val 2 data ffff880610bea000 refcnt 14->14
[   52.023402] IPv6: dst_free(ffff88060b680480) refcnt 0
[   52.023404] IPv6: dst_free(ffff880307f7cf00) refcnt 0
[   52.023407] IPv6: dst_free(ffff880307f7cd80) refcnt 0

(Continue reading)

David Miller | 10 Aug 2012 12:42

Re: [RFC] NETDEV_UNREGISTER_BATCH seems unused nowaday ?

From: Eric Dumazet <eric.dumazet <at> 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?

Eric Dumazet | 10 Aug 2012 13:06

Picon

Re: [RFC] NETDEV_UNREGISTER_BATCH seems unused nowaday ?

On Fri, 2012-08-10 at 03:42 -0700, David Miller wrote:
> From: Eric Dumazet <eric.dumazet <at> 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.

Eric W. Biederman | 10 Aug 2012 16:45

Re: [RFC] NETDEV_UNREGISTER_BATCH seems unused nowaday ?

Eric Dumazet <eric.dumazet <at> gmail.com> writes:

> On Fri, 2012-08-10 at 03:42 -0700, David Miller wrote:
>> From: Eric Dumazet <eric.dumazet <at> 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?

(Continue reading)

Eric Dumazet | 10 Aug 2012 17:01

Picon

Re: [RFC] NETDEV_UNREGISTER_BATCH seems unused nowaday ?

On Fri, 2012-08-10 at 07:45 -0700, Eric W. Biederman wrote:

> Can someone help bring me up to speed.  What has changed in the
> dst ref counting that has invalidated our previous solutions?
> 

In fact your patch (850a545bd8a416484 net: Move rcu_barrier from
rollback_registered_many to netdev_run_todo.) reinstated the problem
again. You didnt notice, but other people can see the problem.

> 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 probably can keep the rcu_barrier() in netdev_run_todo() and kick the
UNREGISTER_FINAL in netdev_run_todo() too.

I'll try that.

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

(Continue reading)

Paul E. McKenney | 10 Aug 2012 19:55

Picon

Re: [RFC] NETDEV_UNREGISTER_BATCH seems unused nowaday ?

On Fri, Aug 10, 2012 at 07:45:48AM -0700, Eric W. Biederman wrote:
> Eric Dumazet <eric.dumazet <at> gmail.com> writes:
> 
> > On Fri, 2012-08-10 at 03:42 -0700, David Miller wrote:
> >> From: Eric Dumazet <eric.dumazet <at> 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

(Continue reading)

Eric Dumazet | 10 Aug 2012 16:46

Picon

[PATCH] net: remove delay at device dismantle

From: Eric Dumazet <edumazet <at> google.com>

I noticed extra one second delay in device dismantle, tracked down to
a call to dst_dev_event() while some call_rcu() are still in RCU queues.

These call_rcu() were posted by rt_free(struct rtable *rt) calls.

We then wait a little (but one second) in netdev_wait_allrefs() before
kicking again NETDEV_UNREGISTER.

As the call_rcu() are now completed, dst_dev_event() can do the needed
device swap on busy dst.

To solve this problem, add a new NETDEV_UNREGISTER_FINAL, called
in rollback_registered_many() after a rcu_barrier().

Change dst_dev_event() handler to react to NETDEV_UNREGISTER_FINAL

Also remove NETDEV_UNREGISTER_BATCH, as its not used anymore after
IP cache removal.

Signed-off-by: Eric Dumazet <edumazet <at> google.com>
Cc: Tom Herbert <therbert <at> google.com>
Cc: Mahesh Bandewar <maheshb <at> google.com>
---
 include/linux/netdevice.h |    2 +-
 net/core/dev.c            |   23 +++++++----------------
 net/core/dst.c            |    2 +-
 net/core/rtnetlink.c      |    2 +-
 net/ipv4/fib_frontend.c   |    2 --

(Continue reading)

Eric Dumazet | 10 Aug 2012 17:42

Picon

[PATCH net-next v2] net: remove delay at device dismantle

From: Eric Dumazet <edumazet <at> google.com>

I noticed extra one second delay in device dismantle, tracked down to
a call to dst_dev_event() while some call_rcu() are still in RCU queues.

These call_rcu() were posted by rt_free(struct rtable *rt) calls.

We then wait a little (but one second) in netdev_wait_allrefs() before
kicking again NETDEV_UNREGISTER.

As the call_rcu() are now completed, dst_dev_event() can do the needed
device swap on busy dst.

To solve this problem, add a new NETDEV_UNREGISTER_FINAL, called
after a rcu_barrier(), but outside of RTNL lock.

Use NETDEV_UNREGISTER_FINAL with care !

Change dst_dev_event() handler to react to NETDEV_UNREGISTER_FINAL

Also remove NETDEV_UNREGISTER_BATCH, as its not used anymore after
IP cache removal.

Signed-off-by: Eric Dumazet <edumazet <at> google.com>
Cc: Tom Herbert <therbert <at> google.com>
Cc: Mahesh Bandewar <maheshb <at> google.com>
Cc: "Eric W. Biederman" <ebiederm <at> xmission.com>
---
v2: NETDEV_UNREGISTER_FINAL called outside of rtnl lock
    as its more risky, base this patch on net-next

(Continue reading)

Eric Dumazet | 11 Aug 2012 07:54

Picon

Re: [PATCH net-next v2] net: remove delay at device dismantle

On Fri, 2012-08-10 at 17:42 +0200, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet <at> google.com>
> 
> I noticed extra one second delay in device dismantle, tracked down to
> a call to dst_dev_event() while some call_rcu() are still in RCU queues.
> 
...
> Signed-off-by: Eric Dumazet <edumazet <at> google.com>
> Cc: Tom Herbert <therbert <at> google.com>
> Cc: Mahesh Bandewar <maheshb <at> google.com>
> Cc: "Eric W. Biederman" <ebiederm <at> xmission.com>
> ---
> v2: NETDEV_UNREGISTER_FINAL called outside of rtnl lock
>     as its more risky, base this patch on net-next

Also I am leaving for a one week vacation with no access to the
Internet, so better hold this patch until my return ;)

Thanks

David Miller | 11 Aug 2012 07:57

Re: [PATCH net-next v2] net: remove delay at device dismantle

From: Eric Dumazet <eric.dumazet <at> gmail.com>
Date: Sat, 11 Aug 2012 07:54:47 +0200

> On Fri, 2012-08-10 at 17:42 +0200, Eric Dumazet wrote:
>> From: Eric Dumazet <edumazet <at> google.com>
>> 
>> I noticed extra one second delay in device dismantle, tracked down to
>> a call to dst_dev_event() while some call_rcu() are still in RCU queues.
>> 
> ...
>> Signed-off-by: Eric Dumazet <edumazet <at> google.com>
>> Cc: Tom Herbert <therbert <at> google.com>
>> Cc: Mahesh Bandewar <maheshb <at> google.com>
>> Cc: "Eric W. Biederman" <ebiederm <at> xmission.com>
>> ---
>> v2: NETDEV_UNREGISTER_FINAL called outside of rtnl lock
>>     as its more risky, base this patch on net-next
> 
> Also I am leaving for a one week vacation with no access to the
> Internet, so better hold this patch until my return ;)

Ok :)

David Miller | 23 Aug 2012 04:18

Re: [PATCH net-next v2] net: remove delay at device dismantle

From: Eric Dumazet <eric.dumazet <at> gmail.com>
Date: Sat, 11 Aug 2012 07:54:47 +0200

> On Fri, 2012-08-10 at 17:42 +0200, Eric Dumazet wrote:
>> From: Eric Dumazet <edumazet <at> google.com>
>> 
>> I noticed extra one second delay in device dismantle, tracked down to
>> a call to dst_dev_event() while some call_rcu() are still in RCU queues.
>> 
> ...
>> Signed-off-by: Eric Dumazet <edumazet <at> google.com>
>> Cc: Tom Herbert <therbert <at> google.com>
>> Cc: Mahesh Bandewar <maheshb <at> google.com>
>> Cc: "Eric W. Biederman" <ebiederm <at> xmission.com>
>> ---
>> v2: NETDEV_UNREGISTER_FINAL called outside of rtnl lock
>>     as its more risky, base this patch on net-next
> 
> Also I am leaving for a one week vacation with no access to the
> Internet, so better hold this patch until my return ;)

Since you're back, I've applied this now.

Thanks.

Gao feng | 23 Aug 2012 04:25

Re: [PATCH net-next v2] net: remove delay at device dismantle

于 2012年08月11日 13:54, Eric Dumazet 写道:
> On Fri, 2012-08-10 at 17:42 +0200, Eric Dumazet wrote:
>> From: Eric Dumazet <edumazet <at> google.com>
>>
>> I noticed extra one second delay in device dismantle, tracked down to
>> a call to dst_dev_event() while some call_rcu() are still in RCU queues.
>>
> ...
>> Signed-off-by: Eric Dumazet <edumazet <at> google.com>
>> Cc: Tom Herbert <therbert <at> google.com>
>> Cc: Mahesh Bandewar <maheshb <at> google.com>
>> Cc: "Eric W. Biederman" <ebiederm <at> xmission.com>
>> ---
>> v2: NETDEV_UNREGISTER_FINAL called outside of rtnl lock
>>     as its more risky, base this patch on net-next
> 
> Also I am leaving for a one week vacation with no access to the
> Internet, so better hold this patch until my return ;)
> 

Hi Eric

I got this warning message with your patch applied.

Aug 22 18:32:56 Donkey kernel: [  124.733048] ===============================
Aug 22 18:32:56 Donkey kernel: [  124.733051] [ INFO: suspicious RCU usage. ]
Aug 22 18:32:56 Donkey kernel: [  124.733054] 3.6.0-rc1+ #14 Not tainted
Aug 22 18:32:56 Donkey kernel: [  124.733057] -------------------------------
Aug 22 18:32:56 Donkey kernel: [  124.733060] include/linux/inetdevice.h:229 suspicious
rcu_dereference_protected() usage!

(Continue reading)

David Miller | 23 Aug 2012 04:51

Re: [PATCH net-next v2] net: remove delay at device dismantle

From: Gao feng <gaofeng <at> cn.fujitsu.com>
Date: Thu, 23 Aug 2012 10:25:00 +0800

> 于 2012年08月11日 13:54, Eric Dumazet 写道:
>> On Fri, 2012-08-10 at 17:42 +0200, Eric Dumazet wrote:
>>> From: Eric Dumazet <edumazet <at> google.com>
>>>
>>> I noticed extra one second delay in device dismantle, tracked down to
>>> a call to dst_dev_event() while some call_rcu() are still in RCU queues.
>>>
>> ...
>>> Signed-off-by: Eric Dumazet <edumazet <at> google.com>
>>> Cc: Tom Herbert <therbert <at> google.com>
>>> Cc: Mahesh Bandewar <maheshb <at> google.com>
>>> Cc: "Eric W. Biederman" <ebiederm <at> xmission.com>
>>> ---
>>> v2: NETDEV_UNREGISTER_FINAL called outside of rtnl lock
>>>     as its more risky, base this patch on net-next
>> 
>> Also I am leaving for a one week vacation with no access to the
>> Internet, so better hold this patch until my return ;)
>> 
> 
> Hi Eric
> 
> I got this warning message with your patch applied.

Ok I won't push this out until this is resolved.

Eric Dumazet | 23 Aug 2012 04:58

Picon

Re: [PATCH net-next v2] net: remove delay at device dismantle

On Wed, 2012-08-22 at 19:51 -0700, David Miller wrote:
> From: Gao feng <gaofeng <at> cn.fujitsu.com>

> > Hi Eric
> > 
> > I got this warning message with your patch applied.
> 
> Ok I won't push this out until this is resolved.

Thanks, I'll send a v2

Return

Return to gmane.linux.network.

Search Archive

Language

Change language

Options

Current view: Threads only / Showing only 20 lines / Not hiding cited text.
Change to All messages, whole messages, or hide cited text.

Post a message
NNTP Newsgroup
Classic Gmane web interface
XML

XML

RSS Feed
List Information

About Gmane