diff mbox series

[01/16] btrfs: remove extent_io_tree_to_inode() and is_inode_io_tree()

Message ID ff5526066ce19b76e1b2b596813d480138e86699.1744046765.git.fdmanana@suse.com (mailing list archive)
State New
Headers show
Series btrfs: some cleanups for extent-io-tree (mostly renames) | expand

Commit Message

Filipe Manana April 7, 2025, 5:36 p.m. UTC 
From: Filipe Manana <fdmanana@suse.com>

These functions aren't used outside extent-io-tree.c, but yet one of them
(extent_io_tree_to_inode()) is unnecessarily exported in the header.

Furthermore their single use is in a pattern like this:

    if (is_inode_io_tree(tree))
        foo(extent_io_tree_to_inode(tree), ...);

So we're effectively unnecessarily adding more indirection, checking
twice if tree->owner == IO_TREE_INODE_IO before getting the inode and
doing a non-inline function call to get tree->inode.

Simplify this by removing these helper functions and instead doing
thing like this:

   if (tree->owner == IO_TREE_INODE_IO)
       foo(tree->inode, ...);

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/extent-io-tree.c | 55 ++++++++++++---------------------------
 fs/btrfs/extent-io-tree.h |  1 -
 2 files changed, 16 insertions(+), 40 deletions(-)

Comments

David Sterba April 7, 2025, 6:22 p.m. UTC | #1 
On Mon, Apr 07, 2025 at 06:36:08PM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> These functions aren't used outside extent-io-tree.c, but yet one of them
> (extent_io_tree_to_inode()) is unnecessarily exported in the header.
> 
> Furthermore their single use is in a pattern like this:
> 
>     if (is_inode_io_tree(tree))
>         foo(extent_io_tree_to_inode(tree), ...);
> 
> So we're effectively unnecessarily adding more indirection, checking
> twice if tree->owner == IO_TREE_INODE_IO before getting the inode and
> doing a non-inline function call to get tree->inode.

I did like that to document the special case and the conditional values
of inode depending on the tree owner.  The duplicated check of tree
owner will be unduplicated by compiler as it sees both functions.

> Simplify this by removing these helper functions and instead doing
> thing like this:
> 
>    if (tree->owner == IO_TREE_INODE_IO)
>        foo(tree->inode, ...);
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
>  fs/btrfs/extent-io-tree.c | 55 ++++++++++++---------------------------
>  fs/btrfs/extent-io-tree.h |  1 -
>  2 files changed, 16 insertions(+), 40 deletions(-)
> 
> diff --git a/fs/btrfs/extent-io-tree.c b/fs/btrfs/extent-io-tree.c
> index d833ab2d69a1..40da61cf3f0e 100644
> --- a/fs/btrfs/extent-io-tree.c
> +++ b/fs/btrfs/extent-io-tree.c
> @@ -80,23 +80,6 @@ static inline void __btrfs_debug_check_extent_io_range(const char *caller,
>  #define btrfs_debug_check_extent_io_range(c, s, e)	do {} while (0)
>  #endif
>  
> -
> -/*
> - * The only tree allowed to set the inode is IO_TREE_INODE_IO.
> - */
> -static bool is_inode_io_tree(const struct extent_io_tree *tree)
> -{
> -	return tree->owner == IO_TREE_INODE_IO;
> -}

This was added to be clear about the semantics, which somehow still is
from the test condition itself though.

> -
> -/* Return the inode if it's valid for the given tree, otherwise NULL. */
> -struct btrfs_inode *extent_io_tree_to_inode(struct extent_io_tree *tree)
> -{
> -	if (tree->owner == IO_TREE_INODE_IO)
> -		return tree->inode;
> -	return NULL;

The two helpers can be used independently, this was meant to prevent
accidental use of fs_info in place of inode.

It's perhaps personal preference of code readability to use or not use
the trivial helpers, as this is fairly static code I don't care if it
gets simplified like that.
diff mbox series

Patch

diff --git a/fs/btrfs/extent-io-tree.c b/fs/btrfs/extent-io-tree.c
index d833ab2d69a1..40da61cf3f0e 100644
--- a/fs/btrfs/extent-io-tree.c
+++ b/fs/btrfs/extent-io-tree.c
@@ -80,23 +80,6 @@  static inline void __btrfs_debug_check_extent_io_range(const char *caller,
 #define btrfs_debug_check_extent_io_range(c, s, e)	do {} while (0)
 #endif
 
-
-/*
- * The only tree allowed to set the inode is IO_TREE_INODE_IO.
- */
-static bool is_inode_io_tree(const struct extent_io_tree *tree)
-{
-	return tree->owner == IO_TREE_INODE_IO;
-}
-
-/* Return the inode if it's valid for the given tree, otherwise NULL. */
-struct btrfs_inode *extent_io_tree_to_inode(struct extent_io_tree *tree)
-{
-	if (tree->owner == IO_TREE_INODE_IO)
-		return tree->inode;
-	return NULL;
-}
-
 /* Read-only access to the inode. */
 const struct btrfs_inode *extent_io_tree_to_inode_const(const struct extent_io_tree *tree)
 {
@@ -362,9 +345,8 @@  static void merge_prev_state(struct extent_io_tree *tree, struct extent_state *s
 
 	prev = prev_state(state);
 	if (prev && prev->end == state->start - 1 && prev->state == state->state) {
-		if (is_inode_io_tree(tree))
-			btrfs_merge_delalloc_extent(extent_io_tree_to_inode(tree),
-						    state, prev);
+		if (tree->owner == IO_TREE_INODE_IO)
+			btrfs_merge_delalloc_extent(tree->inode, state, prev);
 		state->start = prev->start;
 		rb_erase(&prev->rb_node, &tree->state);
 		RB_CLEAR_NODE(&prev->rb_node);
@@ -378,9 +360,8 @@  static void merge_next_state(struct extent_io_tree *tree, struct extent_state *s
 
 	next = next_state(state);
 	if (next && next->start == state->end + 1 && next->state == state->state) {
-		if (is_inode_io_tree(tree))
-			btrfs_merge_delalloc_extent(extent_io_tree_to_inode(tree),
-						    state, next);
+		if (tree->owner == IO_TREE_INODE_IO)
+			btrfs_merge_delalloc_extent(tree->inode, state, next);
 		state->end = next->end;
 		rb_erase(&next->rb_node, &tree->state);
 		RB_CLEAR_NODE(&next->rb_node);
@@ -413,8 +394,8 @@  static void set_state_bits(struct extent_io_tree *tree,
 	u32 bits_to_set = bits & ~EXTENT_CTLBITS;
 	int ret;
 
-	if (is_inode_io_tree(tree))
-		btrfs_set_delalloc_extent(extent_io_tree_to_inode(tree), state, bits);
+	if (tree->owner == IO_TREE_INODE_IO)
+		btrfs_set_delalloc_extent(tree->inode, state, bits);
 
 	ret = add_extent_changeset(state, bits_to_set, changeset, 1);
 	BUG_ON(ret < 0);
@@ -459,10 +440,9 @@  static struct extent_state *insert_state(struct extent_io_tree *tree,
 		if (state->end < entry->start) {
 			if (try_merge && end == entry->start &&
 			    state->state == entry->state) {
-				if (is_inode_io_tree(tree))
-					btrfs_merge_delalloc_extent(
-							extent_io_tree_to_inode(tree),
-							state, entry);
+				if (tree->owner == IO_TREE_INODE_IO)
+					btrfs_merge_delalloc_extent(tree->inode,
+								    state, entry);
 				entry->start = state->start;
 				merge_prev_state(tree, entry);
 				state->state = 0;
@@ -472,10 +452,9 @@  static struct extent_state *insert_state(struct extent_io_tree *tree,
 		} else if (state->end > entry->end) {
 			if (try_merge && entry->end == start &&
 			    state->state == entry->state) {
-				if (is_inode_io_tree(tree))
-					btrfs_merge_delalloc_extent(
-							extent_io_tree_to_inode(tree),
-							state, entry);
+				if (tree->owner == IO_TREE_INODE_IO)
+					btrfs_merge_delalloc_extent(tree->inode,
+								    state, entry);
 				entry->end = state->end;
 				merge_next_state(tree, entry);
 				state->state = 0;
@@ -527,9 +506,8 @@  static int split_state(struct extent_io_tree *tree, struct extent_state *orig,
 	struct rb_node *parent = NULL;
 	struct rb_node **node;
 
-	if (is_inode_io_tree(tree))
-		btrfs_split_delalloc_extent(extent_io_tree_to_inode(tree), orig,
-					    split);
+	if (tree->owner == IO_TREE_INODE_IO)
+		btrfs_split_delalloc_extent(tree->inode, orig, split);
 
 	prealloc->start = orig->start;
 	prealloc->end = split - 1;
@@ -576,9 +554,8 @@  static struct extent_state *clear_state_bit(struct extent_io_tree *tree,
 	u32 bits_to_clear = bits & ~EXTENT_CTLBITS;
 	int ret;
 
-	if (is_inode_io_tree(tree))
-		btrfs_clear_delalloc_extent(extent_io_tree_to_inode(tree), state,
-					    bits);
+	if (tree->owner == IO_TREE_INODE_IO)
+		btrfs_clear_delalloc_extent(tree->inode, state, bits);
 
 	ret = add_extent_changeset(state, bits_to_clear, changeset, 0);
 	BUG_ON(ret < 0);
diff --git a/fs/btrfs/extent-io-tree.h b/fs/btrfs/extent-io-tree.h
index 6dfe8b097d93..bdcb18324516 100644
--- a/fs/btrfs/extent-io-tree.h
+++ b/fs/btrfs/extent-io-tree.h
@@ -134,7 +134,6 @@  struct extent_state {
 #endif
 };
 
-struct btrfs_inode *extent_io_tree_to_inode(struct extent_io_tree *tree);
 const struct btrfs_inode *extent_io_tree_to_inode_const(const struct extent_io_tree *tree);
 const struct btrfs_fs_info *extent_io_tree_to_fs_info(const struct extent_io_tree *tree);