Hello Yosry, On Tue, Feb 28, 2023 at 08:50:00AM +0000, Yosry Ahmed wrote: > Reclaimed pages through other means than LRU-based reclaim are tracked > through reclaim_state in struct scan_control, which is stashed in > current task_struct. These pages are added to the number of reclaimed > pages through LRUs. For memcg reclaim, these pages generally cannot be > linked to the memcg under reclaim and can cause an overestimated count > of reclaimed pages. This short series tries to address that. Could you please add more details on how this manifests as a problem with real workloads? > Patch 1 is just refactoring updating reclaim_state into a helper > function, and renames reclaimed_slab to just reclaimed, with a comment > describing its true purpose. Looking through the code again, I don't think these helpers add value. report_freed_pages() is fairly vague. Report to who? It abstracts only two lines of code, and those two lines are more descriptive of what's happening than the helper is. Just leave them open-coded. add_non_vmanscan_reclaimed() may or may not add anything. But let's take a step back. It only has two callsites because lrugen duplicates the entire reclaim implementation, including the call to shrink_slab() and the transfer of reclaim_state to sc->nr_reclaimed. IMO the resulting code would overall be simpler, less duplicative and easier to follow if you added a common shrink_slab_reclaim() that takes sc, handles the transfer, and documents the memcg exception.
