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:   Chao Yu <chao@kernel.org>
To:     jaegeuk@kernel.org
Cc:     linux-f2fs-devel@lists.sourceforge.net,
        linux-kernel@vger.kernel.org, Chao Yu <chao@kernel.org>
Subject: [PATCH RFC] f2fs: fix to use spinlock to avoid page.private update race
Date:   Mon, 10 Apr 2023 17:39:12 +0800
Message-Id: <20230410093912.2184557-1-chao@kernel.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

There may be subtle race condition, make PagePrivate and page_private
being inconsistent, result in decreasing page count incorrectly,
introduce a per-inode spinlock to avoid such condition.

Signed-off-by: Chao Yu <chao@kernel.org>
---
 fs/f2fs/f2fs.h  | 19 ++++++++++++++++++-
 fs/f2fs/super.c |  2 ++
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index c378aedcadea..6b31bef5853e 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -856,6 +856,8 @@ struct f2fs_inode_info {
 
 	unsigned int atomic_write_cnt;
 	loff_t original_i_size;		/* original i_size before atomic write */
+
+	spinlock_t private_lock;	/* protect page->private */
 };
 
 static inline void get_read_extent_info(struct extent_info *ext,
@@ -1413,21 +1415,28 @@ static inline bool page_private_##name(struct page *page) \
 		test_bit(PAGE_PRIVATE_##flagname, &page_private(page)); \
 }
 
+static inline struct f2fs_inode_info *F2FS_I(struct inode *inode);
 #define PAGE_PRIVATE_SET_FUNC(name, flagname) \
 static inline void set_page_private_##name(struct page *page) \
 { \
+	unsigned long flags; \
+	spin_lock_irqsave(&F2FS_I(page->mapping->host)->private_lock, flags); \
 	if (!PagePrivate(page)) \
 		attach_page_private(page, (void *)page->private); \
 	set_bit(PAGE_PRIVATE_NOT_POINTER, &page_private(page)); \
 	set_bit(PAGE_PRIVATE_##flagname, &page_private(page)); \
+	spin_unlock_irqrestore(&F2FS_I(page->mapping->host)->private_lock, flags); \
 }
 
 #define PAGE_PRIVATE_CLEAR_FUNC(name, flagname) \
 static inline void clear_page_private_##name(struct page *page) \
 { \
+	unsigned long flags; \
+	spin_lock_irqsave(&F2FS_I(page->mapping->host)->private_lock, flags); \
 	clear_bit(PAGE_PRIVATE_##flagname, &page_private(page)); \
-	if (page_private(page) == BIT(PAGE_PRIVATE_NOT_POINTER)) \
+	if (page_private(page) == (BIT(PAGE_PRIVATE_NOT_POINTER))) \
 		detach_page_private(page); \
+	spin_unlock_irqrestore(&F2FS_I(page->mapping->host)->private_lock, flags); \
 }
 
 PAGE_PRIVATE_GET_FUNC(nonpointer, NOT_POINTER);
@@ -1456,17 +1465,25 @@ static inline unsigned long get_page_private_data(struct page *page)
 
 static inline void set_page_private_data(struct page *page, unsigned long data)
 {
+	unsigned long flags;
+
+	spin_lock_irqsave(&F2FS_I(page->mapping->host)->private_lock, flags);
 	if (!PagePrivate(page))
 		attach_page_private(page, 0);
 	set_bit(PAGE_PRIVATE_NOT_POINTER, &page_private(page));
 	page_private(page) |= data << PAGE_PRIVATE_MAX;
+	spin_unlock_irqrestore(&F2FS_I(page->mapping->host)->private_lock, flags);
 }
 
 static inline void clear_page_private_data(struct page *page)
 {
+	unsigned long flags;
+
+	spin_lock_irqsave(&F2FS_I(page->mapping->host)->private_lock, flags);
 	page_private(page) &= GENMASK(PAGE_PRIVATE_MAX - 1, 0);
 	if (page_private(page) == BIT(PAGE_PRIVATE_NOT_POINTER))
 		detach_page_private(page);
+	spin_unlock_irqrestore(&F2FS_I(page->mapping->host)->private_lock, flags);
 }
 
 /* For compression */
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index a1b570a5e50f..555424dd85fd 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1419,6 +1419,8 @@ static struct inode *f2fs_alloc_inode(struct super_block *sb)
 	init_f2fs_rwsem(&fi->i_gc_rwsem[WRITE]);
 	init_f2fs_rwsem(&fi->i_xattr_sem);
 
+	spin_lock_init(&fi->private_lock);
+
 	/* Will be used by directory only */
 	fi->i_dir_level = F2FS_SB(sb)->dir_level;
 
-- 
2.25.1