| Message ID | 20210702130206.30909-1-nborisov@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: Minor improvements to should_alloc_chunk | expand |
On Fri, Jul 02, 2021 at 04:02:06PM +0300, Nikolay Borisov wrote: > Since it's a predicate function make it explicitly return boolean. Also > the 'thresh' variable is only used when force is CHUNK_ALLOC_LIMITED so > reduce the scope of the variable as necessary. Finally, remove the + 2m > used in the final check. Given the granularity of btrfs' allocation I > doubt that the + 2m made a difference when making a decision whether to > allocate a chunk or not. This is mixing 2 cleanups and potentially a functional change in one patch, "I doubt it made a difference" does not sound like a good reasoning, I can say that I doubt you're right so what now? :) Doing the cleanups (return value and moving the scope) in one is probably fine, but the +2M change needs some explanation and should go to a separate patch.
diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c index 3eecbc2b3dae..613527733fb2 100644 --- a/fs/btrfs/block-group.c +++ b/fs/btrfs/block-group.c @@ -3312,30 +3312,29 @@ static void force_metadata_allocation(struct btrfs_fs_info *info) } } -static int should_alloc_chunk(struct btrfs_fs_info *fs_info, +static bool should_alloc_chunk(struct btrfs_fs_info *fs_info, struct btrfs_space_info *sinfo, int force) { u64 bytes_used = btrfs_space_info_used(sinfo, false); - u64 thresh; if (force == CHUNK_ALLOC_FORCE) - return 1; + return true; /* * in limited mode, we want to have some free space up to * about 1% of the FS size. */ if (force == CHUNK_ALLOC_LIMITED) { - thresh = btrfs_super_total_bytes(fs_info->super_copy); + u64 thresh = btrfs_super_total_bytes(fs_info->super_copy); thresh = max_t(u64, SZ_64M, div_factor_fine(thresh, 1)); if (sinfo->total_bytes - bytes_used < thresh) - return 1; + return true; } - if (bytes_used + SZ_2M < div_factor(sinfo->total_bytes, 8)) - return 0; - return 1; + if (bytes_used < div_factor(sinfo->total_bytes, 8)) + return false; + return true; } int btrfs_force_chunk_alloc(struct btrfs_trans_handle *trans, u64 type)
Since it's a predicate function make it explicitly return boolean. Also the 'thresh' variable is only used when force is CHUNK_ALLOC_LIMITED so reduce the scope of the variable as necessary. Finally, remove the + 2m used in the final check. Given the granularity of btrfs' allocation I doubt that the + 2m made a difference when making a decision whether to allocate a chunk or not. Signed-off-by: Nikolay Borisov <nborisov@suse.com> --- fs/btrfs/block-group.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)