Re: [PATCH] mm/vmscan: add pgstall to memcg

[Chuyi Zhou <zhouchuyi@xxxxxxxxxxxxx>]

在 2022/6/25 上午4:42, Andrew Morton 写道:
> (cc memcg developers)
> On Thu, 23 Jun 2022 16:01:03 +0800 zhouchuyi <zhouchuyi@xxxxxxxxxxxxx> wrote:
>
> > Add pgstall to indicate the times of entering direct reclaim of memcg, use
> > the item of ALLOCSTALL_NORMAL in vmevents to save the data.
> >
> > Signed-off-by: zhouchuyi <zhouchuyi@xxxxxxxxxxxxx>
> > ---
> >
> >
> > Hi，
> >
> > I notice memcg uses some items to indicate the situation of direct reclaim in memory.stat，such as pgsteal and pgscan.
> > I think there is no harm in adding another item to show the times of entering direct reclaim.
>
> Why do you consider this useful?  Do you have a use case?  If so,
> please describe it in detail.
Since there are allocstall in proc/vmstat which indicate the number of 
entering direct reclaim of !cgroup_reclaim, So I think it maybe useful. 
For example, users could adjust memory.max to prevent entering direct 
reclaim through this item.So what do you think about it ?
> > ...
> >
> > diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
> > index 176298f2f..44c5b15bd 100644
> > --- a/Documentation/admin-guide/cgroup-v2.rst
> > +++ b/Documentation/admin-guide/cgroup-v2.rst
> > @@ -1447,8 +1447,11 @@ PAGE_SIZE multiple when read back.
> >   
> >   	  pgsteal (npn)
> >   		Amount of reclaimed pages
> > -
> > -	  pgactivate (npn)
> > +
> > +          pgstall (npn)
> > +	  	Number of entering direct reclaim
> > +	
> > +          pgactivate (npn)
> >   		Amount of pages moved to the active LRU list
> >   
> >   	  pgdeactivate (npn)
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index abec50f31..f407084b2 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -1505,6 +1505,8 @@ static char *memory_stat_format(struct mem_cgroup *memcg)
> >   	seq_buf_printf(&s, "pgscan %lu\n",
> >   		       memcg_events(memcg, PGSCAN_KSWAPD) +
> >   		       memcg_events(memcg, PGSCAN_DIRECT));
> > +	seq_buf_printf(&s, "pgstall %lu\n",
> > +			   memcg_events(memcg, ALLOCSTALL_NORMAL));
> >   	seq_buf_printf(&s, "pgsteal %lu\n",
> >   		       memcg_events(memcg, PGSTEAL_KSWAPD) +
> >   		       memcg_events(memcg, PGSTEAL_DIRECT));
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index f7d9a683e..107cec59a 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -3532,7 +3532,10 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
> >   
> >   	if (!cgroup_reclaim(sc))
> >   		__count_zid_vm_events(ALLOCSTALL, sc->reclaim_idx, 1);
> > -
> > +	else {
> > +		count_memcg_events(sc->target_mem_cgroup,
> > +					ALLOCSTALL_NORMAL, 1);
> > +	}
> >   	do {
> >   		vmpressure_prio(sc->gfp_mask, sc->target_mem_cgroup,
> >   				sc->priority);
> > --
> > 2.20.1