On 22/05/21 09:42PM, Baokun Li wrote:Yes, exactly.
When either of the "start + size <= ac->ac_o_ex.fe_logical" orSounds about right to me based on the logic in ext4_mb_use_inode_pa().
"start > ac->ac_o_ex.fe_logical" conditions is met, it indicates
that the fe_logical is not in the allocated range.
We try to allocate/preallocate such that ac->ac_o_ex.fe_logical should fall
within the preallocated range. So if our start or start + size doesn't include
fe_logical then it is a bug in the ext4_mb_normalize_request() logic.
But should we be so harsh to hit a bug_on() or make it warn_on()?I don't think hit a bug_on() is a problem. BUG_ON is not triggered here and will be triggered later.
Also did you run any fs tests with this change.Yes, I ran xfstests on ext3 and ext4 and found no problems.
Since it looks like thisYes, on our coverage report, those lines of code never seem to get there.
logic existed since mballoc was introduced.
Okay, I'm going to remove this tag.In this case, it should be bug_ON.No, there is no issue with this patch. It correctly just removes the duplicate
Fixes: dfe076c106f6 ("ext4: get rid of code duplication")
logic.
Signed-off-by: Baokun Li<libaokun1@xxxxxxxxxx>.
---
fs/ext4/mballoc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 32410b79b664..d0fb57970648 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -4190,7 +4190,7 @@ ext4_mb_normalize_request(struct ext4_allocation_context *ac,
}
rcu_read_unlock();
- if (start + size <= ac->ac_o_ex.fe_logical &&
+ if (start + size <= ac->ac_o_ex.fe_logical ||
start > ac->ac_o_ex.fe_logical) {
ext4_msg(ac->ac_sb, KERN_ERR,
"start %lu, size %lu, fe_logical %lu",
--
2.31.1