In ext4_mb_choose_next_group_best_avail(), we want the start order to be
1 less than goal length and the min_order to be, at max, 1 more than the
original length. This commit fixes an off by one issue that arose due to
the fact that 1 << fls(n) > (n).
After all the processing:
order = 1 order below goal len
min_order = maximum of the three:-
- order - trim_order
- 1 order below B2C(s_stripe)
- 1 order above original len
Fixes: 33122aa930 ("ext4: Add allocation criteria 1.5 (CR1_5)")
Signed-off-by: Ojaswin Mujoo <[email protected]>
---
fs/ext4/mballoc.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 4f2a1df98141..d890495127d8 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -1007,14 +1007,11 @@ static void ext4_mb_choose_next_group_best_avail(struct ext4_allocation_context
* fls() instead since we need to know the actual length while modifying
* goal length.
*/
- order = fls(ac->ac_g_ex.fe_len);
+ order = fls(ac->ac_g_ex.fe_len) - 1;
min_order = order - sbi->s_mb_best_avail_max_trim_order;
if (min_order < 0)
min_order = 0;
- if (1 << min_order < ac->ac_o_ex.fe_len)
- min_order = fls(ac->ac_o_ex.fe_len) + 1;
-
if (sbi->s_stripe > 0) {
/*
* We are assuming that stripe size is always a multiple of
@@ -1022,9 +1019,16 @@ static void ext4_mb_choose_next_group_best_avail(struct ext4_allocation_context
*/
num_stripe_clusters = EXT4_NUM_B2C(sbi, sbi->s_stripe);
if (1 << min_order < num_stripe_clusters)
- min_order = fls(num_stripe_clusters);
+ /*
+ * We consider 1 order less because later we round
+ * up the goal len to num_stripe_clusters
+ */
+ min_order = fls(num_stripe_clusters) - 1;
}
+ if (1 << min_order < ac->ac_o_ex.fe_len)
+ min_order = fls(ac->ac_o_ex.fe_len);
+
for (i = order; i >= min_order; i--) {
int frag_order;
/*
@@ -1038,9 +1042,6 @@ static void ext4_mb_choose_next_group_best_avail(struct ext4_allocation_context
/*
* Try to round up the adjusted goal to stripe size
* (in cluster units) multiple for efficiency.
- *
- * XXX: Is s->stripe always a power of 2? In that case
- * we can use the faster round_up() variant.
*/
ac->ac_g_ex.fe_len = roundup(ac->ac_g_ex.fe_len,
num_stripe_clusters);
--
2.31.1
On Fri, 09 Jun 2023 16:04:03 +0530, Ojaswin Mujoo wrote:
> In ext4_mb_choose_next_group_best_avail(), we want the start order to be
> 1 less than goal length and the min_order to be, at max, 1 more than the
> original length. This commit fixes an off by one issue that arose due to
> the fact that 1 << fls(n) > (n).
>
> After all the processing:
>
> [...]
Applied, thanks!
[1/1] ext4: fix off by one issue in ext4_mb_choose_next_group_best_avail()
commit: 7c79210b15ef69a5702d1fb817bad9a30bffb097
Best regards,
--
Theodore Ts'o <[email protected]>