Changelog
v3 --> v4:
* Removed "memcg_" prefix for shrinker_maps related functions per Roman.
* Use write lock instead of read lock per Kirill. Also removed Johannes's ack
since write lock is used.
* Incorporated the comments from Kirill.
* Removed RFC.
* Rebased to v5.11-rc4.
v2 --> v3:
* Moved shrinker_maps related code to vmscan.c per Dave.
* Removed memcg_shrinker_map_size. Calcuated the size of map via shrinker_nr_max
per Johannes.
* Consolidated shrinker_deferred with shrinker_maps into one struct per Dave.
* Simplified the nr_deferred related code.
* Dropped the memory barrier from v2.
* Moved nr_deferred reparent code to vmscan.c per Dave.
* Added test coverage information in patch #11. Dave is concerned about the
potential regression. I didn't notice regression with my tests, but suggestions
about more test coverage is definitely welcome. And it may help spot regression
with this patch in -mm tree then linux-next tree so I keep it in this version.
* The code cleanup and consolidation resulted in the series grow to 11 patches.
* Rebased onto 5.11-rc2.
v1 --> v2:
* Use shrinker->flags to store the new SHRINKER_REGISTERED flag per Roman.
* Folded patch #1 into patch #6 per Roman.
* Added memory barrier to prevent shrink_slab_memcg from seeing NULL shrinker_maps/
shrinker_deferred per Kirill.
* Removed memcg_shrinker_map_mutex. Protcted shrinker_map/shrinker_deferred
allocations from expand with shrinker_rwsem per Johannes.
Recently huge amount one-off slab drop was seen on some vfs metadata heavy workloads,
it turned out there were huge amount accumulated nr_deferred objects seen by the
shrinker.
On our production machine, I saw absurd number of nr_deferred shown as the below
tracing result:
<...>-48776 [032] .... 27970562.458916: mm_shrink_slab_start:
super_cache_scan+0x0/0x1a0 ffff9a83046f3458: nid: 0 objects to shrink
2531805877005 gfp_flags GFP_HIGHUSER_MOVABLE pgs_scanned 32 lru_pgs
9300 cache items 1667 delta 11 total_scan 833
There are 2.5 trillion deferred objects on one node, assuming all of them
are dentry (192 bytes per object), so the total size of deferred on
one node is ~480TB. It is definitely ridiculous.
I managed to reproduce this problem with kernel build workload plus negative dentry
generator.
First step, run the below kernel build test script:
NR_CPUS=`cat /proc/cpuinfo | grep -e processor | wc -l`
cd /root/Buildarea/linux-stable
for i in `seq 1500`; do
cgcreate -g memory:kern_build
echo 4G > /sys/fs/cgroup/memory/kern_build/memory.limit_in_bytes
echo 3 > /proc/sys/vm/drop_caches
cgexec -g memory:kern_build make clean > /dev/null 2>&1
cgexec -g memory:kern_build make -j$NR_CPUS > /dev/null 2>&1
cgdelete -g memory:kern_build
done
Then run the below negative dentry generator script:
NR_CPUS=`cat /proc/cpuinfo | grep -e processor | wc -l`
mkdir /sys/fs/cgroup/memory/test
echo $$ > /sys/fs/cgroup/memory/test/tasks
for i in `seq $NR_CPUS`; do
while true; do
FILE=`head /dev/urandom | tr -dc A-Za-z0-9 | head -c 64`
cat $FILE 2>/dev/null
done &
done
Then kswapd will shrink half of dentry cache in just one loop as the below tracing result
showed:
kswapd0-475 [028] .... 305968.252561: mm_shrink_slab_start: super_cache_scan+0x0/0x190 0000000024acf00c: nid: 0
objects to shrink 4994376020 gfp_flags GFP_KERNEL cache items 93689873 delta 45746 total_scan 46844936 priority 12
kswapd0-475 [021] .... 306013.099399: mm_shrink_slab_end: super_cache_scan+0x0/0x190 0000000024acf00c: nid: 0 unused
scan count 4994376020 new scan count 4947576838 total_scan 8 last shrinker return val 46844928
There were huge number of deferred objects before the shrinker was called, the behavior
does match the code but it might be not desirable from the user's stand of point.
The excessive amount of nr_deferred might be accumulated due to various reasons, for example:
* GFP_NOFS allocation
* Significant times of small amount scan (< scan_batch, 1024 for vfs metadata)
However the LRUs of slabs are per memcg (memcg-aware shrinkers) but the deferred objects
is per shrinker, this may have some bad effects:
* Poor isolation among memcgs. Some memcgs which happen to have frequent limit
reclaim may get nr_deferred accumulated to a huge number, then other innocent
memcgs may take the fall. In our case the main workload was hit.
* Unbounded deferred objects. There is no cap for deferred objects, it can outgrow
ridiculously as the tracing result showed.
* Easy to get out of control. Although shrinkers take into account deferred objects,
but it can go out of control easily. One misconfigured memcg could incur absurd
amount of deferred objects in a period of time.
* Sort of reclaim problems, i.e. over reclaim, long reclaim latency, etc. There may be
hundred GB slab caches for vfe metadata heavy workload, shrink half of them may take
minutes. We observed latency spike due to the prolonged reclaim.
These issues also have been discussed in https://lore.kernel.org/linux-mm/[email protected]/.
The patchset is the outcome of that discussion.
So this patchset makes nr_deferred per-memcg to tackle the problem. It does:
* Have memcg_shrinker_deferred per memcg per node, just like what shrinker_map
does. Instead it is an atomic_long_t array, each element represent one shrinker
even though the shrinker is not memcg aware, this simplifies the implementation.
For memcg aware shrinkers, the deferred objects are just accumulated to its own
memcg. The shrinkers just see nr_deferred from its own memcg. Non memcg aware
shrinkers still use global nr_deferred from struct shrinker.
* Once the memcg is offlined, its nr_deferred will be reparented to its parent along
with LRUs.
* The root memcg has memcg_shrinker_deferred array too. It simplifies the handling of
reparenting to root memcg.
* Cap nr_deferred to 2x of the length of lru. The idea is borrowed from Dave Chinner's
series (https://lore.kernel.org/linux-xfs/[email protected]/)
The downside is each memcg has to allocate extra memory to store the nr_deferred array.
On our production environment, there are typically around 40 shrinkers, so each memcg
needs ~320 bytes. 10K memcgs would need ~3.2MB memory. It seems fine.
We have been running the patched kernel on some hosts of our fleet (test and production) for
months, it works very well. The monitor data shows the working set is sustained as expected.
Yang Shi (11):
mm: vmscan: use nid from shrink_control for tracepoint
mm: vmscan: consolidate shrinker_maps handling code
mm: vmscan: use shrinker_rwsem to protect shrinker_maps allocation
mm: vmscan: remove memcg_shrinker_map_size
mm: memcontrol: rename shrinker_map to shrinker_info
mm: vmscan: use a new flag to indicate shrinker is registered
mm: vmscan: add per memcg shrinker nr_deferred
mm: vmscan: use per memcg nr_deferred of shrinker
mm: vmscan: don't need allocate shrinker->nr_deferred for memcg aware shrinkers
mm: memcontrol: reparent nr_deferred when memcg offline
mm: vmscan: shrink deferred objects proportional to priority
include/linux/memcontrol.h | 24 +++---
include/linux/shrinker.h | 7 +-
mm/huge_memory.c | 4 +-
mm/list_lru.c | 6 +-
mm/memcontrol.c | 131 +--------------------------------
mm/vmscan.c | 371 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------------
6 files changed, 305 insertions(+), 238 deletions(-)
The number of deferred objects might get windup to an absurd number, and it results in
clamp of slab objects. It is undesirable for sustaining workingset.
So shrink deferred objects proportional to priority and cap nr_deferred to twice of
cache items.
The idea is borrowed fron Dave Chinner's patch:
https://lore.kernel.org/linux-xfs/[email protected]/
Tested with kernel build and vfs metadata heavy workload, no regression is spotted
so far. But it still may have regression for some corner cases.
Signed-off-by: Yang Shi <[email protected]>
---
mm/vmscan.c | 40 +++++-----------------------------------
1 file changed, 5 insertions(+), 35 deletions(-)
diff --git a/mm/vmscan.c b/mm/vmscan.c
index e73f200ffd2d..bb254d39339f 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -659,7 +659,6 @@ static unsigned long do_shrink_slab(struct shrink_control *shrinkctl,
*/
nr = count_nr_deferred(shrinker, shrinkctl);
- total_scan = nr;
if (shrinker->seeks) {
delta = freeable >> priority;
delta *= 4;
@@ -673,37 +672,9 @@ static unsigned long do_shrink_slab(struct shrink_control *shrinkctl,
delta = freeable / 2;
}
+ total_scan = nr >> priority;
total_scan += delta;
- if (total_scan < 0) {
- pr_err("shrink_slab: %pS negative objects to delete nr=%ld\n",
- shrinker->scan_objects, total_scan);
- total_scan = freeable;
- next_deferred = nr;
- } else
- next_deferred = total_scan;
-
- /*
- * We need to avoid excessive windup on filesystem shrinkers
- * due to large numbers of GFP_NOFS allocations causing the
- * shrinkers to return -1 all the time. This results in a large
- * nr being built up so when a shrink that can do some work
- * comes along it empties the entire cache due to nr >>>
- * freeable. This is bad for sustaining a working set in
- * memory.
- *
- * Hence only allow the shrinker to scan the entire cache when
- * a large delta change is calculated directly.
- */
- if (delta < freeable / 4)
- total_scan = min(total_scan, freeable / 2);
-
- /*
- * Avoid risking looping forever due to too large nr value:
- * never try to free more than twice the estimate number of
- * freeable entries.
- */
- if (total_scan > freeable * 2)
- total_scan = freeable * 2;
+ total_scan = min(total_scan, (2 * freeable));
trace_mm_shrink_slab_start(shrinker, shrinkctl, nr,
freeable, delta, total_scan, priority);
@@ -742,10 +713,9 @@ static unsigned long do_shrink_slab(struct shrink_control *shrinkctl,
cond_resched();
}
- if (next_deferred >= scanned)
- next_deferred -= scanned;
- else
- next_deferred = 0;
+ next_deferred = max_t(long, (nr - scanned), 0) + total_scan;
+ next_deferred = min(next_deferred, (2 * freeable));
+
/*
* move the unused scan count back into the shrinker in a
* manner that handles concurrent updates.
--
2.26.2
Since memcg_shrinker_map_size just can be changed under holding shrinker_rwsem
exclusively, the read side can be protected by holding read lock, so it sounds
superfluous to have a dedicated mutex.
Kirill Tkhai suggested use write lock since:
* We want the assignment to shrinker_maps is visible for shrink_slab_memcg().
* The rcu_dereference_protected() dereferrencing in shrink_slab_memcg(), but
in case of we use READ lock in alloc_shrinker_maps(), the dereferrencing
is not actually protected.
* READ lock makes alloc_shrinker_info() racy against memory allocation fail.
alloc_shrinker_info()->free_shrinker_info() may free memory right after
shrink_slab_memcg() dereferenced it. You may say
shrink_slab_memcg()->mem_cgroup_online() protects us from it? Yes, sure,
but this is not the thing we want to remember in the future, since this
spreads modularity.
And a test with heavy paging workload didn't show write lock makes things worse.
Signed-off-by: Yang Shi <[email protected]>
---
mm/vmscan.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/mm/vmscan.c b/mm/vmscan.c
index d950cead66ca..d3f3701dfcd2 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -187,7 +187,6 @@ static DECLARE_RWSEM(shrinker_rwsem);
#ifdef CONFIG_MEMCG
static int memcg_shrinker_map_size;
-static DEFINE_MUTEX(memcg_shrinker_map_mutex);
static void free_shrinker_map_rcu(struct rcu_head *head)
{
@@ -200,8 +199,6 @@ static int expand_one_shrinker_map(struct mem_cgroup *memcg,
struct memcg_shrinker_map *new, *old;
int nid;
- lockdep_assert_held(&memcg_shrinker_map_mutex);
-
for_each_node(nid) {
old = rcu_dereference_protected(
mem_cgroup_nodeinfo(memcg, nid)->shrinker_map, true);
@@ -250,7 +247,7 @@ int alloc_shrinker_maps(struct mem_cgroup *memcg)
if (mem_cgroup_is_root(memcg))
return 0;
- mutex_lock(&memcg_shrinker_map_mutex);
+ down_write(&shrinker_rwsem);
size = memcg_shrinker_map_size;
for_each_node(nid) {
map = kvzalloc_node(sizeof(*map) + size, GFP_KERNEL, nid);
@@ -261,7 +258,7 @@ int alloc_shrinker_maps(struct mem_cgroup *memcg)
}
rcu_assign_pointer(memcg->nodeinfo[nid]->shrinker_map, map);
}
- mutex_unlock(&memcg_shrinker_map_mutex);
+ up_write(&shrinker_rwsem);
return ret;
}
@@ -276,9 +273,8 @@ static int expand_shrinker_maps(int new_id)
if (size <= old_size)
return 0;
- mutex_lock(&memcg_shrinker_map_mutex);
if (!root_mem_cgroup)
- goto unlock;
+ goto out;
memcg = mem_cgroup_iter(NULL, NULL, NULL);
do {
@@ -287,13 +283,13 @@ static int expand_shrinker_maps(int new_id)
ret = expand_one_shrinker_map(memcg, size, old_size);
if (ret) {
mem_cgroup_iter_break(NULL, memcg);
- goto unlock;
+ goto out;
}
} while ((memcg = mem_cgroup_iter(NULL, memcg, NULL)) != NULL);
-unlock:
+out:
if (!ret)
memcg_shrinker_map_size = size;
- mutex_unlock(&memcg_shrinker_map_mutex);
+
return ret;
}
--
2.26.2