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().
Unified · Split
| @@ -1290,16 +1290,18 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi, · 1289 unmodified lines | |||
| 1290 | wait_list, issued); | 1290 | wait_list, issued); |
| 1291 | return 0; | 1291 | return 0; |
| 1292 | } | 1292 | } |
| 1293 | |||
| 1294 | /* | ||
| 1295 | * Issue discard for conventional zones only if the device | ||
| 1296 | * supports discard. | ||
| 1297 | */ | ||
| 1298 | if (!bdev_max_discard_sectors(bdev)) | ||
| 1299 | return -EOPNOTSUPP; | ||
| 1300 | } | 1293 | } |
| 1301 | #endif | 1294 | #endif |
| 1302 | 1295 | ||
| 1296 | /* | ||
| 1297 | * stop issuing discard for any of below cases: | ||
| 1298 | * 1. device is conventional zone, but it doesn't support discard. | ||
| 1299 | * 2. device is regulare device, after snapshot it doesn't support | ||
| 1300 | * discard. | ||
| 1301 | */ | ||
| 1302 | if (!bdev_max_discard_sectors(bdev)) | ||
| 1303 | return -EOPNOTSUPP; | ||
| 1304 | |||
| 1303 | trace_f2fs_issue_discard(bdev, dc->di.start, dc->di.len); | 1305 | trace_f2fs_issue_discard(bdev, dc->di.start, dc->di.len); |
| 1304 | 1306 | ||
| 1305 | lstart = dc->di.lstart; | 1307 | lstart = dc->di.lstart; |
| @@ -1760,6 +1760,18 @@ static int f2fs_freeze(struct super_block *sb) · 1759 unmodified lines | |||
| 1760 | 1760 | ||
| 1761 | static int f2fs_unfreeze(struct super_block *sb) | 1761 | static int f2fs_unfreeze(struct super_block *sb) |
| 1762 | { | 1762 | { |
| 1763 | struct f2fs_sb_info *sbi = F2FS_SB(sb); | ||
| 1764 | |||
| 1765 | /* | ||
| 1766 | * It will update discard_max_bytes of mounted lvm device to zero | ||
| 1767 | * after creating snapshot on this lvm device, let's drop all | ||
| 1768 | * remained discards. | ||
| 1769 | * We don't need to disable real-time discard because discard_max_bytes | ||
| 1770 | * will recover after removal of snapshot. | ||
| 1771 | */ | ||
| 1772 | if (test_opt(sbi, DISCARD) && !f2fs_hw_support_discard(sbi)) | ||
| 1773 | f2fs_issue_discard_timeout(sbi); | ||
| 1774 | |||
| 1763 | clear_sbi_flag(F2FS_SB(sb), SBI_IS_FREEZING); | 1775 | clear_sbi_flag(F2FS_SB(sb), SBI_IS_FREEZING); |
| 1764 | return 0; | 1776 | return 0; |
| 1765 | } | 1777 | } |