On 8/13/20 2:14 PM, Andreas Dilger wrote:
On Aug 7, 2020, at 5:32 AM, brookxu <brookxu.cn@xxxxxxxxx> wrote:
Delete invalid ac_b_extent backup inside ext4_mb_use_best_found(),
we have done this operation in ext4_mb_new_group_pa() and
ext4_mb_new_inode_pa().
I'm not sure I understand this patch completely.
The calls to ext4_mb_new_group_pa() and ext4_mb_new_inode_pa() are
done from ext4_mb_new_preallocation(), which is called at the *end*
of ext4_mb_use_best_found() (i.e. after the lines that are being
deleted).
Maybe I'm confused by the description "we *have done* this operation"
makes it seem like it was already done, but really it should be
"we *will do* this operation in ..."?
That said, it would make more sense to keep the one line here in
ext4_mb_use_best_found() and remove the two duplicate lines in
ext4_mb_new_group_pa() and ext4_mb_new_inode_pa()? In that case,
the patch description would be more correct, like:
Delete duplicate ac_b_extent backup in ext4_mb_new_group_pa()
and ext4_mb_new_inode_pa(), since we have done this operation
in ext4_mb_use_best_found() already.
Looked into the mballoc code and I agree with Andreas points here.
-ritesh
