[Devel] [PATCH rh7 v3 2/2] ve/fs/sync: fix CT's mountpoints traversal

[Andrey Ryabinin]

On 02/26/2016 02:10 PM, Vladimir Davydov wrote:
> On Thu, Feb 25, 2016 at 07:19:38PM +0300, Andrey Ryabinin wrote:
> ...
>> @@ -2673,7 +2676,10 @@ static struct mnt_namespace *create_mnt_ns(struct vfsmount *m)
>>  		struct mount *mnt = real_mount(m);
>>  		mnt->mnt_ns = new_ns;
>>  		new_ns->root = mnt;
>> +		down_write(&namespace_sem);
>>  		list_add(&mnt->mnt_list, &new_ns->list);
>> +		list_add(&new_ns->mntns_list, &get_exec_env()->mntns_list);
> 
> Do we really need to link such mnt_ns?
> 

I think, it shouldn't hurt at least.
Do we have a reason to not link these namespaces? 

>> +		up_write(&namespace_sem);
>>  	} else {
>>  		mntput(m);
>>  	}
>> @@ -2954,6 +2960,7 @@ void put_mnt_ns(struct mnt_namespace *ns)
>>  	namespace_lock();
>>  	br_write_lock(&vfsmount_lock);
>>  	umount_tree(ns->root, 0);
>> +	list_del(&ns->mntns_list);
> 
> Why under br_write_lock? namespace_lock should be enough.

Whether this this under br_write_lock or not doesn't matter absolutely.
It equally could be placed under lock or out of it.

>>  	br_write_unlock(&vfsmount_lock);
>>  	namespace_unlock();
>>  	free_mnt_ns(ns);
> ...
>> @@ -80,6 +80,7 @@ struct ve_struct ve0 = {
>>  	.sched_lat_ve.cur	= &ve0_lat_stats,
>>  	.init_cred		= &init_cred,
>>  	.mnt_nr			= 0,
>> +	.mntns_list		= LIST_HEAD_INIT(ve0.mntns_list),
> 
> It is initialized in ve_create below. Why is it not enough?
> 

create_mnt_ns() called too early, before ve_create().

>>  };
>>  EXPORT_SYMBOL(ve0);
>>  
>> @@ -652,6 +653,7 @@ do_init:
>>  	INIT_LIST_HEAD(&ve->devices);
>>  	INIT_LIST_HEAD(&ve->ve_list);
>>  	INIT_LIST_HEAD(&ve->devmnt_list);
>> +	INIT_LIST_HEAD(&ve->mntns_list);
>>  	mutex_init(&ve->devmnt_mutex);
>>  	kmapset_init_key(&ve->ve_sysfs_perms);
>>  
>> @@ -706,6 +708,8 @@ static void ve_destroy(struct cgroup *cg)
>>  	kmapset_unlink(&ve->ve_sysfs_perms, &ve_sysfs_perms);
>>  	free_ve_devmnts(ve);
>>  
>> +	WARN_ON(!list_empty(&ve->mntns_list));
>> +
>>  	ve_log_destroy(ve);
>>  #if IS_ENABLED(CONFIG_BINFMT_MISC)
>>  	kfree(ve->binfmt_misc);