Re: [PATCH 00/12] btrfs-progs: refactor and generalize chunk/dev_extent allocation

[Naohiro Aota <naohiro.aota@wdc.com>]

On Tue, Apr 06, 2021 at 06:54:37PM +0800, Su Yue wrote:
> 
> On Tue 06 Apr 2021 at 16:05, Naohiro Aota <naohiro.aota@wdc.com> wrote:
> 
> > This is the userland counterpart of the following series.
> > 
> > https://lore.kernel.org/linux-btrfs/20200225035626.1049501-1-naohiro.aota@wdc.com/
> > 
> > This series refactors chunk allocation and device_extent allocation
> > functions and make them generalized to be able to implement other
> > allocation policy easily.
> > 
> > On top of this series, we can simplify userland side of the zoned series
> > as
> > adding a new type of chunk allocator and extent allocator for zoned
> > block
> > devices. Furthermore, we will be able to implement and test some other
> > allocator in the idea page of the wiki e.g. SSD caching, dedicated
> > metadata
> > drive, chunk allocation groups, and so on.
> > 
> > This series also fixes a bug of calculating the stripe size in DUP
> > profile,
> > and cleans up the code.
> > 
> > * Refactoring chunk/dev_extent allocator
> > 
> > Two functions are separated from find_free_dev_extent_start().
> > dev_extent_search_start() decides the starting position of the search.
> > dev_extent_hole_check() checks if a hole found is suitable for device
> > extent allocation.
> > 
> > Split some parts of btrfs_alloc_chunk() into three functions.
> > init_alloc_chunk_policy() initializes the parameters of an allocation.
> > decide_stripe_size() decides the size of chunk and device_extent. And,
> > create_chunk() creates a chunk and device extents.
> > 
> > * Patch organization
> > 
> > Patches 1 and 2 refactor find_free_dev_extent_start().
> > 
> > Patches 3 to 6 refactor btrfs_alloc_chunk() by moving the code into
> > three
> > other functions.
> > 
> > Patch 7 uses create_chunk() to simplify btrfs_alloc_data_chunk().
> > 
> > Patch 8 fixes a bug of calculating stripe size in DUP profile.
> > 
> > Patches 9 to 12 clean up btrfs_alloc_chunk() code by dropping
> > unnecessary
> > parameters, and using better macro/variable name to clarify the meaning.
> > 
> > 
> gcc10 complains following warnings:
> kernel-shared/volumes.c: In function ‘decide_stripe_size’:
> kernel-shared/volumes.c:1119:1: warning: control reaches end of non-void
> function [-Wreturn-type]
> 1119 | }
>      | ^
> kernel-shared/volumes.c: In function ‘dev_extent_search_start’:
> kernel-shared/volumes.c:465:1: warning: control reaches end of non-void
> function [-Wreturn-type]
>  465 | }
>      | ^
> 
> Looked at locations just two nits about 'switch'. Care to fix?

These are actually false-positve warnings. They never reach the end of
the function because BUG() in the default case will abort the program.

The warnings are showing up because the BUG() macro is not marked as
unreachable. This is addressed in the following patch. So, once the
following patch is merged, the warnings will disappear.

https://lore.kernel.org/linux-btrfs/5c7b703beca572514a28677df0caaafab28bfff8.1617265419.git.naohiro.aota@wdc.com/T/#u

> --
> Su
> > Naohiro Aota (12):
> >   btrfs-progs: introduce chunk allocation policy
> >   btrfs-progs: refactor find_free_dev_extent_start()
> >   btrfs-progs: convert type of alloc_chunk_ctl::type
> >   btrfs-progs: consolidate parameter initialization of regular
> > allocator
> >   btrfs-progs: factor out decide_stripe_size()
> >   btrfs-progs: factor out create_chunk()
> >   btrfs-progs: rewrite btrfs_alloc_data_chunk() using   create_chunk()
> >   btrfs-progs: fix to use half the available space for DUP   profile
> >   btrfs-progs: use round_down for allocation calcs
> >   btrfs-progs: drop alloc_chunk_ctl::stripe_len
> >   btrfs-progs: simplify arguments of chunk_bytes_by_type()
> >   btrfs-progs: rename calc_size to stripe_size
> > 
> >  kernel-shared/volumes.c | 514  +++++++++++++++++++++-------------------
> >  kernel-shared/volumes.h |   6 +
> >  2 files changed, 274 insertions(+), 246 deletions(-)
>