Re: [PATCH v3] memcg: keep folio's objcg same as its node

On 8/7/26 10:24 PM, Shakeel Butt wrote:
> memcg_reparent_objcgs() has an inherent assumption that a folio's objcg
> is the objcg of the folio's node.  Folio migration across nodes breaks
> that assumption: the new folio simply inherits the old folio's objcg
> while living on a different node.
> 
> Once the assumption is broken, the reparenting of the folio's objcg and
> the reparenting of the folio's LRU list are no longer atomic.
> memcg_reparent_objcgs() handles one node per iteration and drops all the
> locks in between, so the objcg gets reparented in the iteration for the
> objcg's node while the LRU list gets spliced in the iteration for the
> folio's node.  Any LRU operation on that folio in between resolves its
> lruvec through the objcg, and thus takes the lru_lock of the wrong
> memcg, not the lru_lock of the list the folio is actually on.
> 
> Fix this by selecting the objcg by folio_nid() at charge time, and by
> re-deriving it for the destination node in mem_cgroup_migrate() and
> mem_cgroup_replace_folio().
> 
> Reported-by: Karl Erik Hofseth <[email protected]>
> Closes: https://lore.kernel.org/all/anMmd1ADrDVwMO6v@work/
> Fixes: f1cf8d2f36dc ("mm: memcontrol: eliminate the problem of dying memory cgroup for LRU folios")
> Cc: [email protected]
> Co-developed-by: Johannes Weiner <[email protected]>
> Signed-off-by: Johannes Weiner <[email protected]>
> Signed-off-by: Shakeel Butt <[email protected]>
> ---
> 
> Changes since v2:
> http://lore.kernel.org/[email protected]
> 
> - Refactor common code between mem_cgroup_replace_folio and mem_cgroup_migrate
>    (Johannes)
> - Always commit the destination node's objcg. (Johannes)
> - In mem_cgroup_replace_folio, force charge based on committed objcg (Johannes)
> - In mem_cgroup_migrate, if destination node's objcg is root, uncharge the
>    source node's objcg. (Johannes)
> 
> 
> Changes since v1:
> http://lore.kernel.org/[email protected]
> 
> - In mem_cgroup_migrate, do obj_cgroup_put at the end (Sashiko)
> - Handle scenario where destination node has been reparented to the root but the
>    source node's objcg has not yet (Sashiko)
> - Add comment explaining the race between migration and reparenting (Johannes)
> 
>   mm/memcontrol.c | 100 ++++++++++++++++++++++++++++++++++++++++--------
>   1 file changed, 83 insertions(+), 17 deletions(-)
> 

Thanks for the fix!

Acked-by: Qi Zheng <[email protected]>