Re: [PATCH v1 1/2] mm:vmscan: fix workingset eviction memcg issue

[zhiguojiang]

在 2024/1/11 21:41, Johannes Weiner 写道:
> On Thu, Jan 11, 2024 at 08:24:50PM +0800, Zhiguo Jiang wrote:
> > The parameter of target_memcg is NULL in workingset_eviction(), and
> > the lruvec obtained by mem_cgroup_lruvec(target_memcg, pgdat) is always
> > root_mem_cgroup->lruvec, rather than the lruvec of mem_cgroup where
> > folio is actually located.
> WTF? No!
>
> 	/*
> 	 * The memory cgroup that hit its limit and as a result is the
> 	 * primary target of this reclaim invocation.
> 	 */
> 	struct mem_cgroup *target_mem_cgroup;
>
> The cgroup that is stored in the eviction cookie is the one whose
> limit triggered the reclaim cycle. This is often several levels above
> the cgroups that own the pages. Subsequent refaults need to be
> evaluated at the eviction level:
>
> 	/*
> 	 * The activation decision for this folio is made at the level
> 	 * where the eviction occurred, as that is where the LRU order
> 	 * during folio reclaim is being determined.
> 	 *
> 	 * However, the cgroup that will own the folio is the one that
> 	 * is actually experiencing the refault event.
> 	 */
May I ask three questions: 1.I don't understand the meaning of this 
paragraph. Can you explain it in detail ? 2.What are the characteristics 
of folios managed by the root_mem_cgroup differring from other memcgs? 
3.If shrink flow uses target_lruvec->anon_cost/file_cost, what is the 
purposr of calculating the actual memcg's lruvec->anon_cost/file_cost in 
lru_note_cost(), and the actual memcg's lruvec->anon_cost/file_cost 
values are unused in shrink flow ? Thanks
> > Fix target_memcg to the memcg obtained by folio_memcg(folio).
> >
> > Signed-off-by: Zhiguo Jiang <justinjiang@xxxxxxxx>
> Nacked-by: Johannes Weiner <hannes@xxxxxxxxxxx>
>
> Please take more time to read into the code you're proposing to
> change. You made it sound like a trivial simplification, but this
> totally screws up aging and pressure detection in containers.