Re: [PATCH v2 2/4] memcg: uint16_t for nr_bytes in obj_stock_pcp

[Shakeel Butt]

On Fri, May 22, 2026 at 10:23:31AM +0800, Qi Zheng wrote:
> 
> 
> On 5/22/26 9:19 AM, Shakeel Butt wrote:
> > Currently struct obj_stock_pcp stores nr_bytes in an 'unsigned int'
> > which is 4 bytes on 64-bit machines. Switch the field to uint16_t to
> > shrink the per-CPU cache.
> > 
> > The kernel supports PAGE_SIZE_4KB, _8KB, _16KB, _32KB, _64KB and
> > _256KB (see HAVE_PAGE_SIZE_* in arch/Kconfig). After the
> > PAGE_SIZE-aligned flush in __refill_obj_stock(), the sub-page
> > remainder fits in uint16_t up through 64KiB pages where PAGE_SIZE - 1
> > == U16_MAX, but on 256KiB pages PAGE_SIZE - 1 == 0x3FFFF exceeds
> > U16_MAX. The accumulator also needs to stay within uint16_t between
> > page-aligned flushes on 64KiB pages where PAGE_SIZE itself is
> > U16_MAX + 1.
> > 
> > Accumulate the new total in an 'unsigned int' local, then:
> > 
> >    1. Flush whenever the accumulator would hit U16_MAX. Together with
> >       the existing allow_uncharge flush at PAGE_SIZE, this keeps the
> >       uint16_t safe on PAGE_SIZE <= 64KiB.
> > 
> >    2. On configs with PAGE_SHIFT > 16 (PAGE_SIZE_256KB on hexagon and
> >       powerpc 44x), push any sub-page remainder above U16_MAX into
> >       objcg->nr_charged_bytes via atomic_add before storing back, so
> >       the store cannot silently truncate. The PAGE_SHIFT > 16 guard
> >       folds the branch out at compile time on smaller page sizes.
> > 
> > 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 v1:
> > - Collected tags
> > - Rearrange fields of obj_stock_pcp (David Laight)
> > - Fix comparison operator (Harry)
> > 
> >   mm/memcontrol.c | 33 +++++++++++++++++++++++++++------
> >   1 file changed, 27 insertions(+), 6 deletions(-)
> > 
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index d7c162946719..e4f00a8159d5 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -2019,8 +2019,8 @@ static DEFINE_PER_CPU_ALIGNED(struct memcg_stock_pcp, memcg_stock) = {
> >   struct obj_stock_pcp {
> >   	local_trylock_t lock;
> > -	unsigned int nr_bytes;
> >   	struct obj_cgroup *cached_objcg;
> > +	uint16_t nr_bytes;
> >   	int16_t node_id;
> >   	int nr_slab_reclaimable_b;
> >   	int nr_slab_unreclaimable_b;
> > @@ -3331,6 +3331,7 @@ static void __refill_obj_stock(struct obj_cgroup *objcg,
> >   			       bool allow_uncharge)
> >   {
> >   	unsigned int nr_pages = 0;
> > +	unsigned int stock_nr_bytes;
> >   	if (!stock) {
> >   		nr_pages = nr_bytes >> PAGE_SHIFT;
> > @@ -3339,21 +3340,41 @@ static void __refill_obj_stock(struct obj_cgroup *objcg,
> >   		goto out;
> >   	}
> > +	stock_nr_bytes = stock->nr_bytes;
> >   	if (READ_ONCE(stock->cached_objcg) != objcg) { /* reset if necessary */
> >   		drain_obj_stock(stock);
> >   		obj_cgroup_get(objcg);
> > -		stock->nr_bytes = atomic_read(&objcg->nr_charged_bytes)
> > +		stock_nr_bytes = atomic_read(&objcg->nr_charged_bytes)
> >   				? atomic_xchg(&objcg->nr_charged_bytes, 0) : 0;
> >   		WRITE_ONCE(stock->cached_objcg, objcg);
> >   		allow_uncharge = true;	/* Allow uncharge when objcg changes */
> >   	}
> > -	stock->nr_bytes += nr_bytes;
> > +	stock_nr_bytes += nr_bytes;
> > +
> > +	/* Since stock->nr_bytes is uint16_t, don't refill >= U16_MAX */
> 
>                                                            ^
> 
> should also be changed to: don't refill > U16_MAX ?
> 
> Otherwise:
> 
> Acked-by: Qi Zheng <qi.zheng@xxxxxxxxx>

Thanks. If I send a new version, I will fix this otherwise I will ask Andrew to
fix this inplace.