[PATCH] btrfs: remove NULL transaction support for btrfs_lookup_extent_info()

[PATCH] btrfs: remove NULL transaction support for btrfs_lookup_extent_info()

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

There are no callers of btrfs_lookup_extent_info() that pass a NULL value
for the transaction handle argument, so there's no point in having special
logic to deal with the NULL. The last caller that passed a NULL value was
removed in commit 19b546d7a1b2 ("btrfs: relocation:
Use btrfs_find_all_leafs to locate data extent parent tree leaves").

So remove the NULL handling from btrfs_lookup_extent_info().

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/extent-tree.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 58a72a57414a..a52e52144fa2 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -126,11 +126,6 @@ int btrfs_lookup_extent_info(struct btrfs_trans_handle *trans,
 	if (!path)
 		return -ENOMEM;
 
-	if (!trans) {
-		path->skip_locking = 1;
-		path->search_commit_root = 1;
-	}
-
 search_again:
 	key.objectid = bytenr;
 	key.offset = offset;
@@ -186,9 +181,6 @@ int btrfs_lookup_extent_info(struct btrfs_trans_handle *trans,
 		ret = 0;
 	}
 
-	if (!trans)
-		goto out;
-
 	delayed_refs = &trans->transaction->delayed_refs;
 	spin_lock(&delayed_refs->lock);
 	head = btrfs_find_delayed_ref_head(delayed_refs, bytenr);
@@ -219,7 +211,7 @@ int btrfs_lookup_extent_info(struct btrfs_trans_handle *trans,
 		mutex_unlock(&head->mutex);
 	}
 	spin_unlock(&delayed_refs->lock);
-out:
+
 	WARN_ON(num_refs == 0);
 	if (refs)
 		*refs = num_refs;
-- 
2.43.0

Re: [PATCH] btrfs: remove NULL transaction support for btrfs_lookup_extent_info()

[Qu Wenruo]

在 2024/6/18 17:02, fdmanana@kernel.org 写道:
> From: Filipe Manana <fdmanana@suse.com>
>
> There are no callers of btrfs_lookup_extent_info() that pass a NULL value
> for the transaction handle argument, so there's no point in having special
> logic to deal with the NULL. The last caller that passed a NULL value was
> removed in commit 19b546d7a1b2 ("btrfs: relocation:
> Use btrfs_find_all_leafs to locate data extent parent tree leaves").
>
> So remove the NULL handling from btrfs_lookup_extent_info().
>
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>   fs/btrfs/extent-tree.c | 10 +---------
>   1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 58a72a57414a..a52e52144fa2 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -126,11 +126,6 @@ int btrfs_lookup_extent_info(struct btrfs_trans_handle *trans,
>   	if (!path)
>   		return -ENOMEM;
>
> -	if (!trans) {
> -		path->skip_locking = 1;
> -		path->search_commit_root = 1;
> -	}
> -
>   search_again:
>   	key.objectid = bytenr;
>   	key.offset = offset;
> @@ -186,9 +181,6 @@ int btrfs_lookup_extent_info(struct btrfs_trans_handle *trans,
>   		ret = 0;
>   	}
>
> -	if (!trans)
> -		goto out;
> -
>   	delayed_refs = &trans->transaction->delayed_refs;
>   	spin_lock(&delayed_refs->lock);
>   	head = btrfs_find_delayed_ref_head(delayed_refs, bytenr);
> @@ -219,7 +211,7 @@ int btrfs_lookup_extent_info(struct btrfs_trans_handle *trans,
>   		mutex_unlock(&head->mutex);
>   	}
>   	spin_unlock(&delayed_refs->lock);
> -out:
> +
>   	WARN_ON(num_refs == 0);
>   	if (refs)
>   		*refs = num_refs;

[PATCH v2] btrfs: remove NULL transaction support for btrfs_lookup_extent_info()

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

There are no callers of btrfs_lookup_extent_info() that pass a NULL value
for the transaction handle argument, so there's no point in having special
logic to deal with the NULL. The last caller that passed a NULL value was
removed in commit 19b546d7a1b2 ("btrfs: relocation:
Use btrfs_find_all_leafs to locate data extent parent tree leaves").

So remove the NULL handling from btrfs_lookup_extent_info().

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---

V2: Remove the transaction abort logic check for NULL transaction too.

 fs/btrfs/extent-tree.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 58a72a57414a..21d123d392c0 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -126,11 +126,6 @@ int btrfs_lookup_extent_info(struct btrfs_trans_handle *trans,
 	if (!path)
 		return -ENOMEM;
 
-	if (!trans) {
-		path->skip_locking = 1;
-		path->search_commit_root = 1;
-	}
-
 search_again:
 	key.objectid = bytenr;
 	key.offset = offset;
@@ -171,11 +166,7 @@ int btrfs_lookup_extent_info(struct btrfs_trans_handle *trans,
 			btrfs_err(fs_info,
 			"unexpected extent item size, has %u expect >= %zu",
 				  item_size, sizeof(*ei));
-			if (trans)
-				btrfs_abort_transaction(trans, ret);
-			else
-				btrfs_handle_fs_error(fs_info, ret, NULL);
-
+			btrfs_abort_transaction(trans, ret);
 			goto out_free;
 		}
 
@@ -186,9 +177,6 @@ int btrfs_lookup_extent_info(struct btrfs_trans_handle *trans,
 		ret = 0;
 	}
 
-	if (!trans)
-		goto out;
-
 	delayed_refs = &trans->transaction->delayed_refs;
 	spin_lock(&delayed_refs->lock);
 	head = btrfs_find_delayed_ref_head(delayed_refs, bytenr);
@@ -219,7 +207,7 @@ int btrfs_lookup_extent_info(struct btrfs_trans_handle *trans,
 		mutex_unlock(&head->mutex);
 	}
 	spin_unlock(&delayed_refs->lock);
-out:
+
 	WARN_ON(num_refs == 0);
 	if (refs)
 		*refs = num_refs;
-- 
2.43.0

Re: [PATCH v2] btrfs: remove NULL transaction support for btrfs_lookup_extent_info()

[David Sterba]

On Tue, Jun 18, 2024 at 11:22:01AM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> There are no callers of btrfs_lookup_extent_info() that pass a NULL value
> for the transaction handle argument, so there's no point in having special
> logic to deal with the NULL. The last caller that passed a NULL value was
> removed in commit 19b546d7a1b2 ("btrfs: relocation:
> Use btrfs_find_all_leafs to locate data extent parent tree leaves").
> 
> So remove the NULL handling from btrfs_lookup_extent_info().
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
> 
> V2: Remove the transaction abort logic check for NULL transaction too.
> 
>  fs/btrfs/extent-tree.c | 16 ++--------------
>  1 file changed, 2 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 58a72a57414a..21d123d392c0 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -126,11 +126,6 @@ int btrfs_lookup_extent_info(struct btrfs_trans_handle *trans,
>  	if (!path)
>  		return -ENOMEM;
>  
> -	if (!trans) {
> -		path->skip_locking = 1;
> -		path->search_commit_root = 1;
> -	}
> -
>  search_again:
>  	key.objectid = bytenr;
>  	key.offset = offset;
> @@ -171,11 +166,7 @@ int btrfs_lookup_extent_info(struct btrfs_trans_handle *trans,
>  			btrfs_err(fs_info,
>  			"unexpected extent item size, has %u expect >= %zu",
>  				  item_size, sizeof(*ei));
> -			if (trans)
> -				btrfs_abort_transaction(trans, ret);
> -			else
> -				btrfs_handle_fs_error(fs_info, ret, NULL);

I have a WIP to remove btrfs_handle_fs_error() and for this one I
remember there was a NULL passed somehow to the function, it was
indirectly from the tree walking helpers. But I can't find it and after
another look there seems to be a valid pointer passed all times.

Reviewed-by: David Sterba <dsterba@suse.com>

Re: [PATCH v2] btrfs: remove NULL transaction support for btrfs_lookup_extent_info()

[Qu Wenruo]

在 2024/6/18 19:52, fdmanana@kernel.org 写道:
> From: Filipe Manana <fdmanana@suse.com>
>
> There are no callers of btrfs_lookup_extent_info() that pass a NULL value
> for the transaction handle argument, so there's no point in having special
> logic to deal with the NULL. The last caller that passed a NULL value was
> removed in commit 19b546d7a1b2 ("btrfs: relocation:
> Use btrfs_find_all_leafs to locate data extent parent tree leaves").
>
> So remove the NULL handling from btrfs_lookup_extent_info().
>
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>
> V2: Remove the transaction abort logic check for NULL transaction too.
>
>   fs/btrfs/extent-tree.c | 16 ++--------------
>   1 file changed, 2 insertions(+), 14 deletions(-)
>
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 58a72a57414a..21d123d392c0 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -126,11 +126,6 @@ int btrfs_lookup_extent_info(struct btrfs_trans_handle *trans,
>   	if (!path)
>   		return -ENOMEM;
>
> -	if (!trans) {
> -		path->skip_locking = 1;
> -		path->search_commit_root = 1;
> -	}
> -
>   search_again:
>   	key.objectid = bytenr;
>   	key.offset = offset;
> @@ -171,11 +166,7 @@ int btrfs_lookup_extent_info(struct btrfs_trans_handle *trans,
>   			btrfs_err(fs_info,
>   			"unexpected extent item size, has %u expect >= %zu",
>   				  item_size, sizeof(*ei));
> -			if (trans)
> -				btrfs_abort_transaction(trans, ret);
> -			else
> -				btrfs_handle_fs_error(fs_info, ret, NULL);
> -
> +			btrfs_abort_transaction(trans, ret);
>   			goto out_free;
>   		}
>
> @@ -186,9 +177,6 @@ int btrfs_lookup_extent_info(struct btrfs_trans_handle *trans,
>   		ret = 0;
>   	}
>
> -	if (!trans)
> -		goto out;
> -
>   	delayed_refs = &trans->transaction->delayed_refs;
>   	spin_lock(&delayed_refs->lock);
>   	head = btrfs_find_delayed_ref_head(delayed_refs, bytenr);
> @@ -219,7 +207,7 @@ int btrfs_lookup_extent_info(struct btrfs_trans_handle *trans,
>   		mutex_unlock(&head->mutex);
>   	}
>   	spin_unlock(&delayed_refs->lock);
> -out:
> +
>   	WARN_ON(num_refs == 0);
>   	if (refs)
>   		*refs = num_refs;