| Message ID | cover.1617694997.git.naohiro.aota@wdc.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | btrfs-progs: refactor and generalize chunk/dev_extent allocation | expand |
On 06/04/2021 10:07, Naohiro Aota 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. > For the whole series, Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
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? -- 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(-)
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(-) >
On Tue 06 Apr 2021 at 21:24, Naohiro Aota <naohiro.aota@wdc.com> wrote: > 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. > I see. Thanks. -- Su > 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(-) >>
On Tue, Apr 06, 2021 at 05:05:42PM +0900, Naohiro Aota 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. > > > 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 Added to devel, thanks.