On 2023/12/15 19:41, Jan Kara wrote:
Thank you very much for your clear explanation of the patch. I'll update patch'sOn Thu 14-12-23 14:46:35, Ye Bin wrote:There will not issue discard cmd when do segment fstrim for ext4 fs, however, if full fstrim for the same fs will issue discard cmd. Above issue may happens as follows: Precondition: 1. Fstrim range [0, 15] and [16, 31]; 2. Discard granularity is 16; Range1 Range2 1111000000000000 0000111010101011 There's no free space length large or equal than 16 in 'Range1' or 'Range2'. As ext4_try_to_trim_range() only search free space among range which user specified. However, there's maximum free space length 16 in 'Range1'+ 'Range2'. To solve above issue, we need to find the longest free space to discard.The patch looks good so feel free to add: Reviewed-by: Jan Kara <jack@xxxxxxx> I'd just rephrase the changelog to make it a bit easier to read: Suppose we issue two FITRIM ioctls for ranges [0,15] and [16,31] with mininum length of trimmed range set to 8 blocks. If we have say a range of blocks 10-22 free, this range will not be trimmed because it straddles the boundary of the two FITRIM ranges and neither part is big enough. This is a bit surprising to some users that call FITRIM on smaller ranges of blocks to limit impact on the system. Also XFS trims all free space extents that overlap with the specified range so we are inconsistent among filesystems. Let's change ext4_try_to_trim_range() to consider for trimming the whole free space extent that straddles the end of specified range, not just the part of it within the range. Honza
changelog and resend a version.
Signed-off-by: Ye Bin <yebin10@xxxxxxxxxx> --- fs/ext4/mballoc.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c index d72b5e3c92ec..d195461123d8 100644 --- a/fs/ext4/mballoc.c +++ b/fs/ext4/mballoc.c @@ -6753,13 +6753,15 @@ static int ext4_try_to_trim_range(struct super_block *sb, __acquires(ext4_group_lock_ptr(sb, e4b->bd_group)) __releases(ext4_group_lock_ptr(sb, e4b->bd_group)) { - ext4_grpblk_t next, count, free_count; + ext4_grpblk_t next, count, free_count, last, origin_start; bool set_trimmed = false; void *bitmap;+ last = ext4_last_grp_cluster(sb, e4b->bd_group);bitmap = e4b->bd_bitmap; - if (start == 0 && max >= ext4_last_grp_cluster(sb, e4b->bd_group)) + if (start == 0 && max >= last) set_trimmed = true; + origin_start = start; start = max(e4b->bd_info->bb_first_free, start); count = 0; free_count = 0; @@ -6768,7 +6770,10 @@ __releases(ext4_group_lock_ptr(sb, e4b->bd_group)) start = mb_find_next_zero_bit(bitmap, max + 1, start); if (start > max) break; - next = mb_find_next_bit(bitmap, max + 1, start); + + next = mb_find_next_bit(bitmap, last + 1, start); + if (origin_start == 0 && next >= last) + set_trimmed = true;if ((next - start) >= minblocks) {int ret = ext4_trim_extent(sb, start, next - start, e4b); -- 2.31.1
