[v4,4/8] mm: vmscan: add shrinker_srcu_generation

Commit Message

Qi Zheng March 7, 2023, 6:56 a.m. UTC

From: Kirill Tkhai <[e-mail]>

After we make slab shrink lockless with SRCU, the longest
sleep unregister_shrinker() will be a sleep waiting for
all do_shrink_slab() calls.

To aviod long unbreakable action in the unregister_shrinker(),
add shrinker_srcu_generation to restore a check similar to the
rwsem_is_contendent() check that we had before.

And for memcg slab shrink, we unlock SRCU and continue
iterations from the next shrinker id.

Signed-off-by: Kirill Tkhai <[e-mail]>
Signed-off-by: Qi Zheng <[e-mail]>
---
 mm/vmscan.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

Comments

Vlastimil Babka March 9, 2023, 9:23 a.m. UTC | #1

On 3/7/23 07:56, Qi Zheng wrote:
> From: Kirill Tkhai <[e-mail]>
> 
> After we make slab shrink lockless with SRCU, the longest
> sleep unregister_shrinker() will be a sleep waiting for
> all do_shrink_slab() calls.
> 
> To aviod long unbreakable action in the unregister_shrinker(),

     ^ avoid

> add shrinker_srcu_generation to restore a check similar to the
> rwsem_is_contendent() check that we had before.
> 
> And for memcg slab shrink, we unlock SRCU and continue
> iterations from the next shrinker id.
> 
> Signed-off-by: Kirill Tkhai <[e-mail]>
> Signed-off-by: Qi Zheng <[e-mail]>

Acked-by: Vlastimil Babka <[e-mail]>

Qi Zheng March 9, 2023, 10:12 a.m. UTC | #2

On 2023/3/9 17:23, Vlastimil Babka wrote:
> On 3/7/23 07:56, Qi Zheng wrote:
>> From: Kirill Tkhai <[e-mail]>
>>
>> After we make slab shrink lockless with SRCU, the longest
>> sleep unregister_shrinker() will be a sleep waiting for
>> all do_shrink_slab() calls.
>>
>> To aviod long unbreakable action in the unregister_shrinker(),
> 
>       ^ avoid

will change.

> 
>> add shrinker_srcu_generation to restore a check similar to the
>> rwsem_is_contendent() check that we had before.
>>
>> And for memcg slab shrink, we unlock SRCU and continue
>> iterations from the next shrinker id.
>>
>> Signed-off-by: Kirill Tkhai <[e-mail]>
>> Signed-off-by: Qi Zheng <[e-mail]>
> 
> Acked-by: Vlastimil Babka <[e-mail]>

Thanks.

> 
>

Patch

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 1de9bc3e5aa2..9a5a3da5c8b5 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -204,6 +204,7 @@  static void set_task_reclaim_state(struct task_struct *task,
 LIST_HEAD(shrinker_list);
 DECLARE_RWSEM(shrinker_rwsem);
 DEFINE_SRCU(shrinker_srcu);
+static atomic_t shrinker_srcu_generation = ATOMIC_INIT(0);
 
 #ifdef CONFIG_MEMCG
 static int shrinker_nr_max;
@@ -782,6 +783,7 @@  void unregister_shrinker(struct shrinker *shrinker)
 	debugfs_entry = shrinker_debugfs_remove(shrinker);
 	up_write(&shrinker_rwsem);
 
+	atomic_inc(&shrinker_srcu_generation);
 	synchronize_srcu(&shrinker_srcu);
 
 	debugfs_remove_recursive(debugfs_entry);
@@ -803,6 +805,7 @@  void synchronize_shrinkers(void)
 {
 	down_write(&shrinker_rwsem);
 	up_write(&shrinker_rwsem);
+	atomic_inc(&shrinker_srcu_generation);
 	synchronize_srcu(&shrinker_srcu);
 }
 EXPORT_SYMBOL(synchronize_shrinkers);
@@ -912,18 +915,20 @@  static unsigned long shrink_slab_memcg(gfp_t gfp_mask, int nid,
 {
 	struct shrinker_info *info;
 	unsigned long ret, freed = 0;
-	int srcu_idx;
-	int i;
+	int srcu_idx, generation;
+	int i = 0;
 
 	if (!mem_cgroup_online(memcg))
 		return 0;
 
+again:
 	srcu_idx = srcu_read_lock(&shrinker_srcu);
 	info = shrinker_info_srcu(memcg, nid);
 	if (unlikely(!info))
 		goto unlock;
 
-	for_each_set_bit(i, info->map, info->map_nr_max) {
+	generation = atomic_read(&shrinker_srcu_generation);
+	for_each_set_bit_from(i, info->map, info->map_nr_max) {
 		struct shrink_control sc = {
 			.gfp_mask = gfp_mask,
 			.nid = nid,
@@ -969,6 +974,11 @@  static unsigned long shrink_slab_memcg(gfp_t gfp_mask, int nid,
 				set_shrinker_bit(memcg, nid, i);
 		}
 		freed += ret;
+		if (atomic_read(&shrinker_srcu_generation) != generation) {
+			srcu_read_unlock(&shrinker_srcu, srcu_idx);
+			i++;
+			goto again;
+		}
 	}
 unlock:
 	srcu_read_unlock(&shrinker_srcu, srcu_idx);
@@ -1008,7 +1018,7 @@  static unsigned long shrink_slab(gfp_t gfp_mask, int nid,
 {
 	unsigned long ret, freed = 0;
 	struct shrinker *shrinker;
-	int srcu_idx;
+	int srcu_idx, generation;
 
 	/*
 	 * The root memcg might be allocated even though memcg is disabled
@@ -1022,6 +1032,7 @@  static unsigned long shrink_slab(gfp_t gfp_mask, int nid,
 
 	srcu_idx = srcu_read_lock(&shrinker_srcu);
 
+	generation = atomic_read(&shrinker_srcu_generation);
 	list_for_each_entry_srcu(shrinker, &shrinker_list, list,
 				 srcu_read_lock_held(&shrinker_srcu)) {
 		struct shrink_control sc = {
@@ -1034,6 +1045,11 @@  static unsigned long shrink_slab(gfp_t gfp_mask, int nid,
 		if (ret == SHRINK_EMPTY)
 			ret = 0;
 		freed += ret;
+
+		if (atomic_read(&shrinker_srcu_generation) != generation) {
+			freed = freed ? : 1;
+			break;
+		}
 	}
 
 	srcu_read_unlock(&shrinker_srcu, srcu_idx);