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.
