2023-06-15 11:15:53

by Fengnan Chang

[permalink] [raw]
Subject: [RFC PATCH] ext4: improve discard efficiency

In commit a015434480dc("ext4: send parallel discards on commit
completions"), issue all discard commands in parallel make all
bios could merged into one request, so lowlevel drive can issue
multi segments in one time which is more efficiency, but commit
55cdd0af2bc5 ("ext4: get discard out of jbd2 commit kthread contex")
seems broke this way, let's fix it.
In my test, the time of fstrim fs with multi big sparse file
reduce from 6.7s to 1.3s.

Signed-off-by: Fengnan Chang <[email protected]>
---
fs/ext4/mballoc.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index a2475b8c9fb5..e5a27fd2e959 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -6790,7 +6790,7 @@ int ext4_group_add_blocks(handle_t *handle, struct super_block *sb,
* be called with under the group lock.
*/
static int ext4_trim_extent(struct super_block *sb,
- int start, int count, struct ext4_buddy *e4b)
+ int start, int count, struct ext4_buddy *e4b, struct bio **biop)
__releases(bitlock)
__acquires(bitlock)
{
@@ -6812,7 +6812,7 @@ __acquires(bitlock)
*/
mb_mark_used(e4b, &ex);
ext4_unlock_group(sb, group);
- ret = ext4_issue_discard(sb, group, start, count, NULL);
+ ret = ext4_issue_discard(sb, group, start, count, biop);
ext4_lock_group(sb, group);
mb_free_blocks(NULL, e4b, start, ex.fe_len);
return ret;
@@ -6826,12 +6826,15 @@ __releases(ext4_group_lock_ptr(sb, e4b->bd_group))
{
ext4_grpblk_t next, count, free_count;
void *bitmap;
+ struct bio *discard_bio = NULL;
+ struct blk_plug plug;

bitmap = e4b->bd_bitmap;
start = (e4b->bd_info->bb_first_free > start) ?
e4b->bd_info->bb_first_free : start;
count = 0;
free_count = 0;
+ blk_start_plug(&plug);

while (start <= max) {
start = mb_find_next_zero_bit(bitmap, max + 1, start);
@@ -6840,7 +6843,7 @@ __releases(ext4_group_lock_ptr(sb, e4b->bd_group))
next = mb_find_next_bit(bitmap, max + 1, start);

if ((next - start) >= minblocks) {
- int ret = ext4_trim_extent(sb, start, next - start, e4b);
+ int ret = ext4_trim_extent(sb, start, next - start, e4b, &discard_bio);

if (ret && ret != -EOPNOTSUPP)
break;
@@ -6864,6 +6867,14 @@ __releases(ext4_group_lock_ptr(sb, e4b->bd_group))
break;
}

+ if (discard_bio) {
+ ext4_unlock_group(sb, e4b->bd_group);
+ submit_bio_wait(discard_bio);
+ bio_put(discard_bio);
+ ext4_lock_group(sb, e4b->bd_group);
+ }
+ blk_finish_plug(&plug);
+
return count;
}

--
2.37.1 (Apple Git-137.1)



2023-06-19 03:19:24

by Fengnan Chang

[permalink] [raw]
Subject: Re: [RFC PATCH] ext4: improve discard efficiency

This patch free blocks too earily when do discard, maybe cause data
corrupt, I'll fix this in
next version.

Fengnan Chang <[email protected]> 于2023年6月15日周四 19:12写道:
>
> In commit a015434480dc("ext4: send parallel discards on commit
> completions"), issue all discard commands in parallel make all
> bios could merged into one request, so lowlevel drive can issue
> multi segments in one time which is more efficiency, but commit
> 55cdd0af2bc5 ("ext4: get discard out of jbd2 commit kthread contex")
> seems broke this way, let's fix it.
> In my test, the time of fstrim fs with multi big sparse file
> reduce from 6.7s to 1.3s.
>
> Signed-off-by: Fengnan Chang <[email protected]>
> ---
> fs/ext4/mballoc.c | 17 ++++++++++++++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index a2475b8c9fb5..e5a27fd2e959 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -6790,7 +6790,7 @@ int ext4_group_add_blocks(handle_t *handle, struct super_block *sb,
> * be called with under the group lock.
> */
> static int ext4_trim_extent(struct super_block *sb,
> - int start, int count, struct ext4_buddy *e4b)
> + int start, int count, struct ext4_buddy *e4b, struct bio **biop)
> __releases(bitlock)
> __acquires(bitlock)
> {
> @@ -6812,7 +6812,7 @@ __acquires(bitlock)
> */
> mb_mark_used(e4b, &ex);
> ext4_unlock_group(sb, group);
> - ret = ext4_issue_discard(sb, group, start, count, NULL);
> + ret = ext4_issue_discard(sb, group, start, count, biop);
> ext4_lock_group(sb, group);
> mb_free_blocks(NULL, e4b, start, ex.fe_len);
> return ret;
> @@ -6826,12 +6826,15 @@ __releases(ext4_group_lock_ptr(sb, e4b->bd_group))
> {
> ext4_grpblk_t next, count, free_count;
> void *bitmap;
> + struct bio *discard_bio = NULL;
> + struct blk_plug plug;
>
> bitmap = e4b->bd_bitmap;
> start = (e4b->bd_info->bb_first_free > start) ?
> e4b->bd_info->bb_first_free : start;
> count = 0;
> free_count = 0;
> + blk_start_plug(&plug);
>
> while (start <= max) {
> start = mb_find_next_zero_bit(bitmap, max + 1, start);
> @@ -6840,7 +6843,7 @@ __releases(ext4_group_lock_ptr(sb, e4b->bd_group))
> next = mb_find_next_bit(bitmap, max + 1, start);
>
> if ((next - start) >= minblocks) {
> - int ret = ext4_trim_extent(sb, start, next - start, e4b);
> + int ret = ext4_trim_extent(sb, start, next - start, e4b, &discard_bio);
>
> if (ret && ret != -EOPNOTSUPP)
> break;
> @@ -6864,6 +6867,14 @@ __releases(ext4_group_lock_ptr(sb, e4b->bd_group))
> break;
> }
>
> + if (discard_bio) {
> + ext4_unlock_group(sb, e4b->bd_group);
> + submit_bio_wait(discard_bio);
> + bio_put(discard_bio);
> + ext4_lock_group(sb, e4b->bd_group);
> + }
> + blk_finish_plug(&plug);
> +
> return count;
> }
>
> --
> 2.37.1 (Apple Git-137.1)
>