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 <ebied...@xmission.com>
> Signed-off-by: Kirill Tkhai <ktk...@virtuozzo.com>
> ---
> 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)
>
