While reviewing recent ext4 patch[1], Sashiko raised the following concern[2]: > If the filesystem is initially mounted with the discard option, > deleting files will populate sbi->s_discard_list and queue > s_discard_work. If it is then remounted with nodiscard, the > EXT4_MOUNT_DISCARD flag is cleared, but the pending s_discard_work is > neither cancelled nor flushed. [1] https://lore.kernel.org/r/20260319094545.19291-1-qiang.zhang@linux.dev/ [2] https://sashiko.dev/#/patchset/20260319094545.19291-1-qiang.zhang%40linux.dev The concern was valid, but it had nothing to do with the patch[1]. One of the problems with Sashiko in its current (early) form is that it will detect pre-existing issues and report it as a problem with the patch that it is reviewing. In practice, it would be hard to hit deliberately (unless you are a malicious syzkaller fuzzer), since it would involve mounting the file system with -o discard, and then deleting a large number of files, remounting the file system with -o nodiscard, and then immediately unmounting the file system before the queued discard work has a change to drain on its own. Fix it because it's a real bug, and to avoid Sashiko from raising this concern when analyzing future patches to mballoc.c.
Unified · Split
| @@ -3893,13 +3893,11 @@ void ext4_mb_release(struct super_block *sb) · 3892 unmodified lines | ||
| 3893 | 3893 | struct kmem_cache *cachep = get_groupinfo_cache(sb->s_blocksize_bits); |
| 3894 | 3894 | int count; |
| 3895 | 3895 | |
| 3896 | − if (test_opt(sb, DISCARD)) { | |
| 3897 | − /* | |
| 3898 | − * wait the discard work to drain all of ext4_free_data | |
| 3899 | − */ | |
| 3900 | − flush_work(&sbi->s_discard_work); | |
| 3901 | − WARN_ON_ONCE(!list_empty(&sbi->s_discard_list)); | |
| 3902 | − } | |
| 3896 | + /* | |
| 3897 | + * wait the discard work to drain all of ext4_free_data | |
| 3898 | + */ | |
| 3899 | + flush_work(&sbi->s_discard_work); | |
| 3900 | + WARN_ON_ONCE(!list_empty(&sbi->s_discard_list)); | |
| 3903 | 3901 | |
| 3904 | 3902 | group_info = rcu_access_pointer(sbi->s_group_info); |
| 3905 | 3903 | if (group_info) { |