Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Guowei Du <duguoweisz@gmail.com>
To:     richard@nod.at
Cc:     linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org,
        duguowei <duguowei@xiaomi.com>
Subject: [PATCH] ubifs: sequence each shrink task
Date:   Thu,  7 Jul 2022 20:30:33 +0800
Message-Id: <20220707123033.29042-1-duguoweisz@gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

From: duguowei <duguowei@xiaomi.com>

Because of 'list_move_tail', if two or more tasks are shrinking, there
will be different results for them.

For example:
After the first round, if shrink_run_no of entry equals run_no of task,
task will break directly at the beginning of next round; if they are
not equal, task will continue to shrink until encounter one entry
which has the same number.

It is difficult to confirm the real results of all tasks, so add a lock
to only allow one task to shrink at the same time.

Signed-off-by: duguowei <duguowei@xiaomi.com>
---
 fs/ubifs/shrinker.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/fs/ubifs/shrinker.c b/fs/ubifs/shrinker.c
index d00a6f20ac7b..900644ef432d 100644
--- a/fs/ubifs/shrinker.c
+++ b/fs/ubifs/shrinker.c
@@ -42,6 +42,9 @@ static unsigned int shrinker_run_no;
 /* Protects 'ubifs_infos' list */
 DEFINE_SPINLOCK(ubifs_infos_lock);
 
+/* Sequence each shrink task*/
+DEFINE_SPINLOCK(ubifs_shrink_lock);
+
 /* Global clean znode counter (for all mounted UBIFS instances) */
 atomic_long_t ubifs_clean_zn_cnt;
 
@@ -145,13 +148,12 @@ static int shrink_tnc_trees(int nr, int age, int *contention)
 {
 	struct ubifs_info *c;
 	struct list_head *p;
-	unsigned int run_no;
 	int freed = 0;
 
 	spin_lock(&ubifs_infos_lock);
-	do {
-		run_no = ++shrinker_run_no;
-	} while (run_no == 0);
+	shrinker_run_no++;
+	if (shrinker_run_no == 0)
+		shrinker_run_no = 1;
 	/* Iterate over all mounted UBIFS file-systems and try to shrink them */
 	p = ubifs_infos.next;
 	while (p != &ubifs_infos) {
@@ -160,7 +162,7 @@ static int shrink_tnc_trees(int nr, int age, int *contention)
 		 * We move the ones we do to the end of the list, so we stop
 		 * when we see one we have already done.
 		 */
-		if (c->shrinker_run_no == run_no)
+		if (c->shrinker_run_no == shrinker_run_no)
 			break;
 		if (!mutex_trylock(&c->umount_mutex)) {
 			/* Some un-mount is in progress, try next FS */
@@ -183,7 +185,7 @@ static int shrink_tnc_trees(int nr, int age, int *contention)
 		 * OK, now we have TNC locked, the file-system cannot go away -
 		 * it is safe to reap the cache.
 		 */
-		c->shrinker_run_no = run_no;
+		c->shrinker_run_no = shrinker_run_no;
 		freed += shrink_tnc(c, nr, age, contention);
 		mutex_unlock(&c->tnc_mutex);
 		spin_lock(&ubifs_infos_lock);
@@ -283,7 +285,9 @@ unsigned long ubifs_shrink_scan(struct shrinker *shrink,
 	int contention = 0;
 	unsigned long freed;
 	long clean_zn_cnt = atomic_long_read(&ubifs_clean_zn_cnt);
+	int ret = 0;
 
+	spin_lock(&ubifs_shrink_lock);
 	if (!clean_zn_cnt) {
 		/*
 		 * No clean znodes, nothing to reap. All we can do in this case
@@ -293,7 +297,9 @@ unsigned long ubifs_shrink_scan(struct shrinker *shrink,
 		 * later.
 		 */
 		dbg_tnc("no clean znodes, kick a thread");
-		return kick_a_thread();
+		ret = kick_a_thread();
+		spin_unlock(&ubifs_shrink_lock);
+		return ret;
 	}
 
 	freed = shrink_tnc_trees(nr, OLD_ZNODE_AGE, &contention);
@@ -310,10 +316,12 @@ unsigned long ubifs_shrink_scan(struct shrinker *shrink,
 
 	if (!freed && contention) {
 		dbg_tnc("freed nothing, but contention");
+		spin_unlock(&ubifs_shrink_lock);
 		return SHRINK_STOP;
 	}
 
 out:
 	dbg_tnc("%lu znodes were freed, requested %lu", freed, nr);
+	spin_unlock(&ubifs_shrink_lock);
 	return freed;
 }
-- 
2.36.1