| Message ID | 20181004212443.26519-3-hans.van.kranenburg@mendix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Chunk allocator DUP fix and cleanups | expand |
On 10/04/2018 11:24 PM, Hans van Kranenburg wrote: > num_bytes is really a way too generic name for a variable in this > function. There are a dozen other variables that hold a number of bytes > as value. > > Give it a name that actually describes what it does, which is holding > the size of the chunk that we're allocating. > > Signed-off-by: Hans van Kranenburg <hans.van.kranenburg@mendix.com> > --- > fs/btrfs/volumes.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index 9c9bb127eeee..40fa85e68b1f 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -123,6 +123,8 @@ const char *get_raid_name(enum btrfs_raid_types type) > return btrfs_raid_array[type].raid_name; > } > > + > + Meh, missed that one, will remove. > static int init_first_rw_device(struct btrfs_trans_handle *trans, > struct btrfs_fs_info *fs_info); > static int btrfs_relocate_sys_chunks(struct btrfs_fs_info *fs_info); > @@ -4599,7 +4601,7 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans, > u64 max_stripe_size; > u64 max_chunk_size; > u64 stripe_size; > - u64 num_bytes; > + u64 chunk_size; > int ndevs; > int i; > int j; > @@ -4801,9 +4803,9 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans, > map->type = type; > map->sub_stripes = sub_stripes; > > - num_bytes = stripe_size * data_stripes; > + chunk_size = stripe_size * data_stripes; > > - trace_btrfs_chunk_alloc(info, map, start, num_bytes); > + trace_btrfs_chunk_alloc(info, map, start, chunk_size); > > em = alloc_extent_map(); > if (!em) { > @@ -4814,7 +4816,7 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans, > set_bit(EXTENT_FLAG_FS_MAPPING, &em->flags); > em->map_lookup = map; > em->start = start; > - em->len = num_bytes; > + em->len = chunk_size; > em->block_start = 0; > em->block_len = em->len; > em->orig_block_len = stripe_size; > @@ -4832,7 +4834,7 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans, > refcount_inc(&em->refs); > write_unlock(&em_tree->lock); > > - ret = btrfs_make_block_group(trans, 0, type, start, num_bytes); > + ret = btrfs_make_block_group(trans, 0, type, start, chunk_size); > if (ret) > goto error_del_extent; > >
On 5.10.2018 00:24, Hans van Kranenburg wrote: > num_bytes is really a way too generic name for a variable in this > function. There are a dozen other variables that hold a number of bytes > as value. > > Give it a name that actually describes what it does, which is holding > the size of the chunk that we're allocating. > > Signed-off-by: Hans van Kranenburg <hans.van.kranenburg@mendix.com> Apart from the extra 2 spaces you can add: Reviewed-by: Nikolay Borisov <nborisov@suse.com> > --- > fs/btrfs/volumes.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index 9c9bb127eeee..40fa85e68b1f 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -123,6 +123,8 @@ const char *get_raid_name(enum btrfs_raid_types type) > return btrfs_raid_array[type].raid_name; > } > > + > + > static int init_first_rw_device(struct btrfs_trans_handle *trans, > struct btrfs_fs_info *fs_info); > static int btrfs_relocate_sys_chunks(struct btrfs_fs_info *fs_info); > @@ -4599,7 +4601,7 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans, > u64 max_stripe_size; > u64 max_chunk_size; > u64 stripe_size; > - u64 num_bytes; > + u64 chunk_size; > int ndevs; > int i; > int j; > @@ -4801,9 +4803,9 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans, > map->type = type; > map->sub_stripes = sub_stripes; > > - num_bytes = stripe_size * data_stripes; > + chunk_size = stripe_size * data_stripes; > > - trace_btrfs_chunk_alloc(info, map, start, num_bytes); > + trace_btrfs_chunk_alloc(info, map, start, chunk_size); > > em = alloc_extent_map(); > if (!em) { > @@ -4814,7 +4816,7 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans, > set_bit(EXTENT_FLAG_FS_MAPPING, &em->flags); > em->map_lookup = map; > em->start = start; > - em->len = num_bytes; > + em->len = chunk_size; > em->block_start = 0; > em->block_len = em->len; > em->orig_block_len = stripe_size; > @@ -4832,7 +4834,7 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans, > refcount_inc(&em->refs); > write_unlock(&em_tree->lock); > > - ret = btrfs_make_block_group(trans, 0, type, start, num_bytes); > + ret = btrfs_make_block_group(trans, 0, type, start, chunk_size); > if (ret) > goto error_del_extent; > >
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 9c9bb127eeee..40fa85e68b1f 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -123,6 +123,8 @@ const char *get_raid_name(enum btrfs_raid_types type) return btrfs_raid_array[type].raid_name; } + + static int init_first_rw_device(struct btrfs_trans_handle *trans, struct btrfs_fs_info *fs_info); static int btrfs_relocate_sys_chunks(struct btrfs_fs_info *fs_info); @@ -4599,7 +4601,7 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans, u64 max_stripe_size; u64 max_chunk_size; u64 stripe_size; - u64 num_bytes; + u64 chunk_size; int ndevs; int i; int j; @@ -4801,9 +4803,9 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans, map->type = type; map->sub_stripes = sub_stripes; - num_bytes = stripe_size * data_stripes; + chunk_size = stripe_size * data_stripes; - trace_btrfs_chunk_alloc(info, map, start, num_bytes); + trace_btrfs_chunk_alloc(info, map, start, chunk_size); em = alloc_extent_map(); if (!em) { @@ -4814,7 +4816,7 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans, set_bit(EXTENT_FLAG_FS_MAPPING, &em->flags); em->map_lookup = map; em->start = start; - em->len = num_bytes; + em->len = chunk_size; em->block_start = 0; em->block_len = em->len; em->orig_block_len = stripe_size; @@ -4832,7 +4834,7 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans, refcount_inc(&em->refs); write_unlock(&em_tree->lock); - ret = btrfs_make_block_group(trans, 0, type, start, num_bytes); + ret = btrfs_make_block_group(trans, 0, type, start, chunk_size); if (ret) goto error_del_extent;
num_bytes is really a way too generic name for a variable in this function. There are a dozen other variables that hold a number of bytes as value. Give it a name that actually describes what it does, which is holding the size of the chunk that we're allocating. Signed-off-by: Hans van Kranenburg <hans.van.kranenburg@mendix.com> --- fs/btrfs/volumes.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)