[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <3p5udvc7fgd73kruz563pi4dmc6vjxvszmnegyym2xhuuauw5j@sjudcmk7idht>
Date: Fri, 27 Jun 2025 20:15:05 +0200
From: Jan Kara <jack@...e.cz>
To: Baokun Li <libaokun1@...wei.com>
Cc: linux-ext4@...r.kernel.org, tytso@....edu, jack@...e.cz,
adilger.kernel@...ger.ca, ojaswin@...ux.ibm.com, linux-kernel@...r.kernel.org,
yi.zhang@...wei.com, yangerkun@...wei.com
Subject: Re: [PATCH v2 02/16] ext4: remove unnecessary s_mb_last_start
On Mon 23-06-25 15:32:50, Baokun Li wrote:
> ac->ac_g_ex.fe_start is only used in ext4_mb_find_by_goal(), but STREAM
> ALLOC is activated after ext4_mb_find_by_goal() fails, so there's no need
> to update ac->ac_g_ex.fe_start, remove the unnecessary s_mb_last_start.
>
> Signed-off-by: Baokun Li <libaokun1@...wei.com>
I'd just note that ac->ac_g_ex.fe_start is also used in
ext4_mb_collect_stats() so this change may impact the statistics gathered
there. OTOH it is questionable whether we even want to account streaming
allocation as a goal hit... Anyway, I'm fine with this, I'd just mention it
in the changelog.
Also one nit below but feel free to add:
Reviewed-by: Jan Kara <jack@...e.cz>
> @@ -2849,7 +2848,6 @@ 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;
Maybe reset ac->ac_g_ex.fe_start to 0 instead of leaving it at some random
value? Just for the sake of defensive programming...
Honza
--
Jan Kara <jack@...e.com>
SUSE Labs, CR
Powered by blists - more mailing lists