On Wed, Dec 02, 2020 at 10:27:21AM -0800, Yang Shi wrote: > Currently the number of deferred objects are per shrinker, but some slabs, > for example, > vfs inode/dentry cache are per memcg, this would result in poor isolation > among memcgs. > > The deferred objects typically are generated by __GFP_NOFS allocations, one > memcg with > excessive __GFP_NOFS allocations may blow up deferred objects, then other > innocent memcgs > may suffer from over shrink, excessive reclaim latency, etc. > > For example, two workloads run in memcgA and memcgB respectively, workload in > B is vfs > heavy workload. Workload in A generates excessive deferred objects, then B's > vfs cache > might be hit heavily (drop half of caches) by B's limit reclaim or global > reclaim. > > We observed this hit in our production environment which was running vfs > heavy workload > shown as the below tracing log: > > <...>-409454 [016] .... 28286961.747146: mm_shrink_slab_start: > super_cache_scan+0x0/0x1a0 ffff9a83046f3458: > nid: 1 objects to shrink 3641681686040 gfp_flags > GFP_HIGHUSER_MOVABLE|__GFP_ZERO pgs_scanned 1 lru_pgs 15721 > cache items 246404277 delta 31345 total_scan 123202138 > <...>-409454 [022] .... 28287105.928018: mm_shrink_slab_end: > super_cache_scan+0x0/0x1a0 ffff9a83046f3458: > nid: 1 unused scan count 3641681686040 new scan count 3641798379189 > total_scan 602 > last shrinker return val 123186855 > > The vfs cache and page cache ration was 10:1 on this machine, and half of > caches were dropped. > This also resulted in significant amount of page caches were dropped due to > inodes eviction. > > Make nr_deferred per memcg for memcg aware shrinkers would solve the > unfairness and bring > better isolation. > > When memcg is not enabled (!CONFIG_MEMCG or memcg disabled), the shrinker's > nr_deferred > would be used. And non memcg aware shrinkers use shrinker's nr_deferred all > the time. > > Signed-off-by: Yang Shi <[email protected]> > --- > include/linux/memcontrol.h | 9 +++ > mm/memcontrol.c | 112 ++++++++++++++++++++++++++++++++++++- > mm/vmscan.c | 4 ++ > 3 files changed, 123 insertions(+), 2 deletions(-) > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h > index 922a7f600465..1b343b268359 100644 > --- a/include/linux/memcontrol.h > +++ b/include/linux/memcontrol.h > @@ -92,6 +92,13 @@ struct lruvec_stat { > long count[NR_VM_NODE_STAT_ITEMS]; > }; > > + > +/* Shrinker::id indexed nr_deferred of memcg-aware shrinkers. */ > +struct memcg_shrinker_deferred { > + struct rcu_head rcu; > + atomic_long_t nr_deferred[]; > +};
The idea makes total sense to me. But I wonder if we can add nr_deferred to struct list_lru_one, instead of adding another per-memcg per-shrinker entity? I guess it can simplify the code quite a lot. What do you think?
