Re: [PATCH v2 3/5] fs: Basic infrastructure for offloading inode reclaim

[Andreas Gruenbacher <agruenba@redhat.com>]

Jan,

it seems that we don't really need struct inodes_to_prune; see below.

diff --git a/fs/inode.c b/fs/inode.c
index 7c69ef44619a..f8303af66fcd 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -938,12 +938,6 @@ void evict_inodes(struct super_block *sb)
 }
 EXPORT_SYMBOL_GPL(evict_inodes);
 
-struct inodes_to_prune {
-	struct list_head freeable;
-	struct list_head deferred;
-	unsigned int deferred_count;
-};
-
 /*
  * Isolate the inode from the LRU in preparation for freeing it.
  *
@@ -958,7 +952,7 @@ struct inodes_to_prune {
 static enum lru_status inode_lru_isolate(struct list_head *item,
 		struct list_lru_one *lru, void *arg)
 {
-	struct inodes_to_prune *lists = arg;
+	struct list_head *freeable = arg;
 	struct inode	*inode = container_of(item, struct inode, i_lru);
 
 	/*
@@ -1013,13 +1007,7 @@ static enum lru_status inode_lru_isolate(struct list_head *item,
 
 	WARN_ON(inode_state_read(inode) & I_NEW);
 	inode_state_set(inode, I_FREEING);
-	/* Inode will take long time to cleanup. Offload that to worker. */
-	if (inode_state_read(inode) & I_DEFER_RECLAIM) {
-		list_lru_isolate_move(lru, &inode->i_lru, &lists->deferred);
-		lists->deferred_count++;
-	} else {
-		list_lru_isolate_move(lru, &inode->i_lru, &lists->freeable);
-	}
+	list_lru_isolate_move(lru, &inode->i_lru, freeable);
 	spin_unlock(&inode->i_lock);
 
 	this_cpu_dec(nr_unused);
@@ -1042,27 +1030,34 @@ static int get_deferred_sb_id(struct super_block *sb)
  */
 long prune_icache_sb(struct super_block *sb, struct shrink_control *sc)
 {
-	struct inodes_to_prune lists = {
-		.freeable = LIST_HEAD_INIT(lists.freeable),
-		.deferred = LIST_HEAD_INIT(lists.deferred),
-	};
-	long freed;
+	struct inode *inode, *tmp;
+	long freed, deferred = 0;
+	LIST_HEAD(freeable);
 
 	freed = list_lru_shrink_walk(&sb->s_inode_lru, sc,
-				     inode_lru_isolate, &lists);
-	dispose_list(&lists.freeable);
-	if (!list_empty(&lists.deferred)) {
+				     inode_lru_isolate, &freeable);
+
+	list_for_each_entry_safe(inode, tmp, &freeable, i_lru) {
+		if (inode_state_read(inode) & I_DEFER_RECLAIM) {
+			deferred++;
+			continue;
+		}
+		list_del_init(&inode->i_lru);
+		evict(inode);
+		cond_resched();
+	}
+
+	if (!list_empty(&freeable)) {
 		int id = get_deferred_sb_id(sb);
 		int i, wake_count;
 
 		spin_lock(&deferred_reclaim.queues[id].lock);
-		list_splice_tail(&lists.deferred,
-				 &deferred_reclaim.queues[id].list);
-		atomic_add(lists.deferred_count, &sb->s_deferred_reclaim_count);
+		list_splice_tail(&freeable, &deferred_reclaim.queues[id].list);
+		atomic_add(deferred, &sb->s_deferred_reclaim_count);
 		spin_unlock(&deferred_reclaim.queues[id].lock);
 
 		/* Queue works to process inodes we've added to the list */
-		wake_count = (lists.deferred_count + INODE_RECLAIM_BATCH_SIZE)
+		wake_count = (deferred + INODE_RECLAIM_BATCH_SIZE)
 						/ INODE_RECLAIM_BATCH_SIZE;
 		if (wake_count > deferred_reclaim.workers)
 			wake_count = deferred_reclaim.workers;
-- 
2.55.0

Thanks,
Andreas