| From 55e21b8a0c9fe784979ddce7e4a6989ee7514e05 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Sat, 25 Mar 2023 13:43:39 +0530 |
| Subject: ext4: Fix best extent lstart adjustment logic in |
| ext4_mb_new_inode_pa() |
| |
| From: Ojaswin Mujoo <ojaswin@linux.ibm.com> |
| |
| [ Upstream commit 93cdf49f6eca5e23f6546b8f28457b2e6a6961d9 ] |
| |
| When the length of best extent found is less than the length of goal extent |
| we need to make sure that the best extent atleast covers the start of the |
| original request. This is done by adjusting the ac_b_ex.fe_logical (logical |
| start) of the extent. |
| |
| While doing so, the current logic sometimes results in the best extent's |
| logical range overflowing the goal extent. Since this best extent is later |
| added to the inode preallocation list, we have a possibility of introducing |
| overlapping preallocations. This is discussed in detail here [1]. |
| |
| As per Jan's suggestion, to fix this, replace the existing logic with the |
| below logic for adjusting best extent as it keeps fragmentation in check |
| while ensuring logical range of best extent doesn't overflow out of goal |
| extent: |
| |
| 1. Check if best extent can be kept at end of goal range and still cover |
| original start. |
| 2. Else, check if best extent can be kept at start of goal range and still |
| cover original start. |
| 3. Else, keep the best extent at start of original request. |
| |
| Also, add a few extra BUG_ONs that might help catch errors faster. |
| |
| [1] https://lore.kernel.org/r/Y+OGkVvzPN0RMv0O@li-bb2b2a4c-3307-11b2-a85c-8fa5c3a69313.ibm.com |
| |
| Suggested-by: Jan Kara <jack@suse.cz> |
| Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com> |
| Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com> |
| Reviewed-by: Jan Kara <jack@suse.cz> |
| Link: https://lore.kernel.org/r/f96aca6d415b36d1f90db86c1a8cd7e2e9d7ab0e.1679731817.git.ojaswin@linux.ibm.com |
| Signed-off-by: Theodore Ts'o <tytso@mit.edu> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| fs/ext4/mballoc.c | 49 ++++++++++++++++++++++++++++++----------------- |
| 1 file changed, 31 insertions(+), 18 deletions(-) |
| |
| diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c |
| index 21f09bcffbbaa..32d88757a780e 100644 |
| --- a/fs/ext4/mballoc.c |
| +++ b/fs/ext4/mballoc.c |
| @@ -4321,6 +4321,7 @@ static void ext4_mb_use_inode_pa(struct ext4_allocation_context *ac, |
| BUG_ON(start < pa->pa_pstart); |
| BUG_ON(end > pa->pa_pstart + EXT4_C2B(sbi, pa->pa_len)); |
| BUG_ON(pa->pa_free < len); |
| + BUG_ON(ac->ac_b_ex.fe_len <= 0); |
| pa->pa_free -= len; |
| |
| mb_debug(ac->ac_sb, "use %llu/%d from inode pa %p\n", start, len, pa); |
| @@ -4650,10 +4651,8 @@ ext4_mb_new_inode_pa(struct ext4_allocation_context *ac) |
| pa = ac->ac_pa; |
| |
| if (ac->ac_b_ex.fe_len < ac->ac_g_ex.fe_len) { |
| - int winl; |
| - int wins; |
| - int win; |
| - int offs; |
| + int new_bex_start; |
| + int new_bex_end; |
| |
| /* we can't allocate as much as normalizer wants. |
| * so, found space must get proper lstart |
| @@ -4661,26 +4660,40 @@ ext4_mb_new_inode_pa(struct ext4_allocation_context *ac) |
| BUG_ON(ac->ac_g_ex.fe_logical > ac->ac_o_ex.fe_logical); |
| BUG_ON(ac->ac_g_ex.fe_len < ac->ac_o_ex.fe_len); |
| |
| - /* we're limited by original request in that |
| - * logical block must be covered any way |
| - * winl is window we can move our chunk within */ |
| - winl = ac->ac_o_ex.fe_logical - ac->ac_g_ex.fe_logical; |
| + /* |
| + * Use the below logic for adjusting best extent as it keeps |
| + * fragmentation in check while ensuring logical range of best |
| + * extent doesn't overflow out of goal extent: |
| + * |
| + * 1. Check if best ex can be kept at end of goal and still |
| + * cover original start |
| + * 2. Else, check if best ex can be kept at start of goal and |
| + * still cover original start |
| + * 3. Else, keep the best ex at start of original request. |
| + */ |
| + new_bex_end = ac->ac_g_ex.fe_logical + |
| + EXT4_C2B(sbi, ac->ac_g_ex.fe_len); |
| + new_bex_start = new_bex_end - EXT4_C2B(sbi, ac->ac_b_ex.fe_len); |
| + if (ac->ac_o_ex.fe_logical >= new_bex_start) |
| + goto adjust_bex; |
| |
| - /* also, we should cover whole original request */ |
| - wins = EXT4_C2B(sbi, ac->ac_b_ex.fe_len - ac->ac_o_ex.fe_len); |
| + new_bex_start = ac->ac_g_ex.fe_logical; |
| + new_bex_end = |
| + new_bex_start + EXT4_C2B(sbi, ac->ac_b_ex.fe_len); |
| + if (ac->ac_o_ex.fe_logical < new_bex_end) |
| + goto adjust_bex; |
| |
| - /* the smallest one defines real window */ |
| - win = min(winl, wins); |
| + new_bex_start = ac->ac_o_ex.fe_logical; |
| + new_bex_end = |
| + new_bex_start + EXT4_C2B(sbi, ac->ac_b_ex.fe_len); |
| |
| - offs = ac->ac_o_ex.fe_logical % |
| - EXT4_C2B(sbi, ac->ac_b_ex.fe_len); |
| - if (offs && offs < win) |
| - win = offs; |
| +adjust_bex: |
| + ac->ac_b_ex.fe_logical = new_bex_start; |
| |
| - ac->ac_b_ex.fe_logical = ac->ac_o_ex.fe_logical - |
| - EXT4_NUM_B2C(sbi, win); |
| BUG_ON(ac->ac_o_ex.fe_logical < ac->ac_b_ex.fe_logical); |
| BUG_ON(ac->ac_o_ex.fe_len > ac->ac_b_ex.fe_len); |
| + BUG_ON(new_bex_end > (ac->ac_g_ex.fe_logical + |
| + EXT4_C2B(sbi, ac->ac_g_ex.fe_len))); |
| } |
| |
| /* preallocation can change ac_b_ex, thus we store actually |
| -- |
| 2.39.2 |
| |