[f2fs-dev] [PATCH] f2fs: cache: count the temporary pins apart from the regular references

[Daeho Jeong <daeho43@gmail.com>]

From: Daeho Jeong <daehojeong@google.com>

f2fs_unlock_cache() and f2fs_end_cache_writeback() take a temporary
reference on the entry around clear_and_wake_up_bit(), so that the entry
is not freed before wake_up_bit() returns. f2fs_destroy_cache() waits
for the LOCKED and WRITEBACK bits and then expects the refcount to be 1,
but the bit waits can return as soon as the bit is cleared, before the
I/O completion drops the temporary reference. When the completion runs
in process context and is preempted there (e.g. dm-flakey in
generic/311), umount hits:

  kernel BUG at fs/f2fs/cache.c:567!
  f2fs_destroy_cache+0x260/0x268
  f2fs_put_super+0x1fc/0x428

Count the temporary references in units of F2FS_CACHE_PIN_BIAS, like
GUP_PIN_COUNTING_BIAS for folios, and check only the regular references
in f2fs_destroy_cache(). A leaked regular reference is still caught
there, without waiting for the completion.

The pinned entry is freed by whichever of f2fs_cache_put() and
f2fs_cache_unpin() drops the refcount to zero, as before. The shrinker
still skips an entry while it is pinned, since the refcount is then
neither 1 nor below 3.

Fixes: 6e392158cf54 ("f2fs: cache: pin cached block in f2fs_end_cache_writeback()")
Fixes: 61ba87b33e87 ("f2fs: cache: pin cached block in f2fs_unlock_cache()")
Signed-off-by: Daeho Jeong <daehojeong@google.com>
---
 fs/f2fs/cache.c | 40 ++++++++++++++++++++++++++++++++++------
 1 file changed, 34 insertions(+), 6 deletions(-)

diff --git a/fs/f2fs/cache.c b/fs/f2fs/cache.c
index 04b5408cbc5..a75578cd4e3 100644
--- a/fs/f2fs/cache.c
+++ b/fs/f2fs/cache.c
@@ -18,7 +18,7 @@
 #include <trace/events/f2fs.h>
 #include "segment.h"
 
-static bool f2fs_cache_put(struct f2fs_cached_block *entry);
+static void f2fs_free_cache(struct f2fs_cached_block *entry);
 
 void f2fs_cache_wait_writeback_cond(struct f2fs_cached_block *entry,
 					enum page_type type)
@@ -111,6 +111,27 @@ void f2fs_start_cache_writeback(struct f2fs_cached_block *entry)
 				F2FS_CACHE_TAG_WRITEBACK);
 }
 
+/*
+ * The temporary references taken around clear_and_wake_up_bit() are counted
+ * in units of F2FS_CACHE_PIN_BIAS, like GUP_PIN_COUNTING_BIAS for folios, so
+ * that they can be told apart from the regular references.
+ */
+#define F2FS_CACHE_PIN_BIAS	(1 << 16)
+
+static void f2fs_cache_pin(struct f2fs_cached_block *entry)
+{
+	atomic_add(F2FS_CACHE_PIN_BIAS, &entry->refcount);
+}
+
+static void f2fs_cache_unpin(struct f2fs_cached_block *entry)
+{
+	int ref = atomic_sub_return(F2FS_CACHE_PIN_BIAS, &entry->refcount);
+
+	WARN_ON_ONCE(ref < 0);
+	if (!ref)
+		f2fs_free_cache(entry);
+}
+
 void f2fs_end_cache_writeback(struct f2fs_cached_block *entry)
 {
 	/*
@@ -125,9 +146,9 @@ void f2fs_end_cache_writeback(struct f2fs_cached_block *entry)
 	 * But here we must make sure that the entry is not freed and
 	 * reused before clear_and_wake_up_bit().
 	 */
-	f2fs_cache_get(entry);
+	f2fs_cache_pin(entry);
 	clear_and_wake_up_bit(F2FS_BLOCK_WRITEBACK, &entry->state);
-	f2fs_cache_put(entry);
+	f2fs_cache_unpin(entry);
 }
 
 static int f2fs_cache_refcount(struct f2fs_cached_block *entry)
@@ -135,6 +156,12 @@ static int f2fs_cache_refcount(struct f2fs_cached_block *entry)
 	return atomic_read(&entry->refcount);
 }
 
+/* the number of regular references, excluding the temporary ones */
+static int f2fs_cache_users(struct f2fs_cached_block *entry)
+{
+	return f2fs_cache_refcount(entry) & (F2FS_CACHE_PIN_BIAS - 1);
+}
+
 static void f2fs_do_free_cache(struct f2fs_cached_block *entry)
 {
 	kfree(entry->data);
@@ -326,9 +353,9 @@ void f2fs_unlock_cache(struct f2fs_cached_block *entry)
 	 * Pin the entry here to make sure it is not freed before wake_up_bit()
 	 * completes.
 	 */
-	f2fs_cache_get(entry);
+	f2fs_cache_pin(entry);
 	clear_and_wake_up_bit(F2FS_BLOCK_LOCKED, &entry->state);
-	f2fs_cache_put(entry);
+	f2fs_cache_unpin(entry);
 }
 
 bool f2fs_put_cache(struct f2fs_cached_block *entry, bool unlock)
@@ -564,7 +591,8 @@ void f2fs_destroy_cache(struct f2fs_cached_block_list *cache)
 	f2fs_bug_on(cache->sbi, f2fs_cache_test_dirty(entry));
 	f2fs_bug_on(cache->sbi, f2fs_cache_test_writeback(entry));
 	f2fs_bug_on(cache->sbi, !list_empty(&entry->list));
-	f2fs_bug_on(cache->sbi, f2fs_cache_refcount(entry) != 1);
+	/* the I/O completion may not have unpinned the entry yet */
+	f2fs_bug_on(cache->sbi, f2fs_cache_users(entry) != 1);
 	f2fs_put_cache(entry, true);
 	goto next;
 }
-- 
2.56.0.rc1.315.gc6ed9934b7-goog



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel