On 2025/7/17 18:29, Ojaswin Mujoo wrote:
On Mon, Jul 14, 2025 at 09:03:12PM +0800, Baokun Li wrote:In ext4_mb_regular_allocator(), after the call to ext4_mb_find_by_goal() fails to achieve the inode goal, allocation continues with the stream allocation global goal. Currently, hits for both are combined in sbi->s_bal_goals, hindering accurate optimization. This commit separates global goal hits into sbi->s_bal_stream_goals. Since stream allocation doesn't use ac->ac_g_ex.fe_start, set fe_start to -1. This prevents stream allocations from being counted in s_bal_goals. Also clear EXT4_MB_HINT_TRY_GOAL to avoid calling ext4_mb_find_by_goal again. After adding `stream_goal_hits`, `/proc/fs/ext4/sdx/mb_stats` will show: mballoc: reqs: 840347 success: 750992 groups_scanned: 1230506 cr_p2_aligned_stats: hits: 21531 groups_considered: 411664 extents_scanned: 21531 useless_loops: 0 bad_suggestions: 6 cr_goal_fast_stats: hits: 111222 groups_considered: 1806728 extents_scanned: 467908 useless_loops: 0 bad_suggestions: 13 cr_best_avail_stats: hits: 36267 groups_considered: 1817631 extents_scanned: 156143 useless_loops: 0 bad_suggestions: 204 cr_goal_slow_stats: hits: 106396 groups_considered: 5671710 extents_scanned: 22540056 useless_loops: 123747 cr_any_free_stats: hits: 138071 groups_considered: 724692 extents_scanned: 23615593 useless_loops: 585 extents_scanned: 46804261 goal_hits: 1307 stream_goal_hits: 236317 len_goal_hits: 155549 2^n_hits: 21531 breaks: 225096 lost: 35062 buddies_generated: 40/40 buddies_time_used: 48004 preallocated: 5962467 discarded: 4847560 Signed-off-by: Baokun Li <libaokun1@xxxxxxxxxx> --- fs/ext4/ext4.h | 1 + fs/ext4/mballoc.c | 11 +++++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h index 9df74123e7e6..8750ace12935 100644 --- a/fs/ext4/ext4.h +++ b/fs/ext4/ext4.h @@ -1646,6 +1646,7 @@ struct ext4_sb_info { atomic_t s_bal_cX_ex_scanned[EXT4_MB_NUM_CRS]; /* total extents scanned */ atomic_t s_bal_groups_scanned; /* number of groups scanned */ atomic_t s_bal_goals; /* goal hits */ + atomic_t s_bal_stream_goals; /* stream allocation global goal hits */ atomic_t s_bal_len_goals; /* len goal hits */ atomic_t s_bal_breaks; /* too long searches */ atomic_t s_bal_2orders; /* 2^order hits */ diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c index 336d65c4f6a2..f56ac477c464 100644 --- a/fs/ext4/mballoc.c +++ b/fs/ext4/mballoc.c @@ -2849,8 +2849,9 @@ ext4_mb_regular_allocator(struct ext4_allocation_context *ac) /* TBD: may be hot point */ spin_lock(&sbi->s_md_lock); ac->ac_g_ex.fe_group = sbi->s_mb_last_group; - ac->ac_g_ex.fe_start = sbi->s_mb_last_start; spin_unlock(&sbi->s_md_lock); + ac->ac_g_ex.fe_start = -1; + ac->ac_flags &= ~EXT4_MB_HINT_TRY_GOAL;Hey Baokun, I was a bit late to review this in v2 so I'll add the comment here: So this is mostly to account for retires right? Maybe rather than disabling goal allocation a better way to do this is resetting the original goal group and goal start in the retry logic of ext4_mb_new_blocks()? Since we drop preallocations before retrying, this way we might actually find our goal during the retry. Its a slim chance though but still feels like the right way to do it. Thoughts?
It's true that successfully acquiring the goal on retry is possible, but the probability is too low; I think attempting the inode goal on retry is not very meaningful. The lack of trylock logic in ext4_mb_find_by_goal() also introduces some performance overhead. Additionally, since preallocations might be dropped before retrying, the inode's preallocation could also be discarded. Therefore, pa overlap needs to be re-adjusted. For data block allocation, we should call ext4_mb_normalize_request() to regenerate a new ac_g_ex instead of directly resetting the original goal. ext4_mb_normalize_request() will also determine whether to reset EXT4_MB_HINT_TRY_GOAL. For metadata block allocation, EXT4_MB_STREAM_ALLOC is not set, so there's no need to worry about EXT4_MB_HINT_TRY_GOAL being cleared. Clearing EXT4_MB_HINT_TRY_GOAL here is only to avoid inode goal allocation with -1. If you insist that we should attempt the inode goal on retry, I will send a separate patch to address that. Cheers, Baokun
