[Devel] [PATCH VZ10 v9 0/8] Add per-VE failcount support

[Devel] [PATCH VZ10 v9 0/8] Add per-VE failcount support

[Vladimir Riabchun]

This patchset introduces per-VE failcount support.
We have several resources, that have per-VE limits,
the users might want to know what limit hits make
operations fail.

- Patch 1 is cosmetic, nothing special.
- Patches 2-4 change mount number and limit processing,
  unifying it with other resources. VE selftest is updated
  in patch 3.
- Patch 5 greatly reduces the amount of copy-pasted code
  by introducing some useful macros.
- Patch 6 adds per-VE failcount file.
- Patch 7 fixes kernel log leak to containers from interrupts.
- Patch 8 introduces new VE selftest, that checks mounts
  accounting, limits and failcount updates.

NOTE: for running new selftest this change should be applied on
top of "[PATCH VZ10 v2 0/3] Improvements for ve selftests" patchset.

https://virtuozzo.atlassian.net/browse/VSTOR-135520

Changes since v8:
 - Added new patch 7 to fix logs leakage from interrupts.
   We are fine if we miss some failure logs in container
   dmesg - CT can check failcounts anyway, and bringing
   new ratelimited printks with specified VE is too hard
   for such a small thing.
 - Selftest in patch 8 got major rework:
   1. Correct mmap error handling.
   2. Dropped mnt_cost value. Let's force 1, it is expected
      value in fresh mount+VE namespaces.
   3. Proper solution for pending mount destructions. It used
      to be just sleep(1) which may not be enoght on loaded
      machines. Instead, issue rcu_barrier and wait on mnt_avail_nr.
      Since we forced mnt_cost == 1, it know precisely what the
      new value should be.

Vladimir Riabchun (8):
  ve/ve.{h,c}: Farewell to spaces as indents
  ve/fs: Rework per-ve mount count
  selftests/ve: Update ve_ns_owner_test
  ve: Move from global VE mounts limit to per-VE limit
  ve/ve.c: Generate VE resource accessors using macros
  ve: Introduce per-VE failcount
  ve/printk: Don't use ve log outside task context
  selftests/ve: Add mount accounting selftest

 fs/aio.c                                      |   1 +
 fs/namespace.c                                |  80 ++-
 include/linux/ve.h                            |  35 +-
 kernel/bpf/syscall.c                          |   1 +
 kernel/printk/printk.c                        |   7 +-
 kernel/ve/ve.c                                | 277 ++++-----
 kernel/ve/veowner.c                           |  19 -
 net/core/dev.c                                |  10 +-
 net/core/neighbour.c                          |   3 +-
 net/core/net_namespace.c                      |   4 +-
 tools/testing/selftests/ve/.gitignore         |   1 +
 tools/testing/selftests/ve/Makefile           |   1 +
 .../selftests/ve/ve_mount_accounting_test.c   | 550 ++++++++++++++++++
 tools/testing/selftests/ve/ve_ns_owner_test.c |  37 +-
 tools/testing/selftests/ve/ve_selftest.h      |  21 +
 15 files changed, 822 insertions(+), 225 deletions(-)
 create mode 100644 tools/testing/selftests/ve/ve_mount_accounting_test.c

-- 
2.47.1

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

[Devel] [PATCH VZ10 v9 1/8] ve/ve.{h, c}: Farewell to spaces as indents

[Vladimir Riabchun]

Fix formatting while here.

Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
---
 include/linux/ve.h |  8 ++++----
 kernel/ve/ve.c     | 48 +++++++++++++++++++++++-----------------------
 2 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/include/linux/ve.h b/include/linux/ve.h
index 2dc93e42784b..f946abe4d859 100644
--- a/include/linux/ve.h
+++ b/include/linux/ve.h
@@ -71,7 +71,7 @@ struct ve_struct {
 	void			*log_state;
 #define VE_LOG_BUF_LEN		4096
 
-	struct kstat_lat_pcpu_struct    sched_lat_ve;
+	struct kstat_lat_pcpu_struct	sched_lat_ve;
 	int			odirect_enable;
 
 	int			fsync_enable;
@@ -108,8 +108,8 @@ struct ve_struct {
 	struct kthread_worker	*kthreadd_worker;
 	struct task_struct	*kthreadd_task;
 
-	atomic_t                umh_running_helpers;
-	struct wait_queue_head  umh_helpers_waitq;
+	atomic_t		umh_running_helpers;
+	struct wait_queue_head	umh_helpers_waitq;
 	struct kthread_worker	umh_worker;
 	struct task_struct	*umh_task;
 
@@ -130,7 +130,7 @@ struct ve_struct {
 struct ve_devmnt {
 	struct list_head	link;
 
-	dev_t                   dev;
+	dev_t			dev;
 	char			*allowed_options;
 	char			*hidden_options; /* balloon_ino, etc. */
 };
diff --git a/kernel/ve/ve.c b/kernel/ve/ve.c
index b2788b80e765..2d2934e1ec10 100644
--- a/kernel/ve/ve.c
+++ b/kernel/ve/ve.c
@@ -84,9 +84,9 @@ struct ve_struct ve0 = {
 	.nd_neigh_nr		= ATOMIC_INIT(0),
 	.mnt_nr			= ATOMIC_INIT(0),
 	.meminfo_val		= VE_MEMINFO_SYSTEM,
-	.umh_running_helpers    = ATOMIC_INIT(0),
-	.umh_helpers_waitq      = __WAIT_QUEUE_HEAD_INITIALIZER(ve0.umh_helpers_waitq),
 #ifdef CONFIG_X86
+	.umh_running_helpers	= ATOMIC_INIT(0),
+	.umh_helpers_waitq	= __WAIT_QUEUE_HEAD_INITIALIZER(ve0.umh_helpers_waitq),
 	.vdso_64		= (struct vdso_image*)&vdso_image_64,
 	.vdso_32		= (struct vdso_image*)&vdso_image_32,
 #endif
@@ -289,7 +289,7 @@ EXPORT_SYMBOL(is_ve_init_net);
 
 int nr_threads_ve(struct ve_struct *ve)
 {
-        return cgroup_task_count(ve->css.cgroup);
+	return cgroup_task_count(ve->css.cgroup);
 }
 EXPORT_SYMBOL(nr_threads_ve);
 
@@ -330,8 +330,8 @@ static void ve_drop_context(struct ve_struct *ve)
 	ve_nsproxy = rcu_dereference_protected(ve->ve_nsproxy, lockdep_is_held(&ve->op_sem));
 
 	/* Allows to dereference init_cred and init_task if ve_nsproxy is set */
-        rcu_assign_pointer(ve->ve_nsproxy, NULL);
-        synchronize_rcu();
+	rcu_assign_pointer(ve->ve_nsproxy, NULL);
+	synchronize_rcu();
 	put_nsproxy(ve_nsproxy);
 
 	put_cred(ve->init_cred);
@@ -895,23 +895,23 @@ static int ve_state_show(struct seq_file *sf, void *v)
 	struct ve_struct *ve = css_to_ve(css);
 
 	down_read(&ve->op_sem);
-        switch (ve->state) {
-                case VE_STATE_STARTING:
-                        seq_puts(sf, "STARTING");
-                        break;
-                case VE_STATE_RUNNING:
-                        seq_puts(sf, "RUNNING");
-                        break;
-                case VE_STATE_STOPPING:
-                        seq_puts(sf, "STOPPING");
-                        break;
-                case VE_STATE_STOPPED:
-                case VE_STATE_DEAD:
-                        seq_puts(sf, "STOPPED");
-                        break;
-                default:
-                        seq_puts(sf, "UNKNOWN");
-        }
+	switch (ve->state) {
+	case VE_STATE_STARTING:
+		seq_puts(sf, "STARTING");
+		break;
+	case VE_STATE_RUNNING:
+		seq_puts(sf, "RUNNING");
+		break;
+	case VE_STATE_STOPPING:
+		seq_puts(sf, "STOPPING");
+		break;
+	case VE_STATE_STOPPED:
+	case VE_STATE_DEAD:
+		seq_puts(sf, "STOPPED");
+		break;
+	default:
+		seq_puts(sf, "UNKNOWN");
+	}
 	seq_putc(sf, '\n');
 	up_read(&ve->op_sem);
 
@@ -923,7 +923,7 @@ static ssize_t ve_state_write(struct kernfs_open_file *of, char *buf,
 
 {
 	struct cgroup_subsys_state *css = of_css(of);
-        struct ve_struct *ve = css_to_ve(css);
+	struct ve_struct *ve = css_to_ve(css);
 	int ret = -EINVAL;
 
 	if (!strcmp(buf, "START")) {
@@ -1354,7 +1354,7 @@ static int ve_parse_mount_options(const char *ptr, const char *endp,
 			goto next;
 		}
 
-	        options = kmalloc(options_size, GFP_KERNEL);
+		options = kmalloc(options_size, GFP_KERNEL);
 		if (!options)
 			return -ENOMEM;
 
-- 
2.47.1

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

Re: [Devel] [PATCH VZ10 v9 1/8] ve/ve.{h, c}: Farewell to spaces as indents

[Konstantin Khorenko]

On 9/15/26 19:22, Vladimir Riabchun wrote:
> +++ b/kernel/ve/ve.c
> @@ -84,9 +84,9 @@ struct ve_struct ve0 = {
>  	.nd_neigh_nr		= ATOMIC_INIT(0),
>  	.mnt_nr			= ATOMIC_INIT(0),
>  	.meminfo_val		= VE_MEMINFO_SYSTEM,
> -	.umh_running_helpers    = ATOMIC_INIT(0),
> -	.umh_helpers_waitq      = __WAIT_QUEUE_HEAD_INITIALIZER(ve0.umh_helpers_waitq),
>  #ifdef CONFIG_X86
> +	.umh_running_helpers	= ATOMIC_INIT(0),
> +	.umh_helpers_waitq	= __WAIT_QUEUE_HEAD_INITIALIZER(ve0.umh_helpers_waitq),
>  	.vdso_64		= (struct vdso_image*)&vdso_image_64,
>  	.vdso_32		= (struct vdso_image*)&vdso_image_32,
>  #endif
> @@ -289,7 +289,7 @@ EXPORT_SYMBOL(is_ve_init_net);
>  

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

[Devel] [PATCH VZ10 v9 2/8] ve/fs: Rework per-ve mount count

[Vladimir Riabchun]

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>
---
 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 4adc1db84b4c..1d6e66cd7174 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 f946abe4d859..65c058c0f34d 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 2d2934e1ec10..58a0c4b8e36e 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,
 #ifdef CONFIG_X86
 	.umh_running_helpers	= ATOMIC_INIT(0),
@@ -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.47.1

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

Re: [Devel] [PATCH VZ10 v9 2/8] ve/fs: Rework per-ve mount count

[Vladimir Riabchun]

Sashiko review:

On 9/15/26 19:22, Vladimir Riabchun wrote:
> 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>
> ---
>   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 4adc1db84b4c..1d6e66cd7174 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;

Sashiko says:
  In the function alloc_vfsmnt, can the decrement in ve_try_reserve_mount be
  invalid if the 'mnt_alloc_id' fails subsequently?
  This logic might lead to an incorrect mount count when the ve or the mount fails.

No, it can't. Passing ve_try_reserve_mount means mnt_avail was updated, so on
error path we must unconditionally increment it back.

>   
> @@ -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 */

Sashiko says:
  The comment suggests that ve_mount_put sees a non-null mount in an error case,
  however, in test scenarios, isn't it possible for the ve to be incorrect?

No. ve is always correct, mnt = NULL is handled correctly by ve_mount_put.

> +	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;

Sashiko says:
  Is there a chance that, in ve_try_reserve_mount, the return logic misleads the condition
  when ve is either ve0 or pseudosuper, thus bypassing necessary checks?
  This counting might allow unexpected mounts while avoiding constraints.

We want to ignore avail checks in ve0 and pseudosuper case.

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

Sashio says:
  The unconditional increment in ve_mount_put may cause unintended changes in mnt_avail_nr
  when ve conditions are different. Could this result in inaccurate resource tracking?

No, ve_mount_put is called only when ve_try_reserve_mount is 1. It means it decremented
avail counter.

>   	put_ve(ve);
> -	mnt->ve_owner = NULL;
> +	if (mnt)
> +		mnt->ve_owner = NULL;
>   }
>   
>   ...

-- 
Best regards, Riabchun Vladimir
Linux Kernel Developer, Virtuozzo

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

[Devel] [PATCH VZ10 v9 3/8] selftests/ve: Update ve_ns_owner_test

[Vladimir Riabchun]

Mount accounting interface has changed, now it shows the number
of available mounts.

https://virtuozzo.atlassian.net/browse/VSTOR-135520

Feature: per-ve failcounters
Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
---
 tools/testing/selftests/ve/ve_ns_owner_test.c | 33 ++++++++++---------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/tools/testing/selftests/ve/ve_ns_owner_test.c b/tools/testing/selftests/ve/ve_ns_owner_test.c
index 82c31dff4b1b..c3fa7d7b7f9c 100644
--- a/tools/testing/selftests/ve/ve_ns_owner_test.c
+++ b/tools/testing/selftests/ve/ve_ns_owner_test.c
@@ -18,7 +18,7 @@
  * with a new netns and mntns, the parent reads the new ve's counters
  * via cgroupfs and asserts they reflect the just-created namespaces:
  *   - ve.netns_avail_nr drops by exactly one (the new netns);
- *   - ve.mnt_nr is strictly greater than zero (mounts copied into the
+ *   - ve.mnt_avail_nr drops by some value >= 1 (mounts copied into the
  *     new mntns are accounted to the new ve).
  *
  * We never assert against the parent ve's counters: those are shared
@@ -50,6 +50,7 @@
  * any spurious accounting against the parent ve would overflow it.
  */
 #define VE_NETNS_MAX		3
+#define VE_MOUNTS_MAX		4096
 
 /*
  * Synchronisation across the clone() boundary: child does its setup,
@@ -106,7 +107,7 @@ static int clone_child_func(void *arg)
 /*
  * Before fix:
  *   - clone path: ve.netns_avail_nr stays at VE_NETNS_MAX and
- *     ve.mnt_nr stays at 0 because copy_net_ns()/copy_mnt_ns()
+ *     ve.mnt_avail_nr stays at VE_MOUNTS_MAX because copy_net_ns()/copy_mnt_ns()
  *     charged the parent ve via get_exec_env().
  *   - unshare path: the syscall itself returned -EINVAL, so this
  *     check was unreachable.
@@ -117,16 +118,16 @@ static int clone_child_func(void *arg)
 static void check_new_ve_owner(struct __test_metadata *_metadata,
 			       int cgv2_fd, int ctid)
 {
-	unsigned long long avail, mnt;
-	char path[PATH_MAX];
+	unsigned long long avail_netns, avail_mnt;
+	char path[64];
 
 	snprintf(path, sizeof(path), "%d/ve.netns_avail_nr", ctid);
-	ASSERT_EQ(read_u64_at(cgv2_fd, path, &avail), 0);
-	EXPECT_EQ(avail, VE_NETNS_MAX - 1);
+	ASSERT_EQ(read_u64_at(cgv2_fd, path, &avail_netns), 0);
+	EXPECT_EQ(avail_netns, VE_NETNS_MAX - 1);
 
-	snprintf(path, sizeof(path), "%d/ve.mnt_nr", ctid);
-	ASSERT_EQ(read_u64_at(cgv2_fd, path, &mnt), 0);
-	EXPECT_GT(mnt, 0);
+	snprintf(path, sizeof(path), "%d/ve.mnt_avail_nr", ctid);
+	ASSERT_EQ(read_u64_at(cgv2_fd, path, &avail_mnt), 0);
+	EXPECT_LT(avail_mnt, VE_MOUNTS_MAX);
 }
 
 FIXTURE(ve_ns_owner)
@@ -137,7 +138,7 @@ FIXTURE(ve_ns_owner)
 
 FIXTURE_SETUP(ve_ns_owner)
 {
-	unsigned long long initial_mnt_nr;
+	unsigned long long initial_mnt_avail_nr;
 	char val[16];
 	char path[PATH_MAX];
 
@@ -174,13 +175,13 @@ FIXTURE_SETUP(ve_ns_owner)
 
 	/*
 	 * The new ve cgroup has not been entered by anything yet, so its
-	 * mnt_nr counter must start at 0. Each test below verifies that
-	 * the clone/unshare populates the new mntns under this ve, i.e.
-	 * mnt_nr rises strictly above zero.
+	 * mnt_avail_nr counter should be VE_MOUNTS_MAX. Each test below
+	 * verifies that the clone/unshare populates the new mntns
+	 * under this ve, i.e. mnt_avail_nr strictly decreases.
 	 */
-	snprintf(path, sizeof(path), "%d/ve.mnt_nr", self->ctid);
-	ASSERT_EQ(read_u64_at(self->cgv2_fd, path, &initial_mnt_nr), 0);
-	ASSERT_EQ(initial_mnt_nr, 0);
+	snprintf(path, sizeof(path), "%d/ve.mnt_avail_nr", self->ctid);
+	ASSERT_EQ(read_u64_at(self->cgv2_fd, path, &initial_mnt_avail_nr), 0);
+	ASSERT_EQ(initial_mnt_avail_nr, VE_MOUNTS_MAX);
 };
 
 FIXTURE_TEARDOWN(ve_ns_owner)
-- 
2.47.1

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

Re: [Devel] [PATCH VZ10 v9 3/8] selftests/ve: Update ve_ns_owner_test

[Vladimir Riabchun]

Sashiko review:

On 9/15/26 19:22, Vladimir Riabchun wrote:
> Mount accounting interface has changed, now it shows the number
> of available mounts.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-135520
> 
> Feature: per-ve failcounters
> Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
> ---
>   tools/testing/selftests/ve/ve_ns_owner_test.c | 33 ++++++++++---------
>   1 file changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/tools/testing/selftests/ve/ve_ns_owner_test.c b/tools/testing/selftests/ve/ve_ns_owner_test.c
> index 82c31dff4b1b..c3fa7d7b7f9c 100644
> --- a/tools/testing/selftests/ve/ve_ns_owner_test.c
> +++ b/tools/testing/selftests/ve/ve_ns_owner_test.c
> @@ -18,7 +18,7 @@
>    * with a new netns and mntns, the parent reads the new ve's counters
>    * via cgroupfs and asserts they reflect the just-created namespaces:
>    *   - ve.netns_avail_nr drops by exactly one (the new netns);
> - *   - ve.mnt_nr is strictly greater than zero (mounts copied into the
> + *   - ve.mnt_avail_nr drops by some value >= 1 (mounts copied into the
>    *     new mntns are accounted to the new ve).
>    *
>    * We never assert against the parent ve's counters: those are shared
> @@ -50,6 +50,7 @@
>    * any spurious accounting against the parent ve would overflow it.
>    */
>   #define VE_NETNS_MAX		3
> +#define VE_MOUNTS_MAX		4096
>   
>   /*
>    * Synchronisation across the clone() boundary: child does its setup,
> @@ -106,7 +107,7 @@ static int clone_child_func(void *arg)
>   /*
>    * Before fix:
>    *   - clone path: ve.netns_avail_nr stays at VE_NETNS_MAX and
> - *     ve.mnt_nr stays at 0 because copy_net_ns()/copy_mnt_ns()
> + *     ve.mnt_avail_nr stays at VE_MOUNTS_MAX because copy_net_ns()/copy_mnt_ns()
>    *     charged the parent ve via get_exec_env().
>    *   - unshare path: the syscall itself returned -EINVAL, so this
>    *     check was unreachable.
> @@ -117,16 +118,16 @@ static int clone_child_func(void *arg)
>   static void check_new_ve_owner(struct __test_metadata *_metadata,
>   			       int cgv2_fd, int ctid)
>   {
> -	unsigned long long avail, mnt;
> -	char path[PATH_MAX];
> +	unsigned long long avail_netns, avail_mnt;
> +	char path[64];
>   
>   	snprintf(path, sizeof(path), "%d/ve.netns_avail_nr", ctid);
> -	ASSERT_EQ(read_u64_at(cgv2_fd, path, &avail), 0);
> -	EXPECT_EQ(avail, VE_NETNS_MAX - 1);
> +	ASSERT_EQ(read_u64_at(cgv2_fd, path, &avail_netns), 0);
> +	EXPECT_EQ(avail_netns, VE_NETNS_MAX - 1);
>   
> -	snprintf(path, sizeof(path), "%d/ve.mnt_nr", ctid);
> -	ASSERT_EQ(read_u64_at(cgv2_fd, path, &mnt), 0);
> -	EXPECT_GT(mnt, 0);
> +	snprintf(path, sizeof(path), "%d/ve.mnt_avail_nr", ctid);
> +	ASSERT_EQ(read_u64_at(cgv2_fd, path, &avail_mnt), 0);
> +	EXPECT_LT(avail_mnt, VE_MOUNTS_MAX);

Sashiko says:
  This is a pre-existing issue, but the logic in check_new_ve_owner to verify that
  've.mnt_avail_nr' strictly decreases seems unsound, potentially causing
  intermittent test failures.
  Could the test environment allow for external factors that might not ensure
  this decrement happens logically?
  Is there a possibility that concurrent operations could influence the
  've.mnt_avail_nr' in a way that disrupts this test's reliability?

No, this is our namespace and nobody should touch it. If value is different, a bug
is somewhere.

>   }
>   
>   FIXTURE(ve_ns_owner)
> @@ -137,7 +138,7 @@ FIXTURE(ve_ns_owner)
>   
>   FIXTURE_SETUP(ve_ns_owner)
>   {
> -	unsigned long long initial_mnt_nr;
> +	unsigned long long initial_mnt_avail_nr;
>   	char val[16];
>   	char path[PATH_MAX];
>   
> @@ -174,13 +175,13 @@ FIXTURE_SETUP(ve_ns_owner)
>   
>   	/*
>   	 * The new ve cgroup has not been entered by anything yet, so its
> -	 * mnt_nr counter must start at 0. Each test below verifies that
> -	 * the clone/unshare populates the new mntns under this ve, i.e.
> -	 * mnt_nr rises strictly above zero.
> +	 * mnt_avail_nr counter should be VE_MOUNTS_MAX. Each test below
> +	 * verifies that the clone/unshare populates the new mntns
> +	 * under this ve, i.e. mnt_avail_nr strictly decreases.
>   	 */
> -	snprintf(path, sizeof(path), "%d/ve.mnt_nr", self->ctid);
> -	ASSERT_EQ(read_u64_at(self->cgv2_fd, path, &initial_mnt_nr), 0);
> -	ASSERT_EQ(initial_mnt_nr, 0);
> +	snprintf(path, sizeof(path), "%d/ve.mnt_avail_nr", self->ctid);
> +	ASSERT_EQ(read_u64_at(self->cgv2_fd, path, &initial_mnt_avail_nr), 0);
> +	ASSERT_EQ(initial_mnt_avail_nr, VE_MOUNTS_MAX);
>   };
>   
>   FIXTURE_TEARDOWN(ve_ns_owner)

-- 
Best regards, Riabchun Vladimir
Linux Kernel Developer, Virtuozzo

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

[Devel] [PATCH VZ10 v9 4/8] ve: Move from global VE mounts limit to per-VE limit

[Vladimir Riabchun]

This change unifies mount limit handling with other VE limits.
This will simplify refactoring later and allow more precise limits
control from one place.

https://virtuozzo.atlassian.net/browse/VSTOR-135520

Feature: per-ve failcounters
Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
---
 include/linux/ve.h                            |  9 +++--
 kernel/ve/ve.c                                | 34 ++++++++++++++++++-
 kernel/ve/veowner.c                           | 19 -----------
 tools/testing/selftests/ve/ve_ns_owner_test.c |  6 +++-
 4 files changed, 45 insertions(+), 23 deletions(-)

diff --git a/include/linux/ve.h b/include/linux/ve.h
index 65c058c0f34d..fabc608e31f5 100644
--- a/include/linux/ve.h
+++ b/include/linux/ve.h
@@ -96,7 +96,13 @@ struct ve_struct {
 	atomic_t		nd_neigh_nr;
 	unsigned long		meminfo_val;
 
+	/*
+	 * Operations with a big amount of mount points can require a lot of time.
+	 * These operations take the global lock namespace_sem, so they can affect
+	 * other containers.
+	 */
 	atomic_t		mnt_avail_nr; /* number of available VE mounts */
+	int			mnt_max_nr;
 
 #ifdef CONFIG_COREDUMP
 	char			core_pattern[CORENAME_MAX_SIZE];
@@ -143,8 +149,7 @@ extern int nr_ve;
 #define NETNS_MAX_NR_DEFAULT	256	/* number of net-namespaces per-VE */
 #define NETIF_MAX_NR_DEFAULT	256	/* number of net-interfaces per-VE */
 #define BPF_PROG_MAX_NR_DEFAULT	256	/* number of loaded BPF progs per-VE */
-
-extern unsigned int sysctl_ve_mount_nr;
+#define MNT_MAX_NR_DEFAULT	4096	/* number of mounts per-VE */
 
 #ifdef CONFIG_VE
 static inline void ve_set_state(struct ve_struct *ve, int new_state)
diff --git a/kernel/ve/ve.c b/kernel/ve/ve.c
index 58a0c4b8e36e..a33e55991e52 100644
--- a/kernel/ve/ve.c
+++ b/kernel/ve/ve.c
@@ -83,6 +83,7 @@ struct ve_struct ve0 = {
 	.arp_neigh_nr		= ATOMIC_INIT(0),
 	.nd_neigh_nr		= ATOMIC_INIT(0),
 	.mnt_avail_nr		= ATOMIC_INIT(INT_MAX),
+	.mnt_max_nr		= INT_MAX,
 	.meminfo_val		= VE_MEMINFO_SYSTEM,
 #ifdef CONFIG_X86
 	.umh_running_helpers	= ATOMIC_INIT(0),
@@ -787,7 +788,8 @@ 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_avail_nr, sysctl_ve_mount_nr);
+	ve->mnt_max_nr = MNT_MAX_NR_DEFAULT;
+	atomic_set(&ve->mnt_avail_nr, MNT_MAX_NR_DEFAULT);
 
 #ifdef CONFIG_COREDUMP
 	strcpy(ve->core_pattern, "core");
@@ -1068,6 +1070,30 @@ static s64 ve_mnt_avail_nr_read(struct cgroup_subsys_state *css, struct cftype *
 	return atomic_read(&css_to_ve(css)->mnt_avail_nr);
 }
 
+static u64 ve_mnt_max_nr_read(struct cgroup_subsys_state *css, struct cftype *cft)
+{
+	return css_to_ve(css)->mnt_max_nr;
+}
+
+static int ve_mnt_max_nr_write(struct cgroup_subsys_state *css, struct cftype *cft, u64 val)
+{
+	struct ve_struct *ve = css_to_ve(css);
+	int delta;
+
+	if (!ve_is_super(get_exec_env()))
+		return -EPERM;
+
+	if (val > INT_MAX)
+		return -EOVERFLOW;
+
+	down_write(&ve->op_sem);
+	delta = val - ve->mnt_max_nr;
+	ve->mnt_max_nr = val;
+	atomic_add(delta, &ve->mnt_avail_nr);
+	up_write(&ve->op_sem);
+	return 0;
+}
+
 static u64 ve_netif_max_nr_read(struct cgroup_subsys_state *css, struct cftype *cft)
 {
 	return css_to_ve(css)->netif_max_nr;
@@ -1630,6 +1656,12 @@ static struct cftype ve_cftypes[] = {
 		.name			= "mnt_avail_nr",
 		.read_s64		= ve_mnt_avail_nr_read,
 	},
+	{
+		.name			= "mnt_max_nr",
+		.flags			= CFTYPE_NOT_ON_ROOT,
+		.read_u64		= ve_mnt_max_nr_read,
+		.write_u64		= ve_mnt_max_nr_write,
+	},
 	{
 		.name			= "netif_max_nr",
 		.flags			= CFTYPE_NOT_ON_ROOT,
diff --git a/kernel/ve/veowner.c b/kernel/ve/veowner.c
index 1e631baf0de9..c77735f8c035 100644
--- a/kernel/ve/veowner.c
+++ b/kernel/ve/veowner.c
@@ -48,26 +48,7 @@ static void prepare_proc(void)
  * ------------------------------------------------------------------------
  */
 
-/*
- * Operations with a big amount of mount points can require a lot of time.
- * These operations take the global lock namespace_sem, so they can affect
- * other containers. Let us allow no more than sysctl_ve_mount_nr mount
- * points for a VE.
- */
-unsigned int sysctl_ve_mount_nr = 4096;
-static int ve_mount_nr_min = 0;
-static int ve_mount_nr_max = INT_MAX;
-
 static struct ctl_table vz_fs_table[] = {
-	{
-		.procname	= "ve-mount-nr",
-		.data		= &sysctl_ve_mount_nr,
-		.maxlen		= sizeof(sysctl_ve_mount_nr),
-		.mode		= 0644,
-		.proc_handler	= proc_dointvec_minmax,
-		.extra1		= &ve_mount_nr_min,
-		.extra2		= &ve_mount_nr_max,
-	},
 	{
 		.procname	= "fsync-enable",
 		.data		= &ve0.fsync_enable,
diff --git a/tools/testing/selftests/ve/ve_ns_owner_test.c b/tools/testing/selftests/ve/ve_ns_owner_test.c
index c3fa7d7b7f9c..dcff4d54172d 100644
--- a/tools/testing/selftests/ve/ve_ns_owner_test.c
+++ b/tools/testing/selftests/ve/ve_ns_owner_test.c
@@ -50,7 +50,7 @@
  * any spurious accounting against the parent ve would overflow it.
  */
 #define VE_NETNS_MAX		3
-#define VE_MOUNTS_MAX		4096
+#define VE_MOUNTS_MAX		32
 
 /*
  * Synchronisation across the clone() boundary: child does its setup,
@@ -173,6 +173,10 @@ FIXTURE_SETUP(ve_ns_owner)
 	snprintf(val, sizeof(val), "%d", VE_NETNS_MAX);
 	ASSERT_EQ(write_file_at(self->cgv2_fd, path, val), 0);
 
+	snprintf(path, sizeof(path), "%d/ve.mnt_max_nr", self->ctid);
+	snprintf(val, sizeof(val), "%d", VE_MOUNTS_MAX);
+	ASSERT_EQ(write_file_at(self->cgv2_fd, path, val), 0);
+
 	/*
 	 * The new ve cgroup has not been entered by anything yet, so its
 	 * mnt_avail_nr counter should be VE_MOUNTS_MAX. Each test below
-- 
2.47.1

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

Re: [Devel] [PATCH VZ10 v9 4/8] ve: Move from global VE mounts limit to per-VE limit

[Vladimir Riabchun]

Sashiko review:

On 9/15/26 19:22, Vladimir Riabchun wrote:
> This change unifies mount limit handling with other VE limits.
> This will simplify refactoring later and allow more precise limits
> control from one place.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-135520
> 
> Feature: per-ve failcounters
> Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
> ---
>   include/linux/ve.h                            |  9 +++--
>   kernel/ve/ve.c                                | 34 ++++++++++++++++++-
>   kernel/ve/veowner.c                           | 19 -----------
>   tools/testing/selftests/ve/ve_ns_owner_test.c |  6 +++-
>   4 files changed, 45 insertions(+), 23 deletions(-)
> 
> diff --git a/include/linux/ve.h b/include/linux/ve.h
> index 65c058c0f34d..fabc608e31f5 100644
> --- a/include/linux/ve.h
> +++ b/include/linux/ve.h
> @@ -96,7 +96,13 @@ struct ve_struct {
>   	atomic_t		nd_neigh_nr;
>   	unsigned long		meminfo_val;
>   
> +	/*
> +	 * Operations with a big amount of mount points can require a lot of time.
> +	 * These operations take the global lock namespace_sem, so they can affect
> +	 * other containers.
> +	 */
>   	atomic_t		mnt_avail_nr; /* number of available VE mounts */
> +	int			mnt_max_nr;
>   
>   #ifdef CONFIG_COREDUMP
>   	char			core_pattern[CORENAME_MAX_SIZE];
> @@ -143,8 +149,7 @@ extern int nr_ve;
>   #define NETNS_MAX_NR_DEFAULT	256	/* number of net-namespaces per-VE */
>   #define NETIF_MAX_NR_DEFAULT	256	/* number of net-interfaces per-VE */
>   #define BPF_PROG_MAX_NR_DEFAULT	256	/* number of loaded BPF progs per-VE */
> -
> -extern unsigned int sysctl_ve_mount_nr;
> +#define MNT_MAX_NR_DEFAULT	4096	/* number of mounts per-VE */
>   
>   #ifdef CONFIG_VE
>   static inline void ve_set_state(struct ve_struct *ve, int new_state)
> diff --git a/kernel/ve/ve.c b/kernel/ve/ve.c
> index 58a0c4b8e36e..a33e55991e52 100644
> --- a/kernel/ve/ve.c
> +++ b/kernel/ve/ve.c
> @@ -83,6 +83,7 @@ struct ve_struct ve0 = {
>   	.arp_neigh_nr		= ATOMIC_INIT(0),
>   	.nd_neigh_nr		= ATOMIC_INIT(0),
>   	.mnt_avail_nr		= ATOMIC_INIT(INT_MAX),
> +	.mnt_max_nr		= INT_MAX,
>   	.meminfo_val		= VE_MEMINFO_SYSTEM,
>   #ifdef CONFIG_X86
>   	.umh_running_helpers	= ATOMIC_INIT(0),
> @@ -787,7 +788,8 @@ 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_avail_nr, sysctl_ve_mount_nr);
> +	ve->mnt_max_nr = MNT_MAX_NR_DEFAULT;
> +	atomic_set(&ve->mnt_avail_nr, MNT_MAX_NR_DEFAULT);
>   
>   #ifdef CONFIG_COREDUMP
>   	strcpy(ve->core_pattern, "core");
> @@ -1068,6 +1070,30 @@ static s64 ve_mnt_avail_nr_read(struct cgroup_subsys_state *css, struct cftype *
>   	return atomic_read(&css_to_ve(css)->mnt_avail_nr);
>   }
>   
> +static u64 ve_mnt_max_nr_read(struct cgroup_subsys_state *css, struct cftype *cft)
> +{
> +	return css_to_ve(css)->mnt_max_nr;
> +}
> +
> +static int ve_mnt_max_nr_write(struct cgroup_subsys_state *css, struct cftype *cft, u64 val)
> +{
> +	struct ve_struct *ve = css_to_ve(css);
> +	int delta;
> +
> +	if (!ve_is_super(get_exec_env()))
> +		return -EPERM;
> +
> +	if (val > INT_MAX)
> +		return -EOVERFLOW;
> +
> +	down_write(&ve->op_sem);
> +	delta = val - ve->mnt_max_nr;
> +	ve->mnt_max_nr = val;
> +	atomic_add(delta, &ve->mnt_avail_nr);

Sashiko says:
  In the function ve_mnt_max_nr_write, the computation delta = val - ve->mnt_max_nr
  followed by atomic_add(delta, &ve->mnt_avail_nr) might lead to an integer
  overflow. What checks or safeguards are in place to ensure that this operation
  will not lead to an overflow, potentially affecting the VE system state?

0 <= val <= INT_MAX
0 <= ve->mnt_max_nr <= INT_MAX
INT_MIN < 0 - INT_MAX <= delta <= INT_MAX - 0

So delta never overflows. Can mnt_avail_nr overflow?
No, it can't. By definition mnt_avail_nr = mnt_max_nr - have_mounts.
It can't overflow INT_MAX since it's always <= mnt_max_nr <= INT_MAX.
It can't overflow INT_MIN - it means we have > |INT_MIN| mounts, which
is not the real case.

> +	up_write(&ve->op_sem);
> +	return 0;
> +}
> +
>   static u64 ve_netif_max_nr_read(struct cgroup_subsys_state *css, struct cftype *cft)
>   {
>   	return css_to_ve(css)->netif_max_nr;
> @@ -1630,6 +1656,12 @@ static struct cftype ve_cftypes[] = {
>   		.name			= "mnt_avail_nr",
>   		.read_s64		= ve_mnt_avail_nr_read,
>   	},
> +	{
> +		.name			= "mnt_max_nr",
> +		.flags			= CFTYPE_NOT_ON_ROOT,
> +		.read_u64		= ve_mnt_max_nr_read,
> +		.write_u64		= ve_mnt_max_nr_write,
> +	},
>   	{
>   		.name			= "netif_max_nr",
>   		.flags			= CFTYPE_NOT_ON_ROOT,
> diff --git a/kernel/ve/veowner.c b/kernel/ve/veowner.c
> index 1e631baf0de9..c77735f8c035 100644
> --- a/kernel/ve/veowner.c
> +++ b/kernel/ve/veowner.c
> @@ -48,26 +48,7 @@ static void prepare_proc(void)
>    * ------------------------------------------------------------------------
>    */
>   
> -/*
> - * Operations with a big amount of mount points can require a lot of time.
> - * These operations take the global lock namespace_sem, so they can affect
> - * other containers. Let us allow no more than sysctl_ve_mount_nr mount
> - * points for a VE.
> - */
> -unsigned int sysctl_ve_mount_nr = 4096;
> -static int ve_mount_nr_min = 0;
> -static int ve_mount_nr_max = INT_MAX;
> -
>   static struct ctl_table vz_fs_table[] = {
> -	{
> -		.procname	= "ve-mount-nr",
> -		.data		= &sysctl_ve_mount_nr,
> -		.maxlen		= sizeof(sysctl_ve_mount_nr),
> -		.mode		= 0644,
> -		.proc_handler	= proc_dointvec_minmax,
> -		.extra1		= &ve_mount_nr_min,
> -		.extra2		= &ve_mount_nr_max,
> -	},

Sashiko says:
  The removal of sysctl_ve_mount_nr potentially affects users relying
  on this sysctl for VE mount limits, impacting backward compatibility.
  Is there an alternative provided for monitoring or configuring
  VE mount limits dynamically that maintains backward compatibility?

This sysctl mechanism doesn't work well with per-VE limits,
the only solution to maintain the behavior is to iterate over all
VEs and update avail numbers.

IMO this doesn't add that much value to have two separate limiting
mechanisms in different places.
>   	{
>   		.procname	= "fsync-enable",
>   		.data		= &ve0.fsync_enable,
> diff --git a/tools/testing/selftests/ve/ve_ns_owner_test.c b/tools/testing/selftests/ve/ve_ns_owner_test.c
> index c3fa7d7b7f9c..dcff4d54172d 100644
> --- a/tools/testing/selftests/ve/ve_ns_owner_test.c
> +++ b/tools/testing/selftests/ve/ve_ns_owner_test.c
> @@ -50,7 +50,7 @@
>    * any spurious accounting against the parent ve would overflow it.
>    */
>   #define VE_NETNS_MAX		3
> -#define VE_MOUNTS_MAX		4096
> +#define VE_MOUNTS_MAX		32
>   
>   /*
>    * Synchronisation across the clone() boundary: child does its setup,
> @@ -173,6 +173,10 @@ FIXTURE_SETUP(ve_ns_owner)
>   	snprintf(val, sizeof(val), "%d", VE_NETNS_MAX);
>   	ASSERT_EQ(write_file_at(self->cgv2_fd, path, val), 0);
>   
> +	snprintf(path, sizeof(path), "%d/ve.mnt_max_nr", self->ctid);
> +	snprintf(val, sizeof(val), "%d", VE_MOUNTS_MAX);
> +	ASSERT_EQ(write_file_at(self->cgv2_fd, path, val), 0);
> +
>   	/*
>   	 * The new ve cgroup has not been entered by anything yet, so its
>   	 * mnt_avail_nr counter should be VE_MOUNTS_MAX. Each test below

-- 
Best regards, Riabchun Vladimir
Linux Kernel Developer, Virtuozzo

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

[Devel] [PATCH VZ10 v9 5/8] ve/ve.c: Generate VE resource accessors using macros

[Vladimir Riabchun]

We have a bunch of almost identical code, which takes place
and has some unclear variations in it. Unify all resource
getters and setters using macros.

This changes behavior a little bit: some resources (netns number
for example) required stopped VE. Now such limits could be updated
with a running VE.
Also, some max_nr_write functions didn't verify provided value,
now it is not possible to pass overflowing value.

https://virtuozzo.atlassian.net/browse/VSTOR-135520

Feature: per-ve failcounters
Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
---
 kernel/ve/ve.c | 209 +++++++++++++------------------------------------
 1 file changed, 54 insertions(+), 155 deletions(-)

diff --git a/kernel/ve/ve.c b/kernel/ve/ve.c
index a33e55991e52..5ebdb4b7baa7 100644
--- a/kernel/ve/ve.c
+++ b/kernel/ve/ve.c
@@ -43,6 +43,39 @@
 #include <linux/cpuset.h>		/* For css_tg() */
 #include "../sched/sched.h"		/* For css_tg() */
 
+#define VE_RESOURCE(name)						\
+static u64 ve_##name##_max_nr_read(struct cgroup_subsys_state *css,	\
+				 struct cftype *cft)			\
+{									\
+	return css_to_ve(css)->name##_max_nr;				\
+}									\
+									\
+static int ve_##name##_max_nr_write(struct cgroup_subsys_state *css,	\
+				  struct cftype *cft, u64 val)		\
+{									\
+	struct ve_struct *ve = css_to_ve(css);				\
+	int delta;							\
+									\
+	if (!ve_is_super(get_exec_env()))				\
+		return -EPERM;						\
+									\
+	if (val > INT_MAX)						\
+		return -EOVERFLOW;					\
+									\
+	down_write(&ve->op_sem);					\
+	delta = val - ve->name##_max_nr;				\
+	ve->name##_max_nr = val;					\
+	atomic_add(delta, &ve->name##_avail_nr);			\
+	up_write(&ve->op_sem);						\
+	return 0;							\
+}									\
+									\
+static s64 ve_##name##_avail_nr_read(struct cgroup_subsys_state *css,	\
+				   struct cftype *cft)			\
+{									\
+	return atomic_read(&css_to_ve(css)->name##_avail_nr);		\
+}
+
 extern struct kmapset_set sysfs_ve_perms_set;
 #ifdef CONFIG_PROC_FS
 extern struct kmapset_set proc_ve_perms_set;
@@ -1036,121 +1069,10 @@ static int ve_features_write(struct cgroup_subsys_state *css, struct cftype *cft
 	return 0;
 }
 
-static u64 ve_netns_max_nr_read(struct cgroup_subsys_state *css, struct cftype *cft)
-{
-	return css_to_ve(css)->netns_max_nr;
-}
-
-static int ve_netns_max_nr_write(struct cgroup_subsys_state *css, struct cftype *cft, u64 val)
-{
-	struct ve_struct *ve = css_to_ve(css);
-	int delta;
-
-	if (!ve_is_super(get_exec_env()))
-		return -EPERM;
-
-	down_write(&ve->op_sem);
-	if (VE_IS_RUNNING(ve) || ve->ve_nsproxy) {
-		up_write(&ve->op_sem);
-		return -EBUSY;
-	}
-	delta = val - ve->netns_max_nr;
-	ve->netns_max_nr = val;
-	atomic_add(delta, &ve->netns_avail_nr);
-	up_write(&ve->op_sem);
-	return 0;
-}
-static u64 ve_netns_avail_nr_read(struct cgroup_subsys_state *css, struct cftype *cft)
-{
-	return atomic_read(&css_to_ve(css)->netns_avail_nr);
-}
-
-static s64 ve_mnt_avail_nr_read(struct cgroup_subsys_state *css, struct cftype *cft)
-{
-	return atomic_read(&css_to_ve(css)->mnt_avail_nr);
-}
-
-static u64 ve_mnt_max_nr_read(struct cgroup_subsys_state *css, struct cftype *cft)
-{
-	return css_to_ve(css)->mnt_max_nr;
-}
-
-static int ve_mnt_max_nr_write(struct cgroup_subsys_state *css, struct cftype *cft, u64 val)
-{
-	struct ve_struct *ve = css_to_ve(css);
-	int delta;
-
-	if (!ve_is_super(get_exec_env()))
-		return -EPERM;
-
-	if (val > INT_MAX)
-		return -EOVERFLOW;
-
-	down_write(&ve->op_sem);
-	delta = val - ve->mnt_max_nr;
-	ve->mnt_max_nr = val;
-	atomic_add(delta, &ve->mnt_avail_nr);
-	up_write(&ve->op_sem);
-	return 0;
-}
-
-static u64 ve_netif_max_nr_read(struct cgroup_subsys_state *css, struct cftype *cft)
-{
-	return css_to_ve(css)->netif_max_nr;
-}
-
-static int ve_netif_max_nr_write(struct cgroup_subsys_state *css, struct cftype *cft, u64 val)
-{
-	struct ve_struct *ve = css_to_ve(css);
-	int delta;
-
-	if (!ve_is_super(get_exec_env()))
-		return -EPERM;
-
-	if (val > INT_MAX)
-		return -EOVERFLOW;
-
-	down_write(&ve->op_sem);
-	delta = val - ve->netif_max_nr;
-	ve->netif_max_nr = val;
-	atomic_add(delta, &ve->netif_avail_nr);
-	up_write(&ve->op_sem);
-	return 0;
-}
-
-static s64 ve_netif_avail_nr_read(struct cgroup_subsys_state *css, struct cftype *cft)
-{
-	return atomic_read(&css_to_ve(css)->netif_avail_nr);
-}
-
-static u64 ve_bpf_prog_max_nr_read(struct cgroup_subsys_state *css, struct cftype *cft)
-{
-	return css_to_ve(css)->bpf_prog_max_nr;
-}
-
-static int ve_bpf_prog_max_nr_write(struct cgroup_subsys_state *css, struct cftype *cft, u64 val)
-{
-	struct ve_struct *ve = css_to_ve(css);
-	int delta;
-
-	if (!ve_is_super(get_exec_env()))
-		return -EPERM;
-
-	if (val > INT_MAX)
-		return -EOVERFLOW;
-
-	down_write(&ve->op_sem);
-	delta = val - ve->bpf_prog_max_nr;
-	ve->bpf_prog_max_nr = val;
-	atomic_add(delta, &ve->bpf_prog_avail_nr);
-	up_write(&ve->op_sem);
-	return 0;
-}
-
-static s64 ve_bpf_prog_avail_nr_read(struct cgroup_subsys_state *css, struct cftype *cft)
-{
-	return atomic_read(&css_to_ve(css)->bpf_prog_avail_nr);
-}
+VE_RESOURCE(netns);
+VE_RESOURCE(mnt);
+VE_RESOURCE(netif);
+VE_RESOURCE(bpf_prog);
 
 static int ve_os_release_read(struct seq_file *sf, void *v)
 {
@@ -1598,6 +1520,19 @@ static int ve_rpc_kill_write(struct cgroup_subsys_state *css,
 	return 0;
 }
 
+/* checkpatch will report it, can't enclose this in parentheses. */
+#define VE_RESOURCE_CFTYPE(res)					\
+{								\
+	.name			= #res "_max_nr",		\
+	.flags			= CFTYPE_NOT_ON_ROOT,		\
+	.read_u64		= ve_##res##_max_nr_read,	\
+	.write_u64		= ve_##res##_max_nr_write,	\
+},								\
+{								\
+	.name			= #res "_avail_nr",		\
+	.read_s64		= ve_##res##_avail_nr_read,	\
+}
+
 static struct cftype ve_cftypes[] = {
 
 	{
@@ -1642,46 +1577,10 @@ static struct cftype ve_cftypes[] = {
 		.read_u64		= ve_pid_max_read_u64,
 		.write_u64		= ve_pid_max_write_running_u64,
 	},
-	{
-		.name			= "netns_max_nr",
-		.flags			= CFTYPE_NOT_ON_ROOT,
-		.read_u64		= ve_netns_max_nr_read,
-		.write_u64		= ve_netns_max_nr_write,
-	},
-	{
-		.name			= "netns_avail_nr",
-		.read_u64		= ve_netns_avail_nr_read,
-	},
-	{
-		.name			= "mnt_avail_nr",
-		.read_s64		= ve_mnt_avail_nr_read,
-	},
-	{
-		.name			= "mnt_max_nr",
-		.flags			= CFTYPE_NOT_ON_ROOT,
-		.read_u64		= ve_mnt_max_nr_read,
-		.write_u64		= ve_mnt_max_nr_write,
-	},
-	{
-		.name			= "netif_max_nr",
-		.flags			= CFTYPE_NOT_ON_ROOT,
-		.read_u64		= ve_netif_max_nr_read,
-		.write_u64		= ve_netif_max_nr_write,
-	},
-	{
-		.name			= "netif_avail_nr",
-		.read_s64		= ve_netif_avail_nr_read,
-	},
-	{
-		.name			= "bpf_prog_max_nr",
-		.flags			= CFTYPE_NOT_ON_ROOT,
-		.read_u64		= ve_bpf_prog_max_nr_read,
-		.write_u64		= ve_bpf_prog_max_nr_write,
-	},
-	{
-		.name			= "bpf_prog_avail_nr",
-		.read_s64		= ve_bpf_prog_avail_nr_read,
-	},
+	VE_RESOURCE_CFTYPE(netns),
+	VE_RESOURCE_CFTYPE(mnt),
+	VE_RESOURCE_CFTYPE(netif),
+	VE_RESOURCE_CFTYPE(bpf_prog),
 	{
 		.name			= "os_release",
 		.max_write_len		= __NEW_UTS_LEN + 1,
-- 
2.47.1

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

Re: [Devel] [PATCH VZ10 v9 5/8] ve/ve.c: Generate VE resource accessors using macros

[Vladimir Riabchun]

Sashiko review:

On 9/15/26 19:22, Vladimir Riabchun wrote:
> We have a bunch of almost identical code, which takes place
> and has some unclear variations in it. Unify all resource
> getters and setters using macros.
> 
> This changes behavior a little bit: some resources (netns number
> for example) required stopped VE. Now such limits could be updated
> with a running VE.
> Also, some max_nr_write functions didn't verify provided value,
> now it is not possible to pass overflowing value.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-135520
> 
> Feature: per-ve failcounters
> Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
> ---
>   kernel/ve/ve.c | 209 +++++++++++++------------------------------------
>   1 file changed, 54 insertions(+), 155 deletions(-)
> 
> diff --git a/kernel/ve/ve.c b/kernel/ve/ve.c
> index a33e55991e52..5ebdb4b7baa7 100644
> --- a/kernel/ve/ve.c
> +++ b/kernel/ve/ve.c
> @@ -43,6 +43,39 @@
>   #include <linux/cpuset.h>		/* For css_tg() */
>   #include "../sched/sched.h"		/* For css_tg() */
>   
> +#define VE_RESOURCE(name)						\
> +static u64 ve_##name##_max_nr_read(struct cgroup_subsys_state *css,	\
> +				 struct cftype *cft)			\
> +{									\
> +	return css_to_ve(css)->name##_max_nr;				\
> +}									\
> +									\
> +static int ve_##name##_max_nr_write(struct cgroup_subsys_state *css,	\
> +				  struct cftype *cft, u64 val)		\
> +{									\
> +	struct ve_struct *ve = css_to_ve(css);				\
> +	int delta;							\
> +									\
> +	if (!ve_is_super(get_exec_env()))				\
> +		return -EPERM;						\
> +									\
> +	if (val > INT_MAX)						\
> +		return -EOVERFLOW;					\
> +									\
> +	down_write(&ve->op_sem);					\

Sashiko says:
  Does the change to use macros for VE resource management impact the safety
  of runtime VE modifications? The original ve_netns_max_nr_write function
  included a check on VE_IS_RUNNING(ve) || ve->ve_nsproxy, which prevented
  modifications unless these conditions were met.
  The new macro VE_RESOURCE seems to omit this check, which might lead to
  inconsistencies if resource limits are altered during VE operation.

max values are used only in these helpers to adjust avail number.
There is no reason for enforcing stopped VE - only host can update
the value anyway. And having the ability to adjust limits without stopping
CT seems really useful.

This is the anser for the next Sashiko comment:
  Is the current permission check sufficient in the VE_RESOURCE macro? By checking only
  ve_is_super(get_exec_env()), it seems you now allow modifications without verifying
  the VE's actual running state.
  Would this oversight instate authorizations in inappropriate scenarios where
  VE should disallow updates?
> +	delta = val - ve->name##_max_nr;				\

Sashiko says:
  Could the calculation of delta = val - ve->name_max_nr in VE_RESOURCE lead
  to integer overflow?

No, same reasoning as in previous patch.

> +	ve->name##_max_nr = val;					\
> +	atomic_add(delta, &ve->name##_avail_nr);			\
> +	up_write(&ve->op_sem);						\
> +	return 0;							\
> +}									\
> +									\
> +static s64 ve_##name##_avail_nr_read(struct cgroup_subsys_state *css,	\
> +				   struct cftype *cft)			\
> +{									\
> +	return atomic_read(&css_to_ve(css)->name##_avail_nr);		\
> +}
> +
>   extern struct kmapset_set sysfs_ve_perms_set;
>   #ifdef CONFIG_PROC_FS
>   extern struct kmapset_set proc_ve_perms_set;
> @@ -1036,121 +1069,10 @@ static int ve_features_write(struct cgroup_subsys_state *css, struct cftype *cft
>   	return 0;
>   }
>   
> -static u64 ve_netns_max_nr_read(struct cgroup_subsys_state *css, struct cftype *cft)
> -{
> -	return css_to_ve(css)->netns_max_nr;
> -}
> -
> -static int ve_netns_max_nr_write(struct cgroup_subsys_state *css, struct cftype *cft, u64 val)
> -{
> -	struct ve_struct *ve = css_to_ve(css);
> -	int delta;
> -
> -	if (!ve_is_super(get_exec_env()))
> -		return -EPERM;
> -
> -	down_write(&ve->op_sem);
> -	if (VE_IS_RUNNING(ve) || ve->ve_nsproxy) {
> -		up_write(&ve->op_sem);
> -		return -EBUSY;
> -	}
> -	delta = val - ve->netns_max_nr;
> -	ve->netns_max_nr = val;
> -	atomic_add(delta, &ve->netns_avail_nr);
> -	up_write(&ve->op_sem);
> -	return 0;
> -}
> -static u64 ve_netns_avail_nr_read(struct cgroup_subsys_state *css, struct cftype *cft)
> -{
> -	return atomic_read(&css_to_ve(css)->netns_avail_nr);
> -}
> -
> -static s64 ve_mnt_avail_nr_read(struct cgroup_subsys_state *css, struct cftype *cft)
> -{
> -	return atomic_read(&css_to_ve(css)->mnt_avail_nr);
> -}
> -
> -static u64 ve_mnt_max_nr_read(struct cgroup_subsys_state *css, struct cftype *cft)
> -{
> -	return css_to_ve(css)->mnt_max_nr;
> -}
> -
> -static int ve_mnt_max_nr_write(struct cgroup_subsys_state *css, struct cftype *cft, u64 val)
> -{
> -	struct ve_struct *ve = css_to_ve(css);
> -	int delta;
> -
> -	if (!ve_is_super(get_exec_env()))
> -		return -EPERM;
> -
> -	if (val > INT_MAX)
> -		return -EOVERFLOW;
> -
> -	down_write(&ve->op_sem);
> -	delta = val - ve->mnt_max_nr;
> -	ve->mnt_max_nr = val;
> -	atomic_add(delta, &ve->mnt_avail_nr);
> -	up_write(&ve->op_sem);
> -	return 0;
> -}
> -
> -static u64 ve_netif_max_nr_read(struct cgroup_subsys_state *css, struct cftype *cft)
> -{
> -	return css_to_ve(css)->netif_max_nr;
> -}
> -
> -static int ve_netif_max_nr_write(struct cgroup_subsys_state *css, struct cftype *cft, u64 val)
> -{
> -	struct ve_struct *ve = css_to_ve(css);
> -	int delta;
> -
> -	if (!ve_is_super(get_exec_env()))
> -		return -EPERM;
> -
> -	if (val > INT_MAX)
> -		return -EOVERFLOW;
> -
> -	down_write(&ve->op_sem);
> -	delta = val - ve->netif_max_nr;
> -	ve->netif_max_nr = val;
> -	atomic_add(delta, &ve->netif_avail_nr);
> -	up_write(&ve->op_sem);
> -	return 0;
> -}
> -
> -static s64 ve_netif_avail_nr_read(struct cgroup_subsys_state *css, struct cftype *cft)
> -{
> -	return atomic_read(&css_to_ve(css)->netif_avail_nr);
> -}
> -
> -static u64 ve_bpf_prog_max_nr_read(struct cgroup_subsys_state *css, struct cftype *cft)
> -{
> -	return css_to_ve(css)->bpf_prog_max_nr;
> -}
> -
> -static int ve_bpf_prog_max_nr_write(struct cgroup_subsys_state *css, struct cftype *cft, u64 val)
> -{
> -	struct ve_struct *ve = css_to_ve(css);
> -	int delta;
> -
> -	if (!ve_is_super(get_exec_env()))
> -		return -EPERM;
> -
> -	if (val > INT_MAX)
> -		return -EOVERFLOW;
> -
> -	down_write(&ve->op_sem);
> -	delta = val - ve->bpf_prog_max_nr;
> -	ve->bpf_prog_max_nr = val;
> -	atomic_add(delta, &ve->bpf_prog_avail_nr);
> -	up_write(&ve->op_sem);
> -	return 0;
> -}
> -
> -static s64 ve_bpf_prog_avail_nr_read(struct cgroup_subsys_state *css, struct cftype *cft)
> -{
> -	return atomic_read(&css_to_ve(css)->bpf_prog_avail_nr);
> -}
> +VE_RESOURCE(netns);
> +VE_RESOURCE(mnt);
> +VE_RESOURCE(netif);
> +VE_RESOURCE(bpf_prog);
>   
>   static int ve_os_release_read(struct seq_file *sf, void *v)
>   {
> @@ -1598,6 +1520,19 @@ static int ve_rpc_kill_write(struct cgroup_subsys_state *css,
>   	return 0;
>   }
>   
> +/* checkpatch will report it, can't enclose this in parentheses. */
> +#define VE_RESOURCE_CFTYPE(res)					\
> +{								\
> +	.name			= #res "_max_nr",		\
> +	.flags			= CFTYPE_NOT_ON_ROOT,		\
> +	.read_u64		= ve_##res##_max_nr_read,	\
> +	.write_u64		= ve_##res##_max_nr_write,	\
> +},								\
> +{								\
> +	.name			= #res "_avail_nr",		\
> +	.read_s64		= ve_##res##_avail_nr_read,	\
> +}
> +
>   static struct cftype ve_cftypes[] = {
>   
>   	{
> @@ -1642,46 +1577,10 @@ static struct cftype ve_cftypes[] = {
>   		.read_u64		= ve_pid_max_read_u64,
>   		.write_u64		= ve_pid_max_write_running_u64,
>   	},
> -	{
> -		.name			= "netns_max_nr",
> -		.flags			= CFTYPE_NOT_ON_ROOT,
> -		.read_u64		= ve_netns_max_nr_read,
> -		.write_u64		= ve_netns_max_nr_write,
> -	},
> -	{
> -		.name			= "netns_avail_nr",
> -		.read_u64		= ve_netns_avail_nr_read,
> -	},
> -	{
> -		.name			= "mnt_avail_nr",
> -		.read_s64		= ve_mnt_avail_nr_read,
> -	},
> -	{
> -		.name			= "mnt_max_nr",
> -		.flags			= CFTYPE_NOT_ON_ROOT,
> -		.read_u64		= ve_mnt_max_nr_read,
> -		.write_u64		= ve_mnt_max_nr_write,
> -	},
> -	{
> -		.name			= "netif_max_nr",
> -		.flags			= CFTYPE_NOT_ON_ROOT,
> -		.read_u64		= ve_netif_max_nr_read,
> -		.write_u64		= ve_netif_max_nr_write,
> -	},
> -	{
> -		.name			= "netif_avail_nr",
> -		.read_s64		= ve_netif_avail_nr_read,
> -	},
> -	{
> -		.name			= "bpf_prog_max_nr",
> -		.flags			= CFTYPE_NOT_ON_ROOT,
> -		.read_u64		= ve_bpf_prog_max_nr_read,
> -		.write_u64		= ve_bpf_prog_max_nr_write,
> -	},
> -	{
> -		.name			= "bpf_prog_avail_nr",
> -		.read_s64		= ve_bpf_prog_avail_nr_read,
> -	},
> +	VE_RESOURCE_CFTYPE(netns),
> +	VE_RESOURCE_CFTYPE(mnt),
> +	VE_RESOURCE_CFTYPE(netif),
> +	VE_RESOURCE_CFTYPE(bpf_prog),
>   	{
>   		.name			= "os_release",
>   		.max_write_len		= __NEW_UTS_LEN + 1,

-- 
Best regards, Riabchun Vladimir
Linux Kernel Developer, Virtuozzo

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

[Devel] [PATCH VZ10 v9 6/8] ve: Introduce per-VE failcount

[Vladimir Riabchun]

It may be useful to have a history of resource limit hits for every VE,
this may simplify debugging and provide some information about the
resources usage.

This information is provided by ve.failcount file, any write to it
resets all failcounts.

To add a new failcounter we need to create a new atomic_t field
name_failcount in ve structure and add a new VE_FC_ENTRY in
ve_failcounts array.

One change, unrelated to failcounts: aio fields are now initialized
in ve0.

https://virtuozzo.atlassian.net/browse/VSTOR-135520

Feature: per-ve failcounters
Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
---
 fs/aio.c                 |  1 +
 fs/namespace.c           |  2 ++
 include/linux/ve.h       | 16 +++++++++
 kernel/bpf/syscall.c     |  1 +
 kernel/ve/ve.c           | 70 ++++++++++++++++++++++++++++++++++++++++
 net/core/dev.c           | 10 ++----
 net/core/neighbour.c     |  3 +-
 net/core/net_namespace.c |  4 ++-
 8 files changed, 96 insertions(+), 11 deletions(-)

diff --git a/fs/aio.c b/fs/aio.c
index cb63416af135..4ae06bcd47f2 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -814,6 +814,7 @@ static struct kioctx *ioctx_alloc(unsigned nr_events)
 	spin_lock(&ve->aio_nr_lock);
 	if (ve->aio_nr + ctx->max_reqs > ve->aio_max_nr ||
 	    ve->aio_nr + ctx->max_reqs < ve->aio_nr) {
+		ve_failcount_inc(ve, aio);
 		spin_unlock(&ve->aio_nr_lock);
 		err = -EAGAIN;
 		goto err_ctx;
diff --git a/fs/namespace.c b/fs/namespace.c
index 1d6e66cd7174..ea01664ff5f2 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -3380,6 +3380,8 @@ static inline int ve_try_reserve_mount(struct ve_struct *ve)
 
 	if (ret)
 		get_ve(ve);
+	else
+		ve_failcount_inc(ve, mnt);
 	return ret;
 }
 
diff --git a/include/linux/ve.h b/include/linux/ve.h
index fabc608e31f5..065129b42768 100644
--- a/include/linux/ve.h
+++ b/include/linux/ve.h
@@ -80,12 +80,15 @@ struct ve_struct {
 	struct kmapset_key	proc_perms_key;
 
 	atomic_t		netns_avail_nr;
+	atomic_t		netns_failcount;
 	int			netns_max_nr;
 
 	atomic_t		netif_avail_nr;
+	atomic_t		netif_failcount;
 	int			netif_max_nr;
 
 	atomic_t		bpf_prog_avail_nr;
+	atomic_t		bpf_prog_failcount;
 	int			bpf_prog_max_nr;
 
 	atomic64_t		_uevent_seqnum;
@@ -94,6 +97,7 @@ struct ve_struct {
 
 	atomic_t		arp_neigh_nr;
 	atomic_t		nd_neigh_nr;
+	atomic_t		neigh_tbl_failcount;
 	unsigned long		meminfo_val;
 
 	/*
@@ -102,6 +106,7 @@ struct ve_struct {
 	 * other containers.
 	 */
 	atomic_t		mnt_avail_nr; /* number of available VE mounts */
+	atomic_t		mnt_failcount;
 	int			mnt_max_nr;
 
 #ifdef CONFIG_COREDUMP
@@ -129,6 +134,7 @@ struct ve_struct {
 	spinlock_t		aio_nr_lock;
 	unsigned long		aio_nr;
 	unsigned long		aio_max_nr;
+	atomic_t		aio_failcount;
 #endif
 	struct vfsmount		*devtmpfs_mnt;
 };
@@ -151,6 +157,16 @@ extern int nr_ve;
 #define BPF_PROG_MAX_NR_DEFAULT	256	/* number of loaded BPF progs per-VE */
 #define MNT_MAX_NR_DEFAULT	4096	/* number of mounts per-VE */
 
+#define ve_failcount_inc(ve, name)					\
+do {									\
+	struct ve_struct *__ve = (ve);					\
+									\
+	atomic_inc(&__ve->name##_failcount);				\
+	ve_pr_warn_ratelimited(VE_LOG_BOTH, "CT%s: hits the " #name	\
+		" limit, see the '" #name "' counter in ve.failcount "	\
+		"of the container's ve cgroup\n", ve_name(__ve));	\
+} while (0)
+
 #ifdef CONFIG_VE
 static inline void ve_set_state(struct ve_struct *ve, int new_state)
 {
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index c94d4240e3d3..8498bc9df7ea 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2891,6 +2891,7 @@ static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size)
 	if (!bpf_cap && type == BPF_PROG_TYPE_CGROUP_DEVICE) {
 		load_ve = get_exec_env();
 		if (atomic_dec_if_positive(&load_ve->bpf_prog_avail_nr) < 0) {
+			ve_failcount_inc(load_ve, bpf_prog);
 			load_ve = NULL;
 			err = -ENOSPC;
 			goto put_token;
diff --git a/kernel/ve/ve.c b/kernel/ve/ve.c
index 5ebdb4b7baa7..9261565208e6 100644
--- a/kernel/ve/ve.c
+++ b/kernel/ve/ve.c
@@ -100,10 +100,13 @@ struct ve_struct ve0 = {
 	.features		= -1,
 	.sched_lat_ve.cur	= &ve0_lat_stats,
 	.netns_avail_nr		= ATOMIC_INIT(INT_MAX),
+	.netns_failcount	= ATOMIC_INIT(0),
 	.netns_max_nr		= INT_MAX,
 	.netif_avail_nr		= ATOMIC_INIT(INT_MAX),
+	.netif_failcount	= ATOMIC_INIT(0),
 	.netif_max_nr		= INT_MAX,
 	.bpf_prog_avail_nr	= ATOMIC_INIT(INT_MAX),
+	.bpf_prog_failcount	= ATOMIC_INIT(0),
 	.bpf_prog_max_nr	= INT_MAX,
 	.fsync_enable		= FSYNC_FILTERED,
 	._randomize_va_space	=
@@ -115,8 +118,16 @@ struct ve_struct ve0 = {
 
 	.arp_neigh_nr		= ATOMIC_INIT(0),
 	.nd_neigh_nr		= ATOMIC_INIT(0),
+	.neigh_tbl_failcount	= ATOMIC_INIT(0),
 	.mnt_avail_nr		= ATOMIC_INIT(INT_MAX),
 	.mnt_max_nr		= INT_MAX,
+	.mnt_failcount		= ATOMIC_INIT(0),
+#ifdef CONFIG_AIO
+	.aio_nr_lock		= __SPIN_LOCK_UNLOCKED(aio_nr_lock),
+	.aio_nr			= 0,
+	.aio_max_nr		= AIO_MAX_NR_DEFAULT,
+	.aio_failcount		= ATOMIC_INIT(0),
+#endif
 	.meminfo_val		= VE_MEMINFO_SYSTEM,
 #ifdef CONFIG_X86
 	.umh_running_helpers	= ATOMIC_INIT(0),
@@ -787,12 +798,15 @@ static struct cgroup_subsys_state *ve_create(struct cgroup_subsys_state *parent_
 	ve->fsync_enable = FSYNC_FILTERED;
 
 	atomic_set(&ve->netns_avail_nr, NETNS_MAX_NR_DEFAULT);
+	atomic_set(&ve->netns_failcount, 0);
 	ve->netns_max_nr = NETNS_MAX_NR_DEFAULT;
 
 	atomic_set(&ve->netif_avail_nr, NETIF_MAX_NR_DEFAULT);
+	atomic_set(&ve->netif_failcount, 0);
 	ve->netif_max_nr = NETIF_MAX_NR_DEFAULT;
 
 	atomic_set(&ve->bpf_prog_avail_nr, BPF_PROG_MAX_NR_DEFAULT);
+	atomic_set(&ve->bpf_prog_failcount, 0);
 	ve->bpf_prog_max_nr = BPF_PROG_MAX_NR_DEFAULT;
 
 	err = ve_log_init(ve);
@@ -821,7 +835,9 @@ 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->neigh_tbl_failcount, 0);
 	ve->mnt_max_nr = MNT_MAX_NR_DEFAULT;
+	atomic_set(&ve->mnt_failcount, 0);
 	atomic_set(&ve->mnt_avail_nr, MNT_MAX_NR_DEFAULT);
 
 #ifdef CONFIG_COREDUMP
@@ -834,6 +850,7 @@ static struct cgroup_subsys_state *ve_create(struct cgroup_subsys_state *parent_
 	spin_lock_init(&ve->aio_nr_lock);
 	ve->aio_nr = 0;
 	ve->aio_max_nr = AIO_MAX_NR_DEFAULT;
+	atomic_set(&ve->aio_failcount, 0);
 #endif
 
 	return &ve->css;
@@ -1074,6 +1091,53 @@ VE_RESOURCE(mnt);
 VE_RESOURCE(netif);
 VE_RESOURCE(bpf_prog);
 
+static const struct ve_failcount_entry {
+	const char *name;
+	size_t offset;
+} ve_failcounts[] = {
+#define VE_FC_ENTRY(name) { #name, offsetof(struct ve_struct, name##_failcount) }
+	VE_FC_ENTRY(netns),
+	VE_FC_ENTRY(mnt),
+	VE_FC_ENTRY(netif),
+	VE_FC_ENTRY(bpf_prog),
+	VE_FC_ENTRY(neigh_tbl),
+#ifdef CONFIG_AIO
+	VE_FC_ENTRY(aio),
+#endif
+	{}
+};
+
+static int ve_failcount_read(struct seq_file *sf, void *v)
+{
+	struct ve_struct *ve = css_to_ve(seq_css(sf));
+	const struct ve_failcount_entry *entry;
+	atomic_t *fc;
+
+	for (entry = ve_failcounts; entry->name; entry++) {
+		fc = (void *)ve + entry->offset;
+		seq_printf(sf, "%s: %d\n", entry->name, atomic_read(fc));
+	}
+	return 0;
+}
+
+static ssize_t ve_failcount_write(struct kernfs_open_file *of, char *buf,
+				  size_t nbytes, loff_t off)
+{
+	struct ve_struct *ve = css_to_ve(of_css(of));
+	const struct ve_failcount_entry *entry;
+	atomic_t *fc;
+
+	if (!ve_is_super(get_exec_env()) && !ve->is_pseudosuper)
+		return -EPERM;
+
+	for (entry = ve_failcounts; entry->name; entry++) {
+		fc = (void *)ve + entry->offset;
+		atomic_set(fc, 0);
+	}
+
+	return nbytes;
+}
+
 static int ve_os_release_read(struct seq_file *sf, void *v)
 {
 	struct cgroup_subsys_state *css = seq_css(sf);
@@ -1613,6 +1677,12 @@ static struct cftype ve_cftypes[] = {
 		.flags			= CFTYPE_NOT_ON_ROOT,
 		.write_u64		= ve_rpc_kill_write,
 	},
+	{
+		.name			= "failcount",
+		.flags			= CFTYPE_NOT_ON_ROOT,
+		.seq_show		= ve_failcount_read,
+		.write			= ve_failcount_write,
+	},
 	{ }
 };
 
diff --git a/net/core/dev.c b/net/core/dev.c
index c7dddb200489..7632ad1fd015 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -10997,10 +10997,7 @@ int register_netdevice(struct net_device *dev)
 
 	ret = -ENOMEM;
 	if (atomic_dec_if_positive(&net->owner_ve->netif_avail_nr) < 0) {
-		ve_pr_warn_ratelimited(VE_LOG_BOTH,
-			"CT%s: hits max number of network devices, "
-			"increase ve::netif_max_nr parameter\n",
-			ve_name(net->owner_ve));
+		ve_failcount_inc(net->owner_ve, netif);
 		return ret;
 	}
 
@@ -12211,10 +12208,7 @@ int __dev_change_net_namespace(struct net_device *dev, struct net *net,
 
 	err = -ENOMEM;
 	if (atomic_dec_if_positive(&net->owner_ve->netif_avail_nr) < 0) {
-		ve_pr_warn_ratelimited(VE_LOG_BOTH,
-			"CT%s: hits max number of network devices, "
-			"increase ve::netif_max_nr parameter\n",
-			ve_name(net->owner_ve));
+		ve_failcount_inc(net->owner_ve, netif);
 		goto out;
 	}
 	atomic_inc(&dev_net(dev)->owner_ve->netif_avail_nr);
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index f90deb17fb25..c7d5e6df50c2 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -520,8 +520,7 @@ static struct neighbour *neigh_alloc(struct neigh_table *tbl,
 	    (glob_entries >= READ_ONCE(tbl->gc_thresh2) &&
 	     time_after(now, READ_ONCE(tbl->last_flush) + 5 * HZ))) {
 		if (!neigh_forced_gc(tbl, ve) && entries >= gc_thresh3) {
-			net_info_ratelimited("%s: neighbor table overflow!\n",
-					     tbl->id);
+			ve_failcount_inc(ve, neigh_tbl);
 			NEIGH_CACHE_STAT_INC(tbl, table_fulls);
 			goto out_entries;
 		}
diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index b3d54cad984a..328e1c96b6f4 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -486,8 +486,10 @@ void net_drop_ns(void *p)
 #ifdef CONFIG_VE
 static int dec_netns_avail(struct ve_struct *ve)
 {
-	if (atomic_dec_if_positive(&ve->netns_avail_nr) < 0)
+	if (atomic_dec_if_positive(&ve->netns_avail_nr) < 0) {
+		ve_failcount_inc(ve, netns);
 		return -ENOSPC;
+	}
 	return 0;
 }
 
-- 
2.47.1

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

Re: [Devel] [PATCH VZ10 v9 6/8] ve: Introduce per-VE failcount

[Vladimir Riabchun]

Sashiko review:

On 9/15/26 19:22, Vladimir Riabchun wrote:
> It may be useful to have a history of resource limit hits for every VE,
> this may simplify debugging and provide some information about the
> resources usage.
> 
> This information is provided by ve.failcount file, any write to it
> resets all failcounts.
> 
> To add a new failcounter we need to create a new atomic_t field
> name_failcount in ve structure and add a new VE_FC_ENTRY in
> ve_failcounts array.
> 
> One change, unrelated to failcounts: aio fields are now initialized
> in ve0.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-135520
> 
> Feature: per-ve failcounters
> Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
> ---
>   fs/aio.c                 |  1 +
>   fs/namespace.c           |  2 ++
>   include/linux/ve.h       | 16 +++++++++
>   kernel/bpf/syscall.c     |  1 +
>   kernel/ve/ve.c           | 70 ++++++++++++++++++++++++++++++++++++++++
>   net/core/dev.c           | 10 ++----
>   net/core/neighbour.c     |  3 +-
>   net/core/net_namespace.c |  4 ++-
>   8 files changed, 96 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/aio.c b/fs/aio.c
> index cb63416af135..4ae06bcd47f2 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -814,6 +814,7 @@ static struct kioctx *ioctx_alloc(unsigned nr_events)
>   	spin_lock(&ve->aio_nr_lock);
>   	if (ve->aio_nr + ctx->max_reqs > ve->aio_max_nr ||
>   	    ve->aio_nr + ctx->max_reqs < ve->aio_nr) {
> +		ve_failcount_inc(ve, aio);
>   		spin_unlock(&ve->aio_nr_lock);
>   		err = -EAGAIN;
>   		goto err_ctx;
> diff --git a/fs/namespace.c b/fs/namespace.c
> index 1d6e66cd7174..ea01664ff5f2 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -3380,6 +3380,8 @@ static inline int ve_try_reserve_mount(struct ve_struct *ve)
>   
>   	if (ret)
>   		get_ve(ve);
> +	else
> +		ve_failcount_inc(ve, mnt);
>   	return ret;
>   }
>   
> diff --git a/include/linux/ve.h b/include/linux/ve.h
> index fabc608e31f5..065129b42768 100644
> --- a/include/linux/ve.h
> +++ b/include/linux/ve.h
> @@ -80,12 +80,15 @@ struct ve_struct {
>   	struct kmapset_key	proc_perms_key;
>   
>   	atomic_t		netns_avail_nr;
> +	atomic_t		netns_failcount;
>   	int			netns_max_nr;
>   
>   	atomic_t		netif_avail_nr;
> +	atomic_t		netif_failcount;
>   	int			netif_max_nr;
>   
>   	atomic_t		bpf_prog_avail_nr;
> +	atomic_t		bpf_prog_failcount;
>   	int			bpf_prog_max_nr;
>   
>   	atomic64_t		_uevent_seqnum;
> @@ -94,6 +97,7 @@ struct ve_struct {
>   
>   	atomic_t		arp_neigh_nr;
>   	atomic_t		nd_neigh_nr;
> +	atomic_t		neigh_tbl_failcount;
>   	unsigned long		meminfo_val;
>   
>   	/*
> @@ -102,6 +106,7 @@ struct ve_struct {
>   	 * other containers.
>   	 */
>   	atomic_t		mnt_avail_nr; /* number of available VE mounts */
> +	atomic_t		mnt_failcount;
>   	int			mnt_max_nr;
>   
>   #ifdef CONFIG_COREDUMP
> @@ -129,6 +134,7 @@ struct ve_struct {
>   	spinlock_t		aio_nr_lock;
>   	unsigned long		aio_nr;
>   	unsigned long		aio_max_nr;
> +	atomic_t		aio_failcount;
>   #endif
>   	struct vfsmount		*devtmpfs_mnt;
>   };
> @@ -151,6 +157,16 @@ extern int nr_ve;
>   #define BPF_PROG_MAX_NR_DEFAULT	256	/* number of loaded BPF progs per-VE */
>   #define MNT_MAX_NR_DEFAULT	4096	/* number of mounts per-VE */
>   
> +#define ve_failcount_inc(ve, name)					\
> +do {									\
> +	struct ve_struct *__ve = (ve);					\
> +									\
> +	atomic_inc(&__ve->name##_failcount);				\
> +	ve_pr_warn_ratelimited(VE_LOG_BOTH, "CT%s: hits the " #name	\
> +		" limit, see the '" #name "' counter in ve.failcount "	\
> +		"of the container's ve cgroup\n", ve_name(__ve));	\
> +} while (0)

Sashiko says:
  How does this handle concurrent modifications? The macro ve_failcount_inc increments
  the failcount atomically, but when combined with the reset operation in
  ve_failcount_write, concurrent operations may lead to race conditions
  affecting data integrity.

No.

> +
>   #ifdef CONFIG_VE
>   static inline void ve_set_state(struct ve_struct *ve, int new_state)
>   {
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index c94d4240e3d3..8498bc9df7ea 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -2891,6 +2891,7 @@ static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size)
>   	if (!bpf_cap && type == BPF_PROG_TYPE_CGROUP_DEVICE) {
>   		load_ve = get_exec_env();
>   		if (atomic_dec_if_positive(&load_ve->bpf_prog_avail_nr) < 0) {
> +			ve_failcount_inc(load_ve, bpf_prog);
>   			load_ve = NULL;
>   			err = -ENOSPC;
>   			goto put_token;
> diff --git a/kernel/ve/ve.c b/kernel/ve/ve.c
> index 5ebdb4b7baa7..9261565208e6 100644
> --- a/kernel/ve/ve.c
> +++ b/kernel/ve/ve.c
> @@ -100,10 +100,13 @@ struct ve_struct ve0 = {
>   	.features		= -1,
>   	.sched_lat_ve.cur	= &ve0_lat_stats,
>   	.netns_avail_nr		= ATOMIC_INIT(INT_MAX),
> +	.netns_failcount	= ATOMIC_INIT(0),
>   	.netns_max_nr		= INT_MAX,
>   	.netif_avail_nr		= ATOMIC_INIT(INT_MAX),
> +	.netif_failcount	= ATOMIC_INIT(0),
>   	.netif_max_nr		= INT_MAX,
>   	.bpf_prog_avail_nr	= ATOMIC_INIT(INT_MAX),
> +	.bpf_prog_failcount	= ATOMIC_INIT(0),
>   	.bpf_prog_max_nr	= INT_MAX,
>   	.fsync_enable		= FSYNC_FILTERED,
>   	._randomize_va_space	=
> @@ -115,8 +118,16 @@ struct ve_struct ve0 = {
>   
>   	.arp_neigh_nr		= ATOMIC_INIT(0),
>   	.nd_neigh_nr		= ATOMIC_INIT(0),
> +	.neigh_tbl_failcount	= ATOMIC_INIT(0),
>   	.mnt_avail_nr		= ATOMIC_INIT(INT_MAX),
>   	.mnt_max_nr		= INT_MAX,
> +	.mnt_failcount		= ATOMIC_INIT(0),
> +#ifdef CONFIG_AIO
> +	.aio_nr_lock		= __SPIN_LOCK_UNLOCKED(aio_nr_lock),
> +	.aio_nr			= 0,
> +	.aio_max_nr		= AIO_MAX_NR_DEFAULT,
> +	.aio_failcount		= ATOMIC_INIT(0),
> +#endif
>   	.meminfo_val		= VE_MEMINFO_SYSTEM,
>   #ifdef CONFIG_X86
>   	.umh_running_helpers	= ATOMIC_INIT(0),
> @@ -787,12 +798,15 @@ static struct cgroup_subsys_state *ve_create(struct cgroup_subsys_state *parent_
>   	ve->fsync_enable = FSYNC_FILTERED;
>   
>   	atomic_set(&ve->netns_avail_nr, NETNS_MAX_NR_DEFAULT);
> +	atomic_set(&ve->netns_failcount, 0);
>   	ve->netns_max_nr = NETNS_MAX_NR_DEFAULT;
>   
>   	atomic_set(&ve->netif_avail_nr, NETIF_MAX_NR_DEFAULT);
> +	atomic_set(&ve->netif_failcount, 0);
>   	ve->netif_max_nr = NETIF_MAX_NR_DEFAULT;
>   
>   	atomic_set(&ve->bpf_prog_avail_nr, BPF_PROG_MAX_NR_DEFAULT);
> +	atomic_set(&ve->bpf_prog_failcount, 0);
>   	ve->bpf_prog_max_nr = BPF_PROG_MAX_NR_DEFAULT;
>   
>   	err = ve_log_init(ve);
> @@ -821,7 +835,9 @@ 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->neigh_tbl_failcount, 0);
>   	ve->mnt_max_nr = MNT_MAX_NR_DEFAULT;
> +	atomic_set(&ve->mnt_failcount, 0);
>   	atomic_set(&ve->mnt_avail_nr, MNT_MAX_NR_DEFAULT);
>   
>   #ifdef CONFIG_COREDUMP
> @@ -834,6 +850,7 @@ static struct cgroup_subsys_state *ve_create(struct cgroup_subsys_state *parent_
>   	spin_lock_init(&ve->aio_nr_lock);
>   	ve->aio_nr = 0;
>   	ve->aio_max_nr = AIO_MAX_NR_DEFAULT;
> +	atomic_set(&ve->aio_failcount, 0);
>   #endif
>   
>   	return &ve->css;
> @@ -1074,6 +1091,53 @@ VE_RESOURCE(mnt);
>   VE_RESOURCE(netif);
>   VE_RESOURCE(bpf_prog);
>   
> +static const struct ve_failcount_entry {
> +	const char *name;
> +	size_t offset;
> +} ve_failcounts[] = {
> +#define VE_FC_ENTRY(name) { #name, offsetof(struct ve_struct, name##_failcount) }
> +	VE_FC_ENTRY(netns),
> +	VE_FC_ENTRY(mnt),
> +	VE_FC_ENTRY(netif),
> +	VE_FC_ENTRY(bpf_prog),
> +	VE_FC_ENTRY(neigh_tbl),
> +#ifdef CONFIG_AIO
> +	VE_FC_ENTRY(aio),
> +#endif
> +	{}
> +};
> +
> +static int ve_failcount_read(struct seq_file *sf, void *v)
> +{
> +	struct ve_struct *ve = css_to_ve(seq_css(sf));
> +	const struct ve_failcount_entry *entry;
> +	atomic_t *fc;
> +
> +	for (entry = ve_failcounts; entry->name; entry++) {
> +		fc = (void *)ve + entry->offset;
> +		seq_printf(sf, "%s: %d\n", entry->name, atomic_read(fc));
> +	}
> +	return 0;
> +}
> +

Sashiko says:
  Could this cause synchronization issues?
  The function ve_failcount_write resets the failcount, potentially leading to
  race conditions as it does not account for concurrent increments of the failcount.
+
  Is there a potential for incorrect reads?
  This loop in the function ve_failcount_read reads the failcount values,
  but does so without synchronization measures to handle concurrent writes,
  which might result in stale or inconsistent data being read.

It uses atomic operations. The only race that could be is parallel read and flush.
Not a big deal - part of failcounts will be 0 and part - original ones.

> +static ssize_t ve_failcount_write(struct kernfs_open_file *of, char *buf,
> +				  size_t nbytes, loff_t off)
> +{
> +	struct ve_struct *ve = css_to_ve(of_css(of));
> +	const struct ve_failcount_entry *entry;
> +	atomic_t *fc;
> +
> +	if (!ve_is_super(get_exec_env()) && !ve->is_pseudosuper)
> +		return -EPERM;
> +
> +	for (entry = ve_failcounts; entry->name; entry++) {
> +		fc = (void *)ve + entry->offset;
> +		atomic_set(fc, 0);
> +	}
> +
> +	return nbytes;
> +}
> +
>   static int ve_os_release_read(struct seq_file *sf, void *v)
>   {
>   	struct cgroup_subsys_state *css = seq_css(sf);
> @@ -1613,6 +1677,12 @@ static struct cftype ve_cftypes[] = {
>   		.flags			= CFTYPE_NOT_ON_ROOT,
>   		.write_u64		= ve_rpc_kill_write,
>   	},
> +	{
> +		.name			= "failcount",
> +		.flags			= CFTYPE_NOT_ON_ROOT,
> +		.seq_show		= ve_failcount_read,
> +		.write			= ve_failcount_write,
> +	},
>   	{ }
>   };
>   
> diff --git a/net/core/dev.c b/net/core/dev.c
> index c7dddb200489..7632ad1fd015 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -10997,10 +10997,7 @@ int register_netdevice(struct net_device *dev)
>   
>   	ret = -ENOMEM;
>   	if (atomic_dec_if_positive(&net->owner_ve->netif_avail_nr) < 0) {
> -		ve_pr_warn_ratelimited(VE_LOG_BOTH,
> -			"CT%s: hits max number of network devices, "
> -			"increase ve::netif_max_nr parameter\n",
> -			ve_name(net->owner_ve));
> +		ve_failcount_inc(net->owner_ve, netif);
>   		return ret;
>   	}
>   
> @@ -12211,10 +12208,7 @@ int __dev_change_net_namespace(struct net_device *dev, struct net *net,
>   
>   	err = -ENOMEM;
>   	if (atomic_dec_if_positive(&net->owner_ve->netif_avail_nr) < 0) {
> -		ve_pr_warn_ratelimited(VE_LOG_BOTH,
> -			"CT%s: hits max number of network devices, "
> -			"increase ve::netif_max_nr parameter\n",
> -			ve_name(net->owner_ve));
> +		ve_failcount_inc(net->owner_ve, netif);
>   		goto out;
>   	}
>   	atomic_inc(&dev_net(dev)->owner_ve->netif_avail_nr);
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index f90deb17fb25..c7d5e6df50c2 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -520,8 +520,7 @@ static struct neighbour *neigh_alloc(struct neigh_table *tbl,
>   	    (glob_entries >= READ_ONCE(tbl->gc_thresh2) &&
>   	     time_after(now, READ_ONCE(tbl->last_flush) + 5 * HZ))) {
>   		if (!neigh_forced_gc(tbl, ve) && entries >= gc_thresh3) {
> -			net_info_ratelimited("%s: neighbor table overflow!\n",
> -					     tbl->id);
> +			ve_failcount_inc(ve, neigh_tbl);
>   			NEIGH_CACHE_STAT_INC(tbl, table_fulls);
>   			goto out_entries;
>   		}
> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
> index b3d54cad984a..328e1c96b6f4 100644
> --- a/net/core/net_namespace.c
> +++ b/net/core/net_namespace.c
> @@ -486,8 +486,10 @@ void net_drop_ns(void *p)
>   #ifdef CONFIG_VE
>   static int dec_netns_avail(struct ve_struct *ve)
>   {
> -	if (atomic_dec_if_positive(&ve->netns_avail_nr) < 0)
> +	if (atomic_dec_if_positive(&ve->netns_avail_nr) < 0) {
> +		ve_failcount_inc(ve, netns);
>   		return -ENOSPC;
> +	}
>   	return 0;
>   }
>   

-- 
Best regards, Riabchun Vladimir
Linux Kernel Developer, Virtuozzo

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

Re: [Devel] [PATCH VZ10 v9 6/8] ve: Introduce per-VE failcount

[Konstantin Khorenko]

i will drop neighbor related failcounter from this patch as it increases the amount of printks inside irq

--
Best regards,

Konstantin Khorenko,
Virtuozzo Linux Kernel Team

On 9/15/26 19:22, Vladimir Riabchun wrote:
> It may be useful to have a history of resource limit hits for every VE,
> this may simplify debugging and provide some information about the
> resources usage.
> 
> This information is provided by ve.failcount file, any write to it
> resets all failcounts.
> 
> To add a new failcounter we need to create a new atomic_t field
> name_failcount in ve structure and add a new VE_FC_ENTRY in
> ve_failcounts array.
> 
> One change, unrelated to failcounts: aio fields are now initialized
> in ve0.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-135520
> 
> Feature: per-ve failcounters
> Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
> ---
>  fs/aio.c                 |  1 +
>  fs/namespace.c           |  2 ++
>  include/linux/ve.h       | 16 +++++++++
>  kernel/bpf/syscall.c     |  1 +
>  kernel/ve/ve.c           | 70 ++++++++++++++++++++++++++++++++++++++++
>  net/core/dev.c           | 10 ++----
>  net/core/neighbour.c     |  3 +-
>  net/core/net_namespace.c |  4 ++-
>  8 files changed, 96 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/aio.c b/fs/aio.c
> index cb63416af135..4ae06bcd47f2 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -814,6 +814,7 @@ static struct kioctx *ioctx_alloc(unsigned nr_events)
>  	spin_lock(&ve->aio_nr_lock);
>  	if (ve->aio_nr + ctx->max_reqs > ve->aio_max_nr ||
>  	    ve->aio_nr + ctx->max_reqs < ve->aio_nr) {
> +		ve_failcount_inc(ve, aio);
>  		spin_unlock(&ve->aio_nr_lock);
>  		err = -EAGAIN;
>  		goto err_ctx;
> diff --git a/fs/namespace.c b/fs/namespace.c
> index 1d6e66cd7174..ea01664ff5f2 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -3380,6 +3380,8 @@ static inline int ve_try_reserve_mount(struct ve_struct *ve)
>  
>  	if (ret)
>  		get_ve(ve);
> +	else
> +		ve_failcount_inc(ve, mnt);
>  	return ret;
>  }
>  
> diff --git a/include/linux/ve.h b/include/linux/ve.h
> index fabc608e31f5..065129b42768 100644
> --- a/include/linux/ve.h
> +++ b/include/linux/ve.h
> @@ -80,12 +80,15 @@ struct ve_struct {
>  	struct kmapset_key	proc_perms_key;
>  
>  	atomic_t		netns_avail_nr;
> +	atomic_t		netns_failcount;
>  	int			netns_max_nr;
>  
>  	atomic_t		netif_avail_nr;
> +	atomic_t		netif_failcount;
>  	int			netif_max_nr;
>  
>  	atomic_t		bpf_prog_avail_nr;
> +	atomic_t		bpf_prog_failcount;
>  	int			bpf_prog_max_nr;
>  
>  	atomic64_t		_uevent_seqnum;
> @@ -94,6 +97,7 @@ struct ve_struct {
>  
>  	atomic_t		arp_neigh_nr;
>  	atomic_t		nd_neigh_nr;
> +	atomic_t		neigh_tbl_failcount;
>  	unsigned long		meminfo_val;
>  
>  	/*
> @@ -102,6 +106,7 @@ struct ve_struct {
>  	 * other containers.
>  	 */
>  	atomic_t		mnt_avail_nr; /* number of available VE mounts */
> +	atomic_t		mnt_failcount;
>  	int			mnt_max_nr;
>  
>  #ifdef CONFIG_COREDUMP
> @@ -129,6 +134,7 @@ struct ve_struct {
>  	spinlock_t		aio_nr_lock;
>  	unsigned long		aio_nr;
>  	unsigned long		aio_max_nr;
> +	atomic_t		aio_failcount;
>  #endif
>  	struct vfsmount		*devtmpfs_mnt;
>  };
> @@ -151,6 +157,16 @@ extern int nr_ve;
>  #define BPF_PROG_MAX_NR_DEFAULT	256	/* number of loaded BPF progs per-VE */
>  #define MNT_MAX_NR_DEFAULT	4096	/* number of mounts per-VE */
>  
> +#define ve_failcount_inc(ve, name)					\
> +do {									\
> +	struct ve_struct *__ve = (ve);					\
> +									\
> +	atomic_inc(&__ve->name##_failcount);				\
> +	ve_pr_warn_ratelimited(VE_LOG_BOTH, "CT%s: hits the " #name	\
> +		" limit, see the '" #name "' counter in ve.failcount "	\
> +		"of the container's ve cgroup\n", ve_name(__ve));	\
> +} while (0)
> +
>  #ifdef CONFIG_VE
>  static inline void ve_set_state(struct ve_struct *ve, int new_state)
>  {
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index c94d4240e3d3..8498bc9df7ea 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -2891,6 +2891,7 @@ static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size)
>  	if (!bpf_cap && type == BPF_PROG_TYPE_CGROUP_DEVICE) {
>  		load_ve = get_exec_env();
>  		if (atomic_dec_if_positive(&load_ve->bpf_prog_avail_nr) < 0) {
> +			ve_failcount_inc(load_ve, bpf_prog);
>  			load_ve = NULL;
>  			err = -ENOSPC;
>  			goto put_token;
> diff --git a/kernel/ve/ve.c b/kernel/ve/ve.c
> index 5ebdb4b7baa7..9261565208e6 100644
> --- a/kernel/ve/ve.c
> +++ b/kernel/ve/ve.c
> @@ -100,10 +100,13 @@ struct ve_struct ve0 = {
>  	.features		= -1,
>  	.sched_lat_ve.cur	= &ve0_lat_stats,
>  	.netns_avail_nr		= ATOMIC_INIT(INT_MAX),
> +	.netns_failcount	= ATOMIC_INIT(0),
>  	.netns_max_nr		= INT_MAX,
>  	.netif_avail_nr		= ATOMIC_INIT(INT_MAX),
> +	.netif_failcount	= ATOMIC_INIT(0),
>  	.netif_max_nr		= INT_MAX,
>  	.bpf_prog_avail_nr	= ATOMIC_INIT(INT_MAX),
> +	.bpf_prog_failcount	= ATOMIC_INIT(0),
>  	.bpf_prog_max_nr	= INT_MAX,
>  	.fsync_enable		= FSYNC_FILTERED,
>  	._randomize_va_space	=
> @@ -115,8 +118,16 @@ struct ve_struct ve0 = {
>  
>  	.arp_neigh_nr		= ATOMIC_INIT(0),
>  	.nd_neigh_nr		= ATOMIC_INIT(0),
> +	.neigh_tbl_failcount	= ATOMIC_INIT(0),
>  	.mnt_avail_nr		= ATOMIC_INIT(INT_MAX),
>  	.mnt_max_nr		= INT_MAX,
> +	.mnt_failcount		= ATOMIC_INIT(0),
> +#ifdef CONFIG_AIO
> +	.aio_nr_lock		= __SPIN_LOCK_UNLOCKED(aio_nr_lock),
> +	.aio_nr			= 0,
> +	.aio_max_nr		= AIO_MAX_NR_DEFAULT,
> +	.aio_failcount		= ATOMIC_INIT(0),
> +#endif
>  	.meminfo_val		= VE_MEMINFO_SYSTEM,
>  #ifdef CONFIG_X86
>  	.umh_running_helpers	= ATOMIC_INIT(0),
> @@ -787,12 +798,15 @@ static struct cgroup_subsys_state *ve_create(struct cgroup_subsys_state *parent_
>  	ve->fsync_enable = FSYNC_FILTERED;
>  
>  	atomic_set(&ve->netns_avail_nr, NETNS_MAX_NR_DEFAULT);
> +	atomic_set(&ve->netns_failcount, 0);
>  	ve->netns_max_nr = NETNS_MAX_NR_DEFAULT;
>  
>  	atomic_set(&ve->netif_avail_nr, NETIF_MAX_NR_DEFAULT);
> +	atomic_set(&ve->netif_failcount, 0);
>  	ve->netif_max_nr = NETIF_MAX_NR_DEFAULT;
>  
>  	atomic_set(&ve->bpf_prog_avail_nr, BPF_PROG_MAX_NR_DEFAULT);
> +	atomic_set(&ve->bpf_prog_failcount, 0);
>  	ve->bpf_prog_max_nr = BPF_PROG_MAX_NR_DEFAULT;
>  
>  	err = ve_log_init(ve);
> @@ -821,7 +835,9 @@ 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->neigh_tbl_failcount, 0);
>  	ve->mnt_max_nr = MNT_MAX_NR_DEFAULT;
> +	atomic_set(&ve->mnt_failcount, 0);
>  	atomic_set(&ve->mnt_avail_nr, MNT_MAX_NR_DEFAULT);
>  
>  #ifdef CONFIG_COREDUMP
> @@ -834,6 +850,7 @@ static struct cgroup_subsys_state *ve_create(struct cgroup_subsys_state *parent_
>  	spin_lock_init(&ve->aio_nr_lock);
>  	ve->aio_nr = 0;
>  	ve->aio_max_nr = AIO_MAX_NR_DEFAULT;
> +	atomic_set(&ve->aio_failcount, 0);
>  #endif
>  
>  	return &ve->css;
> @@ -1074,6 +1091,53 @@ VE_RESOURCE(mnt);
>  VE_RESOURCE(netif);
>  VE_RESOURCE(bpf_prog);
>  
> +static const struct ve_failcount_entry {
> +	const char *name;
> +	size_t offset;
> +} ve_failcounts[] = {
> +#define VE_FC_ENTRY(name) { #name, offsetof(struct ve_struct, name##_failcount) }
> +	VE_FC_ENTRY(netns),
> +	VE_FC_ENTRY(mnt),
> +	VE_FC_ENTRY(netif),
> +	VE_FC_ENTRY(bpf_prog),
> +	VE_FC_ENTRY(neigh_tbl),
> +#ifdef CONFIG_AIO
> +	VE_FC_ENTRY(aio),
> +#endif
> +	{}
> +};
> +
> +static int ve_failcount_read(struct seq_file *sf, void *v)
> +{
> +	struct ve_struct *ve = css_to_ve(seq_css(sf));
> +	const struct ve_failcount_entry *entry;
> +	atomic_t *fc;
> +
> +	for (entry = ve_failcounts; entry->name; entry++) {
> +		fc = (void *)ve + entry->offset;
> +		seq_printf(sf, "%s: %d\n", entry->name, atomic_read(fc));
> +	}
> +	return 0;
> +}
> +
> +static ssize_t ve_failcount_write(struct kernfs_open_file *of, char *buf,
> +				  size_t nbytes, loff_t off)
> +{
> +	struct ve_struct *ve = css_to_ve(of_css(of));
> +	const struct ve_failcount_entry *entry;
> +	atomic_t *fc;
> +
> +	if (!ve_is_super(get_exec_env()) && !ve->is_pseudosuper)
> +		return -EPERM;
> +
> +	for (entry = ve_failcounts; entry->name; entry++) {
> +		fc = (void *)ve + entry->offset;
> +		atomic_set(fc, 0);
> +	}
> +
> +	return nbytes;
> +}
> +
>  static int ve_os_release_read(struct seq_file *sf, void *v)
>  {
>  	struct cgroup_subsys_state *css = seq_css(sf);
> @@ -1613,6 +1677,12 @@ static struct cftype ve_cftypes[] = {
>  		.flags			= CFTYPE_NOT_ON_ROOT,
>  		.write_u64		= ve_rpc_kill_write,
>  	},
> +	{
> +		.name			= "failcount",
> +		.flags			= CFTYPE_NOT_ON_ROOT,
> +		.seq_show		= ve_failcount_read,
> +		.write			= ve_failcount_write,
> +	},
>  	{ }
>  };
>  
> diff --git a/net/core/dev.c b/net/core/dev.c
> index c7dddb200489..7632ad1fd015 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -10997,10 +10997,7 @@ int register_netdevice(struct net_device *dev)
>  
>  	ret = -ENOMEM;
>  	if (atomic_dec_if_positive(&net->owner_ve->netif_avail_nr) < 0) {
> -		ve_pr_warn_ratelimited(VE_LOG_BOTH,
> -			"CT%s: hits max number of network devices, "
> -			"increase ve::netif_max_nr parameter\n",
> -			ve_name(net->owner_ve));
> +		ve_failcount_inc(net->owner_ve, netif);
>  		return ret;
>  	}
>  
> @@ -12211,10 +12208,7 @@ int __dev_change_net_namespace(struct net_device *dev, struct net *net,
>  
>  	err = -ENOMEM;
>  	if (atomic_dec_if_positive(&net->owner_ve->netif_avail_nr) < 0) {
> -		ve_pr_warn_ratelimited(VE_LOG_BOTH,
> -			"CT%s: hits max number of network devices, "
> -			"increase ve::netif_max_nr parameter\n",
> -			ve_name(net->owner_ve));
> +		ve_failcount_inc(net->owner_ve, netif);
>  		goto out;
>  	}
>  	atomic_inc(&dev_net(dev)->owner_ve->netif_avail_nr);
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index f90deb17fb25..c7d5e6df50c2 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -520,8 +520,7 @@ static struct neighbour *neigh_alloc(struct neigh_table *tbl,
>  	    (glob_entries >= READ_ONCE(tbl->gc_thresh2) &&
>  	     time_after(now, READ_ONCE(tbl->last_flush) + 5 * HZ))) {
>  		if (!neigh_forced_gc(tbl, ve) && entries >= gc_thresh3) {
> -			net_info_ratelimited("%s: neighbor table overflow!\n",
> -					     tbl->id);
> +			ve_failcount_inc(ve, neigh_tbl);
>  			NEIGH_CACHE_STAT_INC(tbl, table_fulls);
>  			goto out_entries;
>  		}
> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
> index b3d54cad984a..328e1c96b6f4 100644
> --- a/net/core/net_namespace.c
> +++ b/net/core/net_namespace.c
> @@ -486,8 +486,10 @@ void net_drop_ns(void *p)
>  #ifdef CONFIG_VE
>  static int dec_netns_avail(struct ve_struct *ve)
>  {
> -	if (atomic_dec_if_positive(&ve->netns_avail_nr) < 0)
> +	if (atomic_dec_if_positive(&ve->netns_avail_nr) < 0) {
> +		ve_failcount_inc(ve, netns);
>  		return -ENOSPC;
> +	}
>  	return 0;
>  }
>  

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

Re: [Devel] [PATCH VZ10 v9 6/8] ve: Introduce per-VE failcount

[Konstantin Khorenko]

On 9/15/26 19:22, Vladimir Riabchun wrote:
> It may be useful to have a history of resource limit hits for every VE,
> this may simplify debugging and provide some information about the
> resources usage.
> 
> This information is provided by ve.failcount file, any write to it
> resets all failcounts.
> 
> To add a new failcounter we need to create a new atomic_t field
> name_failcount in ve structure and add a new VE_FC_ENTRY in
> ve_failcounts array.
> 
> One change, unrelated to failcounts: aio fields are now initialized
> in ve0.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-135520
> 
> Feature: per-ve failcounters
> Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
> ---
>  fs/aio.c                 |  1 +
>  fs/namespace.c           |  2 ++
>  include/linux/ve.h       | 16 +++++++++
>  kernel/bpf/syscall.c     |  1 +
>  kernel/ve/ve.c           | 70 ++++++++++++++++++++++++++++++++++++++++
>  net/core/dev.c           | 10 ++----
>  net/core/neighbour.c     |  3 +-
>  net/core/net_namespace.c |  4 ++-
>  8 files changed, 96 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/aio.c b/fs/aio.c
> index cb63416af135..4ae06bcd47f2 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -814,6 +814,7 @@ static struct kioctx *ioctx_alloc(unsigned nr_events)
>  	spin_lock(&ve->aio_nr_lock);
>  	if (ve->aio_nr + ctx->max_reqs > ve->aio_max_nr ||
>  	    ve->aio_nr + ctx->max_reqs < ve->aio_nr) {
> +		ve_failcount_inc(ve, aio);
>  		spin_unlock(&ve->aio_nr_lock);
>  		err = -EAGAIN;
>  		goto err_ctx;
> diff --git a/fs/namespace.c b/fs/namespace.c
> index 1d6e66cd7174..ea01664ff5f2 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -3380,6 +3380,8 @@ static inline int ve_try_reserve_mount(struct ve_struct *ve)
>  
>  	if (ret)
>  		get_ve(ve);
> +	else
> +		ve_failcount_inc(ve, mnt);
>  	return ret;
>  }

  Now every failed mount produces two ratelimited warnings in the host
  log: the pre-existing one in alloc_vfsmnt()

        pr_warn_ratelimited("CT#%s reached the limit on mounts.\n", ...);

  and the one printed by ve_failcount_inc(). They are ratelimited
  independently, so the log gets both messages for the same event.

  Please drop the alloc_vfsmnt() one in this patch: the
  ve_failcount_inc() message carries strictly more information (it also
  names the counter to look at) and additionally goes to the container
  log.

>  
> diff --git a/include/linux/ve.h b/include/linux/ve.h
> index fabc608e31f5..065129b42768 100644
> --- a/include/linux/ve.h
> +++ b/include/linux/ve.h
> @@ -80,12 +80,15 @@ struct ve_struct {
>  	struct kmapset_key	proc_perms_key;
>  
>  	atomic_t		netns_avail_nr;
> +	atomic_t		netns_failcount;
>  	int			netns_max_nr;
>  
>  	atomic_t		netif_avail_nr;
> +	atomic_t		netif_failcount;
>  	int			netif_max_nr;
>  
>  	atomic_t		bpf_prog_avail_nr;
> +	atomic_t		bpf_prog_failcount;
>  	int			bpf_prog_max_nr;
>  
>  	atomic64_t		_uevent_seqnum;
> @@ -94,6 +97,7 @@ struct ve_struct {
>  
>  	atomic_t		arp_neigh_nr;
>  	atomic_t		nd_neigh_nr;
> +	atomic_t		neigh_tbl_failcount;
>  	unsigned long		meminfo_val;
>  
>  	/*
> @@ -102,6 +106,7 @@ struct ve_struct {
>  	 * other containers.
>  	 */
>  	atomic_t		mnt_avail_nr; /* number of available VE mounts */
> +	atomic_t		mnt_failcount;
>  	int			mnt_max_nr;
>  
>  #ifdef CONFIG_COREDUMP
> @@ -129,6 +134,7 @@ struct ve_struct {
>  	spinlock_t		aio_nr_lock;
>  	unsigned long		aio_nr;
>  	unsigned long		aio_max_nr;
> +	atomic_t		aio_failcount;
>  #endif
>  	struct vfsmount		*devtmpfs_mnt;
>  };
> @@ -151,6 +157,16 @@ extern int nr_ve;
>  #define BPF_PROG_MAX_NR_DEFAULT	256	/* number of loaded BPF progs per-VE */
>  #define MNT_MAX_NR_DEFAULT	4096	/* number of mounts per-VE */
>  
> +#define ve_failcount_inc(ve, name)					\
> +do {									\
> +	struct ve_struct *__ve = (ve);					\
> +									\
> +	atomic_inc(&__ve->name##_failcount);				\
> +	ve_pr_warn_ratelimited(VE_LOG_BOTH, "CT%s: hits the " #name	\
> +		" limit, see the '" #name "' counter in ve.failcount "	\
> +		"of the container's ve cgroup\n", ve_name(__ve));	\
> +} while (0)
> +
>  #ifdef CONFIG_VE
>  static inline void ve_set_state(struct ve_struct *ve, int new_state)
>  {
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index c94d4240e3d3..8498bc9df7ea 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -2891,6 +2891,7 @@ static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size)
>  	if (!bpf_cap && type == BPF_PROG_TYPE_CGROUP_DEVICE) {
>  		load_ve = get_exec_env();
>  		if (atomic_dec_if_positive(&load_ve->bpf_prog_avail_nr) < 0) {
> +			ve_failcount_inc(load_ve, bpf_prog);
>  			load_ve = NULL;
>  			err = -ENOSPC;
>  			goto put_token;
> diff --git a/kernel/ve/ve.c b/kernel/ve/ve.c
> index 5ebdb4b7baa7..9261565208e6 100644
> --- a/kernel/ve/ve.c
> +++ b/kernel/ve/ve.c
> @@ -100,10 +100,13 @@ struct ve_struct ve0 = {
>  	.features		= -1,
>  	.sched_lat_ve.cur	= &ve0_lat_stats,
>  	.netns_avail_nr		= ATOMIC_INIT(INT_MAX),
> +	.netns_failcount	= ATOMIC_INIT(0),
>  	.netns_max_nr		= INT_MAX,
>  	.netif_avail_nr		= ATOMIC_INIT(INT_MAX),
> +	.netif_failcount	= ATOMIC_INIT(0),
>  	.netif_max_nr		= INT_MAX,
>  	.bpf_prog_avail_nr	= ATOMIC_INIT(INT_MAX),
> +	.bpf_prog_failcount	= ATOMIC_INIT(0),
>  	.bpf_prog_max_nr	= INT_MAX,
>  	.fsync_enable		= FSYNC_FILTERED,
>  	._randomize_va_space	=
> @@ -115,8 +118,16 @@ struct ve_struct ve0 = {
>  
>  	.arp_neigh_nr		= ATOMIC_INIT(0),
>  	.nd_neigh_nr		= ATOMIC_INIT(0),
> +	.neigh_tbl_failcount	= ATOMIC_INIT(0),
>  	.mnt_avail_nr		= ATOMIC_INIT(INT_MAX),
>  	.mnt_max_nr		= INT_MAX,
> +	.mnt_failcount		= ATOMIC_INIT(0),
> +#ifdef CONFIG_AIO
> +	.aio_nr_lock		= __SPIN_LOCK_UNLOCKED(aio_nr_lock),
> +	.aio_nr			= 0,
> +	.aio_max_nr		= AIO_MAX_NR_DEFAULT,
> +	.aio_failcount		= ATOMIC_INIT(0),
> +#endif
>  	.meminfo_val		= VE_MEMINFO_SYSTEM,
>  #ifdef CONFIG_X86
>  	.umh_running_helpers	= ATOMIC_INIT(0),
> @@ -787,12 +798,15 @@ static struct cgroup_subsys_state *ve_create(struct cgroup_subsys_state *parent_
>  	ve->fsync_enable = FSYNC_FILTERED;
>  
>  	atomic_set(&ve->netns_avail_nr, NETNS_MAX_NR_DEFAULT);
> +	atomic_set(&ve->netns_failcount, 0);
>  	ve->netns_max_nr = NETNS_MAX_NR_DEFAULT;
>  
>  	atomic_set(&ve->netif_avail_nr, NETIF_MAX_NR_DEFAULT);
> +	atomic_set(&ve->netif_failcount, 0);
>  	ve->netif_max_nr = NETIF_MAX_NR_DEFAULT;
>  
>  	atomic_set(&ve->bpf_prog_avail_nr, BPF_PROG_MAX_NR_DEFAULT);
> +	atomic_set(&ve->bpf_prog_failcount, 0);
>  	ve->bpf_prog_max_nr = BPF_PROG_MAX_NR_DEFAULT;
>  
>  	err = ve_log_init(ve);
> @@ -821,7 +835,9 @@ 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->neigh_tbl_failcount, 0);
>  	ve->mnt_max_nr = MNT_MAX_NR_DEFAULT;
> +	atomic_set(&ve->mnt_failcount, 0);
>  	atomic_set(&ve->mnt_avail_nr, MNT_MAX_NR_DEFAULT);
>  
>  #ifdef CONFIG_COREDUMP
> @@ -834,6 +850,7 @@ static struct cgroup_subsys_state *ve_create(struct cgroup_subsys_state *parent_
>  	spin_lock_init(&ve->aio_nr_lock);
>  	ve->aio_nr = 0;
>  	ve->aio_max_nr = AIO_MAX_NR_DEFAULT;
> +	atomic_set(&ve->aio_failcount, 0);
>  #endif
>  
>  	return &ve->css;
> @@ -1074,6 +1091,53 @@ VE_RESOURCE(mnt);
>  VE_RESOURCE(netif);
>  VE_RESOURCE(bpf_prog);
>  
> +static const struct ve_failcount_entry {
> +	const char *name;
> +	size_t offset;
> +} ve_failcounts[] = {
> +#define VE_FC_ENTRY(name) { #name, offsetof(struct ve_struct, name##_failcount) }
> +	VE_FC_ENTRY(netns),
> +	VE_FC_ENTRY(mnt),
> +	VE_FC_ENTRY(netif),
> +	VE_FC_ENTRY(bpf_prog),
> +	VE_FC_ENTRY(neigh_tbl),
> +#ifdef CONFIG_AIO
> +	VE_FC_ENTRY(aio),
> +#endif
> +	{}
> +};
> +
> +static int ve_failcount_read(struct seq_file *sf, void *v)
> +{
> +	struct ve_struct *ve = css_to_ve(seq_css(sf));
> +	const struct ve_failcount_entry *entry;
> +	atomic_t *fc;
> +
> +	for (entry = ve_failcounts; entry->name; entry++) {
> +		fc = (void *)ve + entry->offset;
> +		seq_printf(sf, "%s: %d\n", entry->name, atomic_read(fc));
> +	}
> +	return 0;
> +}
> +
> +static ssize_t ve_failcount_write(struct kernfs_open_file *of, char *buf,
> +				  size_t nbytes, loff_t off)
> +{
> +	struct ve_struct *ve = css_to_ve(of_css(of));
> +	const struct ve_failcount_entry *entry;
> +	atomic_t *fc;
> +
> +	if (!ve_is_super(get_exec_env()) && !ve->is_pseudosuper)
> +		return -EPERM;
> +
> +	for (entry = ve_failcounts; entry->name; entry++) {
> +		fc = (void *)ve + entry->offset;
> +		atomic_set(fc, 0);
> +	}
> +
> +	return nbytes;
> +}
> +
>  static int ve_os_release_read(struct seq_file *sf, void *v)
>  {
>  	struct cgroup_subsys_state *css = seq_css(sf);
> @@ -1613,6 +1677,12 @@ static struct cftype ve_cftypes[] = {
>  		.flags			= CFTYPE_NOT_ON_ROOT,
>  		.write_u64		= ve_rpc_kill_write,
>  	},
> +	{
> +		.name			= "failcount",
> +		.flags			= CFTYPE_NOT_ON_ROOT,
> +		.seq_show		= ve_failcount_read,
> +		.write			= ve_failcount_write,
> +	},
>  	{ }
>  };
>  
> diff --git a/net/core/dev.c b/net/core/dev.c
> index c7dddb200489..7632ad1fd015 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -10997,10 +10997,7 @@ int register_netdevice(struct net_device *dev)
>  
>  	ret = -ENOMEM;
>  	if (atomic_dec_if_positive(&net->owner_ve->netif_avail_nr) < 0) {
> -		ve_pr_warn_ratelimited(VE_LOG_BOTH,
> -			"CT%s: hits max number of network devices, "
> -			"increase ve::netif_max_nr parameter\n",
> -			ve_name(net->owner_ve));
> +		ve_failcount_inc(net->owner_ve, netif);
>  		return ret;
>  	}
>  
> @@ -12211,10 +12208,7 @@ int __dev_change_net_namespace(struct net_device *dev, struct net *net,
>  
>  	err = -ENOMEM;
>  	if (atomic_dec_if_positive(&net->owner_ve->netif_avail_nr) < 0) {
> -		ve_pr_warn_ratelimited(VE_LOG_BOTH,
> -			"CT%s: hits max number of network devices, "
> -			"increase ve::netif_max_nr parameter\n",
> -			ve_name(net->owner_ve));
> +		ve_failcount_inc(net->owner_ve, netif);
>  		goto out;
>  	}
>  	atomic_inc(&dev_net(dev)->owner_ve->netif_avail_nr);
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index f90deb17fb25..c7d5e6df50c2 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -520,8 +520,7 @@ static struct neighbour *neigh_alloc(struct neigh_table *tbl,
>  	    (glob_entries >= READ_ONCE(tbl->gc_thresh2) &&
>  	     time_after(now, READ_ONCE(tbl->last_flush) + 5 * HZ))) {
>  		if (!neigh_forced_gc(tbl, ve) && entries >= gc_thresh3) {
> -			net_info_ratelimited("%s: neighbor table overflow!\n",
> -					     tbl->id);
> +			ve_failcount_inc(ve, neigh_tbl);
>  			NEIGH_CACHE_STAT_INC(tbl, table_fulls);
>  			goto out_entries;
>  		}
> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
> index b3d54cad984a..328e1c96b6f4 100644
> --- a/net/core/net_namespace.c
> +++ b/net/core/net_namespace.c
> @@ -486,8 +486,10 @@ void net_drop_ns(void *p)
>  #ifdef CONFIG_VE
>  static int dec_netns_avail(struct ve_struct *ve)
>  {
> -	if (atomic_dec_if_positive(&ve->netns_avail_nr) < 0)
> +	if (atomic_dec_if_positive(&ve->netns_avail_nr) < 0) {
> +		ve_failcount_inc(ve, netns);
>  		return -ENOSPC;
> +	}
>  	return 0;
>  }
>  

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

[Devel] [PATCH VZ10 v9 7/8] ve/printk: Don't use ve log outside task context

[Vladimir Riabchun]

get_exec_env() in interrupt context will return VE of the process
that was interrupted.
We don't want to leak logs to unrelated containers if we use
ve_printk in a function that may be called in an interrupt.

https://virtuozzo.atlassian.net/browse/VSTOR-135520
Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
---
 kernel/printk/printk.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 8940b36e790b..90ecd7960800 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -560,7 +560,8 @@ struct log_state *ve_log_state(void)
 {
 	struct log_state *log = &init_log_state;
 #ifdef CONFIG_VE
-	if (get_exec_env()->log_state)
+	/* get_exec_env() is not reliable outside task context */
+	if (get_exec_env()->log_state && in_task())
 		log = get_exec_env()->log_state;
 #endif
 	return log;
@@ -2538,7 +2539,7 @@ static asmlinkage int ve_vprintk(int dst, const char *fmt, va_list args)
 	va_copy(args2, args);
 	if (ve_is_super(get_exec_env()) || (dst & VE0_LOG))
 		r = vprintk(fmt, args);
-	if (!ve_is_super(get_exec_env()) && (dst & VE_LOG))
+	if (!ve_is_super(get_exec_env()) && (dst & VE_LOG) && in_task())
 		r = ve_vprintk_emit(0, LOGLEVEL_DEFAULT, NULL, fmt, args2);
 	va_end(args2);
 
@@ -2570,7 +2571,7 @@ static asmlinkage int ve_vprintk_deferred(int dst, const char *fmt, va_list args
 	va_copy(args2, args);
 	if (ve_is_super(get_exec_env()) || (dst & VE0_LOG))
 		r = vprintk_emit(0, LOGLEVEL_SCHED, NULL, fmt, args);
-	if (!ve_is_super(get_exec_env()) && (dst & VE_LOG))
+	if (!ve_is_super(get_exec_env()) && (dst & VE_LOG) && in_task())
 		r = ve_vprintk_emit(0, LOGLEVEL_SCHED, NULL, fmt, args2);
 	va_end(args2);
 
-- 
2.47.1

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

Re: [Devel] [PATCH VZ10 v9 7/8] ve/printk: Don't use ve log outside task context

[Vladimir Riabchun]

Sashiko review:

On 9/15/26 19:22, Vladimir Riabchun wrote:
> get_exec_env() in interrupt context will return VE of the process
> that was interrupted.
> We don't want to leak logs to unrelated containers if we use
> ve_printk in a function that may be called in an interrupt.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-135520
> Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
> ---
>   kernel/printk/printk.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 8940b36e790b..90ecd7960800 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -560,7 +560,8 @@ struct log_state *ve_log_state(void)
>   {
>   	struct log_state *log = &init_log_state;
>   #ifdef CONFIG_VE
> -	if (get_exec_env()->log_state)
> +	/* get_exec_env() is not reliable outside task context */
> +	if (get_exec_env()->log_state && in_task())

Sashiko says:
  This is a pre-existing issue, but can the condition in "ve_log_state()" at
  "if (get_exec_env()->log_state && in_task())" lead to incorrectly accessing
  log state outside of task contexts, if callers aren't properly enforcing
  the in_task() guarantee?

This is exactly what the patch is doing: this updated check ensures
that even if somebody forgot to enforce this check or we somehow reached
this from interrupt, we leak nothing to CT.

>   		log = get_exec_env()->log_state;
>   #endif
>   	return log;
> @@ -2538,7 +2539,7 @@ static asmlinkage int ve_vprintk(int dst, const char *fmt, va_list args)
>   	va_copy(args2, args);
>   	if (ve_is_super(get_exec_env()) || (dst & VE0_LOG))
>   		r = vprintk(fmt, args);
> -	if (!ve_is_super(get_exec_env()) && (dst & VE_LOG))
> +	if (!ve_is_super(get_exec_env()) && (dst & VE_LOG) && in_task())

Sashiko says:
  In "ve_vprintk", is it possible that the "ve_vprintk_emit()" call could cause
  a use-after-free scenario given that it lacks an in_task() check,
  unlike similar calls that were updated in the patch?
  Could accessing "get_exec_env()->log_state" without this check lead to
  accessing unstable memory, causing kernel panics?

ve_vprintk_emit uses ve_log_state helper, so it has in_task() check.

>   		r = ve_vprintk_emit(0, LOGLEVEL_DEFAULT, NULL, fmt, args2);
>   	va_end(args2);
>   
> @@ -2570,7 +2571,7 @@ static asmlinkage int ve_vprintk_deferred(int dst, const char *fmt, va_list args
>   	va_copy(args2, args);
>   	if (ve_is_super(get_exec_env()) || (dst & VE0_LOG))
>   		r = vprintk_emit(0, LOGLEVEL_SCHED, NULL, fmt, args);
> -	if (!ve_is_super(get_exec_env()) && (dst & VE_LOG))
> +	if (!ve_is_super(get_exec_env()) && (dst & VE_LOG) && in_task())
>   		r = ve_vprintk_emit(0, LOGLEVEL_SCHED, NULL, fmt, args2);
>   	va_end(args2);
>   

-- 
Best regards, Riabchun Vladimir
Linux Kernel Developer, Virtuozzo

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

Re: [Devel] [PATCH VZ10 v9 7/8] ve/printk: Don't use ve log outside task context

[Vladimir Riabchun]

Please, don't take it with this series.

The problem exists without the patches and we'd better add
a proper fix in a separate series under the task VSTOR-146839.

On 9/15/26 19:22, Vladimir Riabchun wrote:
> get_exec_env() in interrupt context will return VE of the process
> that was interrupted.
> We don't want to leak logs to unrelated containers if we use
> ve_printk in a function that may be called in an interrupt.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-135520
> Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
> ---
>   kernel/printk/printk.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 8940b36e790b..90ecd7960800 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -560,7 +560,8 @@ struct log_state *ve_log_state(void)
>   {
>   	struct log_state *log = &init_log_state;
>   #ifdef CONFIG_VE
> -	if (get_exec_env()->log_state)
> +	/* get_exec_env() is not reliable outside task context */
> +	if (get_exec_env()->log_state && in_task())
>   		log = get_exec_env()->log_state;
>   #endif
>   	return log;
> @@ -2538,7 +2539,7 @@ static asmlinkage int ve_vprintk(int dst, const char *fmt, va_list args)
>   	va_copy(args2, args);
>   	if (ve_is_super(get_exec_env()) || (dst & VE0_LOG))
>   		r = vprintk(fmt, args);
> -	if (!ve_is_super(get_exec_env()) && (dst & VE_LOG))
> +	if (!ve_is_super(get_exec_env()) && (dst & VE_LOG) && in_task())
>   		r = ve_vprintk_emit(0, LOGLEVEL_DEFAULT, NULL, fmt, args2);
>   	va_end(args2);
>   
> @@ -2570,7 +2571,7 @@ static asmlinkage int ve_vprintk_deferred(int dst, const char *fmt, va_list args
>   	va_copy(args2, args);
>   	if (ve_is_super(get_exec_env()) || (dst & VE0_LOG))
>   		r = vprintk_emit(0, LOGLEVEL_SCHED, NULL, fmt, args);
> -	if (!ve_is_super(get_exec_env()) && (dst & VE_LOG))
> +	if (!ve_is_super(get_exec_env()) && (dst & VE_LOG) && in_task())
>   		r = ve_vprintk_emit(0, LOGLEVEL_SCHED, NULL, fmt, args2);
>   	va_end(args2);
>   

-- 
Best regards, Riabchun Vladimir
Linux Kernel Developer, Virtuozzo

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

Re: [Devel] [PATCH VZ10 v9 7/8] ve/printk: Don't use ve log outside task context

[Konstantin Khorenko]

On 9/22/26 13:21, Vladimir Riabchun wrote:
> Please, don't take it with this series.
> 
> The problem exists without the patches and we'd better add
> a proper fix in a separate series under the task VSTOR-146839.

ok, i won't apply this patch as well as i will drop neighbor related failcounter from the previous patch as it increases the amount of printks inside irq.

> 
> On 9/15/26 19:22, Vladimir Riabchun wrote:
>> get_exec_env() in interrupt context will return VE of the process
>> that was interrupted.
>> We don't want to leak logs to unrelated containers if we use
>> ve_printk in a function that may be called in an interrupt.
>>
>> https://virtuozzo.atlassian.net/browse/VSTOR-135520
>> Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
>> ---
>>   kernel/printk/printk.c | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
>> index 8940b36e790b..90ecd7960800 100644
>> --- a/kernel/printk/printk.c
>> +++ b/kernel/printk/printk.c
>> @@ -560,7 +560,8 @@ struct log_state *ve_log_state(void)
>>   {
>>   	struct log_state *log = &init_log_state;
>>   #ifdef CONFIG_VE
>> -	if (get_exec_env()->log_state)
>> +	/* get_exec_env() is not reliable outside task context */
>> +	if (get_exec_env()->log_state && in_task())
>>   		log = get_exec_env()->log_state;
>>   #endif
>>   	return log;
>> @@ -2538,7 +2539,7 @@ static asmlinkage int ve_vprintk(int dst, const char *fmt, va_list args)
>>   	va_copy(args2, args);
>>   	if (ve_is_super(get_exec_env()) || (dst & VE0_LOG))
>>   		r = vprintk(fmt, args);
>> -	if (!ve_is_super(get_exec_env()) && (dst & VE_LOG))
>> +	if (!ve_is_super(get_exec_env()) && (dst & VE_LOG) && in_task())
>>   		r = ve_vprintk_emit(0, LOGLEVEL_DEFAULT, NULL, fmt, args2);
>>   	va_end(args2);
>>   
>> @@ -2570,7 +2571,7 @@ static asmlinkage int ve_vprintk_deferred(int dst, const char *fmt, va_list args
>>   	va_copy(args2, args);
>>   	if (ve_is_super(get_exec_env()) || (dst & VE0_LOG))
>>   		r = vprintk_emit(0, LOGLEVEL_SCHED, NULL, fmt, args);
>> -	if (!ve_is_super(get_exec_env()) && (dst & VE_LOG))
>> +	if (!ve_is_super(get_exec_env()) && (dst & VE_LOG) && in_task())
>>   		r = ve_vprintk_emit(0, LOGLEVEL_SCHED, NULL, fmt, args2);
>>   	va_end(args2);
>>   
> 

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

[Devel] [PATCH VZ10 v9 8/8] selftests/ve: Add mount accounting selftest

[Vladimir Riabchun]

There are 6 test cases, covered in the new test:
1. Simple mount accouting correctness, just mount/umount.
2. Verification of correct limit hits and changes, including
   negative values.
3. Partial mounts test, when mount limit is hit in the middle
   of creation.
4. Test that enabled pseudosuper allows overuse.
5. Test that pseudosuper doesn't affect mount accounting.
6. Failcount feature verification.

https://virtuozzo.atlassian.net/browse/VSTOR-135520

Feature: per-ve failcounters
Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
---
 tools/testing/selftests/ve/.gitignore         |   1 +
 tools/testing/selftests/ve/Makefile           |   1 +
 .../selftests/ve/ve_mount_accounting_test.c   | 550 ++++++++++++++++++
 tools/testing/selftests/ve/ve_selftest.h      |  21 +
 4 files changed, 573 insertions(+)
 create mode 100644 tools/testing/selftests/ve/ve_mount_accounting_test.c

diff --git a/tools/testing/selftests/ve/.gitignore b/tools/testing/selftests/ve/.gitignore
index afa4c568c2c9..3df4d05888dc 100644
--- a/tools/testing/selftests/ve/.gitignore
+++ b/tools/testing/selftests/ve/.gitignore
@@ -1,2 +1,3 @@
 ve_ns_owner_test
 ve_perms_test
+ve_mount_accounting_test
diff --git a/tools/testing/selftests/ve/Makefile b/tools/testing/selftests/ve/Makefile
index ec40cbc7b3a1..c6efe7c4b4fb 100644
--- a/tools/testing/selftests/ve/Makefile
+++ b/tools/testing/selftests/ve/Makefile
@@ -4,5 +4,6 @@ CFLAGS += -g -Wall -O2
 
 TEST_GEN_PROGS += ve_ns_owner_test
 TEST_GEN_PROGS += ve_perms_test
+TEST_GEN_PROGS += ve_mount_accounting_test
 
 include ../lib.mk
diff --git a/tools/testing/selftests/ve/ve_mount_accounting_test.c b/tools/testing/selftests/ve/ve_mount_accounting_test.c
new file mode 100644
index 000000000000..6ba13dcdcc87
--- /dev/null
+++ b/tools/testing/selftests/ve/ve_mount_accounting_test.c
@@ -0,0 +1,550 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ve_mount_accounting selftests
+ *
+ * Tests to check the correctness of mount accounting.
+ */
+#define _GNU_SOURCE
+#include <asm/unistd.h>
+#include <linux/sched.h>
+#include <linux/limits.h>
+#include <sys/wait.h>
+#include <sys/syscall.h>
+#include <sys/stat.h>
+#include <sys/mount.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <sched.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <string.h>
+#include <time.h>
+
+#include "../kselftest_harness.h"
+#include "ve_selftest.h"
+
+#define TMP_DIR			"/ve-mnt-tmp/"
+#define VE_MOUNTS_MAX		128
+
+/*
+ * These values take a while to calculate, the first test will
+ * do this and store the values for those who come after.
+ */
+int *need_avail;
+
+FIXTURE(ve_mnt_acc)
+{
+	int cgv2_fd;
+	int ctid;
+	struct namespace_ctx ns_ctx;
+	int orig_have_mnt;
+};
+
+/*
+ * free_vfsmnt is called via call_rcu, so we need to ensure
+ * that all pending call_rcu tasks are finished.
+ * However, one barrier is not enough:
+ * mntput_no_expire
+ *   init_task_work(&mnt->mnt_rcu, __cleanup_mnt);
+ *   task_work_add(task, &mnt->mnt_rcu, TWA_RESUME)
+ *     OR
+ *   schedule_delayed_work(&delayed_mntput_work, 1)
+ *   # Both later call cleanup_mnt
+ *     cleanup_mnt
+ *       call_rcu(&mnt->mnt_rcu, delayed_free_vfsmnt);
+ *
+ * We can't enforce workqueue flush from userspace, so instead
+ * read several times until the value is the one we expect.
+ */
+static int wait_on_mnt_avail(FIXTURE_DATA(ve_mnt_acc) * self, int expected)
+{
+#define WAIT_TIMEOUT_NS		(4096UL * 1000 * 1000)
+	char path_avail[64];
+	int mnt_avail_nr = 0;
+	struct timespec delay = { .tv_nsec = 1000000 };
+
+	snprintf(path_avail, sizeof(path_avail), "%d/ve.mnt_avail_nr", self->ctid);
+
+	while (delay.tv_nsec <= WAIT_TIMEOUT_NS) {
+		if (read_s32_at(self->cgv2_fd, path_avail, &mnt_avail_nr)) {
+			fprintf(stderr, "Failed to read from %s\n", path_avail);
+			return 1;
+		}
+		if (mnt_avail_nr == expected)
+			return 0;
+
+		do_rcu_barrier();
+		if (nanosleep(&delay, NULL)) {
+			fprintf(stderr, "nanosleep failed: %s\n", strerror(errno));
+			return 1;
+		}
+		delay.tv_nsec *= 2;
+	}
+	fprintf(stderr, "Didn't get expected value at %s\n", path_avail);
+	fprintf(stderr, "Expected %d, last read %d\n", expected, mnt_avail_nr);
+	return -1;
+}
+
+static int set_pseudosuper(FIXTURE_DATA(ve_mnt_acc) * self, int value)
+{
+	char path[64];
+
+	snprintf(path, sizeof(path), "%d/ve.pseudosuper", self->ctid);
+	return write_u64_at(self->cgv2_fd, path, value);
+}
+
+static int _create_mount(void *id_ptr)
+{
+	char path[PATH_MAX];
+	int id = *(int *)id_ptr, ret;
+
+	snprintf(path, sizeof(path), TMP_DIR "%d", id);
+
+	if (mkdir(path, 0755) < 0) {
+		fprintf(stderr, "Failed to create directory %s: %s\n", path, strerror(errno));
+		return -1;
+	}
+	ret = mount("tmpfs", path, "tmpfs", 0, "size=1M");
+	if (!ret)
+		return 0;
+	fprintf(stderr, "Failed to mount tmpfs to %s: %s\n", path, strerror(errno));
+
+	rmdir(path);
+	return -1;
+}
+
+/* Create a mount and wait until mount counter is valid */
+static int create_mount(FIXTURE_DATA(ve_mnt_acc) * self, int id)
+{
+	int ret, avail_before;
+	char path[64];
+
+	snprintf(path, sizeof(path), "%d/ve.mnt_avail_nr", self->ctid);
+
+	if (read_s32_at(self->cgv2_fd, path, &avail_before)) {
+		fprintf(stderr, "Failed to read mnt_avail_nr in create_mount\n");
+		return 1;
+	}
+
+	ret = run_in_ve(self->cgv2_fd, self->ctid, &self->ns_ctx,
+		CLONE_NEWVE | CLONE_NEWNS, _create_mount, &id);
+
+	if (ret)
+		wait_on_mnt_avail(self, avail_before);
+	else
+		ret = wait_on_mnt_avail(self, avail_before - 1);
+	return ret;
+}
+
+static int _destroy_mount(void *id_ptr)
+{
+	char path[PATH_MAX];
+	struct stat st;
+	int id = *(int *)id_ptr;
+
+	snprintf(path, sizeof(path), TMP_DIR "%d", id);
+
+	if (stat(path, &st))
+		return 1;
+	if (umount(path)) {
+		fprintf(stderr, "failed to umount directory %s: %s\n", path, strerror(errno));
+		return -1;
+	}
+	if (rmdir(path)) {
+		fprintf(stderr, "failed to remove directory %s: %s\n", path, strerror(errno));
+		return -1;
+	}
+	return 0;
+}
+
+/* Do umount and ensure that mnt_avail_nr is valid on exit(+1) */
+static int destroy_mount(FIXTURE_DATA(ve_mnt_acc) * self, int id)
+{
+	int ret, avail_before;
+	char path[64];
+
+	snprintf(path, sizeof(path), "%d/ve.mnt_avail_nr", self->ctid);
+
+	if (read_s32_at(self->cgv2_fd, path, &avail_before)) {
+		fprintf(stderr, "Failed to read mnt_avail_nr in create_mount\n");
+		return 1;
+	}
+
+	ret = run_in_ve(self->cgv2_fd, self->ctid, &self->ns_ctx,
+		CLONE_NEWVE | CLONE_NEWNS, _destroy_mount, &id);
+
+	/* Cost must be exactly 1 */
+	if (!ret)
+		ret = wait_on_mnt_avail(self, avail_before + 1);
+
+	return ret;
+}
+
+#define MAX_MNT_ID 32
+
+static int get_free_mnt_id(void)
+{
+	int i;
+	struct stat st;
+	char path[PATH_MAX];
+
+	for (i = 0; i < MAX_MNT_ID; i++) {
+		snprintf(path, sizeof(path), TMP_DIR "%d", i);
+		if (stat(path, &st))
+			return i;
+	}
+	return -1;
+}
+
+/* Returns 0 if mount failed OR mount cost is not 1 */
+static int try_mount(FIXTURE_DATA(ve_mnt_acc) * self)
+{
+	int mnt_id = get_free_mnt_id();
+
+	if (mnt_id < 0 || create_mount(self, mnt_id))
+		return 0;
+
+	return destroy_mount(self, mnt_id) == 0;
+}
+
+/*
+ * Current kernel code first creates many mounts with ve_owner != ve0,
+ * so this VE needs more mounts than would be really consumed after commit_tree.
+ */
+static int get_required_avail_mnts(FIXTURE_DATA(ve_mnt_acc) * self)
+{
+	int l = 0, r = VE_MOUNTS_MAX + 1, mid;
+	char path_max_nr[64];
+
+	snprintf(path_max_nr, sizeof(path_max_nr), "%d/ve.mnt_max_nr", self->ctid);
+
+	while (r - l > 1) {
+		mid = (l + r) / 2;
+		if (write_u64_at(self->cgv2_fd, path_max_nr, mid + self->orig_have_mnt))
+			return -1;
+
+		if (try_mount(self))
+			r = mid;
+		else
+			l = mid;
+	}
+	if (r == VE_MOUNTS_MAX + 1) {
+		fprintf(stderr, "%s: %d avail mounts is not enough\n", __func__, r);
+		return -1;
+	}
+	if (write_u64_at(self->cgv2_fd, path_max_nr, VE_MOUNTS_MAX))
+		return -1;
+	return r;
+}
+
+/* Expect mount success and return new avail value */
+static int mount_and_get_avail(struct __test_metadata *_metadata,
+			FIXTURE_DATA(ve_mnt_acc) * self, int mnt_id)
+{
+	char path_avail[64];
+	int mnt_avail_nr;
+
+	snprintf(path_avail, sizeof(path_avail), "%d/ve.mnt_avail_nr", self->ctid);
+
+	ASSERT_EQ(create_mount(self, mnt_id), 0);
+	ASSERT_EQ(read_s32_at(self->cgv2_fd, path_avail, &mnt_avail_nr), 0);
+	return mnt_avail_nr;
+}
+
+/* Expect mount failure and ensure intact avail number */
+static int check_mount_fails(struct __test_metadata *_metadata,
+			FIXTURE_DATA(ve_mnt_acc) * self,
+			int mnt_id, int avail_count)
+{
+	char path_avail[64];
+	int mnt_avail_nr;
+
+	snprintf(path_avail, sizeof(path_avail), "%d/ve.mnt_avail_nr", self->ctid);
+
+	if (read_s32_at(self->cgv2_fd, path_avail, &mnt_avail_nr))
+		return 1;
+	if (mnt_avail_nr != avail_count) {
+		fprintf(stderr, "%s: mnt_avail(%d) != avail_count(%d) before mount\n",
+			__func__, mnt_avail_nr, avail_count);
+		return -1;
+	}
+
+	if (create_mount(self, mnt_id) == 0) {
+		fprintf(stderr, "%s: successful mount\n", __func__);
+		return -1;
+	}
+
+	if (read_s32_at(self->cgv2_fd, path_avail, &mnt_avail_nr))
+		return 1;
+	if (mnt_avail_nr != avail_count) {
+		fprintf(stderr, "%s: mnt_avail(%d) != avail_count(%d) after mount\n",
+			__func__, mnt_avail_nr, avail_count);
+		return -1;
+	}
+	return 0;
+}
+
+FIXTURE_SETUP(ve_mnt_acc)
+{
+	int initial_mnt_avail_nr;
+	char path[64];
+
+	self->cgv2_fd = mount_cg2_fd();
+	ASSERT_GE(self->cgv2_fd, 0);
+	mkdir(TMP_DIR, 0755);
+
+	ASSERT_EQ(write_file_at(self->cgv2_fd, "cgroup.subtree_control",
+		  VE_CONTROLLERS), 0);
+
+	self->ctid = make_ve(self->cgv2_fd, CTID_MIN);
+	ASSERT_GE(self->ctid, 0);
+
+	snprintf(path, sizeof(path), "%d/ve.mnt_max_nr", self->ctid);
+	ASSERT_EQ(write_u64_at(self->cgv2_fd, path, VE_MOUNTS_MAX), 0);
+
+	/*
+	 * The new ve cgroup has not been entered by anything yet, so its
+	 * mnt_avail_nr counter should be VE_MOUNTS_MAX.
+	 */
+	snprintf(path, sizeof(path), "%d/ve.mnt_avail_nr", self->ctid);
+	ASSERT_EQ(read_s32_at(self->cgv2_fd, path, &initial_mnt_avail_nr), 0);
+	ASSERT_EQ(initial_mnt_avail_nr, VE_MOUNTS_MAX);
+
+	ASSERT_EQ(prepare_namespaces(&self->ns_ctx, CLONE_NEWNS | CLONE_NEWVE,
+				self->cgv2_fd, self->ctid), 0);
+
+	ASSERT_EQ(read_s32_at(self->cgv2_fd, path, &initial_mnt_avail_nr), 0);
+	self->orig_have_mnt = VE_MOUNTS_MAX - initial_mnt_avail_nr;
+
+	/* Calculate once - it is time-consuming */
+	if (*need_avail == -1)
+		*need_avail = get_required_avail_mnts(self);
+
+	ASSERT_GE(*need_avail, 1);
+	/* try_mount ensures cost is 1 */
+	ASSERT_EQ(try_mount(self), 1);
+};
+
+FIXTURE_TEARDOWN(ve_mnt_acc)
+{
+	for (int i = 0; i < MAX_MNT_ID; i++)
+		_destroy_mount((void *)&i);
+
+	close_namespace_fds(&self->ns_ctx);
+	destroy_ve(self->cgv2_fd, self->ctid);
+	close(self->cgv2_fd);
+	rmdir(TMP_DIR);
+}
+
+/* Simple test to check mount/umount accounting correctness */
+TEST_F(ve_mnt_acc, mount_umount)
+{
+	int original_mnt_avail, mnt_avail_nr;
+	char path[64];
+
+	snprintf(path, sizeof(path), "%d/ve.mnt_avail_nr", self->ctid);
+
+	ASSERT_EQ(read_s32_at(self->cgv2_fd, path, &original_mnt_avail), 0);
+
+	ASSERT_LT(mount_and_get_avail(_metadata, self, 0),
+		  original_mnt_avail);
+
+	ASSERT_EQ(destroy_mount(self, 0), 0);
+	ASSERT_EQ(read_s32_at(self->cgv2_fd, path, &mnt_avail_nr), 0);
+	ASSERT_EQ(mnt_avail_nr, original_mnt_avail);
+}
+
+/* Test mount limit hits */
+TEST_F(ve_mnt_acc, hit_limits)
+{
+	int mnt_avail_nr;
+	char path_avail[64], path_max_nr[64];
+
+	snprintf(path_avail, sizeof(path_avail), "%d/ve.mnt_avail_nr", self->ctid);
+	snprintf(path_max_nr, sizeof(path_max_nr), "%d/ve.mnt_max_nr", self->ctid);
+
+	/* Step 1: reduce number of available mounts to *need_avail */
+	ASSERT_EQ(write_u64_at(self->cgv2_fd, path_max_nr,
+		self->orig_have_mnt + 1 * (*need_avail)), 0);
+	ASSERT_EQ(read_s32_at(self->cgv2_fd, path_avail, &mnt_avail_nr), 0);
+	ASSERT_EQ(mnt_avail_nr, 1 * *need_avail);
+
+	/* Step 2: do one mount, no mounts should be available */
+	ASSERT_EQ(mount_and_get_avail(_metadata, self, 0), *need_avail - 1);
+
+	/* Step 3: check that one more mount fails */
+	ASSERT_EQ(check_mount_fails(_metadata, self, 1, (*need_avail) - 1), 0);
+
+	/* Step 4: increase by 1 and win now */
+	ASSERT_EQ(write_u64_at(self->cgv2_fd, path_max_nr,
+		self->orig_have_mnt + 1 + *need_avail), 0);
+	ASSERT_EQ(read_s32_at(self->cgv2_fd, path_avail, &mnt_avail_nr), 0);
+	ASSERT_EQ(mnt_avail_nr, *need_avail);
+
+	ASSERT_EQ(mount_and_get_avail(_metadata, self, 1), *need_avail - 1);
+
+	/* Step 5: reduce mnt_max_nr so we have more mounts than allowed */
+	ASSERT_EQ(write_u64_at(self->cgv2_fd, path_max_nr,
+				self->orig_have_mnt + 1), 0);
+	ASSERT_EQ(read_s32_at(self->cgv2_fd, path_avail, &mnt_avail_nr), 0);
+	ASSERT_EQ(mnt_avail_nr, -1);
+
+	/* Step 6: try to do mount when avail < 0, ensure number is intact */
+	ASSERT_EQ(check_mount_fails(_metadata, self, 2, -1), 0);
+
+	/* Step 7: remove one mount, check avail value update, mount should fail */
+	ASSERT_EQ(destroy_mount(self, 0), 0);
+	ASSERT_EQ(check_mount_fails(_metadata, self, 2, 0), 0);
+
+	/* Step 8: remove one more mount and check that new mount succeeds */
+	ASSERT_EQ(destroy_mount(self, 1), 0);
+	ASSERT_EQ(read_s32_at(self->cgv2_fd, path_avail, &mnt_avail_nr), 0);
+	ASSERT_EQ(mnt_avail_nr, 1);
+	ASSERT_EQ(write_u64_at(self->cgv2_fd, path_max_nr, self->orig_have_mnt + *need_avail), 0);
+	ASSERT_EQ(mount_and_get_avail(_metadata, self, 2), *need_avail - 1);
+
+	ASSERT_EQ(destroy_mount(self, 2), 0);
+}
+
+/*
+ * Mount propagation makes one mount cost more.
+ * This test checks that if we run out of mounts in the middle of creating
+ * a new one, everything is restored smoothly and nothing leaks.
+ */
+TEST_F(ve_mnt_acc, partial_mounts)
+{
+	char path_avail[64], path_max_nr[64];
+	int i;
+
+	snprintf(path_max_nr, sizeof(path_max_nr), "%d/ve.mnt_max_nr", self->ctid);
+	snprintf(path_avail, sizeof(path_avail), "%d/ve.mnt_avail_nr", self->ctid);
+
+	if (*need_avail == 1)
+		SKIP(return, "mount cost is 1, no partial mounts possible");
+
+	for (i = 0; i < *need_avail; i++) {
+		ASSERT_EQ(write_u64_at(self->cgv2_fd, path_max_nr, self->orig_have_mnt + i), 0);
+		ASSERT_EQ(check_mount_fails(_metadata, self, 0, i), 0);
+	}
+}
+
+/* Test that pseudosuper allows negative avail with correct accounting. */
+TEST_F(ve_mnt_acc, pseudosuper_allows_overuse)
+{
+	char path_max_nr[64];
+
+	snprintf(path_max_nr, sizeof(path_max_nr), "%d/ve.mnt_max_nr", self->ctid);
+
+	ASSERT_EQ(write_u64_at(self->cgv2_fd, path_max_nr, self->orig_have_mnt + *need_avail), 0);
+	ASSERT_EQ(set_pseudosuper(self, 1), 0);
+
+	ASSERT_EQ(mount_and_get_avail(_metadata, self, 0), *need_avail - 1);
+	ASSERT_EQ(mount_and_get_avail(_metadata, self, 1), *need_avail - 2);
+
+	/* Go below zero */
+	ASSERT_EQ(write_u64_at(self->cgv2_fd, path_max_nr, self->orig_have_mnt), 0);
+	ASSERT_EQ(mount_and_get_avail(_metadata, self, 2), -3);
+
+	ASSERT_EQ(set_pseudosuper(self, 0), 0);
+
+	/* Below zero should fail */
+	ASSERT_EQ(check_mount_fails(_metadata, self, 3, -3), 0);
+	/* Now need 2 more to add a new mount */
+	ASSERT_EQ(write_u64_at(self->cgv2_fd, path_max_nr,
+		self->orig_have_mnt + 3 + *need_avail - 2), 0);
+	ASSERT_EQ(check_mount_fails(_metadata, self, 3, *need_avail - 2), 0);
+
+	ASSERT_EQ(destroy_mount(self, 0), 0);
+	ASSERT_EQ(check_mount_fails(_metadata, self, 3, *need_avail - 1), 0);
+	ASSERT_EQ(destroy_mount(self, 2), 0);
+
+	ASSERT_EQ(try_mount(self), 1);
+}
+
+/* Test that pseudosuper doesn't disable accounting. */
+TEST_F(ve_mnt_acc, pseudosuper_continues_accounting)
+{
+	int orig_mnt_avail, mnt_avail_nr;
+	char path_avail[64];
+
+	snprintf(path_avail, sizeof(path_avail), "%d/ve.mnt_avail_nr", self->ctid);
+	ASSERT_EQ(read_s32_at(self->cgv2_fd, path_avail, &orig_mnt_avail), 0);
+
+	ASSERT_EQ(set_pseudosuper(self, 0), 0);
+
+	/* mnt 0 - mounted without pseudosuper, umounted with it. */
+	ASSERT_EQ(mount_and_get_avail(_metadata, self, 0), orig_mnt_avail - 1);
+	ASSERT_EQ(set_pseudosuper(self, 1), 0);
+
+	/* Cost is the same when mount/umount happen under pseudosuper. */
+	ASSERT_EQ(try_mount(self), 1);
+
+	/* mnt 1 - mounted with pseudosuper, umounted without it. */
+	ASSERT_EQ(mount_and_get_avail(_metadata, self, 1),
+		  orig_mnt_avail - 2);
+
+	ASSERT_EQ(destroy_mount(self, 0), 0);
+	ASSERT_EQ(read_s32_at(self->cgv2_fd, path_avail, &mnt_avail_nr), 0);
+	ASSERT_EQ(mnt_avail_nr, orig_mnt_avail - 1);
+
+	ASSERT_EQ(set_pseudosuper(self, 0), 0);
+
+	ASSERT_EQ(destroy_mount(self, 1), 0);
+	ASSERT_EQ(read_s32_at(self->cgv2_fd, path_avail, &mnt_avail_nr), 0);
+	ASSERT_EQ(mnt_avail_nr, orig_mnt_avail);
+}
+
+/* Test failcount feature */
+TEST_F(ve_mnt_acc, failcount)
+{
+	char path_fc[64], failcount_str[512], path_max_nr[64];
+
+	snprintf(path_fc, sizeof(path_fc), "%d/ve.failcount", self->ctid);
+	snprintf(path_max_nr, sizeof(path_max_nr), "%d/ve.mnt_max_nr", self->ctid);
+
+	ASSERT_GE(read_file_at(self->cgv2_fd, path_fc,
+			       failcount_str, sizeof(failcount_str)), 0);
+	ASSERT_TRUE(strstr(failcount_str, "mnt: 0\n") != NULL);
+
+	/* Check successful mount doesn't affect failcount */
+	mount_and_get_avail(_metadata, self, 0);
+	ASSERT_GE(read_file_at(self->cgv2_fd, path_fc,
+			       failcount_str, sizeof(failcount_str)), 0);
+	ASSERT_TRUE(strstr(failcount_str, "mnt: 0\n") != NULL);
+	ASSERT_EQ(destroy_mount(self, 0), 0);
+
+	/* Check failcount update when mount fails */
+	ASSERT_EQ(write_u64_at(self->cgv2_fd, path_max_nr, 0), 0);
+	ASSERT_LT(create_mount(self, 1), 0);
+	ASSERT_GE(read_file_at(self->cgv2_fd, path_fc,
+			       failcount_str, sizeof(failcount_str)), 0);
+	ASSERT_TRUE(strstr(failcount_str, "mnt: 1\n") != NULL);
+
+	/* Check failcount flush */
+	ASSERT_EQ(write_u64_at(self->cgv2_fd, path_fc, 0), 0);
+	ASSERT_GE(read_file_at(self->cgv2_fd, path_fc,
+			       failcount_str, sizeof(failcount_str)), 0);
+	ASSERT_TRUE(strstr(failcount_str, "mnt: 0\n") != NULL);
+
+	/* Check failcount update when mount fails again */
+	ASSERT_EQ(write_u64_at(self->cgv2_fd, path_max_nr, 0), 0);
+	ASSERT_LT(create_mount(self, 1), 0);
+	ASSERT_GE(read_file_at(self->cgv2_fd, path_fc,
+			       failcount_str, sizeof(failcount_str)), 0);
+	ASSERT_TRUE(strstr(failcount_str, "mnt: 1\n") != NULL);
+}
+
+int main(int argc, char **argv)
+{
+	need_avail = mmap(NULL, sizeof(int),
+		PROT_READ | PROT_WRITE, MAP_SHARED | MAP_ANONYMOUS, -1, 0);
+
+	if (need_avail == MAP_FAILED) {
+		fprintf(stderr, "Failed to mmap shared region: %s\n", strerror(errno));
+		return 1;
+	}
+	*need_avail = -1;
+
+	return test_harness_run(argc, argv);
+}
diff --git a/tools/testing/selftests/ve/ve_selftest.h b/tools/testing/selftests/ve/ve_selftest.h
index bf9b89b59a6c..8cc9b3238aaf 100644
--- a/tools/testing/selftests/ve/ve_selftest.h
+++ b/tools/testing/selftests/ve/ve_selftest.h
@@ -413,4 +413,25 @@ static inline int is_mounted(const char *path)
 	return errno == EXDEV ? 1 : -1;
 }
 
+static inline int do_rcu_barrier(void)
+{
+	int fd;
+
+	errno = 0;
+	fd = open("/sys/module/rcutree/parameters/do_rcu_barrier", O_WRONLY);
+
+	if (fd < 0) {
+		fprintf(stderr, "Failed to open do_rcu_barrier: %s\n", strerror(errno));
+		return -1;
+	}
+	errno = 0;
+	if (write(fd, "1", 2) != 2) {
+		fprintf(stderr, "Failed to issue rcu_barrier: %s\n", strerror(errno));
+		close(fd);
+		return -1;
+	}
+	close(fd);
+	return 0;
+}
+
 #endif /* __SELFTESTS_VE_VE_SELFTEST_H */
-- 
2.47.1

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

Re: [Devel] [PATCH VZ10 v9 8/8] selftests/ve: Add mount accounting selftest

[Vladimir Riabchun]

Sashiko review:

On 9/15/26 19:22, Vladimir Riabchun wrote:
> There are 6 test cases, covered in the new test:
> 1. Simple mount accouting correctness, just mount/umount.
> 2. Verification of correct limit hits and changes, including
>     negative values.
> 3. Partial mounts test, when mount limit is hit in the middle
>     of creation.
> 4. Test that enabled pseudosuper allows overuse.
> 5. Test that pseudosuper doesn't affect mount accounting.
> 6. Failcount feature verification.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-135520
> 
> Feature: per-ve failcounters
> Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
> ---
>   tools/testing/selftests/ve/.gitignore         |   1 +
>   tools/testing/selftests/ve/Makefile           |   1 +
>   .../selftests/ve/ve_mount_accounting_test.c   | 550 ++++++++++++++++++
>   tools/testing/selftests/ve/ve_selftest.h      |  21 +
>   4 files changed, 573 insertions(+)
>   create mode 100644 tools/testing/selftests/ve/ve_mount_accounting_test.c
> 
> diff --git a/tools/testing/selftests/ve/.gitignore b/tools/testing/selftests/ve/.gitignore
> index afa4c568c2c9..3df4d05888dc 100644
> --- a/tools/testing/selftests/ve/.gitignore
> +++ b/tools/testing/selftests/ve/.gitignore
> @@ -1,2 +1,3 @@
>   ve_ns_owner_test
>   ve_perms_test
> +ve_mount_accounting_test
> diff --git a/tools/testing/selftests/ve/Makefile b/tools/testing/selftests/ve/Makefile
> index ec40cbc7b3a1..c6efe7c4b4fb 100644
> --- a/tools/testing/selftests/ve/Makefile
> +++ b/tools/testing/selftests/ve/Makefile
> @@ -4,5 +4,6 @@ CFLAGS += -g -Wall -O2
>   
>   TEST_GEN_PROGS += ve_ns_owner_test
>   TEST_GEN_PROGS += ve_perms_test
> +TEST_GEN_PROGS += ve_mount_accounting_test
>   
>   include ../lib.mk
> diff --git a/tools/testing/selftests/ve/ve_mount_accounting_test.c b/tools/testing/selftests/ve/ve_mount_accounting_test.c
> new file mode 100644
> index 000000000000..6ba13dcdcc87
> --- /dev/null
> +++ b/tools/testing/selftests/ve/ve_mount_accounting_test.c
> @@ -0,0 +1,550 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * ve_mount_accounting selftests
> + *
> + * Tests to check the correctness of mount accounting.
> + */
> +#define _GNU_SOURCE
> +#include <asm/unistd.h>
> +#include <linux/sched.h>
> +#include <linux/limits.h>
> +#include <sys/wait.h>
> +#include <sys/syscall.h>
> +#include <sys/stat.h>
> +#include <sys/mount.h>
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <sched.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <string.h>
> +#include <time.h>
> +
> +#include "../kselftest_harness.h"
> +#include "ve_selftest.h"
> +
> +#define TMP_DIR			"/ve-mnt-tmp/"
> +#define VE_MOUNTS_MAX		128
> +
> +/*
> + * These values take a while to calculate, the first test will
> + * do this and store the values for those who come after.
> + */
> +int *need_avail;
> +
> +FIXTURE(ve_mnt_acc)
> +{
> +	int cgv2_fd;
> +	int ctid;
> +	struct namespace_ctx ns_ctx;
> +	int orig_have_mnt;
> +};
> +
> +/*
> + * free_vfsmnt is called via call_rcu, so we need to ensure
> + * that all pending call_rcu tasks are finished.
> + * However, one barrier is not enough:
> + * mntput_no_expire
> + *   init_task_work(&mnt->mnt_rcu, __cleanup_mnt);
> + *   task_work_add(task, &mnt->mnt_rcu, TWA_RESUME)
> + *     OR
> + *   schedule_delayed_work(&delayed_mntput_work, 1)
> + *   # Both later call cleanup_mnt
> + *     cleanup_mnt
> + *       call_rcu(&mnt->mnt_rcu, delayed_free_vfsmnt);
> + *
> + * We can't enforce workqueue flush from userspace, so instead
> + * read several times until the value is the one we expect.
> + */

Sashiko says:
  This segment of the `wait_on_mnt_avail` function can potentially lead
  to an infinite wait if the value `mnt_avail_nr` does not match
  the expected value before timeout.
  While this isn't a bug introduced by this patch, could we include
  some mechanism  to handle timeouts more gracefully, such as an error code
  return or retry limit?

Something is off here.
1. There was no bug before this patch.
2. Retry and error return mechanism is what this code does.

> +static int wait_on_mnt_avail(FIXTURE_DATA(ve_mnt_acc) * self, int expected)
> +{
> +#define WAIT_TIMEOUT_NS		(4096UL * 1000 * 1000)
> +	char path_avail[64];
> +	int mnt_avail_nr = 0;
> +	struct timespec delay = { .tv_nsec = 1000000 };
> +
> +	snprintf(path_avail, sizeof(path_avail), "%d/ve.mnt_avail_nr", self->ctid);
> +
> +	while (delay.tv_nsec <= WAIT_TIMEOUT_NS) {
> +		if (read_s32_at(self->cgv2_fd, path_avail, &mnt_avail_nr)) {
> +			fprintf(stderr, "Failed to read from %s\n", path_avail);
> +			return 1;
> +		}
> +		if (mnt_avail_nr == expected)
> +			return 0;
> +
> +		do_rcu_barrier();
> +		if (nanosleep(&delay, NULL)) {
> +			fprintf(stderr, "nanosleep failed: %s\n", strerror(errno));
> +			return 1;
> +		}
> +		delay.tv_nsec *= 2;
> +	}
> +	fprintf(stderr, "Didn't get expected value at %s\n", path_avail);
> +	fprintf(stderr, "Expected %d, last read %d\n", expected, mnt_avail_nr);
> +	return -1;
> +}
> +
> +static int set_pseudosuper(FIXTURE_DATA(ve_mnt_acc) * self, int value)
> +{
> +	char path[64];
> +
> +	snprintf(path, sizeof(path), "%d/ve.pseudosuper", self->ctid);
> +	return write_u64_at(self->cgv2_fd, path, value);
> +}
> +

Sashiko says:
  In `_create_mount`, can the clean-up logic be more robust to ensure
  that filesystem resources are consistently freed in the event of an error
  or unexpected behavior?

No more cleanup here is possible.

> +static int _create_mount(void *id_ptr)
> +{
> +	char path[PATH_MAX];
> +	int id = *(int *)id_ptr, ret;
> +
> +	snprintf(path, sizeof(path), TMP_DIR "%d", id);
> +
> +	if (mkdir(path, 0755) < 0) {
> +		fprintf(stderr, "Failed to create directory %s: %s\n", path, strerror(errno));
> +		return -1;
> +	}
> +	ret = mount("tmpfs", path, "tmpfs", 0, "size=1M");
> +	if (!ret)
> +		return 0;
> +	fprintf(stderr, "Failed to mount tmpfs to %s: %s\n", path, strerror(errno));
> +
> +	rmdir(path);
> +	return -1;
> +}
> +
> +/* Create a mount and wait until mount counter is valid */
> +static int create_mount(FIXTURE_DATA(ve_mnt_acc) * self, int id)
> +{
> +	int ret, avail_before;
> +	char path[64];
> +
> +	snprintf(path, sizeof(path), "%d/ve.mnt_avail_nr", self->ctid);
> +
> +	if (read_s32_at(self->cgv2_fd, path, &avail_before)) {
> +		fprintf(stderr, "Failed to read mnt_avail_nr in create_mount\n");
> +		return 1;
> +	}
> +

Sashiko says:
  Would it be beneficial to include comprehensive error handling in `create_mount`
  to cover cases when `run_in_ve` fails?

No, it wouldn't. Nothing we can do in this case and nothing
needs to be handled - no resources created.

> +	ret = run_in_ve(self->cgv2_fd, self->ctid, &self->ns_ctx,
> +		CLONE_NEWVE | CLONE_NEWNS, _create_mount, &id);
> +
> +	if (ret)
> +		wait_on_mnt_avail(self, avail_before);
> +	else
> +		ret = wait_on_mnt_avail(self, avail_before - 1);
> +	return ret;
> +}
> +

Sashiko says:
  Does the `_destroy_mount` function correctly handle subsequent errors that may
  occur during the unmount and directory removal stages?

Yes, it does.

> +static int _destroy_mount(void *id_ptr)
> +{
> +	char path[PATH_MAX];
> +	struct stat st;
> +	int id = *(int *)id_ptr;
> +
> +	snprintf(path, sizeof(path), TMP_DIR "%d", id);
> +
> +	if (stat(path, &st))
> +		return 1;
> +	if (umount(path)) {
> +		fprintf(stderr, "failed to umount directory %s: %s\n", path, strerror(errno));
> +		return -1;
> +	}
> +	if (rmdir(path)) {
> +		fprintf(stderr, "failed to remove directory %s: %s\n", path, strerror(errno));
> +		return -1;
> +	}
> +	return 0;
> +}
> +
> ...
> +
> +int main(int argc, char **argv)
> +{
> +	need_avail = mmap(NULL, sizeof(int),
> +		PROT_READ | PROT_WRITE, MAP_SHARED | MAP_ANONYMOUS, -1, 0);
> +
> +	if (need_avail == MAP_FAILED) {
> +		fprintf(stderr, "Failed to mmap shared region: %s\n", strerror(errno));
> +		return 1;
> +	}
> +	*need_avail = -1;
> +
> +	return test_harness_run(argc, argv);

Sashiko says:
  Does the `main` function properly handle the memory allocated by `mmap`,
  especially considering that there might not be a corresponding `munmap` to free
  the memory?

Doesn't matter, we can free need_avail only when test_harness_run finishes.
It is easier to just exit.

> +}
> diff --git a/tools/testing/selftests/ve/ve_selftest.h b/tools/testing/selftests/ve/ve_selftest.h
> index bf9b89b59a6c..8cc9b3238aaf 100644
> --- a/tools/testing/selftests/ve/ve_selftest.h
> +++ b/tools/testing/selftests/ve/ve_selftest.h
> @@ -413,4 +413,25 @@ static inline int is_mounted(const char *path)
>   	return errno == EXDEV ? 1 : -1;
>   }
>   

Sashiko says:
  Could write operations within `do_rcu_barrier` be improperly assumed to be atomic
  leading to potential scheduling issues when invoked in contexts that might sleep?

No, they couldn't. This is user-space code.

> +static inline int do_rcu_barrier(void)
> +{
> +	int fd;
> +
> +	errno = 0;
> +	fd = open("/sys/module/rcutree/parameters/do_rcu_barrier", O_WRONLY);
> +
> +	if (fd < 0) {
> +		fprintf(stderr, "Failed to open do_rcu_barrier: %s\n", strerror(errno));
> +		return -1;
> +	}
> +	errno = 0;
> +	if (write(fd, "1", 2) != 2) {
> +		fprintf(stderr, "Failed to issue rcu_barrier: %s\n", strerror(errno));
> +		close(fd);
> +		return -1;
> +	}
> +	close(fd);
> +	return 0;
> +}
> +
>   #endif /* __SELFTESTS_VE_VE_SELFTEST_H */

-- 
Best regards, Riabchun Vladimir
Linux Kernel Developer, Virtuozzo

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

Re: [Devel] [PATCH VZ10 v9 8/8] selftests/ve: Add mount accounting selftest

[Konstantin Khorenko]

On 9/15/26 19:22, Vladimir Riabchun wrote:
> There are 6 test cases, covered in the new test:
> 1. Simple mount accouting correctness, just mount/umount.
> 2. Verification of correct limit hits and changes, including
>    negative values.
> 3. Partial mounts test, when mount limit is hit in the middle
>    of creation.
> 4. Test that enabled pseudosuper allows overuse.
> 5. Test that pseudosuper doesn't affect mount accounting.
> 6. Failcount feature verification.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-135520
> 
> Feature: per-ve failcounters
> Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
> ---
>  tools/testing/selftests/ve/.gitignore         |   1 +
>  tools/testing/selftests/ve/Makefile           |   1 +
>  .../selftests/ve/ve_mount_accounting_test.c   | 550 ++++++++++++++++++
>  tools/testing/selftests/ve/ve_selftest.h      |  21 +
>  4 files changed, 573 insertions(+)
>  create mode 100644 tools/testing/selftests/ve/ve_mount_accounting_test.c
> 

<skipped>

> + * We can't enforce workqueue flush from userspace, so instead
> + * read several times until the value is the one we expect.
> + */
> +static int wait_on_mnt_avail(FIXTURE_DATA(ve_mnt_acc) * self, int expected)
> +{
> +#define WAIT_TIMEOUT_NS		(4096UL * 1000 * 1000)
> +	char path_avail[64];
> +	int mnt_avail_nr = 0;
> +	struct timespec delay = { .tv_nsec = 1000000 };
> +
> +	snprintf(path_avail, sizeof(path_avail), "%d/ve.mnt_avail_nr", self->ctid);
> +
> +	while (delay.tv_nsec <= WAIT_TIMEOUT_NS) {
> +		if (read_s32_at(self->cgv2_fd, path_avail, &mnt_avail_nr)) {
> +			fprintf(stderr, "Failed to read from %s\n", path_avail);
> +			return 1;
> +		}
> +		if (mnt_avail_nr == expected)
> +			return 0;
> +
> +		do_rcu_barrier();
> +		if (nanosleep(&delay, NULL)) {
> +			fprintf(stderr, "nanosleep failed: %s\n", strerror(errno));
> +			return 1;
> +		}
> +		delay.tv_nsec *= 2;

  nanosleep() requires tv_nsec < 1e9 and fails with EINVAL otherwise.
  The doubling goes 1ms, 2ms, ..., 512ms, and the next step is 1024ms,
  which no longer fits tv_nsec. So the loop never gets anywhere near
  WAIT_TIMEOUT_NS: after ~1s of total sleeping the next iteration bails
  out with "nanosleep failed: Invalid argument" and returns 1, instead
  of the intended ~8s of waiting and the timeout report with the
  expected/read values.

  This defeats the purpose of the helper: on a loaded machine, where
  the counter takes a while to settle, the test fails early with a
  misleading error - exactly the case this function was reworked for
  in v9.

  Please keep the delay in a plain nanoseconds variable and normalize
  it before sleeping:

        delay.tv_sec = ns / 1000000000;
        delay.tv_nsec = ns % 1000000000;

  As a side note, tv_nsec is long, so 2048ms would overflow it on a
  32-bit build; the normalization fixes that too.

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

Re: [Devel] [PATCH VZ10 v9 0/8] Add per-VE failcount support

[Vasileios Almpanis]

> This patchset introduces per-VE failcount support.
> We have several resources, that have per-VE limits,
> the users might want to know what limit hits make
> operations fail.
> 
> - Patch 1 is cosmetic, nothing special.
> - Patches 2-4 change mount number and limit processing,
>   unifying it with other resources. VE selftest is updated
>   in patch 3.
> - Patch 5 greatly reduces the amount of copy-pasted code
>   by introducing some useful macros.
> - Patch 6 adds per-VE failcount file.
> - Patch 7 fixes kernel log leak to containers from interrupts.
> - Patch 8 introduces new VE selftest, that checks mounts
>   accounting, limits and failcount updates.
> 
> NOTE: for running new selftest this change should be applied on
> top of "[PATCH VZ10 v2 0/3] Improvements for ve selftests" patchset.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-135520
> 

LGTM. Note for Kostya maybe apply patch 7 before patch 6 so if we bisect
we won't hit a step where logs are leaked to random CTs.

> Changes since v8:
>  - Added new patch 7 to fix logs leakage from interrupts.
>    We are fine if we miss some failure logs in container
>    dmesg - CT can check failcounts anyway, and bringing
>    new ratelimited printks with specified VE is too hard
>    for such a small thing.
>  - Selftest in patch 8 got major rework:
>    1. Correct mmap error handling.
>    2. Dropped mnt_cost value. Let's force 1, it is expected
>       value in fresh mount+VE namespaces.
>    3. Proper solution for pending mount destructions. It used
>       to be just sleep(1) which may not be enoght on loaded
>       machines. Instead, issue rcu_barrier and wait on mnt_avail_nr.
>       Since we forced mnt_cost == 1, it know precisely what the
>       new value should be.
> 
> Vladimir Riabchun (8):
>   ve/ve.{h,c}: Farewell to spaces as indents
>   ve/fs: Rework per-ve mount count
>   selftests/ve: Update ve_ns_owner_test
>   ve: Move from global VE mounts limit to per-VE limit
>   ve/ve.c: Generate VE resource accessors using macros
>   ve: Introduce per-VE failcount
>   ve/printk: Don't use ve log outside task context
>   selftests/ve: Add mount accounting selftest
> 
>  fs/aio.c                                      |   1 +
>  fs/namespace.c                                |  80 ++-
>  include/linux/ve.h                            |  35 +-
>  kernel/bpf/syscall.c                          |   1 +
>  kernel/printk/printk.c                        |   7 +-
>  kernel/ve/ve.c                                | 277 ++++-----
>  kernel/ve/veowner.c                           |  19 -
>  net/core/dev.c                                |  10 +-
>  net/core/neighbour.c                          |   3 +-
>  net/core/net_namespace.c                      |   4 +-
>  tools/testing/selftests/ve/.gitignore         |   1 +
>  tools/testing/selftests/ve/Makefile           |   1 +
>  .../selftests/ve/ve_mount_accounting_test.c   | 550 ++++++++++++++++++
>  tools/testing/selftests/ve/ve_ns_owner_test.c |  37 +-
>  tools/testing/selftests/ve/ve_selftest.h      |  21 +
>  15 files changed, 822 insertions(+), 225 deletions(-)
>  create mode 100644 tools/testing/selftests/ve/ve_mount_accounting_test.c
> 
> --
> 2.47.1
> 
> _______________________________________________
> Devel mailing list
> Devel@openvz.org
> https://lists.openvz.org/mailman/listinfo/devel

Reviewed-by: Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>

-- 
Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH VZ10 v9 0/8] Add per-VE failcount support

[Konstantin Khorenko]

https://sashiko.vzint.dev/#/patchset/cover.1789491989.git.vladimir.riabchun%40virtuozzo.com

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

Re: [Devel] [PATCH VZ10 v9 0/8] Add per-VE failcount support

[Vladimir Riabchun]

Addressed all comments in replies for patches.

On 9/16/26 19:30, Konstantin Khorenko wrote:
> https://sashiko.vzint.dev/#/patchset/cover.1789491989.git.vladimir.riabchun%40virtuozzo.com
> 
> ?

-- 
Best regards, Riabchun Vladimir
Linux Kernel Developer, Virtuozzo

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

Re: [Devel] [PATCH VZ10 v9 0/8] Add per-VE failcount support

[Vladimir Riabchun]

Please note that this change is applied on top of "[PATCH VZ10 v3 0/3] Improvements
for ve selftests", so that one should be done first.

On 9/16/26 19:30, Konstantin Khorenko wrote:
> https://sashiko.vzint.dev/#/patchset/cover.1789491989.git.vladimir.riabchun%40virtuozzo.com
> 
> ?

-- 
Best regards, Riabchun Vladimir
Linux Kernel Developer, Virtuozzo

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