Re: [PATCH v2 3/4] memcg: int16_t for cached slab stats

[David Laight]

On Thu, 21 May 2026 18:19:07 -0700
Shakeel Butt <shakeel.butt@xxxxxxxxx> wrote:

> Currently struct obj_stock_pcp stores cached slab stats in 'int' which
> is 4 bytes per counter on 64-bit machines. Switch them to int16_t to
> shrink the cached metadata.
> 
> The existing PAGE_SIZE flush in __account_obj_stock() bounds *bytes at
> PAGE_SIZE on 4KiB and 16KiB page archs, well within int16_t. On 64KiB
> pages PAGE_SIZE is well above S16_MAX so that flush never fires, and a
> sufficiently long run of accumulations would overflow the cache. Add
> an explicit S16_MAX guard before each add: when the next add would
> push abs(*bytes) past S16_MAX, fold the cached value into @nr and
> flush directly via mod_objcg_mlstate() before the accumulation.
> 
> Fixes: 01b9da291c49 ("mm: memcontrol: convert objcg to be per-memcg per-node type")
> Tested-by: kernel test robot <oliver.sang@xxxxxxxxx>
> Signed-off-by: Shakeel Butt <shakeel.butt@xxxxxxxxx>
> Reviewed-by: Harry Yoo (Oracle) <harry@xxxxxxxxxx>
> ---
> 
> Changes since v2:
> - Collected tags
> 
>  mm/memcontrol.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index e4f00a8159d5..78c02451312b 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -2022,8 +2022,8 @@ struct obj_stock_pcp {
>  	struct obj_cgroup *cached_objcg;
>  	uint16_t nr_bytes;
>  	int16_t node_id;
> -	int nr_slab_reclaimable_b;
> -	int nr_slab_unreclaimable_b;
> +	int16_t nr_slab_reclaimable_b;
> +	int16_t nr_slab_unreclaimable_b;
>  
>  	struct work_struct work;
>  	unsigned long flags;
> @@ -3158,7 +3158,7 @@ static void __account_obj_stock(struct obj_cgroup *objcg,
>  				struct obj_stock_pcp *stock, int nr,
>  				struct pglist_data *pgdat, enum node_stat_item idx)
>  {
> -	int *bytes;
> +	int16_t *bytes;
>  
>  	/*
>  	 * Though at the moment MAX_NUMNODES <= 1024 in all archs but let's make
> @@ -3195,6 +3195,16 @@ static void __account_obj_stock(struct obj_cgroup *objcg,
>  
>  	bytes = (idx == NR_SLAB_RECLAIMABLE_B) ? &stock->nr_slab_reclaimable_b
>  					       : &stock->nr_slab_unreclaimable_b;
> +	/*
> +	 * To avoid overflow or underflow, flush directly if accumulating @nr
> +	 * would push the cached value past S16_MAX.
> +	 */
> +	if (abs(nr + *bytes) > S16_MAX) {
> +		nr += *bytes;
> +		*bytes = 0;
> +		goto direct;
> +	}
> +

I think you should do the add first:
	nr += *bytes;
	if (abs(nr) < S16_MAX && (!*bytes || abs(nr) < PAGE_SIZE)) {
		*bytes = nr;
	} else {
		*bytes = 0;
		mod_objcg_mlstate(objcg, pgdat, idx, nr);
	}

-- David



>  	/*
>  	 * Even for large object >= PAGE_SIZE, the vmstat data will still be
>  	 * cached locally at least once before pushing it out.