Skip to contentSkip to conversation

f2fs: fix to drop all discards after creating snapshot on lvm device

PATCH Applied Chao Yu 2 participants 2 messages Nov 21, 2024 – Nov 23, 2024

Subsystems (2)
Stable branch F2FS file system
lore
Share & tools
lore

Chao Yu

author maintainer
#0 lore raw

PATCH 1/1 f2fs: fix to drop all discards after creating snapshot on lvm device

Piergiorgio reported a bug in bugzilla as below:

------------[ cut here ]------------
WARNING: CPU: 2 PID: 969 at fs/f2fs/segment.c:1330
RIP: 0010:__submit_discard_cmd+0x27d/0x400 [f2fs]
Call Trace:
 __issue_discard_cmd+0x1ca/0x350 [f2fs]
 issue_discard_thread+0x191/0x480 [f2fs]
 kthread+0xcf/0x100
 ret_from_fork+0x31/0x50
 ret_from_fork_asm+0x1a/0x30

w/ below testcase, it can reproduce this bug quickly:
- pvcreate /dev/vdb
- vgcreate myvg1 /dev/vdb
- lvcreate -L 1024m -n mylv1 myvg1
- mount /dev/myvg1/mylv1 /mnt/f2fs
- dd if=/dev/zero of=/mnt/f2fs/file bs=1M count=20
- sync
- rm /mnt/f2fs/file
- sync
- lvcreate -L 1024m -s -n mylv1-snapshot /dev/myvg1/mylv1
- umount /mnt/f2fs

The root cause is: it will update discard_max_bytes of mounted lvm device to zero after creating snapshot on this lvm device, then, __submit_discard_cmd() will pass parameter @nr_sects w/ zero value to __blkdev_issue_discard(), it returns a NULL bio pointer, result in panic.

This patch changes as below for fixing:
1. Let's drop all remained discards in f2fs_unfreeze() if snapshot of lvm device is created.
2. Checking discard_max_bytes before submitting discard during __submit_discard_cmd().

Cc [email protected]
Fixes 35ec7d574884 ("f2fs: split discard command in prior to block layer")
Reported-by Piergiorgio Sartor <[email protected]>
Closes https://bugzilla.kernel.org/show_bug.cgi?id=219484
Signed-off-by Chao Yu <[email protected]>
---
 fs/f2fs/segment.c | 16 +++++++++-------
 fs/f2fs/super.c   | 12 ++++++++++++
 2 files changed, 21 insertions(+), 7 deletions(-)
fs/f2fs/segment.c +9 -7
index 7bdfe08ce9ea..af3fb3f6d9b5 100644
@@ -1290,16 +1290,18 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi,
 						wait_list, issued);
 			return 0;
 		}
-
-		/*
-		 * Issue discard for conventional zones only if the device
-		 * supports discard.
-		 */
-		if (!bdev_max_discard_sectors(bdev))
-			return -EOPNOTSUPP;
 	}
 #endif
 
+	/*
+	 * stop issuing discard for any of below cases:
+	 * 1. device is conventional zone, but it doesn't support discard.
+	 * 2. device is regulare device, after snapshot it doesn't support
+	 * discard.
+	 */
+	if (!bdev_max_discard_sectors(bdev))
+		return -EOPNOTSUPP;
+
 	trace_f2fs_issue_discard(bdev, dc->di.start, dc->di.len);
 
 	lstart = dc->di.lstart;
fs/f2fs/super.c +12 -0
index c0670cd61956..fc7d463dee15 100644
@@ -1760,6 +1760,18 @@ static int f2fs_freeze(struct super_block *sb)
 
 static int f2fs_unfreeze(struct super_block *sb)
 {
+	struct f2fs_sb_info *sbi = F2FS_SB(sb);
+
+	/*
+	 * It will update discard_max_bytes of mounted lvm device to zero
+	 * after creating snapshot on this lvm device, let's drop all
+	 * remained discards.
+	 * We don't need to disable real-time discard because discard_max_bytes
+	 * will recover after removal of snapshot.
+	 */
+	if (test_opt(sbi, DISCARD) && !f2fs_hw_support_discard(sbi))
+		f2fs_issue_discard_timeout(sbi);
+
 	clear_sbi_flag(F2FS_SB(sb), SBI_IS_FREEZING);
 	return 0;
 }
Signature · 1 line
2.40.1
CI Applied
#1 lore raw
This patch was applied to jaegeuk/f2fs.git (dev) by Jaegeuk Kim <[email protected]>:

Hello:

This patch was applied to jaegeuk/f2fs.git (dev)
by Jaegeuk Kim <[email protected]>:

Quoted 12 lines · On Thu, 21 Nov 2024 22:17:16 +0800 you wrote:

Piergiorgio reported a bug in bugzilla as below:

------------[ cut here ]------------
WARNING: CPU: 2 PID: 969 at fs/f2fs/segment.c:1330
RIP: 0010:__submit_discard_cmd+0x27d/0x400 [f2fs]
Call Trace:
 __issue_discard_cmd+0x1ca/0x350 [f2fs]
 issue_discard_thread+0x191/0x480 [f2fs]
 kthread+0xcf/0x100
 ret_from_fork+0x31/0x50
 ret_from_fork_asm+0x1a/0x30
Quoted 1 line

[...]

Here is the summary with links:
- [f2fs-dev] f2fs: fix to drop all discards after creating snapshot on lvm device

    https://git.kernel.org/jaegeuk/f2fs/c/bc8aeb04fd80

You are awesome, thank you!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html