[PATCH] btrfs: btrfs_backref_link_edge() cleanup

Daniel Vacek posted 1 patch 8 months, 4 weeks ago
fs/btrfs/backref.c | 16 ++++++----------
fs/btrfs/backref.h |  7 -------
2 files changed, 6 insertions(+), 17 deletions(-)
[PATCH] btrfs: btrfs_backref_link_edge() cleanup
Posted by Daniel Vacek 8 months, 4 weeks ago
This function is always called with the LINK_LOWER argument. Thus we can
simplify it and remove the LINK_LOWER and LINK_UPPER macros.

The last call with LINK_UPPER was removed with commit
0097422c0dfe0a ("btrfs: remove clone_backref_node() from relocation")

Signed-off-by: Daniel Vacek <neelx@suse.com>
---
 fs/btrfs/backref.c | 16 ++++++----------
 fs/btrfs/backref.h |  7 -------
 2 files changed, 6 insertions(+), 17 deletions(-)

diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
index ed497f5f8d1bb..d011c64243c0c 100644
--- a/fs/btrfs/backref.c
+++ b/fs/btrfs/backref.c
@@ -3161,18 +3161,14 @@ void btrfs_backref_release_cache(struct btrfs_backref_cache *cache)
 	ASSERT(!cache->nr_edges);
 }
 
-void btrfs_backref_link_edge(struct btrfs_backref_edge *edge,
-			     struct btrfs_backref_node *lower,
-			     struct btrfs_backref_node *upper,
-			     int link_which)
+static void btrfs_backref_link_edge(struct btrfs_backref_edge *edge,
+				    struct btrfs_backref_node *lower,
+				    struct btrfs_backref_node *upper)
 {
 	ASSERT(upper && lower && upper->level == lower->level + 1);
 	edge->node[LOWER] = lower;
 	edge->node[UPPER] = upper;
-	if (link_which & LINK_LOWER)
-		list_add_tail(&edge->list[LOWER], &lower->upper);
-	if (link_which & LINK_UPPER)
-		list_add_tail(&edge->list[UPPER], &upper->lower);
+	list_add_tail(&edge->list[LOWER], &lower->upper);
 }
 /*
  * Handle direct tree backref
@@ -3242,7 +3238,7 @@ static int handle_direct_tree_backref(struct btrfs_backref_cache *cache,
 		ASSERT(upper->checked);
 		INIT_LIST_HEAD(&edge->list[UPPER]);
 	}
-	btrfs_backref_link_edge(edge, cur, upper, LINK_LOWER);
+	btrfs_backref_link_edge(edge, cur, upper);
 	return 0;
 }
 
@@ -3412,7 +3408,7 @@ static int handle_indirect_tree_backref(struct btrfs_trans_handle *trans,
 			if (!upper->owner)
 				upper->owner = btrfs_header_owner(eb);
 		}
-		btrfs_backref_link_edge(edge, lower, upper, LINK_LOWER);
+		btrfs_backref_link_edge(edge, lower, upper);
 
 		if (rb_node) {
 			btrfs_put_root(root);
diff --git a/fs/btrfs/backref.h b/fs/btrfs/backref.h
index 953637115956b..507cfb35a23cd 100644
--- a/fs/btrfs/backref.h
+++ b/fs/btrfs/backref.h
@@ -423,13 +423,6 @@ struct btrfs_backref_node *btrfs_backref_alloc_node(
 struct btrfs_backref_edge *btrfs_backref_alloc_edge(
 		struct btrfs_backref_cache *cache);
 
-#define		LINK_LOWER	(1U << 0)
-#define		LINK_UPPER	(1U << 1)
-
-void btrfs_backref_link_edge(struct btrfs_backref_edge *edge,
-			     struct btrfs_backref_node *lower,
-			     struct btrfs_backref_node *upper,
-			     int link_which);
 void btrfs_backref_free_node(struct btrfs_backref_cache *cache,
 			     struct btrfs_backref_node *node);
 void btrfs_backref_free_edge(struct btrfs_backref_cache *cache,
-- 
2.47.2
Re: [PATCH] btrfs: btrfs_backref_link_edge() cleanup
Posted by David Sterba 8 months, 3 weeks ago
Please write more descriptive subject lines, related to what is being
cleaned up, and not just a generic "function() cleanup".

On Wed, May 14, 2025 at 03:12:39PM +0200, Daniel Vacek wrote:
> This function is always called with the LINK_LOWER argument. Thus we can
> simplify it and remove the LINK_LOWER and LINK_UPPER macros.
> 
> The last call with LINK_UPPER was removed with commit
> 0097422c0dfe0a ("btrfs: remove clone_backref_node() from relocation")

While removing the code is ok, looking to the git history I think
there's probably more, related to the node linking. Here it's removing
one half (LINK_UPPER).
Re: [PATCH] btrfs: btrfs_backref_link_edge() cleanup
Posted by Daniel Vacek 8 months, 3 weeks ago
On Thu, 15 May 2025 at 16:47, David Sterba <dsterba@suse.cz> wrote:
>
> Please write more descriptive subject lines, related to what is being
> cleaned up, and not just a generic "function() cleanup".
>
> On Wed, May 14, 2025 at 03:12:39PM +0200, Daniel Vacek wrote:
> > This function is always called with the LINK_LOWER argument. Thus we can
> > simplify it and remove the LINK_LOWER and LINK_UPPER macros.
> >
> > The last call with LINK_UPPER was removed with commit
> > 0097422c0dfe0a ("btrfs: remove clone_backref_node() from relocation")
>
> While removing the code is ok, looking to the git history I think
> there's probably more, related to the node linking. Here it's removing
> one half (LINK_UPPER).

I'm not sure what you mean here. I mentioned the commit you suggested.
Checking the rest of edge linking I don't see how it could be
trivially simplified further.
Re: [PATCH] btrfs: btrfs_backref_link_edge() cleanup
Posted by David Sterba 8 months, 2 weeks ago
On Mon, May 19, 2025 at 12:38:14PM +0200, Daniel Vacek wrote:
> On Thu, 15 May 2025 at 16:47, David Sterba <dsterba@suse.cz> wrote:
> >
> > Please write more descriptive subject lines, related to what is being
> > cleaned up, and not just a generic "function() cleanup".
> >
> > On Wed, May 14, 2025 at 03:12:39PM +0200, Daniel Vacek wrote:
> > > This function is always called with the LINK_LOWER argument. Thus we can
> > > simplify it and remove the LINK_LOWER and LINK_UPPER macros.
> > >
> > > The last call with LINK_UPPER was removed with commit
> > > 0097422c0dfe0a ("btrfs: remove clone_backref_node() from relocation")
> >
> > While removing the code is ok, looking to the git history I think
> > there's probably more, related to the node linking. Here it's removing
> > one half (LINK_UPPER).
> 
> I'm not sure what you mean here. I mentioned the commit you suggested.
> Checking the rest of edge linking I don't see how it could be
> trivially simplified further.

The idea is to look around if there's possibly more code to be removed,
not trivially but based on the logic related to the removed code for the
LOWER part. The relocation code is complicated so this is beyond the
cleanup and don't have a specific suggestion, so let's take what we have
now.
Re: [PATCH] btrfs: btrfs_backref_link_edge() cleanup
Posted by Johannes Thumshirn 8 months, 4 weeks ago
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Re: [PATCH] btrfs: btrfs_backref_link_edge() cleanup
Posted by Johannes Thumshirn 8 months, 4 weeks ago
On 14.05.25 17:47, Johannes Thumshirn wrote:
> Looks good,
> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> 

Hit sent too early sorry, I'd also mention in the changelog that it 
isn't used outside of backref.c so it can be made static.
Re: [PATCH] btrfs: btrfs_backref_link_edge() cleanup
Posted by Daniel Vacek 8 months, 3 weeks ago
On Wed, 14 May 2025 at 17:48, Johannes Thumshirn
<Johannes.Thumshirn@wdc.com> wrote:
>
> On 14.05.25 17:47, Johannes Thumshirn wrote:
> > Looks good,
> > Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> >
>
> Hit sent too early sorry, I'd also mention in the changelog that it
> isn't used outside of backref.c so it can be made static.

Yeah, that could be mentioned explicitly.