| Message ID | 20200610123258.12382-2-johannes.thumshirn@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs-progs: simplify chunk allocation a bit | expand |
On 2020/6/10 下午8:32, Johannes Thumshirn wrote: > In btrfs_alloc_chunk_ctrl() we have a recurring pattern, first we assign > num stripes, then we test if num_stripes is smaller than a hardcoded > boundary and after that we set min_stripes to this magic value. > > Reverse the logic by first assigning min_stripes and then testing > num_stripes against min_stripes. > > This will help further refactoring. > > Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> I'm wondering why we don't use btrfs_raid_attr::devs_min. In fact, I'm a little surprised why use so little of that structure, but relies on so many if branches to do the check. Thanks, Qu > --- > volumes.c | 30 +++++++++++++++--------------- > 1 file changed, 15 insertions(+), 15 deletions(-) > > diff --git a/volumes.c b/volumes.c > index 7f84fbbaa742..089363f66473 100644 > --- a/volumes.c > +++ b/volumes.c > @@ -1054,25 +1054,25 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans, > } > } > if (type & BTRFS_BLOCK_GROUP_RAID1) { > - num_stripes = min_t(u64, 2, > + min_stripes = 2; > + num_stripes = min_t(u64, min_stripes, > btrfs_super_num_devices(info->super_copy)); > - if (num_stripes < 2) > + if (num_stripes < min_stripes) > return -ENOSPC; > - min_stripes = 2; > } > if (type & BTRFS_BLOCK_GROUP_RAID1C3) { > - num_stripes = min_t(u64, 3, > + min_stripes = 3; > + num_stripes = min_t(u64, min_stripes, > btrfs_super_num_devices(info->super_copy)); > - if (num_stripes < 3) > + if (num_stripes < min_stripes) > return -ENOSPC; > - min_stripes = 3; > } > if (type & BTRFS_BLOCK_GROUP_RAID1C4) { > - num_stripes = min_t(u64, 4, > + min_stripes = 4; > + num_stripes = min_t(u64, min_stripes, > btrfs_super_num_devices(info->super_copy)); > - if (num_stripes < 4) > + if (num_stripes < min_stripes) > return -ENOSPC; > - min_stripes = 4; > } > if (type & BTRFS_BLOCK_GROUP_DUP) { > num_stripes = 2; > @@ -1085,32 +1085,32 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans, > min_stripes = 2; > } > if (type & (BTRFS_BLOCK_GROUP_RAID10)) { > + min_stripes = 4; > num_stripes = btrfs_super_num_devices(info->super_copy); > if (num_stripes > max_stripes) > num_stripes = max_stripes; > - if (num_stripes < 4) > + if (num_stripes < min_stripes) > return -ENOSPC; > num_stripes &= ~(u32)1; > sub_stripes = 2; > - min_stripes = 4; > } > if (type & (BTRFS_BLOCK_GROUP_RAID5)) { > + min_stripes = 2; > num_stripes = btrfs_super_num_devices(info->super_copy); > if (num_stripes > max_stripes) > num_stripes = max_stripes; > - if (num_stripes < 2) > + if (num_stripes < min_stripes) > return -ENOSPC; > - min_stripes = 2; > stripe_len = find_raid56_stripe_len(num_stripes - 1, > btrfs_super_stripesize(info->super_copy)); > } > if (type & (BTRFS_BLOCK_GROUP_RAID6)) { > + min_stripes = 3; > num_stripes = btrfs_super_num_devices(info->super_copy); > if (num_stripes > max_stripes) > num_stripes = max_stripes; > - if (num_stripes < 3) > + if (num_stripes < min_stripes) > return -ENOSPC; > - min_stripes = 3; > stripe_len = find_raid56_stripe_len(num_stripes - 2, > btrfs_super_stripesize(info->super_copy)); > } >
diff --git a/volumes.c b/volumes.c index 7f84fbbaa742..089363f66473 100644 --- a/volumes.c +++ b/volumes.c @@ -1054,25 +1054,25 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans, } } if (type & BTRFS_BLOCK_GROUP_RAID1) { - num_stripes = min_t(u64, 2, + min_stripes = 2; + num_stripes = min_t(u64, min_stripes, btrfs_super_num_devices(info->super_copy)); - if (num_stripes < 2) + if (num_stripes < min_stripes) return -ENOSPC; - min_stripes = 2; } if (type & BTRFS_BLOCK_GROUP_RAID1C3) { - num_stripes = min_t(u64, 3, + min_stripes = 3; + num_stripes = min_t(u64, min_stripes, btrfs_super_num_devices(info->super_copy)); - if (num_stripes < 3) + if (num_stripes < min_stripes) return -ENOSPC; - min_stripes = 3; } if (type & BTRFS_BLOCK_GROUP_RAID1C4) { - num_stripes = min_t(u64, 4, + min_stripes = 4; + num_stripes = min_t(u64, min_stripes, btrfs_super_num_devices(info->super_copy)); - if (num_stripes < 4) + if (num_stripes < min_stripes) return -ENOSPC; - min_stripes = 4; } if (type & BTRFS_BLOCK_GROUP_DUP) { num_stripes = 2; @@ -1085,32 +1085,32 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans, min_stripes = 2; } if (type & (BTRFS_BLOCK_GROUP_RAID10)) { + min_stripes = 4; num_stripes = btrfs_super_num_devices(info->super_copy); if (num_stripes > max_stripes) num_stripes = max_stripes; - if (num_stripes < 4) + if (num_stripes < min_stripes) return -ENOSPC; num_stripes &= ~(u32)1; sub_stripes = 2; - min_stripes = 4; } if (type & (BTRFS_BLOCK_GROUP_RAID5)) { + min_stripes = 2; num_stripes = btrfs_super_num_devices(info->super_copy); if (num_stripes > max_stripes) num_stripes = max_stripes; - if (num_stripes < 2) + if (num_stripes < min_stripes) return -ENOSPC; - min_stripes = 2; stripe_len = find_raid56_stripe_len(num_stripes - 1, btrfs_super_stripesize(info->super_copy)); } if (type & (BTRFS_BLOCK_GROUP_RAID6)) { + min_stripes = 3; num_stripes = btrfs_super_num_devices(info->super_copy); if (num_stripes > max_stripes) num_stripes = max_stripes; - if (num_stripes < 3) + if (num_stripes < min_stripes) return -ENOSPC; - min_stripes = 3; stripe_len = find_raid56_stripe_len(num_stripes - 2, btrfs_super_stripesize(info->super_copy)); }
In btrfs_alloc_chunk_ctrl() we have a recurring pattern, first we assign num stripes, then we test if num_stripes is smaller than a hardcoded boundary and after that we set min_stripes to this magic value. Reverse the logic by first assigning min_stripes and then testing num_stripes against min_stripes. This will help further refactoring. Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> --- volumes.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-)