From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrei Vagin Subject: Re: [PATCH v2 05/31] net: Allow pernet_operations to be executed in parallel Date: Wed, 17 Jan 2018 10:34:57 -0800 Message-ID: <20180117183455.GA3189@outlook.office365.com> References: <151120175301.3159.9577108443167812854.stgit@localhost.localdomain> <151120277590.3159.12461615068657469111.stgit@localhost.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset=koi8-r 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: Kirill Tkhai Return-path: Received: from mail-eopbgr30128.outbound.protection.outlook.com ([40.107.3.128]:65416 "EHLO EUR03-AM5-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1754066AbeAQSfT (ORCPT ); Wed, 17 Jan 2018 13:35:19 -0500 Content-Disposition: inline In-Reply-To: <151120277590.3159.12461615068657469111.stgit@localhost.localdomain> Sender: netdev-owner@vger.kernel.org List-ID: 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? > + 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) >