diff mbox series

ext4: clear EXT4_GROUP_INFO_WAS_TRIMMED_BIT even mount with discard

Message ID 20231230070654.178638-1-yangerkun@huaweicloud.com
State New
Headers show
Series ext4: clear EXT4_GROUP_INFO_WAS_TRIMMED_BIT even mount with discard | expand

Commit Message

Yang Erkun Dec. 30, 2023, 7:06 a.m. UTC
Commit 3d56b8d2c74c ("ext4: Speed up FITRIM by recording flags in
ext4_group_info") speed up fstrim by skipping trim trimmed group. We
also has the chance to clear trimmed once there exists some block free
for this group(mount without discard), and the next trim for this group
will work well too.

For mount with discard, we will issue dicard when we free blocks, so
leave trimmed flag keep alive to skip useless trim trigger from
userspace seems reasonable. But for some case like ext4 build on
dm-thinpool(ext4 blocksize 4K, pool blocksize 128K), discard from ext4
maybe unaligned for dm thinpool, and thinpool will just finish this
discard(see process_discard_bio when begein equals to end) without
actually process discard. For this case, trim from userspace can really
help us to free some thinpool block.

So convert to clear trimmed flag for all case no matter mounted with
discard or not.

Fixes: 3d56b8d2c74c ("ext4: Speed up FITRIM by recording flags in ext4_group_info")
Signed-off-by: yangerkun <yangerkun@huaweicloud.com>
---
 fs/ext4/mballoc.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

Comments

Yang Erkun Jan. 16, 2024, 1:02 a.m. UTC | #1
Ping...

在 2023/12/30 15:06, yangerkun 写道:
> Commit 3d56b8d2c74c ("ext4: Speed up FITRIM by recording flags in
> ext4_group_info") speed up fstrim by skipping trim trimmed group. We
> also has the chance to clear trimmed once there exists some block free
> for this group(mount without discard), and the next trim for this group
> will work well too.
> 
> For mount with discard, we will issue dicard when we free blocks, so
> leave trimmed flag keep alive to skip useless trim trigger from
> userspace seems reasonable. But for some case like ext4 build on
> dm-thinpool(ext4 blocksize 4K, pool blocksize 128K), discard from ext4
> maybe unaligned for dm thinpool, and thinpool will just finish this
> discard(see process_discard_bio when begein equals to end) without
> actually process discard. For this case, trim from userspace can really
> help us to free some thinpool block.
> 
> So convert to clear trimmed flag for all case no matter mounted with
> discard or not.
> 
> Fixes: 3d56b8d2c74c ("ext4: Speed up FITRIM by recording flags in ext4_group_info")
> Signed-off-by: yangerkun <yangerkun@huaweicloud.com>
> ---
>   fs/ext4/mballoc.c | 10 ++++------
>   1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index d72b5e3c92ec..69240ae775f1 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -3855,11 +3855,8 @@ static void ext4_free_data_in_buddy(struct super_block *sb,
>   	/*
>   	 * 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_GRP_CLEAR_TRIMMED(db);
> +	EXT4_MB_GRP_CLEAR_TRIMMED(db);
>   
>   	if (!db->bb_free_root.rb_node) {
>   		/* No more items in the per group rb tree
> @@ -6481,8 +6478,9 @@ static void ext4_mb_clear_bb(handle_t *handle, struct inode *inode,
>   					 " group:%u block:%d count:%lu failed"
>   					 " with %d", block_group, bit, count,
>   					 err);
> -		} else
> -			EXT4_MB_GRP_CLEAR_TRIMMED(e4b.bd_info);
> +		}
> +
> +		EXT4_MB_GRP_CLEAR_TRIMMED(e4b.bd_info);
>   
>   		ext4_lock_group(sb, block_group);
>   		mb_free_blocks(inode, &e4b, bit, count_clusters);
Yang Erkun Feb. 21, 2024, 9:05 a.m. UTC | #2
Gently ping...

在 2024/1/16 9:02, yangerkun 写道:
> Ping...
> 
> 在 2023/12/30 15:06, yangerkun 写道:
>> Commit 3d56b8d2c74c ("ext4: Speed up FITRIM by recording flags in
>> ext4_group_info") speed up fstrim by skipping trim trimmed group. We
>> also has the chance to clear trimmed once there exists some block free
>> for this group(mount without discard), and the next trim for this group
>> will work well too.
>>
>> For mount with discard, we will issue dicard when we free blocks, so
>> leave trimmed flag keep alive to skip useless trim trigger from
>> userspace seems reasonable. But for some case like ext4 build on
>> dm-thinpool(ext4 blocksize 4K, pool blocksize 128K), discard from ext4
>> maybe unaligned for dm thinpool, and thinpool will just finish this
>> discard(see process_discard_bio when begein equals to end) without
>> actually process discard. For this case, trim from userspace can really
>> help us to free some thinpool block.
>>
>> So convert to clear trimmed flag for all case no matter mounted with
>> discard or not.
>>
>> Fixes: 3d56b8d2c74c ("ext4: Speed up FITRIM by recording flags in 
>> ext4_group_info")
>> Signed-off-by: yangerkun <yangerkun@huaweicloud.com>
>> ---
>>   fs/ext4/mballoc.c | 10 ++++------
>>   1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
>> index d72b5e3c92ec..69240ae775f1 100644
>> --- a/fs/ext4/mballoc.c
>> +++ b/fs/ext4/mballoc.c
>> @@ -3855,11 +3855,8 @@ static void ext4_free_data_in_buddy(struct 
>> super_block *sb,
>>       /*
>>        * 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_GRP_CLEAR_TRIMMED(db);
>> +    EXT4_MB_GRP_CLEAR_TRIMMED(db);
>>       if (!db->bb_free_root.rb_node) {
>>           /* No more items in the per group rb tree
>> @@ -6481,8 +6478,9 @@ static void ext4_mb_clear_bb(handle_t *handle, 
>> struct inode *inode,
>>                        " group:%u block:%d count:%lu failed"
>>                        " with %d", block_group, bit, count,
>>                        err);
>> -        } else
>> -            EXT4_MB_GRP_CLEAR_TRIMMED(e4b.bd_info);
>> +        }
>> +
>> +        EXT4_MB_GRP_CLEAR_TRIMMED(e4b.bd_info);
>>           ext4_lock_group(sb, block_group);
>>           mb_free_blocks(inode, &e4b, bit, count_clusters);
Jan Kara Feb. 21, 2024, 11:18 a.m. UTC | #3
On Sat 30-12-23 15:06:54, yangerkun wrote:
> Commit 3d56b8d2c74c ("ext4: Speed up FITRIM by recording flags in
> ext4_group_info") speed up fstrim by skipping trim trimmed group. We
> also has the chance to clear trimmed once there exists some block free
> for this group(mount without discard), and the next trim for this group
> will work well too.
> 
> For mount with discard, we will issue dicard when we free blocks, so
> leave trimmed flag keep alive to skip useless trim trigger from
> userspace seems reasonable. But for some case like ext4 build on
> dm-thinpool(ext4 blocksize 4K, pool blocksize 128K), discard from ext4
> maybe unaligned for dm thinpool, and thinpool will just finish this
> discard(see process_discard_bio when begein equals to end) without
> actually process discard. For this case, trim from userspace can really
> help us to free some thinpool block.
> 
> So convert to clear trimmed flag for all case no matter mounted with
> discard or not.
> 
> Fixes: 3d56b8d2c74c ("ext4: Speed up FITRIM by recording flags in ext4_group_info")
> Signed-off-by: yangerkun <yangerkun@huaweicloud.com>

Thanks for the fix. It looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/mballoc.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index d72b5e3c92ec..69240ae775f1 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -3855,11 +3855,8 @@ static void ext4_free_data_in_buddy(struct super_block *sb,
>  	/*
>  	 * 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_GRP_CLEAR_TRIMMED(db);
> +	EXT4_MB_GRP_CLEAR_TRIMMED(db);
>  
>  	if (!db->bb_free_root.rb_node) {
>  		/* No more items in the per group rb tree
> @@ -6481,8 +6478,9 @@ static void ext4_mb_clear_bb(handle_t *handle, struct inode *inode,
>  					 " group:%u block:%d count:%lu failed"
>  					 " with %d", block_group, bit, count,
>  					 err);
> -		} else
> -			EXT4_MB_GRP_CLEAR_TRIMMED(e4b.bd_info);
> +		}
> +
> +		EXT4_MB_GRP_CLEAR_TRIMMED(e4b.bd_info);
>  
>  		ext4_lock_group(sb, block_group);
>  		mb_free_blocks(inode, &e4b, bit, count_clusters);
> -- 
> 2.39.2
> 
>
Yang Erkun March 30, 2024, 8:04 a.m. UTC | #4
Hi, Ted,

Ping for this patch.


在 2024/2/21 19:18, Jan Kara 写道:
> On Sat 30-12-23 15:06:54, yangerkun wrote:
>> Commit 3d56b8d2c74c ("ext4: Speed up FITRIM by recording flags in
>> ext4_group_info") speed up fstrim by skipping trim trimmed group. We
>> also has the chance to clear trimmed once there exists some block free
>> for this group(mount without discard), and the next trim for this group
>> will work well too.
>>
>> For mount with discard, we will issue dicard when we free blocks, so
>> leave trimmed flag keep alive to skip useless trim trigger from
>> userspace seems reasonable. But for some case like ext4 build on
>> dm-thinpool(ext4 blocksize 4K, pool blocksize 128K), discard from ext4
>> maybe unaligned for dm thinpool, and thinpool will just finish this
>> discard(see process_discard_bio when begein equals to end) without
>> actually process discard. For this case, trim from userspace can really
>> help us to free some thinpool block.
>>
>> So convert to clear trimmed flag for all case no matter mounted with
>> discard or not.
>>
>> Fixes: 3d56b8d2c74c ("ext4: Speed up FITRIM by recording flags in ext4_group_info")
>> Signed-off-by: yangerkun <yangerkun@huaweicloud.com>
> 
> Thanks for the fix. It looks good. Feel free to add:
> 
> Reviewed-by: Jan Kara <jack@suse.cz>
> 
> 								Honza
> 
>> ---
>>   fs/ext4/mballoc.c | 10 ++++------
>>   1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
>> index d72b5e3c92ec..69240ae775f1 100644
>> --- a/fs/ext4/mballoc.c
>> +++ b/fs/ext4/mballoc.c
>> @@ -3855,11 +3855,8 @@ static void ext4_free_data_in_buddy(struct super_block *sb,
>>   	/*
>>   	 * 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_GRP_CLEAR_TRIMMED(db);
>> +	EXT4_MB_GRP_CLEAR_TRIMMED(db);
>>   
>>   	if (!db->bb_free_root.rb_node) {
>>   		/* No more items in the per group rb tree
>> @@ -6481,8 +6478,9 @@ static void ext4_mb_clear_bb(handle_t *handle, struct inode *inode,
>>   					 " group:%u block:%d count:%lu failed"
>>   					 " with %d", block_group, bit, count,
>>   					 err);
>> -		} else
>> -			EXT4_MB_GRP_CLEAR_TRIMMED(e4b.bd_info);
>> +		}
>> +
>> +		EXT4_MB_GRP_CLEAR_TRIMMED(e4b.bd_info);
>>   
>>   		ext4_lock_group(sb, block_group);
>>   		mb_free_blocks(inode, &e4b, bit, count_clusters);
>> -- 
>> 2.39.2
>>
>>
Yang Erkun May 6, 2024, 12:59 a.m. UTC | #5
Hi, Ted,

Ping again...

在 2024/3/30 16:04, yangerkun 写道:
> Hi, Ted,
> 
> Ping for this patch.
> 
> 
> 在 2024/2/21 19:18, Jan Kara 写道:
>> On Sat 30-12-23 15:06:54, yangerkun wrote:
>>> Commit 3d56b8d2c74c ("ext4: Speed up FITRIM by recording flags in
>>> ext4_group_info") speed up fstrim by skipping trim trimmed group. We
>>> also has the chance to clear trimmed once there exists some block free
>>> for this group(mount without discard), and the next trim for this group
>>> will work well too.
>>>
>>> For mount with discard, we will issue dicard when we free blocks, so
>>> leave trimmed flag keep alive to skip useless trim trigger from
>>> userspace seems reasonable. But for some case like ext4 build on
>>> dm-thinpool(ext4 blocksize 4K, pool blocksize 128K), discard from ext4
>>> maybe unaligned for dm thinpool, and thinpool will just finish this
>>> discard(see process_discard_bio when begein equals to end) without
>>> actually process discard. For this case, trim from userspace can really
>>> help us to free some thinpool block.
>>>
>>> So convert to clear trimmed flag for all case no matter mounted with
>>> discard or not.
>>>
>>> Fixes: 3d56b8d2c74c ("ext4: Speed up FITRIM by recording flags in 
>>> ext4_group_info")
>>> Signed-off-by: yangerkun <yangerkun@huaweicloud.com>
>>
>> Thanks for the fix. It looks good. Feel free to add:
>>
>> Reviewed-by: Jan Kara <jack@suse.cz>
>>
>>                                 Honza
>>
>>> ---
>>>   fs/ext4/mballoc.c | 10 ++++------
>>>   1 file changed, 4 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
>>> index d72b5e3c92ec..69240ae775f1 100644
>>> --- a/fs/ext4/mballoc.c
>>> +++ b/fs/ext4/mballoc.c
>>> @@ -3855,11 +3855,8 @@ static void ext4_free_data_in_buddy(struct 
>>> super_block *sb,
>>>       /*
>>>        * 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_GRP_CLEAR_TRIMMED(db);
>>> +    EXT4_MB_GRP_CLEAR_TRIMMED(db);
>>>       if (!db->bb_free_root.rb_node) {
>>>           /* No more items in the per group rb tree
>>> @@ -6481,8 +6478,9 @@ static void ext4_mb_clear_bb(handle_t *handle, 
>>> struct inode *inode,
>>>                        " group:%u block:%d count:%lu failed"
>>>                        " with %d", block_group, bit, count,
>>>                        err);
>>> -        } else
>>> -            EXT4_MB_GRP_CLEAR_TRIMMED(e4b.bd_info);
>>> +        }
>>> +
>>> +        EXT4_MB_GRP_CLEAR_TRIMMED(e4b.bd_info);
>>>           ext4_lock_group(sb, block_group);
>>>           mb_free_blocks(inode, &e4b, bit, count_clusters);
>>> -- 
>>> 2.39.2
>>>
>>>
diff mbox series

Patch

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index d72b5e3c92ec..69240ae775f1 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -3855,11 +3855,8 @@  static void ext4_free_data_in_buddy(struct super_block *sb,
 	/*
 	 * 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_GRP_CLEAR_TRIMMED(db);
+	EXT4_MB_GRP_CLEAR_TRIMMED(db);
 
 	if (!db->bb_free_root.rb_node) {
 		/* No more items in the per group rb tree
@@ -6481,8 +6478,9 @@  static void ext4_mb_clear_bb(handle_t *handle, struct inode *inode,
 					 " group:%u block:%d count:%lu failed"
 					 " with %d", block_group, bit, count,
 					 err);
-		} else
-			EXT4_MB_GRP_CLEAR_TRIMMED(e4b.bd_info);
+		}
+
+		EXT4_MB_GRP_CLEAR_TRIMMED(e4b.bd_info);
 
 		ext4_lock_group(sb, block_group);
 		mb_free_blocks(inode, &e4b, bit, count_clusters);