On May 27, 2020, at 8:08 AM, Wang Shilong <wangshilong1991@xxxxxxxxx> wrote:
>
> From: Wang Shilong <wshilong@xxxxxxx>
>
> Currently WAS_TRIMMED flag is not persistent, whenever filesystem was
> remounted, fstrim need walk all block groups again, the problem with
> this is FSTRIM could be slow on very large LUN SSD based filesystem.
>
> To avoid this kind of problem, we introduce a block group flag
> EXT4_BG_WAS_TRIMMED, the side effect of this is we need introduce
> extra one block group dirty write after trimming block group.
>
> And When clearing TRIMMED flag, block group will be journalled
> anyway, so it won't introduce any overhead.
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index ad2dbf6e4924..23c2dc529a28 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -357,6 +357,7 @@ struct flex_groups {
> #define EXT4_BG_INODE_UNINIT 0x0001 /* Inode table/bitmap not in use */
> #define EXT4_BG_BLOCK_UNINIT 0x0002 /* Block bitmap not in use */
> #define EXT4_BG_INODE_ZEROED 0x0004 /* On-disk itable initialized to zero */
> +#define EXT4_BG_WAS_TRIMMED 0x0008 /* block group was trimmed */
>
> /*
> * Macro-instructions used to manage group descriptors
> @@ -3112,9 +3113,8 @@ struct ext4_group_info {
> };
>
> #define EXT4_GROUP_INFO_NEED_INIT_BIT 0
> -#define EXT4_GROUP_INFO_WAS_TRIMMED_BIT 1
> -#define EXT4_GROUP_INFO_BBITMAP_CORRUPT_BIT 2
> -#define EXT4_GROUP_INFO_IBITMAP_CORRUPT_BIT 3
> +#define EXT4_GROUP_INFO_BBITMAP_CORRUPT_BIT 1
> +#define EXT4_GROUP_INFO_IBITMAP_CORRUPT_BIT 2
(minor) I don't think you need to renumber these bits, just remove the
WAS_TRIMMED_BIT and leave the others as-is. Not a big deal either way.
> diff --git a/fs/ext4/ext4_jbd2.h b/fs/ext4/ext4_jbd2.h
> index 4b9002f0e84c..4094a5b247f7 100644
> --- a/fs/ext4/ext4_jbd2.h
> +++ b/fs/ext4/ext4_jbd2.h
>
> @@ -4939,6 +4929,14 @@ void ext4_free_blocks(handle_t *handle, struct inode *inode,
> ret = ext4_free_group_clusters(sb, gdp) + count_clusters;
> ext4_free_group_clusters_set(sb, gdp, ret);
> ext4_block_bitmap_csum_set(sb, block_group, gdp, bitmap_bh);
> + /*
> + * Clear the trimmed flag for the group so that the next
> + * ext4_trim_fs can trim it.
> + * If the volume is mounted with -o discard, online discard
> + * is supported and the free blocks will be trimmed online.
> + */
> + if (!test_opt(sb, DISCARD))
> + EXT4_MB_GDP_CLEAR_TRIMMED(gdp);
As discussed in my other email, I think a follow-on patch should track (in
ext4_group_info in-memory counter) the number of blocks freed in each group,
and only clear the WAS_TRIMMED flag if there are several blocks freed, or if
the group becomes totally "empty" (i.e. free < num_itable_blocks + 2). That
will avoid overly-aggressive full group trim operations (i.e. we don't want
to trim if only a single block was freed).
Cheers, Andreas
Attachment:
signature.asc
Description: Message signed with OpenPGP
