Re: [Devel] [PATCH VZ10 v6 3/9] ve/fs: Rework per-ve mount count

[Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>]

> Previous approach with current mounts counter had an issue:
> there was a gap between ve_mount_allowed check and ve_mount_nr_inc,
> which could allow CT to have more mounts than expected.
> 
> Fix this by tracking the number of available mounts instead
> of current ones. This also makes resources accounting
> more consistent - we are using ***_avail_nr approach more.
> 
> One more issue with inconsistent ve value is fixed:
> ve_mount_allowed always used ve from get_exec_env, but
> ve_mount_nr_inc operated with owner_ve.
> Now actual ve value is calculated in the beginning of alloc_vfsmnt.
> 
> To avoid incorrect accounting when is_pseudosuper is changed,
> update avail_nr count without > 0 check if VE is ve0 or pseudosuper.
> This also simplifies ve_mount_put, since increment is
> now unconditional.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-135520
> 
> Feature: per-ve failcounters
> Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
>
> diff --git a/fs/namespace.c b/fs/namespace.c
> index 68e0efb73d7c..c9e2ab9b3b57 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -317,18 +317,21 @@ int mnt_get_count(struct mount *mnt)
>  #endif
>  }
>  
> -static inline int ve_mount_allowed(void);
> -static inline void ve_mount_nr_inc(struct mount *mnt, struct ve_struct *ve);
> -static inline void ve_mount_nr_dec(struct mount *mnt);
> +static inline int ve_try_reserve_mount(struct ve_struct *ve);
> +static inline void ve_mount_put(struct mount *mnt, struct ve_struct *ve);
>  
>  static struct mount *alloc_vfsmnt(const char *name, struct ve_struct *owner_ve)
>  {
>  	struct mount *mnt;
> +	struct ve_struct *ve = owner_ve;
>  
> -	if (!ve_mount_allowed()) {
> +	if (!ve)
> +		ve = get_exec_env();
> +
> +	if (!ve_try_reserve_mount(ve)) {
>  		pr_warn_ratelimited(
>  			"CT#%s reached the limit on mounts.\n",
> -			ve_name(get_exec_env()));
> +			ve_name(ve));
>  		return NULL;
>  	}
>  
> @@ -336,6 +339,14 @@ static struct mount *alloc_vfsmnt(const char *name, struct ve_struct *owner_ve)
>  	if (mnt) {
>  		int err;
>  
> +#ifdef CONFIG_VE
> +		/*
> +		 * Got ve reference in ve_try_reserve_mount, set mnt ve data
> +		 * here, so in case of error ve_mount_put sees correct info.
> +		 */
> +		mnt->ve_owner = ve;
> +#endif
> +
>  		err = mnt_alloc_id(mnt);
>  		if (err)
>  			goto out_free_cache;
> @@ -370,7 +381,8 @@ static struct mount *alloc_vfsmnt(const char *name, struct ve_struct *owner_ve)
>  		INIT_LIST_HEAD(&mnt->mnt_umounting);
>  		INIT_HLIST_HEAD(&mnt->mnt_stuck_children);
>  		mnt->mnt.mnt_idmap = &nop_mnt_idmap;
> -		ve_mount_nr_inc(mnt, owner_ve);
> +	} else {
> +		ve_mount_put(mnt, ve);
>  	}
>  	return mnt;
>  
> @@ -381,6 +393,8 @@ static struct mount *alloc_vfsmnt(const char *name, struct ve_struct *owner_ve)
>  out_free_id:
>  	mnt_free_id(mnt);
>  out_free_cache:
> +	/* Got ve reference in ve_try_reserve_mount */
> +	ve_mount_put(mnt, ve);
>  	kmem_cache_free(mnt_cache, mnt);
>  	return NULL;
>  }
> @@ -750,7 +764,7 @@ int sb_prepare_remount_readonly(struct super_block *sb)
>  static void free_vfsmnt(struct mount *mnt)
>  {
>  	mnt_idmap_put(mnt_idmap(&mnt->mnt));
> -	ve_mount_nr_dec(mnt);
> +	ve_mount_put(mnt, mnt->ve_owner);
this breaks compilation with CONFIG_VE=n. ve_onwer doesn't exist there.
Easiest solution would be to just wrap it under ifdef since the getting
stub also does nothing incase CONFIG_VE=n

-- 
Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>