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 <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?
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)
>>
