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

[Kirill Tkhai]

В Вс, 21/06/2015 в 19:48 +0300, Vladimir Davydov пишет:
> 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>

Reviewed-by: Kirill Tkhai <ktkhai at odin.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)