From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kirill Tkhai Subject: Re: [PATCH v2 05/31] net: Allow pernet_operations to be executed in parallel Date: Thu, 18 Jan 2018 13:16:49 +0300 Message-ID: <667f701d-4a53-650c-1289-edc8573e4751@virtuozzo.com> References: <151120175301.3159.9577108443167812854.stgit@localhost.localdomain> <151120277590.3159.12461615068657469111.stgit@localhost.localdomain> <20180117183455.GA3189@outlook.office365.com> Mime-Version: 1.0 Content-Type: text/plain; charset=koi8-r Content-Transfer-Encoding: 7bit Cc: davem@davemloft.net, vyasevic@redhat.com, kstewart@linuxfoundation.org, pombredanne@nexb.com, vyasevich@gmail.com, mark.rutland@arm.com, gregkh@linuxfoundation.org, adobriyan@gmail.com, fw@strlen.de, nicolas.dichtel@6wind.com, xiyou.wangcong@gmail.com, roman.kapl@sysgo.com, paul@paul-moore.com, dsahern@gmail.com, daniel@iogearbox.net, lucien.xin@gmail.com, mschiffer@universe-factory.net, rshearma@brocade.com, linux-kernel@vger.kernel.org, netdev@vger.kernel.org, ebiederm@xmission.com, gorcunov@virtuozzo.com, eric.dumazet@gmail.com, stephen@networkplumber.org To: Andrei Vagin Return-path: Received: from mail-eopbgr10118.outbound.protection.outlook.com ([40.107.1.118]:8256 "EHLO EUR02-HE1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1755719AbeARKQ5 (ORCPT ); Thu, 18 Jan 2018 05:16:57 -0500 In-Reply-To: <20180117183455.GA3189@outlook.office365.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 17.01.2018 21:34, Andrei Vagin wrote: > On Mon, Nov 20, 2017 at 09:32:55PM +0300, Kirill Tkhai wrote: >> This adds new pernet_operations::async flag to indicate operations, >> which ->init(), ->exit() and ->exit_batch() methods are allowed >> to be executed in parallel with the methods of any other pernet_operations. >> >> When there are only asynchronous pernet_operations in the system, >> net_mutex won't be taken for a net construction and destruction. >> >> Also, remove BUG_ON(mutex_is_locked()) from net_assign_generic() >> without replacing with the equivalent net_sem check, as there is >> one more lockdep assert below. >> >> Suggested-by: Eric W. Biederman >> Signed-off-by: Kirill Tkhai >> --- >> include/net/net_namespace.h | 6 ++++++ >> net/core/net_namespace.c | 29 +++++++++++++++++++---------- >> 2 files changed, 25 insertions(+), 10 deletions(-) >> >> diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h >> index 10f99dafd5ac..db978c4755f7 100644 >> --- a/include/net/net_namespace.h >> +++ b/include/net/net_namespace.h >> @@ -303,6 +303,12 @@ struct pernet_operations { >> void (*exit_batch)(struct list_head *net_exit_list); >> unsigned int *id; >> size_t size; >> + /* >> + * Indicates above methods are allowe to be executed in parallel >> + * with methods of any other pernet_operations, i.e. they are not >> + * need synchronization via net_mutex. >> + */ >> + bool async; >> }; >> >> /* >> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c >> index c4f7452906bb..550c766f73aa 100644 >> --- a/net/core/net_namespace.c >> +++ b/net/core/net_namespace.c >> @@ -41,8 +41,9 @@ struct net init_net = { >> EXPORT_SYMBOL(init_net); >> >> static bool init_net_initialized; >> +static unsigned nr_sync_pernet_ops; >> /* >> - * net_sem: protects: pernet_list, net_generic_ids, >> + * net_sem: protects: pernet_list, net_generic_ids, nr_sync_pernet_ops, >> * init_net_initialized and first_device pointer. >> */ >> DECLARE_RWSEM(net_sem); >> @@ -70,11 +71,10 @@ static int net_assign_generic(struct net *net, unsigned int id, void *data) >> { >> struct net_generic *ng, *old_ng; >> >> - BUG_ON(!mutex_is_locked(&net_mutex)); >> BUG_ON(id < MIN_PERNET_OPS_ID); >> >> old_ng = rcu_dereference_protected(net->gen, >> - lockdep_is_held(&net_mutex)); >> + lockdep_is_held(&net_sem)); >> if (old_ng->s.len > id) { >> old_ng->ptr[id] = data; >> return 0; >> @@ -419,11 +419,14 @@ struct net *copy_net_ns(unsigned long flags, >> rv = down_read_killable(&net_sem); >> if (rv < 0) >> goto put_userns; >> - rv = mutex_lock_killable(&net_mutex); >> - if (rv < 0) >> - goto up_read; >> + if (nr_sync_pernet_ops) { >> + rv = mutex_lock_killable(&net_mutex); >> + if (rv < 0) >> + goto up_read; >> + } >> rv = setup_net(net, user_ns); >> - mutex_unlock(&net_mutex); >> + if (nr_sync_pernet_ops) >> + mutex_unlock(&net_mutex); >> up_read: >> up_read(&net_sem); >> if (rv < 0) { >> @@ -453,7 +456,8 @@ static void cleanup_net(struct work_struct *work) >> spin_unlock_irq(&cleanup_list_lock); >> >> down_read(&net_sem); >> - mutex_lock(&net_mutex); >> + if (nr_sync_pernet_ops) >> + mutex_lock(&net_mutex); >> >> /* Don't let anyone else find us. */ >> rtnl_lock(); >> @@ -489,7 +493,8 @@ static void cleanup_net(struct work_struct *work) >> list_for_each_entry_reverse(ops, &pernet_list, list) >> ops_exit_list(ops, &net_exit_list); >> >> - mutex_unlock(&net_mutex); >> + if (nr_sync_pernet_ops) >> + mutex_unlock(&net_mutex); >> >> /* Free the net generic variables */ >> list_for_each_entry_reverse(ops, &pernet_list, list) >> @@ -961,6 +966,9 @@ static int register_pernet_operations(struct list_head *list, >> rcu_barrier(); >> if (ops->id) >> ida_remove(&net_generic_ids, *ops->id); >> + } else if (!ops->async) { >> + pr_info_once("Pernet operations %ps are sync.\n", ops); > > As far as I understand, we have this sync mode for backward > compatibility with non-upstream modules, don't we? If the answer is yes, > it may be better to add WARN_ONCE here? There are 200+ more pernet operations requiring the review and making them async. This pr_info_once() is to help people find unconverted pernet_operations they use and start the work on converting them. Thanks, Kirill >> + nr_sync_pernet_ops++; >> } >> >> return error; >> @@ -968,7 +976,8 @@ static int register_pernet_operations(struct list_head *list, >> >> static void unregister_pernet_operations(struct pernet_operations *ops) >> { >> - >> + if (!ops->async) >> + BUG_ON(nr_sync_pernet_ops-- == 0); >> __unregister_pernet_operations(ops); >> rcu_barrier(); >> if (ops->id) >>