Re: [PATCH 02/21] btrfs: add WARN_ON() on incorrect lock range

[Boris Burkov <boris@bur.io>]

On Thu, Mar 02, 2023 at 04:24:47PM -0600, Goldwyn Rodrigues wrote:
> From: Goldwyn Rodrigues <rgoldwyn@suse.com>
> 
> Add a WARN_ON(start > end) to make sure that the locking happens on the
> correct range and no incorrect nodes (with state->start > state->end)
> are added to the tree.

Looks good, naturally. Quick question about it: do you think that
checking this invariant applies to other extent bit setting operations?

Perhaps it could also make sense to refactor
btrfs_debug_check_extent_io_range s.t. it compiles regardless of
CONFIG_BTRFS_DEBUG, and that it's possible for a caller to opt in to
checking even when the debug setting isn't set. (Perhaps with a
_checked() variant of the set_extent_bit function or something)

> 
> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
> ---
>  fs/btrfs/extent-io-tree.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/btrfs/extent-io-tree.c b/fs/btrfs/extent-io-tree.c
> index 29a225836e28..482721dd1eba 100644
> --- a/fs/btrfs/extent-io-tree.c
> +++ b/fs/btrfs/extent-io-tree.c
> @@ -1710,6 +1710,7 @@ int try_lock_extent(struct extent_io_tree *tree, u64 start, u64 end,
>  	int err;
>  	u64 failed_start;
>  
> +	WARN_ON(start > end);
>  	err = __set_extent_bit(tree, start, end, EXTENT_LOCKED, &failed_start,
>  			       NULL, cached, NULL, GFP_NOFS);
>  	if (err == -EEXIST) {
> @@ -1732,6 +1733,7 @@ int lock_extent(struct extent_io_tree *tree, u64 start, u64 end,
>  	int err;
>  	u64 failed_start;
>  
> +	WARN_ON(start > end);
>  	err = __set_extent_bit(tree, start, end, EXTENT_LOCKED, &failed_start,
>  			       &failed_state, cached_state, NULL, GFP_NOFS);
>  	while (err == -EEXIST) {
> -- 
> 2.39.2
>