Re: [PATCH v3 1/4] mm/filemap: add AS_UNCHARGED

[Shakeel Butt <shakeel.butt@linux.dev>]

On Wed, Aug 20, 2025 at 04:53:08PM -0700, Shakeel Butt wrote:
> On Wed, Aug 20, 2025 at 03:52:22PM -0700, Boris Burkov wrote:
> [...]
> > 
> > Thanks so much for the report and fix! I fear there might be some other
> > paths that try to get memcg from lruvec or folio or whatever without
> > checking it. I feel like in this exact case, I would want to go to the
> > first sign of trouble and fix it at lruvec_memcg(). But then who knows
> > what else we've missed.
> 
> lruvec_memcg() is not an issue but folio_memcg() can be. I found
> following instances of folio_memcg() which are problematic (on
> next-20250819):
> 
> mm/memcontrol.c:3246:   css_get_many(&__folio_memcg(folio)->css, new_refs);
> 
> include/trace/events/writeback.h:269:           __entry->page_cgroup_ino = cgroup_ino(folio_memcg(folio)->css.cgroup);
> 
> mm/workingset.c:244:    struct mem_cgroup *memcg = folio_memcg(folio);
> 
> mm/huge_memory.c:4020:  WARN_ON_ONCE(!mem_cgroup_disabled() && !folio_memcg(folio));
> 
> > 
> > May I ask what you were running to trigger this? My fstests run (clearly
> > not exercising enough interesting memory paths) did not hit it.
> > 
> > This does make me wonder if the superior approach to the original patch
> > isn't just to go back to the very first thing Qu did and account these
> > to the root cgroup rather than do the whole uncharged thing.
> 
> I don't have any strong preference one way or the other.

After thinking a bit more, I think root memcg approach by Qu should be
preferred. Using that we will avoid this unnecessary code churn for NULL
memcg checks and I am pretty sure I might have missed some places I
listed above.