[Devel] [PATCH rh7] Revert "ve/netns: wait for sub net namespaces to exit on ve cleanup"

Vladimir Davydov vdavydov at parallels.com
Sun Jun 21 09:48:38 PDT 2015


This reverts commit da13b22809fc80f5493441b141f4979dedc74fb2.

In RH6 we have a rather sophisticated VE-vs-netns destruction design: on
fini_ve_netns we setup sysfs_completion, then put ve_netns, which is
supposed to complete the completion, and then wait for the completion.
This is needed, because net ns has a reference to ve and therefore it
must die before ve. With sub net namespaces introduced this design gets
even trickier: now we want all sub net namespaces to die before ve. To
achieve that, we made each sub net ns take a reference to the ve_netns -
this is what the patch I'm reverting did.

In RH7 we don't need this crap, because each net ns takes a reference to
the owner ve and therefore ve cannot pass away before ve_netns or any
sub net ns.

Moreover, the original patch contains a bug: it makes net_free, which is
called from cleanup_net -> net_drop_ns, dereference net->owner_ve, the
reference to which is dropped earlier in cleanup_net:

  general protection fault: 0000 [#1] SMP
  CPU: 0 PID: 343 Comm: kworker/u64:7 ve: 0 Not tainted 3.10.0+ #192 ovz.5.15-13-gfd82c5e4b758
  Workqueue: netns cleanup_net
  task: ffff8800aa030000 ti: ffff8800aab3a000 task.ti: ffff8800aab3a000
  RIP: 0010:[<ffffffff814d9626>]  [<ffffffff814d9626>] net_drop_ns+0x56/0x70
  RSP: 0018:ffff8800aab3bd20  EFLAGS: 00010202
  RAX: 0000000000000000 RBX: ffff8800a8d80000 RCX: 0000000000000000
  RDX: 000000000000000e RSI: 0000000000000000 RDI: 0000000000000296
  RBP: ffff8800aab3bd30 R08: 0000000000000000 R09: 0000000100050000
  R10: 0000000000000000 R11: 0000000000000000 R12: 6b6b6b6b6b6b6b6b
  R13: ffff8800a8d80000 R14: ffff8800aab3bcf0 R15: 0000000000000800
  FS:  0000000000000000(0000) GS:ffff88010be00000(0000) knlGS:0000000000000000
  CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
  CR2: 00007f0a79a1a000 CR3: 0000000000024000 CR4: 00000000000006f0
  DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
  DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
  Stack:
   ffff8800aab3bd50 ffff8800aab3bd50 ffff8800aab3bd88 ffffffff814d984e
   ffff8800a8d80050 ffff8800a8d80050 ffff8800aab3bd50 ffff8800aab3bd50
   00000000d74acfe9 ffffffff819f3300 ffff8800aaa1fac8 ffff88010b882520
  Call Trace:
   [<ffffffff814d984e>] cleanup_net+0x20e/0x240
   [<ffffffff8108256a>] process_one_work+0x1ea/0x580
   [<ffffffff81082508>] ? process_one_work+0x188/0x580
   [<ffffffff810834fb>] worker_thread+0x11b/0x3a0
   [<ffffffff810833e0>] ? manage_workers.isra.23+0x2a0/0x2a0
   [<ffffffff8108a38a>] kthread+0xea/0xf0
   [<ffffffff8108a2a0>] ? create_kthread+0x60/0x60
   [<ffffffff8160edec>] ret_from_fork+0x7c/0xb0
   [<ffffffff8108a2a0>] ? create_kthread+0x60/0x60

In fact, it's my fault: I didn't notice all of these at a quick glance
at this patch and let it trickle through.

Signed-off-by: Vladimir Davydov <vdavydov at parallels.com>
---
 net/core/net_namespace.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index 9e73cc47089b..3f7da6adbe0b 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -165,8 +165,6 @@ static __net_init int setup_net(struct net *net, struct user_namespace *user_ns)
 
 #ifdef CONFIG_VE
 	net->owner_ve = get_ve(get_exec_env());
-	if (net->owner_ve->ve_netns)
-		get_net(net->owner_ve->ve_netns);
 #endif
 
 	atomic_set(&net->count, 1);
@@ -235,7 +233,6 @@ out_free:
 
 static void net_free(struct net *net)
 {
-	struct net *ve_netns = net->owner_ve->ve_netns;
 #ifdef NETNS_REFCNT_DEBUG
 	if (unlikely(atomic_read(&net->use_count) != 0)) {
 		pr_emerg("network namespace not free! Usage: %d\n",
@@ -245,9 +242,6 @@ static void net_free(struct net *net)
 #endif
 	kfree(net->gen);
 	kmem_cache_free(net_cachep, net);
-
-	if (ve_netns)
-		put_net(ve_netns);
 }
 
 void net_drop_ns(void *p)
-- 
2.1.4




More information about the Devel mailing list