Re: [Devel] [PATCH VZ10 v7 7/9] ve: Introduce per-VE failcount

[Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>]

On 8/28/26 18:52, Pavel Tikhomirov wrote:
> 
> 
> On 8/24/26 15:54, 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       |  6 ++++
>>   kernel/bpf/syscall.c     |  1 +
>>   kernel/ve/ve.c           | 70 ++++++++++++++++++++++++++++++++++++++++
>>   net/core/dev.c           |  2 ++
>>   net/core/neighbour.c     |  1 +
>>   net/core/net_namespace.c |  4 ++-
>>   8 files changed, 86 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/aio.c b/fs/aio.c
>> index cb63416af135..3fa07cc626f8 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) {
>> +		atomic_inc(&ve->aio_failcount);
>>   		spin_unlock(&ve->aio_nr_lock);
>>   		err = -EAGAIN;
>>   		goto err_ctx;
>> diff --git a/fs/namespace.c b/fs/namespace.c
>> index e97f48204617..c30bbc370f2b 100644
>> --- a/fs/namespace.c
>> +++ b/fs/namespace.c
>> @@ -3371,6 +3371,8 @@ static inline int ve_try_reserve_mount(struct ve_struct *ve)
>>   
>>   	if (ret)
>>   		get_ve(ve);
>> +	else
>> +		atomic_inc(&ve->mnt_failcount);
>>   	return ret;
>>   }
>>   
>> diff --git a/include/linux/ve.h b/include/linux/ve.h
>> index 5687faad46ff..9e73527e970e 100644
>> --- a/include/linux/ve.h
>> +++ b/include/linux/ve.h
>> @@ -72,12 +72,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;
>> @@ -86,6 +89,7 @@ struct ve_struct {
>>   
>>   	atomic_t		arp_neigh_nr;
>>   	atomic_t		nd_neigh_nr;
>> +	atomic_t		neigh_tbl_failcount;
>>   	unsigned long		meminfo_val;
>>   
>>   	/*
>> @@ -94,6 +98,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
>> @@ -121,6 +126,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;
>>   };
>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> index c94d4240e3d3..9d57e7999ae0 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) {
>> +			atomic_inc(&load_ve->bpf_prog_failcount);
>>   			load_ve = NULL;
>>   			err = -ENOSPC;
>>   			goto put_token;
>> diff --git a/kernel/ve/ve.c b/kernel/ve/ve.c
>> index 0f02835765ff..826f72ad0a22 100644
>> --- a/kernel/ve/ve.c
>> +++ b/kernel/ve/ve.c
>> @@ -99,10 +99,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	=
>> @@ -114,8 +117,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,
> 
> This feature is not intended to limit host aio, right? Should it
> be INT_MAX or something big here for ve0?

It preserves original kernel behavior. Upstream kernel has aio_max_nr
sysctl variable with the same value as AIO_MAX_NR_DEFAULT.
Commit 1f6006388601 ("ve/fs/aio: aio_nr & aio_max_nr variables virtualization")
added aio_max_nr to ve structure, but it didn't initialize it in ve0.

Here I just put the correct value in it, so no functional changes.

> 
>> +	.aio_failcount		= ATOMIC_INIT(0),
>> +#endif
>>   	.meminfo_val		= VE_MEMINFO_SYSTEM,
>>   	.umh_running_helpers	= ATOMIC_INIT(0),
>>   	.umh_helpers_waitq	= __WAIT_QUEUE_HEAD_INITIALIZER(ve0.umh_helpers_waitq),
>> @@ -780,12 +791,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);
>> @@ -812,7 +826,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
>> @@ -825,6 +841,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;
>> @@ -1065,6 +1082,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);
>> @@ -1602,6 +1666,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..05e0b9b6ba23 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -10997,6 +10997,7 @@ int register_netdevice(struct net_device *dev)
>>   
>>   	ret = -ENOMEM;
>>   	if (atomic_dec_if_positive(&net->owner_ve->netif_avail_nr) < 0) {
>> +		atomic_inc(&net->owner_ve->netif_failcount);
>>   		ve_pr_warn_ratelimited(VE_LOG_BOTH,
>>   			"CT%s: hits max number of network devices, "
>>   			"increase ve::netif_max_nr parameter\n",
>> @@ -12211,6 +12212,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) {
>> +		atomic_inc(&net->owner_ve->netif_failcount);
>>   		ve_pr_warn_ratelimited(VE_LOG_BOTH,
>>   			"CT%s: hits max number of network devices, "
>>   			"increase ve::netif_max_nr parameter\n",
>> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
>> index f90deb17fb25..57a49d9c98a7 100644
>> --- a/net/core/neighbour.c
>> +++ b/net/core/neighbour.c
>> @@ -520,6 +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) {
>> +			atomic_inc(&ve->neigh_tbl_failcount);
>>   			net_info_ratelimited("%s: neighbor table overflow!\n",
>>   					     tbl->id);
>>   			NEIGH_CACHE_STAT_INC(tbl, table_fulls);
>> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
>> index b3d54cad984a..9a3376d2682f 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) {
>> +		atomic_inc(&ve->netns_failcount);
> 
> Let's add a helper for incrementing our failcounts:
> 
> #define ve_failcount_inc(ve, name)                                    \
> do {                                                                  \
>        struct ve_struct *__ve = (ve);                                  \
>                                                                        \
>        if (atomic_inc_return(&__ve->name##_failcount) == 1)            \
>                pr_warn("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)
> 
> Call sites become one-liners:
> 
> --- a/fs/aio.c
> -             atomic_inc(&ve->aio_failcount);
> +             ve_failcount_inc(ve, aio);
> --- a/fs/namespace.c
> -             atomic_inc(&ve->mnt_failcount);
> +             ve_failcount_inc(ve, mnt);
> --- a/kernel/bpf/syscall.c
> -                     atomic_inc(&load_ve->bpf_prog_failcount);
> +                     ve_failcount_inc(load_ve, bpf_prog);
> --- a/net/core/dev.c          (both register_netdevice() and __dev_change_net_namespace())
> -             atomic_inc(&net->owner_ve->netif_failcount);
> +             ve_failcount_inc(net->owner_ve, netif);
> --- a/net/core/neighbour.c
> -                     atomic_inc(&ve->neigh_tbl_failcount);
> +                     ve_failcount_inc(ve, neigh_tbl);
> --- a/net/core/net_namespace.c
> -             atomic_inc(&ve->netns_failcount);
> +             ve_failcount_inc(ve, netns);
> 
> The idea behind it is to show in dmesg that failcount was reached, to simpify
> the detection of problematic containers for us.
> 
> For reference http://lore.virtuozzo.com/kernel/ccc3cbc8-e225-41cf-b563-699254aeb57b@virtuozzo.com/T/#t
> 

Fair, I'll do it in next version or as a follow-up if everything else is fine here.
Just instead of pr_warn we should use pr_warn_ratelimited, as it is easy
to trigger warning from container and flood dmesg and serial console.

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