[Devel] [PATCH hci-8.1 v10 02/13] ve/fs: count available mounts instead of used ones

[Konstantin Khorenko <khorenko@virtuozzo.com>]

From: Vladimir Riabchun <vladimir.riabchun@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>
Reviewed-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 fs/namespace.c     | 78 ++++++++++++++++++++++++++++++++----------------------
 include/linux/ve.h |  2 +-
 kernel/ve/ve.c     | 12 ++++-----
 3 files changed, 54 insertions(+), 38 deletions(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index 4adc1db84b4c0..1d6e66cd71745 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -317,19 +317,22 @@ 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 inline void ve_mount_reown(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;
 	}
 
@@ -337,6 +340,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;
@@ -371,7 +382,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;
 
@@ -382,6 +394,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;
 }
@@ -751,7 +765,9 @@ 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);
+#ifdef CONFIG_VE
+	ve_mount_put(mnt, mnt->ve_owner);
+#endif
 	kfree_const(mnt->mnt_devname);
 #ifdef CONFIG_SMP
 	free_percpu(mnt->mnt_pcp);
@@ -3214,7 +3230,7 @@ int ve_devmnt_process(struct ve_struct *ve, dev_t dev, void **data_pp, int remou
 		if (devmnt->dev == dev) {
 			err = ve_devmnt_check(data, devmnt->allowed_options);
 			/*
-			 * In case of @is_pseudouser set, ie restore procedure,
+			 * In case of @is_pseudosuper set, ie restore procedure,
 			 * we don't check for allowed options filtering, since
 			 * restore mode is special.
 			 */
@@ -3353,30 +3369,30 @@ int ve_devmnt_verify(struct ve_struct *ve, dev_t dev, char *opts, bool new_mount
 	return err;
 }
 
-static inline int ve_mount_allowed(void)
-{
-	struct ve_struct *ve = get_exec_env();
-
-	return ve_is_super(ve) || ve->is_pseudosuper ||
-		atomic_read(&ve->mnt_nr) < (int)sysctl_ve_mount_nr;
-}
-
-static inline void ve_mount_nr_inc(struct mount *mnt, struct ve_struct *ve)
+static inline int ve_try_reserve_mount(struct ve_struct *ve)
 {
-	if (!ve)
-		ve = get_exec_env();
+	int ret = ve_is_super(ve) || ve->is_pseudosuper;
+	/* Ignore limits in ve0 and pseudosuper cases, but still count. */
+	if (ret)
+		atomic_dec(&ve->mnt_avail_nr);
+	else
+		ret = atomic_dec_if_positive(&ve->mnt_avail_nr) >= 0;
 
-	mnt->ve_owner = get_ve(ve);
-	atomic_inc(&ve->mnt_nr);
+	if (ret)
+		get_ve(ve);
+	return ret;
 }
 
-static inline void ve_mount_nr_dec(struct mount *mnt)
+static inline void ve_mount_put(struct mount *mnt, struct ve_struct *ve)
 {
-	struct ve_struct *ve = mnt->ve_owner;
-
-	atomic_dec(&ve->mnt_nr);
+	/*
+	 * ve argument is needed to reuse this function in alloc_vfsmnt error path.
+	 * Other users should pass mnt->ve_owner value.
+	 */
+	atomic_inc(&ve->mnt_avail_nr);
 	put_ve(ve);
-	mnt->ve_owner = NULL;
+	if (mnt)
+		mnt->ve_owner = NULL;
 }
 
 /*
@@ -3397,9 +3413,9 @@ static inline void ve_mount_reown(struct mount *mnt, struct ve_struct *ve)
 	if (old == ve)
 		return;
 
-	atomic_inc(&ve->mnt_nr);
+	atomic_dec(&ve->mnt_avail_nr);
 	WRITE_ONCE(mnt->ve_owner, get_ve(ve));
-	atomic_dec(&old->mnt_nr);
+	atomic_inc(&old->mnt_avail_nr);
 	put_ve(old);
 }
 
@@ -3422,10 +3438,10 @@ bool is_sb_ve_accessible(struct ve_struct *ve, struct super_block *sb)
 
 #else /* CONFIG_VE */
 
-static inline int ve_mount_allowed(void) { return 1; }
-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) { return 1; }
+static inline void ve_mount_put(struct mount *mnt, struct ve_struct *ve) { }
 static inline void ve_mount_reown(struct mount *mnt, struct ve_struct *ve) { }
+
 #endif /* CONFIG_VE */
 
 /*
diff --git a/include/linux/ve.h b/include/linux/ve.h
index f946abe4d8596..65c058c0f34d4 100644
--- a/include/linux/ve.h
+++ b/include/linux/ve.h
@@ -96,7 +96,7 @@ struct ve_struct {
 	atomic_t		nd_neigh_nr;
 	unsigned long		meminfo_val;
 
-	atomic_t		mnt_nr; /* number of present VE mounts */
+	atomic_t		mnt_avail_nr; /* number of available VE mounts */
 
 #ifdef CONFIG_COREDUMP
 	char			core_pattern[CORENAME_MAX_SIZE];
diff --git a/kernel/ve/ve.c b/kernel/ve/ve.c
index e3ae2e5d9a86c..bed8a58af0853 100644
--- a/kernel/ve/ve.c
+++ b/kernel/ve/ve.c
@@ -82,7 +82,7 @@ struct ve_struct ve0 = {
 
 	.arp_neigh_nr		= ATOMIC_INIT(0),
 	.nd_neigh_nr		= ATOMIC_INIT(0),
-	.mnt_nr			= ATOMIC_INIT(0),
+	.mnt_avail_nr		= ATOMIC_INIT(INT_MAX),
 	.meminfo_val		= VE_MEMINFO_SYSTEM,
 	.umh_running_helpers	= ATOMIC_INIT(0),
 	.umh_helpers_waitq	= __WAIT_QUEUE_HEAD_INITIALIZER(ve0.umh_helpers_waitq),
@@ -787,7 +787,7 @@ static struct cgroup_subsys_state *ve_create(struct cgroup_subsys_state *parent_
 
 	atomic_set(&ve->arp_neigh_nr, 0);
 	atomic_set(&ve->nd_neigh_nr, 0);
-	atomic_set(&ve->mnt_nr, 0);
+	atomic_set(&ve->mnt_avail_nr, sysctl_ve_mount_nr);
 
 #ifdef CONFIG_COREDUMP
 	strcpy(ve->core_pattern, "core");
@@ -1063,9 +1063,9 @@ static u64 ve_netns_avail_nr_read(struct cgroup_subsys_state *css, struct cftype
 	return atomic_read(&css_to_ve(css)->netns_avail_nr);
 }
 
-static u64 ve_mnt_nr_read(struct cgroup_subsys_state *css, struct cftype *cft)
+static s64 ve_mnt_avail_nr_read(struct cgroup_subsys_state *css, struct cftype *cft)
 {
-	return atomic_read(&css_to_ve(css)->mnt_nr);
+	return atomic_read(&css_to_ve(css)->mnt_avail_nr);
 }
 
 static u64 ve_netif_max_nr_read(struct cgroup_subsys_state *css, struct cftype *cft)
@@ -1627,8 +1627,8 @@ static struct cftype ve_cftypes[] = {
 		.read_u64		= ve_netns_avail_nr_read,
 	},
 	{
-		.name			= "mnt_nr",
-		.read_u64		= ve_mnt_nr_read,
+		.name			= "mnt_avail_nr",
+		.read_s64		= ve_mnt_avail_nr_read,
 	},
 	{
 		.name			= "netif_max_nr",

-- 
2.43.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel