public inbox for linux-btrfs@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] Reduce size of extent_io_tree, remove fs_info
@ 2023-11-21 13:20 David Sterba
  2023-11-21 13:20 ` [PATCH 1/5] btrfs: move lockdep class setting out of extent_io_tree_init David Sterba
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: David Sterba @ 2023-11-21 13:20 UTC (permalink / raw)
  To: linux-btrfs; +Cc: David Sterba

We have the fs_info pointer in extent_io_tree for the trace points as
the inode is not always set. This is a bit wasteful and extent_io_tree
is also embedded in other structures. The tree owner can be used to
determine if the inode is expected to be non-NULL, otherwise we can
store the fs_info pointer.

I tried to do it in the cleanest way, union and access wrappers, it's
IMO worth the space savings:

- btrfs_inode		1104 -> 1088
- btrfs_device		 520 ->  512
- btrfs_root		1360 -> 1344
- btrfs_transaction	 456 ->  440
- btrfs_fs_info		3600 -> 3592
- reloc_control		1520 -> 1512

The btrfs_inode structure is getting closer to the 1024 size where it
would pack better in the slab pages.

David Sterba (5):
  btrfs: move lockdep class setting out of extent_io_tree_init
  btrfs: drop error message in extent_io_tree insert_state()
  btrfs: constify fs_info parameter in __btrfs_panic()
  btrfs: enhance extent_io_tree error reports
  btrfs: always set extent_io_tree::inode and drop fs_info

 fs/btrfs/extent-io-tree.c    | 113 ++++++++++++++++++++++-------------
 fs/btrfs/extent-io-tree.h    |  18 +++++-
 fs/btrfs/inode.c             |  14 +++++
 fs/btrfs/messages.c          |   2 +-
 fs/btrfs/messages.h          |   2 +-
 fs/btrfs/tests/btrfs-tests.c |   2 +-
 include/trace/events/btrfs.h |  45 +++++---------
 7 files changed, 118 insertions(+), 78 deletions(-)

-- 
2.42.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/5] btrfs: move lockdep class setting out of extent_io_tree_init
  2023-11-21 13:20 [PATCH 0/5] Reduce size of extent_io_tree, remove fs_info David Sterba
@ 2023-11-21 13:20 ` David Sterba
  2023-11-21 13:20 ` [PATCH 2/5] btrfs: drop error message in extent_io_tree insert_state() David Sterba
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: David Sterba @ 2023-11-21 13:20 UTC (permalink / raw)
  To: linux-btrfs; +Cc: David Sterba

The per-inode file extent tree was added in 41a2ee75aab0 ("btrfs:
introduce per-inode file extent tree"), it's the only tree type
that requires the lockdep class. Move it to the file where it is
actually used.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/extent-io-tree.c | 10 ----------
 fs/btrfs/inode.c          | 11 +++++++++++
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/extent-io-tree.c b/fs/btrfs/extent-io-tree.c
index 76061245a46b..56be64e656da 100644
--- a/fs/btrfs/extent-io-tree.c
+++ b/fs/btrfs/extent-io-tree.c
@@ -78,14 +78,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
 
-/*
- * For the file_extent_tree, we want to hold the inode lock when we lookup and
- * update the disk_i_size, but lockdep will complain because our io_tree we hold
- * the tree lock and get the inode lock when setting delalloc.  These two things
- * are unrelated, so make a class for the file_extent_tree so we don't get the
- * two locking patterns mixed up.
- */
-static struct lock_class_key file_extent_tree_class;
 
 void extent_io_tree_init(struct btrfs_fs_info *fs_info,
 			 struct extent_io_tree *tree, unsigned int owner)
@@ -95,8 +87,6 @@ void extent_io_tree_init(struct btrfs_fs_info *fs_info,
 	spin_lock_init(&tree->lock);
 	tree->inode = NULL;
 	tree->owner = owner;
-	if (owner == IO_TREE_INODE_FILE_EXTENT)
-		lockdep_set_class(&tree->lock, &file_extent_tree_class);
 }
 
 /*
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 844e800160f1..9e7b44c2fbce 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -114,6 +114,15 @@ struct data_reloc_warn {
 	int mirror_num;
 };
 
+/*
+ * For the file_extent_tree, we want to hold the inode lock when we lookup and
+ * update the disk_i_size, but lockdep will complain because our io_tree we hold
+ * the tree lock and get the inode lock when setting delalloc. These two things
+ * are unrelated, so make a class for the file_extent_tree so we don't get the
+ * two locking patterns mixed up.
+ */
+static struct lock_class_key file_extent_tree_class;
+
 static const struct inode_operations btrfs_dir_inode_operations;
 static const struct inode_operations btrfs_symlink_inode_operations;
 static const struct inode_operations btrfs_special_inode_operations;
@@ -8506,6 +8515,8 @@ struct inode *btrfs_alloc_inode(struct super_block *sb)
 	ei->io_tree.inode = ei;
 	extent_io_tree_init(fs_info, &ei->file_extent_tree,
 			    IO_TREE_INODE_FILE_EXTENT);
+	/* Lockdep class is set only for the file extent tree. */
+	lockdep_set_class(&ei->file_extent_tree.lock, &file_extent_tree_class);
 	mutex_init(&ei->log_mutex);
 	spin_lock_init(&ei->ordered_tree_lock);
 	ei->ordered_tree = RB_ROOT;
-- 
2.42.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/5] btrfs: drop error message in extent_io_tree insert_state()
  2023-11-21 13:20 [PATCH 0/5] Reduce size of extent_io_tree, remove fs_info David Sterba
  2023-11-21 13:20 ` [PATCH 1/5] btrfs: move lockdep class setting out of extent_io_tree_init David Sterba
@ 2023-11-21 13:20 ` David Sterba
  2023-11-21 13:20 ` [PATCH 3/5] btrfs: constify fs_info parameter in __btrfs_panic() David Sterba
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: David Sterba @ 2023-11-21 13:20 UTC (permalink / raw)
  To: linux-btrfs; +Cc: David Sterba

The helper insert_state errors are handled in all callers and reported
by extent_io_tree_panic so we don't need to do it twice.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/extent-io-tree.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/fs/btrfs/extent-io-tree.c b/fs/btrfs/extent-io-tree.c
index 56be64e656da..887d9beb7b10 100644
--- a/fs/btrfs/extent-io-tree.c
+++ b/fs/btrfs/extent-io-tree.c
@@ -442,9 +442,6 @@ static struct extent_state *insert_state(struct extent_io_tree *tree,
 			}
 			node = &(*node)->rb_right;
 		} else {
-			btrfs_err(tree->fs_info,
-			       "found node %llu %llu on insert of %llu %llu",
-			       entry->start, entry->end, state->start, state->end);
 			return ERR_PTR(-EEXIST);
 		}
 	}
-- 
2.42.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/5] btrfs: constify fs_info parameter in __btrfs_panic()
  2023-11-21 13:20 [PATCH 0/5] Reduce size of extent_io_tree, remove fs_info David Sterba
  2023-11-21 13:20 ` [PATCH 1/5] btrfs: move lockdep class setting out of extent_io_tree_init David Sterba
  2023-11-21 13:20 ` [PATCH 2/5] btrfs: drop error message in extent_io_tree insert_state() David Sterba
@ 2023-11-21 13:20 ` David Sterba
  2023-11-21 13:20 ` [PATCH 4/5] btrfs: enhance extent_io_tree error reports David Sterba
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: David Sterba @ 2023-11-21 13:20 UTC (permalink / raw)
  To: linux-btrfs; +Cc: David Sterba

The printk helpers take const fs_info if it's used just for the
identifier in the messages, __btrfs_panic() lacks that.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/messages.c | 2 +-
 fs/btrfs/messages.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/messages.c b/fs/btrfs/messages.c
index b8f9c9e56c8c..cdada4865837 100644
--- a/fs/btrfs/messages.c
+++ b/fs/btrfs/messages.c
@@ -287,7 +287,7 @@ void __cold btrfs_err_32bit_limit(struct btrfs_fs_info *fs_info)
  * panic or BUGs, depending on mount options.
  */
 __cold
-void __btrfs_panic(struct btrfs_fs_info *fs_info, const char *function,
+void __btrfs_panic(const struct btrfs_fs_info *fs_info, const char *function,
 		   unsigned int line, int error, const char *fmt, ...)
 {
 	char *s_id = "<unknown>";
diff --git a/fs/btrfs/messages.h b/fs/btrfs/messages.h
index 4d04c1fa5899..08a9272399d2 100644
--- a/fs/btrfs/messages.h
+++ b/fs/btrfs/messages.h
@@ -194,7 +194,7 @@ const char * __attribute_const__ btrfs_decode_error(int error);
 
 __printf(5, 6)
 __cold
-void __btrfs_panic(struct btrfs_fs_info *fs_info, const char *function,
+void __btrfs_panic(const struct btrfs_fs_info *fs_info, const char *function,
 		   unsigned int line, int error, const char *fmt, ...);
 /*
  * If BTRFS_MOUNT_PANIC_ON_FATAL_ERROR is in mount_opt, __btrfs_panic
-- 
2.42.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 4/5] btrfs: enhance extent_io_tree error reports
  2023-11-21 13:20 [PATCH 0/5] Reduce size of extent_io_tree, remove fs_info David Sterba
                   ` (2 preceding siblings ...)
  2023-11-21 13:20 ` [PATCH 3/5] btrfs: constify fs_info parameter in __btrfs_panic() David Sterba
@ 2023-11-21 13:20 ` David Sterba
  2023-11-21 13:20 ` [PATCH 5/5] btrfs: always set extent_io_tree::inode and drop fs_info David Sterba
  2023-11-21 14:19 ` [PATCH 0/5] Reduce size of extent_io_tree, remove fs_info Josef Bacik
  5 siblings, 0 replies; 7+ messages in thread
From: David Sterba @ 2023-11-21 13:20 UTC (permalink / raw)
  To: linux-btrfs; +Cc: David Sterba

Pass the type of the extent io tree operation which failed in the report
helper. The message wording and contents is updated, though locking
might be the cause of the error it's probably not the only one and we're
interested in the state.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/extent-io-tree.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/extent-io-tree.c b/fs/btrfs/extent-io-tree.c
index 887d9beb7b10..2d564ead9dbe 100644
--- a/fs/btrfs/extent-io-tree.c
+++ b/fs/btrfs/extent-io-tree.c
@@ -313,10 +313,14 @@ static inline struct extent_state *tree_search(struct extent_io_tree *tree, u64
 	return tree_search_for_insert(tree, offset, NULL, NULL);
 }
 
-static void extent_io_tree_panic(struct extent_io_tree *tree, int err)
+static void extent_io_tree_panic(const struct extent_io_tree *tree,
+				 const struct extent_state *state,
+				 const char *opname,
+				 int err)
 {
 	btrfs_panic(tree->fs_info, err,
-	"locking error: extent tree was modified by another thread while locked");
+		    "extent io tree error on %s state start %llu end %llu",
+		    opname, state->start, state->end);
 }
 
 static void merge_prev_state(struct extent_io_tree *tree, struct extent_state *state)
@@ -676,7 +680,7 @@ int __clear_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
 			goto search_again;
 		err = split_state(tree, state, prealloc, start);
 		if (err)
-			extent_io_tree_panic(tree, err);
+			extent_io_tree_panic(tree, state, "split", err);
 
 		prealloc = NULL;
 		if (err)
@@ -698,7 +702,7 @@ int __clear_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
 			goto search_again;
 		err = split_state(tree, state, prealloc, end + 1);
 		if (err)
-			extent_io_tree_panic(tree, err);
+			extent_io_tree_panic(tree, state, "split", err);
 
 		if (wake)
 			wake_up(&state->wq);
@@ -1133,7 +1137,7 @@ static int __set_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
 			goto search_again;
 		err = split_state(tree, state, prealloc, start);
 		if (err)
-			extent_io_tree_panic(tree, err);
+			extent_io_tree_panic(tree, state, "split", err);
 
 		prealloc = NULL;
 		if (err)
@@ -1181,7 +1185,7 @@ static int __set_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
 		inserted_state = insert_state(tree, prealloc, bits, changeset);
 		if (IS_ERR(inserted_state)) {
 			err = PTR_ERR(inserted_state);
-			extent_io_tree_panic(tree, err);
+			extent_io_tree_panic(tree, prealloc, "insert", err);
 		}
 
 		cache_state(inserted_state, cached_state);
@@ -1209,7 +1213,7 @@ static int __set_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
 			goto search_again;
 		err = split_state(tree, state, prealloc, end + 1);
 		if (err)
-			extent_io_tree_panic(tree, err);
+			extent_io_tree_panic(tree, state, "split", err);
 
 		set_state_bits(tree, prealloc, bits, changeset);
 		cache_state(prealloc, cached_state);
@@ -1363,7 +1367,7 @@ int convert_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
 		}
 		err = split_state(tree, state, prealloc, start);
 		if (err)
-			extent_io_tree_panic(tree, err);
+			extent_io_tree_panic(tree, state, "split", err);
 		prealloc = NULL;
 		if (err)
 			goto out;
@@ -1411,7 +1415,7 @@ int convert_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
 		inserted_state = insert_state(tree, prealloc, bits, NULL);
 		if (IS_ERR(inserted_state)) {
 			err = PTR_ERR(inserted_state);
-			extent_io_tree_panic(tree, err);
+			extent_io_tree_panic(tree, prealloc, "insert", err);
 		}
 		cache_state(inserted_state, cached_state);
 		if (inserted_state == prealloc)
@@ -1434,7 +1438,7 @@ int convert_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
 
 		err = split_state(tree, state, prealloc, end + 1);
 		if (err)
-			extent_io_tree_panic(tree, err);
+			extent_io_tree_panic(tree, state, "split", err);
 
 		set_state_bits(tree, prealloc, bits, NULL);
 		cache_state(prealloc, cached_state);
-- 
2.42.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 5/5] btrfs: always set extent_io_tree::inode and drop fs_info
  2023-11-21 13:20 [PATCH 0/5] Reduce size of extent_io_tree, remove fs_info David Sterba
                   ` (3 preceding siblings ...)
  2023-11-21 13:20 ` [PATCH 4/5] btrfs: enhance extent_io_tree error reports David Sterba
@ 2023-11-21 13:20 ` David Sterba
  2023-11-21 14:19 ` [PATCH 0/5] Reduce size of extent_io_tree, remove fs_info Josef Bacik
  5 siblings, 0 replies; 7+ messages in thread
From: David Sterba @ 2023-11-21 13:20 UTC (permalink / raw)
  To: linux-btrfs; +Cc: David Sterba

The extent_io_tree is embedded in several structures, notably in struct
btrfs_inode.  The fs_info is only used for reporting errors and for
reference in trace points. We can get to the pointer through the inode,
but not all io trees set it. However, we always know the owner and
can recognize if inode is valid.  For access helpers are provided, const
variant for the trace points.

This reduces size of extent_io_tree by 8 bytes and following structures
in turn:

- btrfs_inode		1104 -> 1088
- btrfs_device		 520 ->  512
- btrfs_root		1360 -> 1344
- btrfs_transaction	 456 ->  440
- btrfs_fs_info		3600 -> 3592
- reloc_control		1520 -> 1512

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/extent-io-tree.c    | 80 ++++++++++++++++++++++++++----------
 fs/btrfs/extent-io-tree.h    | 18 ++++++--
 fs/btrfs/inode.c             |  3 ++
 fs/btrfs/tests/btrfs-tests.c |  2 +-
 include/trace/events/btrfs.h | 45 +++++++-------------
 5 files changed, 93 insertions(+), 55 deletions(-)

diff --git a/fs/btrfs/extent-io-tree.c b/fs/btrfs/extent-io-tree.c
index 2d564ead9dbe..dbd201a99693 100644
--- a/fs/btrfs/extent-io-tree.c
+++ b/fs/btrfs/extent-io-tree.c
@@ -58,12 +58,13 @@ static inline void __btrfs_debug_check_extent_io_range(const char *caller,
 						       struct extent_io_tree *tree,
 						       u64 start, u64 end)
 {
-	struct btrfs_inode *inode = tree->inode;
+	const struct btrfs_inode *inode;
 	u64 isize;
 
-	if (!inode)
+	if (tree->owner != IO_TREE_INODE_IO)
 		return;
 
+	inode = extent_io_tree_to_inode_const(tree);
 	isize = i_size_read(&inode->vfs_inode);
 	if (end >= PAGE_SIZE && (end % 2) == 0 && end != isize - 1) {
 		btrfs_debug_rl(inode->root->fs_info,
@@ -79,13 +80,44 @@ static inline void __btrfs_debug_check_extent_io_range(const char *caller,
 #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)
+{
+	if (tree->owner == IO_TREE_INODE_IO)
+		return tree->inode;
+	return NULL;
+}
+
+/* For read-only access to fs_info. */
+const struct btrfs_fs_info *extent_io_tree_to_fs_info(const struct extent_io_tree *tree)
+{
+	if (tree->owner == IO_TREE_INODE_IO)
+		return tree->inode->root->fs_info;
+	return tree->fs_info;
+}
+
 void extent_io_tree_init(struct btrfs_fs_info *fs_info,
 			 struct extent_io_tree *tree, unsigned int owner)
 {
-	tree->fs_info = fs_info;
 	tree->state = RB_ROOT;
 	spin_lock_init(&tree->lock);
-	tree->inode = NULL;
+	tree->fs_info = fs_info;
 	tree->owner = owner;
 }
 
@@ -318,7 +350,7 @@ static void extent_io_tree_panic(const struct extent_io_tree *tree,
 				 const char *opname,
 				 int err)
 {
-	btrfs_panic(tree->fs_info, err,
+	btrfs_panic(extent_io_tree_to_fs_info(tree), err,
 		    "extent io tree error on %s state start %llu end %llu",
 		    opname, state->start, state->end);
 }
@@ -329,8 +361,9 @@ 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 (tree->inode)
-			btrfs_merge_delalloc_extent(tree->inode, state, prev);
+		if (is_inode_io_tree(tree))
+			btrfs_merge_delalloc_extent(extent_io_tree_to_inode(tree),
+						    state, prev);
 		state->start = prev->start;
 		rb_erase(&prev->rb_node, &tree->state);
 		RB_CLEAR_NODE(&prev->rb_node);
@@ -344,8 +377,9 @@ 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 (tree->inode)
-			btrfs_merge_delalloc_extent(tree->inode, state, next);
+		if (is_inode_io_tree(tree))
+			btrfs_merge_delalloc_extent(extent_io_tree_to_inode(tree),
+						    state, next);
 		state->end = next->end;
 		rb_erase(&next->rb_node, &tree->state);
 		RB_CLEAR_NODE(&next->rb_node);
@@ -378,8 +412,8 @@ static void set_state_bits(struct extent_io_tree *tree,
 	u32 bits_to_set = bits & ~EXTENT_CTLBITS;
 	int ret;
 
-	if (tree->inode)
-		btrfs_set_delalloc_extent(tree->inode, state, bits);
+	if (is_inode_io_tree(tree))
+		btrfs_set_delalloc_extent(extent_io_tree_to_inode(tree), state, bits);
 
 	ret = add_extent_changeset(state, bits_to_set, changeset, 1);
 	BUG_ON(ret < 0);
@@ -424,9 +458,10 @@ 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 (tree->inode)
-					btrfs_merge_delalloc_extent(tree->inode,
-								    state, entry);
+				if (is_inode_io_tree(tree))
+					btrfs_merge_delalloc_extent(
+							extent_io_tree_to_inode(tree),
+							state, entry);
 				entry->start = state->start;
 				merge_prev_state(tree, entry);
 				state->state = 0;
@@ -436,9 +471,10 @@ 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 (tree->inode)
-					btrfs_merge_delalloc_extent(tree->inode,
-								    state, entry);
+				if (is_inode_io_tree(tree))
+					btrfs_merge_delalloc_extent(
+							extent_io_tree_to_inode(tree),
+							state, entry);
 				entry->end = state->end;
 				merge_next_state(tree, entry);
 				state->state = 0;
@@ -490,8 +526,9 @@ static int split_state(struct extent_io_tree *tree, struct extent_state *orig,
 	struct rb_node *parent = NULL;
 	struct rb_node **node;
 
-	if (tree->inode)
-		btrfs_split_delalloc_extent(tree->inode, orig, split);
+	if (is_inode_io_tree(tree))
+		btrfs_split_delalloc_extent(extent_io_tree_to_inode(tree), orig,
+					    split);
 
 	prealloc->start = orig->start;
 	prealloc->end = split - 1;
@@ -538,8 +575,9 @@ static struct extent_state *clear_state_bit(struct extent_io_tree *tree,
 	u32 bits_to_clear = bits & ~EXTENT_CTLBITS;
 	int ret;
 
-	if (tree->inode)
-		btrfs_clear_delalloc_extent(tree->inode, state, bits);
+	if (is_inode_io_tree(tree))
+		btrfs_clear_delalloc_extent(extent_io_tree_to_inode(tree), 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 5602b0137fcd..ebe6390d65e9 100644
--- a/fs/btrfs/extent-io-tree.h
+++ b/fs/btrfs/extent-io-tree.h
@@ -87,9 +87,17 @@ enum {
 
 struct extent_io_tree {
 	struct rb_root state;
-	struct btrfs_fs_info *fs_info;
-	/* Inode associated with this tree, or NULL. */
-	struct btrfs_inode *inode;
+	/*
+	 * The fs_info is needed for trace points, a tree attached to an inode
+	 * needs the inode.
+	 *
+	 * owner == IO_TREE_INODE_IO - then inode is valid and fs_info can be
+	 *                             accessed as inode->root->fs_info
+	 */
+	union {
+		struct btrfs_fs_info *fs_info;
+		struct btrfs_inode *inode;
+	};
 
 	/* Who owns this io tree, should be one of IO_TREE_* */
 	u8 owner;
@@ -112,6 +120,10 @@ 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);
+
 void extent_io_tree_init(struct btrfs_fs_info *fs_info,
 			 struct extent_io_tree *tree, unsigned int owner);
 void extent_io_tree_release(struct extent_io_tree *tree);
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 9e7b44c2fbce..3c23ce73c7a8 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8511,8 +8511,11 @@ struct inode *btrfs_alloc_inode(struct super_block *sb)
 
 	inode = &ei->vfs_inode;
 	extent_map_tree_init(&ei->extent_tree);
+
+	/* This io tree sets the valid inode. */
 	extent_io_tree_init(fs_info, &ei->io_tree, IO_TREE_INODE_IO);
 	ei->io_tree.inode = ei;
+
 	extent_io_tree_init(fs_info, &ei->file_extent_tree,
 			    IO_TREE_INODE_FILE_EXTENT);
 	/* Lockdep class is set only for the file extent tree. */
diff --git a/fs/btrfs/tests/btrfs-tests.c b/fs/btrfs/tests/btrfs-tests.c
index ca09cf9afce8..63afda5eac67 100644
--- a/fs/btrfs/tests/btrfs-tests.c
+++ b/fs/btrfs/tests/btrfs-tests.c
@@ -102,7 +102,7 @@ struct btrfs_device *btrfs_alloc_dummy_device(struct btrfs_fs_info *fs_info)
 	if (!dev)
 		return ERR_PTR(-ENOMEM);
 
-	extent_io_tree_init(NULL, &dev->alloc_state, 0);
+	extent_io_tree_init(fs_info, &dev->alloc_state, 0);
 	INIT_LIST_HEAD(&dev->dev_list);
 	list_add(&dev->dev_list, &fs_info->fs_devices->devices);
 
diff --git a/include/trace/events/btrfs.h b/include/trace/events/btrfs.h
index 279a7a0c90c0..558847e41efd 100644
--- a/include/trace/events/btrfs.h
+++ b/include/trace/events/btrfs.h
@@ -2099,17 +2099,12 @@ TRACE_EVENT(btrfs_set_extent_bit,
 		__field(	unsigned,	set_bits)
 	),
 
-	TP_fast_assign_btrfs(tree->fs_info,
-		__entry->owner = tree->owner;
-		if (tree->inode) {
-			const struct btrfs_inode *inode = tree->inode;
+	TP_fast_assign_btrfs(extent_io_tree_to_fs_info(tree),
+		const struct btrfs_inode *inode = extent_io_tree_to_inode_const(tree);
 
-			__entry->ino	= btrfs_ino(inode);
-			__entry->rootid	= inode->root->root_key.objectid;
-		} else {
-			__entry->ino	= 0;
-			__entry->rootid	= 0;
-		}
+		__entry->owner		= tree->owner;
+		__entry->ino		= inode ? btrfs_ino(inode) : 0;
+		__entry->rootid		= inode ? inode->root->root_key.objectid : 0;
 		__entry->start		= start;
 		__entry->len		= len;
 		__entry->set_bits	= set_bits;
@@ -2137,17 +2132,12 @@ TRACE_EVENT(btrfs_clear_extent_bit,
 		__field(	unsigned,	clear_bits)
 	),
 
-	TP_fast_assign_btrfs(tree->fs_info,
-		__entry->owner = tree->owner;
-		if (tree->inode) {
-			const struct btrfs_inode *inode = tree->inode;
+	TP_fast_assign_btrfs(extent_io_tree_to_fs_info(tree),
+		const struct btrfs_inode *inode = extent_io_tree_to_inode_const(tree);
 
-			__entry->ino	= btrfs_ino(inode);
-			__entry->rootid	= inode->root->root_key.objectid;
-		} else {
-			__entry->ino	= 0;
-			__entry->rootid	= 0;
-		}
+		__entry->owner		= tree->owner;
+		__entry->ino		= inode ? btrfs_ino(inode) : 0;
+		__entry->rootid		= inode ? inode->root->root_key.objectid : 0;
 		__entry->start		= start;
 		__entry->len		= len;
 		__entry->clear_bits	= clear_bits;
@@ -2176,17 +2166,12 @@ TRACE_EVENT(btrfs_convert_extent_bit,
 		__field(	unsigned,	clear_bits)
 	),
 
-	TP_fast_assign_btrfs(tree->fs_info,
-		__entry->owner = tree->owner;
-		if (tree->inode) {
-			const struct btrfs_inode *inode = tree->inode;
+	TP_fast_assign_btrfs(extent_io_tree_to_fs_info(tree),
+		const struct btrfs_inode *inode = extent_io_tree_to_inode_const(tree);
 
-			__entry->ino	= btrfs_ino(inode);
-			__entry->rootid	= inode->root->root_key.objectid;
-		} else {
-			__entry->ino	= 0;
-			__entry->rootid	= 0;
-		}
+		__entry->owner		= tree->owner;
+		__entry->ino		= inode ? btrfs_ino(inode) : 0;
+		__entry->rootid		= inode ? inode->root->root_key.objectid : 0;
 		__entry->start		= start;
 		__entry->len		= len;
 		__entry->set_bits	= set_bits;
-- 
2.42.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/5] Reduce size of extent_io_tree, remove fs_info
  2023-11-21 13:20 [PATCH 0/5] Reduce size of extent_io_tree, remove fs_info David Sterba
                   ` (4 preceding siblings ...)
  2023-11-21 13:20 ` [PATCH 5/5] btrfs: always set extent_io_tree::inode and drop fs_info David Sterba
@ 2023-11-21 14:19 ` Josef Bacik
  5 siblings, 0 replies; 7+ messages in thread
From: Josef Bacik @ 2023-11-21 14:19 UTC (permalink / raw)
  To: David Sterba; +Cc: linux-btrfs

On Tue, Nov 21, 2023 at 02:20:12PM +0100, David Sterba wrote:
> We have the fs_info pointer in extent_io_tree for the trace points as
> the inode is not always set. This is a bit wasteful and extent_io_tree
> is also embedded in other structures. The tree owner can be used to
> determine if the inode is expected to be non-NULL, otherwise we can
> store the fs_info pointer.
> 
> I tried to do it in the cleanest way, union and access wrappers, it's
> IMO worth the space savings:
> 
> - btrfs_inode		1104 -> 1088
> - btrfs_device		 520 ->  512
> - btrfs_root		1360 -> 1344
> - btrfs_transaction	 456 ->  440
> - btrfs_fs_info		3600 -> 3592
> - reloc_control		1520 -> 1512
> 
> The btrfs_inode structure is getting closer to the 1024 size where it
> would pack better in the slab pages.
>

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef 

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-11-21 14:20 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-21 13:20 [PATCH 0/5] Reduce size of extent_io_tree, remove fs_info David Sterba
2023-11-21 13:20 ` [PATCH 1/5] btrfs: move lockdep class setting out of extent_io_tree_init David Sterba
2023-11-21 13:20 ` [PATCH 2/5] btrfs: drop error message in extent_io_tree insert_state() David Sterba
2023-11-21 13:20 ` [PATCH 3/5] btrfs: constify fs_info parameter in __btrfs_panic() David Sterba
2023-11-21 13:20 ` [PATCH 4/5] btrfs: enhance extent_io_tree error reports David Sterba
2023-11-21 13:20 ` [PATCH 5/5] btrfs: always set extent_io_tree::inode and drop fs_info David Sterba
2023-11-21 14:19 ` [PATCH 0/5] Reduce size of extent_io_tree, remove fs_info Josef Bacik

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox