Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] ext4: FITRIM balloon discard merges only logically contiguous extents --
commit 5d29cebdec45f399e32530ba7c9295bebaf3a2e6 Author: Denis V. Lunev <[email protected]> ext4: discard the balloon on FITRIM This commit makes ext4_trim_fs() walk the balloon inode after the free space and discard its unwritten extents that fall into the requested range. Physically adjacent extents are merged into one discard, and long runs are discarded in chunks with checks for fatal signals and freezing. > diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c > --- a/fs/ext4/mballoc.c > +++ b/fs/ext4/mballoc.c [ ... ] > +static int ext4_trim_balloon(struct super_block *sb, ext4_fsblk_t start, > + ext4_fsblk_t end, ext4_fsblk_t minblocks) > +{ [ ... ] > + unwritten = err > 0 && (map.m_flags & EXT4_MAP_UNWRITTEN); > + if (unwritten && run_len && run_pblk + run_len == map.m_pblk) { > + run_len += map.m_len; > + continue; > + } > + > + ret = ext4_trim_balloon_run(sb, run_pblk, run_len, start, end, > + minblocks); > + if (ret) > + break; > + run_pblk = map.m_pblk; > + run_len = unwritten ? map.m_len : 0; [Severity: Low] The commit message says "Physically adjacent extents are merged into one discard, otherwise every cut at the unwritten extent length limit leaves a partly covered cluster of the image below." Is that description accurate for what ext4_trim_balloon() does? When the walk meets a hole or a written extent, the code takes the non-merge path above. It calls ext4_trim_balloon_run() on the pending run and then sets run_len to 0. The next unwritten extent then starts a new run even if its m_pblk equals the end of the previous run: unwritten extent A (pblk P .. P+n-1) hole / written extent in the logical range unwritten extent B (pblk P+n ..) A and B are physically adjacent but are discarded as two separate runs. The boundary cluster can then be left partly covered, which is the case the merging is meant to avoid. Only extents that are consecutive in logical order and also physically adjacent are merged. The common case, a split at the unwritten extent length limit, is handled correctly. Could the commit message be reworded to say that logically consecutive, physically adjacent extents are merged? -- Sashiko AI review ยท https://sashiko.vzint.dev/#/patchset/[email protected]?part=4 _______________________________________________ Devel mailing list [email protected] https://lists.openvz.org/mailman/listinfo/devel
