Skip to content
Snippets Groups Projects
  • Shakeel Butt's avatar
    387d2c54
    memcg: protect concurrent access to mem_cgroup_idr · 387d2c54
    Shakeel Butt authored and Frieder Schrempf's avatar Frieder Schrempf committed
    commit 9972605a upstream.
    
    Commit 73f576c0 ("mm: memcontrol: fix cgroup creation failure after
    many small jobs") decoupled the memcg IDs from the CSS ID space to fix the
    cgroup creation failures.  It introduced IDR to maintain the memcg ID
    space.  The IDR depends on external synchronization mechanisms for
    modifications.  For the mem_cgroup_idr, the idr_alloc() and idr_replace()
    happen within css callback and thus are protected through cgroup_mutex
    from concurrent modifications.  However idr_remove() for mem_cgroup_idr
    was not protected against concurrency and can be run concurrently for
    different memcgs when they hit their refcnt to zero.  Fix that.
    
    We have been seeing list_lru based kernel crashes at a low frequency in
    our fleet for a long time.  These crashes were in different part of
    list_lru code including list_lru_add(), list_lru_del() and reparenting
    code.  Upon further inspection, it looked like for a given object (dentry
    and inode), the super_block's list_lru didn't have list_lru_one for the
    memcg of that object.  The initial suspicions were either the object is
    not allocated through kmem_cache_alloc_lru() or somehow
    memcg_list_lru_alloc() failed to allocate list_lru_one() for a memcg but
    returned success.  No evidence were found for these cases.
    
    Looking more deeply, we started seeing situations where valid memcg's id
    is not present in mem_cgroup_idr and in some cases multiple valid memcgs
    have same id and mem_cgroup_idr is pointing to one of them.  So, the most
    reasonable explanation is that these situations can happen due to race
    between multiple idr_remove() calls or race between
    idr_alloc()/idr_replace() and idr_remove().  These races are causing
    multiple memcgs to acquire the same ID and then offlining of one of them
    would cleanup list_lrus on the system for all of them.  Later access from
    other memcgs to the list_lru cause crashes due to missing list_lru_one.
    
    Link: https://lkml.kernel.org/r/20240802235822.1830976-1-shakeel.butt@linux.dev
    
    
    Fixes: 73f576c0 ("mm: memcontrol: fix cgroup creation failure after many small jobs")
    Signed-off-by: default avatarShakeel Butt <shakeel.butt@linux.dev>
    Acked-by: default avatarMuchun Song <muchun.song@linux.dev>
    Reviewed-by: default avatarRoman Gushchin <roman.gushchin@linux.dev>
    Acked-by: default avatarJohannes Weiner <hannes@cmpxchg.org>
    Cc: Michal Hocko <mhocko@suse.com>
    Cc: <stable@vger.kernel.org>
    Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
    [ Adapted over commit 6f0df8e1 ("memcontrol: ensure memcg acquired by id is
      properly set up") not in the tree ]
    Signed-off-by: default avatarTomas Krcka <krckatom@amazon.de>
    Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
    387d2c54
    History
    memcg: protect concurrent access to mem_cgroup_idr
    Shakeel Butt authored and Frieder Schrempf's avatar Frieder Schrempf committed
    commit 9972605a upstream.
    
    Commit 73f576c0 ("mm: memcontrol: fix cgroup creation failure after
    many small jobs") decoupled the memcg IDs from the CSS ID space to fix the
    cgroup creation failures.  It introduced IDR to maintain the memcg ID
    space.  The IDR depends on external synchronization mechanisms for
    modifications.  For the mem_cgroup_idr, the idr_alloc() and idr_replace()
    happen within css callback and thus are protected through cgroup_mutex
    from concurrent modifications.  However idr_remove() for mem_cgroup_idr
    was not protected against concurrency and can be run concurrently for
    different memcgs when they hit their refcnt to zero.  Fix that.
    
    We have been seeing list_lru based kernel crashes at a low frequency in
    our fleet for a long time.  These crashes were in different part of
    list_lru code including list_lru_add(), list_lru_del() and reparenting
    code.  Upon further inspection, it looked like for a given object (dentry
    and inode), the super_block's list_lru didn't have list_lru_one for the
    memcg of that object.  The initial suspicions were either the object is
    not allocated through kmem_cache_alloc_lru() or somehow
    memcg_list_lru_alloc() failed to allocate list_lru_one() for a memcg but
    returned success.  No evidence were found for these cases.
    
    Looking more deeply, we started seeing situations where valid memcg's id
    is not present in mem_cgroup_idr and in some cases multiple valid memcgs
    have same id and mem_cgroup_idr is pointing to one of them.  So, the most
    reasonable explanation is that these situations can happen due to race
    between multiple idr_remove() calls or race between
    idr_alloc()/idr_replace() and idr_remove().  These races are causing
    multiple memcgs to acquire the same ID and then offlining of one of them
    would cleanup list_lrus on the system for all of them.  Later access from
    other memcgs to the list_lru cause crashes due to missing list_lru_one.
    
    Link: https://lkml.kernel.org/r/20240802235822.1830976-1-shakeel.butt@linux.dev
    
    
    Fixes: 73f576c0 ("mm: memcontrol: fix cgroup creation failure after many small jobs")
    Signed-off-by: default avatarShakeel Butt <shakeel.butt@linux.dev>
    Acked-by: default avatarMuchun Song <muchun.song@linux.dev>
    Reviewed-by: default avatarRoman Gushchin <roman.gushchin@linux.dev>
    Acked-by: default avatarJohannes Weiner <hannes@cmpxchg.org>
    Cc: Michal Hocko <mhocko@suse.com>
    Cc: <stable@vger.kernel.org>
    Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
    [ Adapted over commit 6f0df8e1 ("memcontrol: ensure memcg acquired by id is
      properly set up") not in the tree ]
    Signed-off-by: default avatarTomas Krcka <krckatom@amazon.de>
    Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>