diff mbox series

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

Message ID e12de38182a185ffc0540190a90d97124232103f.1677793433.git.rgoldwyn@suse.com (mailing list archive)
State New, archived
Headers show
Series Lock extents before pages | expand

Commit Message

Goldwyn Rodrigues March 2, 2023, 10:24 p.m. UTC 
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.

Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
 fs/btrfs/extent-io-tree.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Boris Burkov March 8, 2023, 7:28 p.m. UTC | #1 
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
>
diff mbox series

Patch

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) {