[PATCH 0/4] Delayed ref root cleanups

[PATCH 0/4] Delayed ref root cleanups

[David Sterba]

Embed delayed root into btrfs_fs_info.

David Sterba (4):
  btrfs: embed delayed root to struct btrfs_fs_info
  btrfs: reorder members in btrfs_delayed_root for better packing
  btrfs: don't use local variables for fs_info->delayed_root
  btrfs: pass btrfs_fs_info to btrfs_first_delayed_node()

 fs/btrfs/delayed-inode.c | 49 +++++++++++++---------------------------
 fs/btrfs/delayed-inode.h | 15 ------------
 fs/btrfs/disk-io.c       |  8 ++-----
 fs/btrfs/fs.h            | 18 +++++++++++++--
 4 files changed, 34 insertions(+), 56 deletions(-)

-- 
2.51.1

[PATCH 1/4] btrfs: embed delayed root to struct btrfs_fs_info

[David Sterba]

The fs_info::delayed_root is allocated dynamically but there's only one
instance per filesystem so we can embed it into the fs_info itself. The
size grows from 3880 to 3952 on release config, i.e. still under 4K.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/delayed-inode.c | 24 ++++++++++++------------
 fs/btrfs/delayed-inode.h | 15 ---------------
 fs/btrfs/disk-io.c       |  8 ++------
 fs/btrfs/fs.h            | 18 ++++++++++++++++--
 4 files changed, 30 insertions(+), 35 deletions(-)

diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
index 2286bee2c6d3a8..a752646257dff3 100644
--- a/fs/btrfs/delayed-inode.c
+++ b/fs/btrfs/delayed-inode.c
@@ -257,7 +257,7 @@ static struct btrfs_delayed_node *btrfs_next_delayed_node(
 	struct list_head *p;
 	struct btrfs_delayed_node *next = NULL;
 
-	delayed_root = node->root->fs_info->delayed_root;
+	delayed_root = &node->root->fs_info->delayed_root;
 	spin_lock(&delayed_root->lock);
 	if (!test_bit(BTRFS_DELAYED_NODE_IN_LIST, &node->flags)) {
 		/* not in the list */
@@ -287,7 +287,7 @@ static void __btrfs_release_delayed_node(
 	if (!delayed_node)
 		return;
 
-	delayed_root = delayed_node->root->fs_info->delayed_root;
+	delayed_root = &delayed_node->root->fs_info->delayed_root;
 
 	mutex_lock(&delayed_node->mutex);
 	if (delayed_node->count)
@@ -425,7 +425,7 @@ static int __btrfs_add_delayed_item(struct btrfs_delayed_node *delayed_node,
 		delayed_node->index_cnt = ins->index + 1;
 
 	delayed_node->count++;
-	atomic_inc(&delayed_node->root->fs_info->delayed_root->items);
+	atomic_inc(&delayed_node->root->fs_info->delayed_root.items);
 	return 0;
 }
 
@@ -452,7 +452,7 @@ static void __btrfs_remove_delayed_item(struct btrfs_delayed_item *delayed_item)
 	/* If it's in a rbtree, then we need to have delayed node locked. */
 	lockdep_assert_held(&delayed_node->mutex);
 
-	delayed_root = delayed_node->root->fs_info->delayed_root;
+	delayed_root = &delayed_node->root->fs_info->delayed_root;
 
 	if (delayed_item->type == BTRFS_DELAYED_INSERTION_ITEM)
 		root = &delayed_node->ins_root;
@@ -988,7 +988,7 @@ static void btrfs_release_delayed_inode(struct btrfs_delayed_node *delayed_node)
 		clear_bit(BTRFS_DELAYED_NODE_INODE_DIRTY, &delayed_node->flags);
 		delayed_node->count--;
 
-		delayed_root = delayed_node->root->fs_info->delayed_root;
+		delayed_root = &delayed_node->root->fs_info->delayed_root;
 		finish_one_item(delayed_root);
 	}
 }
@@ -1002,7 +1002,7 @@ static void btrfs_release_delayed_iref(struct btrfs_delayed_node *delayed_node)
 		ASSERT(delayed_node->root);
 		delayed_node->count--;
 
-		delayed_root = delayed_node->root->fs_info->delayed_root;
+		delayed_root = &delayed_node->root->fs_info->delayed_root;
 		finish_one_item(delayed_root);
 	}
 }
@@ -1168,7 +1168,7 @@ static int __btrfs_run_delayed_items(struct btrfs_trans_handle *trans, int nr)
 	block_rsv = trans->block_rsv;
 	trans->block_rsv = &fs_info->delayed_block_rsv;
 
-	delayed_root = fs_info->delayed_root;
+	delayed_root = &fs_info->delayed_root;
 
 	curr_node = btrfs_first_delayed_node(delayed_root, &curr_delayed_node_tracker);
 	while (curr_node && (!count || nr--)) {
@@ -1417,7 +1417,7 @@ void btrfs_assert_delayed_root_empty(struct btrfs_fs_info *fs_info)
 	struct btrfs_ref_tracker delayed_node_tracker;
 	struct btrfs_delayed_node *node;
 
-	node = btrfs_first_delayed_node( fs_info->delayed_root, &delayed_node_tracker);
+	node = btrfs_first_delayed_node(&fs_info->delayed_root, &delayed_node_tracker);
 	if (WARN_ON(node)) {
 		btrfs_delayed_node_ref_tracker_free(node,
 						    &delayed_node_tracker);
@@ -1440,7 +1440,7 @@ static bool could_end_wait(struct btrfs_delayed_root *delayed_root, int seq)
 
 void btrfs_balance_delayed_items(struct btrfs_fs_info *fs_info)
 {
-	struct btrfs_delayed_root *delayed_root = fs_info->delayed_root;
+	struct btrfs_delayed_root *delayed_root = &fs_info->delayed_root;
 
 	if ((atomic_read(&delayed_root->items) < BTRFS_DELAYED_BACKGROUND) ||
 		btrfs_workqueue_normal_congested(fs_info->delayed_workers))
@@ -1970,7 +1970,7 @@ int btrfs_delayed_update_inode(struct btrfs_trans_handle *trans,
 	fill_stack_inode_item(trans, &delayed_node->inode_item, inode);
 	set_bit(BTRFS_DELAYED_NODE_INODE_DIRTY, &delayed_node->flags);
 	delayed_node->count++;
-	atomic_inc(&root->fs_info->delayed_root->items);
+	atomic_inc(&root->fs_info->delayed_root.items);
 release_node:
 	mutex_unlock(&delayed_node->mutex);
 	btrfs_release_delayed_node(delayed_node, &delayed_node_tracker);
@@ -2012,7 +2012,7 @@ int btrfs_delayed_delete_inode_ref(struct btrfs_inode *inode)
 	mutex_lock(&delayed_node->mutex);
 	if (!test_and_set_bit(BTRFS_DELAYED_NODE_DEL_IREF, &delayed_node->flags)) {
 		delayed_node->count++;
-		atomic_inc(&fs_info->delayed_root->items);
+		atomic_inc(&fs_info->delayed_root.items);
 	}
 	mutex_unlock(&delayed_node->mutex);
 	btrfs_release_delayed_node(delayed_node, &delayed_node_tracker);
@@ -2118,7 +2118,7 @@ void btrfs_destroy_delayed_inodes(struct btrfs_fs_info *fs_info)
 	struct btrfs_delayed_node *curr_node, *prev_node;
 	struct btrfs_ref_tracker curr_delayed_node_tracker, prev_delayed_node_tracker;
 
-	curr_node = btrfs_first_delayed_node(fs_info->delayed_root,
+	curr_node = btrfs_first_delayed_node(&fs_info->delayed_root,
 					     &curr_delayed_node_tracker);
 	while (curr_node) {
 		__btrfs_kill_delayed_node(curr_node);
diff --git a/fs/btrfs/delayed-inode.h b/fs/btrfs/delayed-inode.h
index b09d4ec8c77dde..fc752863f89bcd 100644
--- a/fs/btrfs/delayed-inode.h
+++ b/fs/btrfs/delayed-inode.h
@@ -30,21 +30,6 @@ enum btrfs_delayed_item_type {
 	BTRFS_DELAYED_DELETION_ITEM
 };
 
-struct btrfs_delayed_root {
-	spinlock_t lock;
-	struct list_head node_list;
-	/*
-	 * Used for delayed nodes which is waiting to be dealt with by the
-	 * worker. If the delayed node is inserted into the work queue, we
-	 * drop it from this list.
-	 */
-	struct list_head prepare_list;
-	atomic_t items;		/* for delayed items */
-	atomic_t items_seq;	/* for delayed items */
-	int nodes;		/* for delayed nodes */
-	wait_queue_head_t wait;
-};
-
 struct btrfs_ref_tracker_dir {
 #ifdef CONFIG_BTRFS_DEBUG
 	struct ref_tracker_dir dir;
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index cecb81d0f9e0c6..52ebb4eefe6362 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -22,6 +22,7 @@
 #include "disk-io.h"
 #include "transaction.h"
 #include "btrfs_inode.h"
+#include "delayed-inode.h"
 #include "bio.h"
 #include "print-tree.h"
 #include "locking.h"
@@ -1237,7 +1238,6 @@ void btrfs_free_fs_info(struct btrfs_fs_info *fs_info)
 	btrfs_free_stripe_hash_table(fs_info);
 	btrfs_free_ref_cache(fs_info);
 	kfree(fs_info->balance_ctl);
-	kfree(fs_info->delayed_root);
 	free_global_roots(fs_info);
 	btrfs_put_root(fs_info->tree_root);
 	btrfs_put_root(fs_info->chunk_root);
@@ -2889,11 +2889,7 @@ static int init_mount_fs_info(struct btrfs_fs_info *fs_info, struct super_block
 	if (ret)
 		return ret;
 
-	fs_info->delayed_root = kmalloc(sizeof(struct btrfs_delayed_root),
-					GFP_KERNEL);
-	if (!fs_info->delayed_root)
-		return -ENOMEM;
-	btrfs_init_delayed_root(fs_info->delayed_root);
+	btrfs_init_delayed_root(&fs_info->delayed_root);
 
 	if (sb_rdonly(sb))
 		set_bit(BTRFS_FS_STATE_RO, &fs_info->fs_state);
diff --git a/fs/btrfs/fs.h b/fs/btrfs/fs.h
index 0dc851b9c51bc2..dd4944f9a98553 100644
--- a/fs/btrfs/fs.h
+++ b/fs/btrfs/fs.h
@@ -44,7 +44,6 @@ struct btrfs_block_group;
 struct btrfs_root;
 struct btrfs_fs_devices;
 struct btrfs_transaction;
-struct btrfs_delayed_root;
 struct btrfs_balance_control;
 struct btrfs_subpage_info;
 struct btrfs_stripe_hash_table;
@@ -455,6 +454,21 @@ struct btrfs_commit_stats {
 	u64 critical_section_start_time;
 };
 
+struct btrfs_delayed_root {
+	spinlock_t lock;
+	struct list_head node_list;
+	/*
+	 * Used for delayed nodes which is waiting to be dealt with by the
+	 * worker. If the delayed node is inserted into the work queue, we
+	 * drop it from this list.
+	 */
+	struct list_head prepare_list;
+	atomic_t items;		/* for delayed items */
+	atomic_t items_seq;	/* for delayed items */
+	int nodes;		/* for delayed nodes */
+	wait_queue_head_t wait;
+};
+
 struct btrfs_fs_info {
 	u8 chunk_tree_uuid[BTRFS_UUID_SIZE];
 	unsigned long flags;
@@ -804,7 +818,7 @@ struct btrfs_fs_info {
 	/* Filesystem state */
 	unsigned long fs_state;
 
-	struct btrfs_delayed_root *delayed_root;
+	struct btrfs_delayed_root delayed_root;
 
 	/* Entries are eb->start >> nodesize_bits */
 	struct xarray buffer_tree;
-- 
2.51.1

[PATCH 2/4] btrfs: reorder members in btrfs_delayed_root for better packing

[David Sterba]

There are two unnecessary 4B holes in btrfs_delayed_root;

struct btrfs_delayed_root {
        spinlock_t                 lock;                 /*     0     4 */

        /* XXX 4 bytes hole, try to pack */

        struct list_head           node_list;            /*     8    16 */
        struct list_head           prepare_list;         /*    24    16 */
        atomic_t                   items;                /*    40     4 */
        atomic_t                   items_seq;            /*    44     4 */
        int                        nodes;                /*    48     4 */

        /* XXX 4 bytes hole, try to pack */

        wait_queue_head_t          wait;                 /*    56    24 */

        /* size: 80, cachelines: 2, members: 7 */
        /* sum members: 72, holes: 2, sum holes: 8 */
        /* last cacheline: 16 bytes */
};

Reordering 'nodes' after 'lock' reduces size by 8B, to 72 on release
config.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/fs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/fs.h b/fs/btrfs/fs.h
index dd4944f9a98553..4d721fbd390c55 100644
--- a/fs/btrfs/fs.h
+++ b/fs/btrfs/fs.h
@@ -456,6 +456,7 @@ struct btrfs_commit_stats {
 
 struct btrfs_delayed_root {
 	spinlock_t lock;
+	int nodes;		/* for delayed nodes */
 	struct list_head node_list;
 	/*
 	 * Used for delayed nodes which is waiting to be dealt with by the
@@ -465,7 +466,6 @@ struct btrfs_delayed_root {
 	struct list_head prepare_list;
 	atomic_t items;		/* for delayed items */
 	atomic_t items_seq;	/* for delayed items */
-	int nodes;		/* for delayed nodes */
 	wait_queue_head_t wait;
 };
 
-- 
2.51.1

[PATCH 3/4] btrfs: don't use local variables for fs_info->delayed_root

[David Sterba]

In all cases the delayed_root is used once in a function, we don't need
to use a local variable for that.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/delayed-inode.c | 24 ++++--------------------
 1 file changed, 4 insertions(+), 20 deletions(-)

diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
index a752646257dff3..fc5926ecc762ff 100644
--- a/fs/btrfs/delayed-inode.c
+++ b/fs/btrfs/delayed-inode.c
@@ -443,7 +443,6 @@ static void __btrfs_remove_delayed_item(struct btrfs_delayed_item *delayed_item)
 {
 	struct btrfs_delayed_node *delayed_node = delayed_item->delayed_node;
 	struct rb_root_cached *root;
-	struct btrfs_delayed_root *delayed_root;
 
 	/* Not inserted, ignore it. */
 	if (RB_EMPTY_NODE(&delayed_item->rb_node))
@@ -452,8 +451,6 @@ static void __btrfs_remove_delayed_item(struct btrfs_delayed_item *delayed_item)
 	/* If it's in a rbtree, then we need to have delayed node locked. */
 	lockdep_assert_held(&delayed_node->mutex);
 
-	delayed_root = &delayed_node->root->fs_info->delayed_root;
-
 	if (delayed_item->type == BTRFS_DELAYED_INSERTION_ITEM)
 		root = &delayed_node->ins_root;
 	else
@@ -462,8 +459,7 @@ static void __btrfs_remove_delayed_item(struct btrfs_delayed_item *delayed_item)
 	rb_erase_cached(&delayed_item->rb_node, root);
 	RB_CLEAR_NODE(&delayed_item->rb_node);
 	delayed_node->count--;
-
-	finish_one_item(delayed_root);
+	finish_one_item(&delayed_node->root->fs_info->delayed_root);
 }
 
 static void btrfs_release_delayed_item(struct btrfs_delayed_item *item)
@@ -980,30 +976,21 @@ static int btrfs_delete_delayed_items(struct btrfs_trans_handle *trans,
 
 static void btrfs_release_delayed_inode(struct btrfs_delayed_node *delayed_node)
 {
-	struct btrfs_delayed_root *delayed_root;
-
 	if (delayed_node &&
 	    test_bit(BTRFS_DELAYED_NODE_INODE_DIRTY, &delayed_node->flags)) {
 		ASSERT(delayed_node->root);
 		clear_bit(BTRFS_DELAYED_NODE_INODE_DIRTY, &delayed_node->flags);
 		delayed_node->count--;
-
-		delayed_root = &delayed_node->root->fs_info->delayed_root;
-		finish_one_item(delayed_root);
+		finish_one_item(&delayed_node->root->fs_info->delayed_root);
 	}
 }
 
 static void btrfs_release_delayed_iref(struct btrfs_delayed_node *delayed_node)
 {
-
 	if (test_and_clear_bit(BTRFS_DELAYED_NODE_DEL_IREF, &delayed_node->flags)) {
-		struct btrfs_delayed_root *delayed_root;
-
 		ASSERT(delayed_node->root);
 		delayed_node->count--;
-
-		delayed_root = &delayed_node->root->fs_info->delayed_root;
-		finish_one_item(delayed_root);
+		finish_one_item(&delayed_node->root->fs_info->delayed_root);
 	}
 }
 
@@ -1150,7 +1137,6 @@ __btrfs_commit_inode_delayed_items(struct btrfs_trans_handle *trans,
 static int __btrfs_run_delayed_items(struct btrfs_trans_handle *trans, int nr)
 {
 	struct btrfs_fs_info *fs_info = trans->fs_info;
-	struct btrfs_delayed_root *delayed_root;
 	struct btrfs_delayed_node *curr_node, *prev_node;
 	struct btrfs_ref_tracker curr_delayed_node_tracker, prev_delayed_node_tracker;
 	struct btrfs_path *path;
@@ -1168,9 +1154,7 @@ static int __btrfs_run_delayed_items(struct btrfs_trans_handle *trans, int nr)
 	block_rsv = trans->block_rsv;
 	trans->block_rsv = &fs_info->delayed_block_rsv;
 
-	delayed_root = &fs_info->delayed_root;
-
-	curr_node = btrfs_first_delayed_node(delayed_root, &curr_delayed_node_tracker);
+	curr_node = btrfs_first_delayed_node(&fs_info->delayed_root, &curr_delayed_node_tracker);
 	while (curr_node && (!count || nr--)) {
 		ret = __btrfs_commit_inode_delayed_items(trans, path,
 							 curr_node);
-- 
2.51.1

[PATCH 4/4] btrfs: pass btrfs_fs_info to btrfs_first_delayed_node()

[David Sterba]

As the delayed root is now in the fs_info we can pass it to
btrfs_first_delayed_node().

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/delayed-inode.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
index fc5926ecc762ff..1739a0b29c49d7 100644
--- a/fs/btrfs/delayed-inode.c
+++ b/fs/btrfs/delayed-inode.c
@@ -232,19 +232,19 @@ static void btrfs_dequeue_delayed_node(struct btrfs_delayed_root *root,
 }
 
 static struct btrfs_delayed_node *btrfs_first_delayed_node(
-			struct btrfs_delayed_root *delayed_root,
+			struct btrfs_fs_info *fs_info,
 			struct btrfs_ref_tracker *tracker)
 {
 	struct btrfs_delayed_node *node;
 
-	spin_lock(&delayed_root->lock);
-	node = list_first_entry_or_null(&delayed_root->node_list,
+	spin_lock(&fs_info->delayed_root.lock);
+	node = list_first_entry_or_null(&fs_info->delayed_root.node_list,
 					struct btrfs_delayed_node, n_list);
 	if (node) {
 		refcount_inc(&node->refs);
 		btrfs_delayed_node_ref_tracker_alloc(node, tracker, GFP_ATOMIC);
 	}
-	spin_unlock(&delayed_root->lock);
+	spin_unlock(&fs_info->delayed_root.lock);
 
 	return node;
 }
@@ -1154,7 +1154,7 @@ static int __btrfs_run_delayed_items(struct btrfs_trans_handle *trans, int nr)
 	block_rsv = trans->block_rsv;
 	trans->block_rsv = &fs_info->delayed_block_rsv;
 
-	curr_node = btrfs_first_delayed_node(&fs_info->delayed_root, &curr_delayed_node_tracker);
+	curr_node = btrfs_first_delayed_node(fs_info, &curr_delayed_node_tracker);
 	while (curr_node && (!count || nr--)) {
 		ret = __btrfs_commit_inode_delayed_items(trans, path,
 							 curr_node);
@@ -1401,7 +1401,7 @@ void btrfs_assert_delayed_root_empty(struct btrfs_fs_info *fs_info)
 	struct btrfs_ref_tracker delayed_node_tracker;
 	struct btrfs_delayed_node *node;
 
-	node = btrfs_first_delayed_node(&fs_info->delayed_root, &delayed_node_tracker);
+	node = btrfs_first_delayed_node(fs_info, &delayed_node_tracker);
 	if (WARN_ON(node)) {
 		btrfs_delayed_node_ref_tracker_free(node,
 						    &delayed_node_tracker);
@@ -2102,8 +2102,7 @@ void btrfs_destroy_delayed_inodes(struct btrfs_fs_info *fs_info)
 	struct btrfs_delayed_node *curr_node, *prev_node;
 	struct btrfs_ref_tracker curr_delayed_node_tracker, prev_delayed_node_tracker;
 
-	curr_node = btrfs_first_delayed_node(&fs_info->delayed_root,
-					     &curr_delayed_node_tracker);
+	curr_node = btrfs_first_delayed_node(fs_info, &curr_delayed_node_tracker);
 	while (curr_node) {
 		__btrfs_kill_delayed_node(curr_node);
 
-- 
2.51.1

Re: [PATCH 0/4] Delayed ref root cleanups

[Boris Burkov]

On Fri, Jan 09, 2026 at 06:17:39PM +0100, David Sterba wrote:
> Embed delayed root into btrfs_fs_info.

The patches all look fine to me, but I think it would be nice to give
some justification for why it is desirable to make this change besides
"it's possible". If anything, it is a regression on the size of struct
btrfs_fs_info as you mention in the first patch.

If the answer is just that it's simpler and there is no need for a
separate allocation, then fair enough. But then why not directly embed
all the one-off structures pointed to by fs_info? Like all the global
roots, for example. Are they too large? What constitutes too large?
Later, when we slowly add stuff to fs_info till it is bigger than 4k,
should we undo this patch set? Or look for other, bigger structs to
unembed first?

Thanks,
Boris

> 
> David Sterba (4):
>   btrfs: embed delayed root to struct btrfs_fs_info
>   btrfs: reorder members in btrfs_delayed_root for better packing
>   btrfs: don't use local variables for fs_info->delayed_root
>   btrfs: pass btrfs_fs_info to btrfs_first_delayed_node()
> 
>  fs/btrfs/delayed-inode.c | 49 +++++++++++++---------------------------
>  fs/btrfs/delayed-inode.h | 15 ------------
>  fs/btrfs/disk-io.c       |  8 ++-----
>  fs/btrfs/fs.h            | 18 +++++++++++++--
>  4 files changed, 34 insertions(+), 56 deletions(-)
> 
> -- 
> 2.51.1
> 

Re: [PATCH 0/4] Delayed ref root cleanups

[David Sterba]

On Fri, Jan 09, 2026 at 10:16:27AM -0800, Boris Burkov wrote:
> On Fri, Jan 09, 2026 at 06:17:39PM +0100, David Sterba wrote:
> > Embed delayed root into btrfs_fs_info.
> 
> The patches all look fine to me, but I think it would be nice to give
> some justification for why it is desirable to make this change besides
> "it's possible". If anything, it is a regression on the size of struct
> btrfs_fs_info as you mention in the first patch.

A regression? That's an unusal way how to look at it and I did not cross
my mind. The motivation is that the two objects have same lifetime and
whe have spare bytes in the slab.

> If the answer is just that it's simpler and there is no need for a
> separate allocation, then fair enough. But then why not directly embed
> all the one-off structures pointed to by fs_info? Like all the global
> roots, for example. Are they too large? What constitutes too large?
> Later, when we slowly add stuff to fs_info till it is bigger than 4k,
> should we undo this patch set? Or look for other, bigger structs to
> unembed first?

Fair questions. If we embed everything the fs_info would be say 16K. The
threshold I'm considering is 4K, which is 4K page on the most common
architecture x86_64. ARM can be configured to have 4K or 64K on the most
common setups, so I'm not making it worse by the 4K choice.

So, if the structure for embedding is small enough not to cross 4K and
still leave some space then I consider it worth doing. In the case of
increasing the fs_info by required and small new members (spinlocks,
atomics, various stats etc) we can first look how to shring the size by
reordering it. Currently I see there are 97 bytes in holes. Then we can
look what is used optionally, eg. depends on a mount option and move it
to a separate structure.

The delayed root is a core data structure so we will not have to
separate it again and revert this patchset. What I'd start looking for
for a separate data structure would be some kind of static
almost-read-only information, like mount option bits or status flags
etc.

Also I don't want people to worry about fs_info size when there's
something new to implement. We have some space to use and I will notice
if we cross the boundary as I do random checks of the patch effects
every now and then. This applies to parameters and stack space
consumption. You may say this is pointless like in the other patchset
but even on machines with terabytes of memory a kernel thread is still
limited to 16K of stack and layering subsystems can use substantial
portions of it. My long term goal is to keep the level the same without
hindering development.

Re: [PATCH 2/4] btrfs: reorder members in btrfs_delayed_root for better packing

[Qu Wenruo]

在 2026/1/10 03:47, David Sterba 写道:
> There are two unnecessary 4B holes in btrfs_delayed_root;
> 
> struct btrfs_delayed_root {
>          spinlock_t                 lock;                 /*     0     4 */
> 
>          /* XXX 4 bytes hole, try to pack */
> 
>          struct list_head           node_list;            /*     8    16 */
>          struct list_head           prepare_list;         /*    24    16 */
>          atomic_t                   items;                /*    40     4 */
>          atomic_t                   items_seq;            /*    44     4 */
>          int                        nodes;                /*    48     4 */
> 
>          /* XXX 4 bytes hole, try to pack */
> 
>          wait_queue_head_t          wait;                 /*    56    24 */
> 
>          /* size: 80, cachelines: 2, members: 7 */
>          /* sum members: 72, holes: 2, sum holes: 8 */
>          /* last cacheline: 16 bytes */
> };
> 
> Reordering 'nodes' after 'lock' reduces size by 8B, to 72 on release
> config.

Not a huge thing, but can we put members in descend order of their sizes 
if they are properly aligned.

For this particular case, wait_queue_heat_t itself isn't properly 
power-of-2 sized, but with 2 32bits member, it can be shrink even futher:

struct btrfs_delayed_root {
         wait_queue_head_t          wait;                 /*     0    24 */
         int                        nodes;                /*    24     4 */
         spinlock_t                 lock;                 /*    28     4 */
         struct list_head           node_list;            /*    32    16 */
         struct list_head           prepare_list;         /*    48    16 */
         /* --- cacheline 1 boundary (64 bytes) --- */
         atomic_t                   items;                /*    64     4 */
         atomic_t                   items_seq;            /*    68     4 */

         /* size: 72, cachelines: 2, members: 7 */
         /* last cacheline: 8 bytes */
};

Thanks,
Qu

> 
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>   fs/btrfs/fs.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/fs.h b/fs/btrfs/fs.h
> index dd4944f9a98553..4d721fbd390c55 100644
> --- a/fs/btrfs/fs.h
> +++ b/fs/btrfs/fs.h
> @@ -456,6 +456,7 @@ struct btrfs_commit_stats {
>   
>   struct btrfs_delayed_root {
>   	spinlock_t lock;
> +	int nodes;		/* for delayed nodes */
>   	struct list_head node_list;
>   	/*
>   	 * Used for delayed nodes which is waiting to be dealt with by the
> @@ -465,7 +466,6 @@ struct btrfs_delayed_root {
>   	struct list_head prepare_list;
>   	atomic_t items;		/* for delayed items */
>   	atomic_t items_seq;	/* for delayed items */
> -	int nodes;		/* for delayed nodes */
>   	wait_queue_head_t wait;
>   };
>   

Re: [PATCH 0/4] Delayed ref root cleanups

[Boris Burkov]

On Fri, Jan 09, 2026 at 10:09:21PM +0100, David Sterba wrote:
> On Fri, Jan 09, 2026 at 10:16:27AM -0800, Boris Burkov wrote:
> > On Fri, Jan 09, 2026 at 06:17:39PM +0100, David Sterba wrote:
> > > Embed delayed root into btrfs_fs_info.
> > 
> > The patches all look fine to me, but I think it would be nice to give
> > some justification for why it is desirable to make this change besides
> > "it's possible". If anything, it is a regression on the size of struct
> > btrfs_fs_info as you mention in the first patch.
> 
> A regression? That's an unusal way how to look at it and I did not cross
> my mind. The motivation is that the two objects have same lifetime and
> whe have spare bytes in the slab.
> 

Sorry for the slightly hyperbolic language. My point was merely that it
is the main observable outcome. I agree with you that it's not an actual
meaningful regression in any way that we care about today.

All I'm saying is we consider it a minor win (all else being equal) to
shrink structs, so it's only fair to consider growing them a minor
regression.

It can be offset by other benefits and be an attractive choice anyway.

> > If the answer is just that it's simpler and there is no need for a
> > separate allocation, then fair enough. But then why not directly embed
> > all the one-off structures pointed to by fs_info? Like all the global
> > roots, for example. Are they too large? What constitutes too large?
> > Later, when we slowly add stuff to fs_info till it is bigger than 4k,
> > should we undo this patch set? Or look for other, bigger structs to
> > unembed first?
> 
> Fair questions. If we embed everything the fs_info would be say 16K. The
> threshold I'm considering is 4K, which is 4K page on the most common
> architecture x86_64. ARM can be configured to have 4K or 64K on the most
> common setups, so I'm not making it worse by the 4K choice.

Agreed.

> 
> So, if the structure for embedding is small enough not to cross 4K and
> still leave some space then I consider it worth doing. In the case of

All I'm really asking is for some durable explanation why it is that you
consider it worth doing. Your email and patch messages just explained
why it was possible.

Even something like "if a non optional object is smaller than ~X, we
prefer not to use up slab" would be a helpful explanation.

Understanding how you think about these things is important to other
developers so we know what principles to try to follow in our own work.

Otherwise, we are just guessing at your taste preferences, and we can
monkey around and you can clean up after us :). I assume that is not the
most desirable.

> increasing the fs_info by required and small new members (spinlocks,
> atomics, various stats etc) we can first look how to shring the size by
> reordering it. Currently I see there are 97 bytes in holes. Then we can
> look what is used optionally, eg. depends on a mount option and move it
> to a separate structure.
> 
> The delayed root is a core data structure so we will not have to
> separate it again and revert this patchset. What I'd start looking for
> for a separate data structure would be some kind of static
> almost-read-only information, like mount option bits or status flags
> etc.

I mean, it's not impossible to imagine a future where the lower hanging
fruit are exhausted and we are butting up on 4k.

> 
> Also I don't want people to worry about fs_info size when there's
> something new to implement. We have some space to use and I will notice
> if we cross the boundary as I do random checks of the patch effects
> every now and then. This applies to parameters and stack space
> consumption. You may say this is pointless like in the other patchset
> but even on machines with terabytes of memory a kernel thread is still
> limited to 16K of stack and layering subsystems can use substantial
> portions of it. My long term goal is to keep the level the same without
> hindering development.

I won't say pointless, since you're right, there is a point to reducing
stack size. Perhaps the better expression of how I feel about that is
"arbitrary". But individual improvements are always useful.

Thanks,
Boris

Re: [PATCH 0/4] Delayed ref root cleanups

[Boris Burkov]

On Fri, Jan 09, 2026 at 10:09:21PM +0100, David Sterba wrote:
> On Fri, Jan 09, 2026 at 10:16:27AM -0800, Boris Burkov wrote:
> > On Fri, Jan 09, 2026 at 06:17:39PM +0100, David Sterba wrote:
> > > Embed delayed root into btrfs_fs_info.
> > 
> > The patches all look fine to me, but I think it would be nice to give
> > some justification for why it is desirable to make this change besides
> > "it's possible". If anything, it is a regression on the size of struct
> > btrfs_fs_info as you mention in the first patch.
> 
> A regression? That's an unusal way how to look at it and I did not cross
> my mind. The motivation is that the two objects have same lifetime and
> whe have spare bytes in the slab.
> 
> > If the answer is just that it's simpler and there is no need for a
> > separate allocation, then fair enough. But then why not directly embed
> > all the one-off structures pointed to by fs_info? Like all the global
> > roots, for example. Are they too large? What constitutes too large?
> > Later, when we slowly add stuff to fs_info till it is bigger than 4k,
> > should we undo this patch set? Or look for other, bigger structs to
> > unembed first?
> 
> Fair questions. If we embed everything the fs_info would be say 16K. The
> threshold I'm considering is 4K, which is 4K page on the most common
> architecture x86_64. ARM can be configured to have 4K or 64K on the most
> common setups, so I'm not making it worse by the 4K choice.
> 
> So, if the structure for embedding is small enough not to cross 4K and
> still leave some space then I consider it worth doing. In the case of
> increasing the fs_info by required and small new members (spinlocks,
> atomics, various stats etc) we can first look how to shring the size by
> reordering it. Currently I see there are 97 bytes in holes. Then we can
> look what is used optionally, eg. depends on a mount option and move it
> to a separate structure.
> 
> The delayed root is a core data structure so we will not have to
> separate it again and revert this patchset. What I'd start looking for
> for a separate data structure would be some kind of static
> almost-read-only information, like mount option bits or status flags
> etc.
> 
> Also I don't want people to worry about fs_info size when there's
> something new to implement. We have some space to use and I will notice
> if we cross the boundary as I do random checks of the patch effects
> every now and then. This applies to parameters and stack space
> consumption. You may say this is pointless like in the other patchset
> but even on machines with terabytes of memory a kernel thread is still
> limited to 16K of stack and layering subsystems can use substantial
> portions of it. My long term goal is to keep the level the same without
> hindering development.

Also, all quibbling aside, I don't want to hold you up on trivialities.

If you can think of a short, specific explanation for why this is
preferable, I would appreciate you adding it. But even if not, please
add:

Reviewed-by: Boris Burkov <boris@bur.io>

Re: [PATCH 2/4] btrfs: reorder members in btrfs_delayed_root for better packing

[David Sterba]

On Sat, Jan 10, 2026 at 07:46:11AM +1030, Qu Wenruo wrote:
> 
> 
> 在 2026/1/10 03:47, David Sterba 写道:
> > There are two unnecessary 4B holes in btrfs_delayed_root;
> > 
> > struct btrfs_delayed_root {
> >          spinlock_t                 lock;                 /*     0     4 */
> > 
> >          /* XXX 4 bytes hole, try to pack */
> > 
> >          struct list_head           node_list;            /*     8    16 */
> >          struct list_head           prepare_list;         /*    24    16 */
> >          atomic_t                   items;                /*    40     4 */
> >          atomic_t                   items_seq;            /*    44     4 */
> >          int                        nodes;                /*    48     4 */
> > 
> >          /* XXX 4 bytes hole, try to pack */
> > 
> >          wait_queue_head_t          wait;                 /*    56    24 */
> > 
> >          /* size: 80, cachelines: 2, members: 7 */
> >          /* sum members: 72, holes: 2, sum holes: 8 */
> >          /* last cacheline: 16 bytes */
> > };
> > 
> > Reordering 'nodes' after 'lock' reduces size by 8B, to 72 on release
> > config.
> 
> Not a huge thing, but can we put members in descend order of their sizes 
> if they are properly aligned.

We can do that but I'm not sure about the effects. Once there are no
holes left the automatic alignment is correct with respect to the types.
The structure is compact and the next thing to consider is cacheline
grouping, so we can decouple locks, atomics or implied locks in other
structures like the wait_queue_head or semaphores.

You suggest to reorder wait from 2nd cacheline to the first, so now the
atomics are in the 2nd. This groups wait + lock, and the items +
items_seq. I can't tell just form that which one is better, this depends
on access patterns and code would need to be analyzed. In many cases
this is not important because the access is not so parallel, so eg. one
access per function is hardly measurable. And we don't care in general,
so in this case I don't think either way is better or worse.

> For this particular case, wait_queue_heat_t itself isn't properly 
> power-of-2 sized, but with 2 32bits member, it can be shrink even futher:

The final size after my patch is the same, 72 bytes, what I posted was
the structure before reordering.

> struct btrfs_delayed_root {
>          wait_queue_head_t          wait;                 /*     0    24 */
>          int                        nodes;                /*    24     4 */
>          spinlock_t                 lock;                 /*    28     4 */
>          struct list_head           node_list;            /*    32    16 */
>          struct list_head           prepare_list;         /*    48    16 */
>          /* --- cacheline 1 boundary (64 bytes) --- */
>          atomic_t                   items;                /*    64     4 */
>          atomic_t                   items_seq;            /*    68     4 */
> 
>          /* size: 72, cachelines: 2, members: 7 */
>          /* last cacheline: 8 bytes */
> };

Re: [PATCH 0/4] Delayed ref root cleanups

[David Sterba]

On Fri, Jan 09, 2026 at 01:39:53PM -0800, Boris Burkov wrote:
> On Fri, Jan 09, 2026 at 10:09:21PM +0100, David Sterba wrote:
> > On Fri, Jan 09, 2026 at 10:16:27AM -0800, Boris Burkov wrote:
> > > On Fri, Jan 09, 2026 at 06:17:39PM +0100, David Sterba wrote:
> > > > Embed delayed root into btrfs_fs_info.
> > > 
> > > The patches all look fine to me, but I think it would be nice to give
> > > some justification for why it is desirable to make this change besides
> > > "it's possible". If anything, it is a regression on the size of struct
> > > btrfs_fs_info as you mention in the first patch.
> > 
> > A regression? That's an unusal way how to look at it and I did not cross
> > my mind. The motivation is that the two objects have same lifetime and
> > whe have spare bytes in the slab.
> > 
> 
> Sorry for the slightly hyperbolic language. My point was merely that it
> is the main observable outcome. I agree with you that it's not an actual
> meaningful regression in any way that we care about today.
> 
> All I'm saying is we consider it a minor win (all else being equal) to
> shrink structs, so it's only fair to consider growing them a minor
> regression.
> 
> It can be offset by other benefits and be an attractive choice anyway.
> 
> > > If the answer is just that it's simpler and there is no need for a
> > > separate allocation, then fair enough. But then why not directly embed
> > > all the one-off structures pointed to by fs_info? Like all the global
> > > roots, for example. Are they too large? What constitutes too large?
> > > Later, when we slowly add stuff to fs_info till it is bigger than 4k,
> > > should we undo this patch set? Or look for other, bigger structs to
> > > unembed first?
> > 
> > Fair questions. If we embed everything the fs_info would be say 16K. The
> > threshold I'm considering is 4K, which is 4K page on the most common
> > architecture x86_64. ARM can be configured to have 4K or 64K on the most
> > common setups, so I'm not making it worse by the 4K choice.
> 
> Agreed.
> 
> > So, if the structure for embedding is small enough not to cross 4K and
> > still leave some space then I consider it worth doing. In the case of
> 
> All I'm really asking is for some durable explanation why it is that you
> consider it worth doing. Your email and patch messages just explained
> why it was possible.
> 
> Even something like "if a non optional object is smaller than ~X, we
> prefer not to use up slab" would be a helpful explanation.
> 
> Understanding how you think about these things is important to other
> developers so we know what principles to try to follow in our own work.
> 
> Otherwise, we are just guessing at your taste preferences, and we can
> monkey around and you can clean up after us :). I assume that is not the
> most desirable.

You're right, it's good to have all the why's answered, sometimes I'm
not sure if I'm explaining trivialities but this is skewed by the time
one stares at the code until it's all obvious. The changelogs also serve
as documentation and gather information that we may use in the future
for similar patterns.

> > increasing the fs_info by required and small new members (spinlocks,
> > atomics, various stats etc) we can first look how to shring the size by
> > reordering it. Currently I see there are 97 bytes in holes. Then we can
> > look what is used optionally, eg. depends on a mount option and move it
> > to a separate structure.
> > 
> > The delayed root is a core data structure so we will not have to
> > separate it again and revert this patchset. What I'd start looking for
> > for a separate data structure would be some kind of static
> > almost-read-only information, like mount option bits or status flags
> > etc.
> 
> I mean, it's not impossible to imagine a future where the lower hanging
> fruit are exhausted and we are butting up on 4k.

Yeah, fs_info gets updated often, comparing to other structures, I'm
sure we'll hit the page limit.

> > Also I don't want people to worry about fs_info size when there's
> > something new to implement. We have some space to use and I will notice
> > if we cross the boundary as I do random checks of the patch effects
> > every now and then. This applies to parameters and stack space
> > consumption. You may say this is pointless like in the other patchset
> > but even on machines with terabytes of memory a kernel thread is still
> > limited to 16K of stack and layering subsystems can use substantial
> > portions of it. My long term goal is to keep the level the same without
> > hindering development.
> 
> I won't say pointless, since you're right, there is a point to reducing
> stack size. Perhaps the better expression of how I feel about that is
> "arbitrary". But individual improvements are always useful.

The general answer to that is that kernel memory is a scarce resource.
We neither waste it because big machines exist or optimize too
aggressively because small machines exist. Fixing low hanging fruit
stuff like reducing size is easy, everything beyond obvious needs some
numbers and analysis and a comment in code why the general principles
may be broken (like leaving a gap in a structure due to forced cacheline
alignment). Anyway, thanks for the discussion, I'll update the
changelogs.

Re: [PATCH 0/4] Delayed ref root cleanups

[David Sterba]

On Fri, Jan 09, 2026 at 02:27:24PM -0800, Boris Burkov wrote:
> On Fri, Jan 09, 2026 at 10:09:21PM +0100, David Sterba wrote:
> > On Fri, Jan 09, 2026 at 10:16:27AM -0800, Boris Burkov wrote:
> > > On Fri, Jan 09, 2026 at 06:17:39PM +0100, David Sterba wrote:
> > > > Embed delayed root into btrfs_fs_info.
> > > 
> > > The patches all look fine to me, but I think it would be nice to give
> > > some justification for why it is desirable to make this change besides
> > > "it's possible". If anything, it is a regression on the size of struct
> > > btrfs_fs_info as you mention in the first patch.
> > 
> > A regression? That's an unusal way how to look at it and I did not cross
> > my mind. The motivation is that the two objects have same lifetime and
> > whe have spare bytes in the slab.
> > 
> > > If the answer is just that it's simpler and there is no need for a
> > > separate allocation, then fair enough. But then why not directly embed
> > > all the one-off structures pointed to by fs_info? Like all the global
> > > roots, for example. Are they too large? What constitutes too large?
> > > Later, when we slowly add stuff to fs_info till it is bigger than 4k,
> > > should we undo this patch set? Or look for other, bigger structs to
> > > unembed first?
> > 
> > Fair questions. If we embed everything the fs_info would be say 16K. The
> > threshold I'm considering is 4K, which is 4K page on the most common
> > architecture x86_64. ARM can be configured to have 4K or 64K on the most
> > common setups, so I'm not making it worse by the 4K choice.
> > 
> > So, if the structure for embedding is small enough not to cross 4K and
> > still leave some space then I consider it worth doing. In the case of
> > increasing the fs_info by required and small new members (spinlocks,
> > atomics, various stats etc) we can first look how to shring the size by
> > reordering it. Currently I see there are 97 bytes in holes. Then we can
> > look what is used optionally, eg. depends on a mount option and move it
> > to a separate structure.
> > 
> > The delayed root is a core data structure so we will not have to
> > separate it again and revert this patchset. What I'd start looking for
> > for a separate data structure would be some kind of static
> > almost-read-only information, like mount option bits or status flags
> > etc.
> > 
> > Also I don't want people to worry about fs_info size when there's
> > something new to implement. We have some space to use and I will notice
> > if we cross the boundary as I do random checks of the patch effects
> > every now and then. This applies to parameters and stack space
> > consumption. You may say this is pointless like in the other patchset
> > but even on machines with terabytes of memory a kernel thread is still
> > limited to 16K of stack and layering subsystems can use substantial
> > portions of it. My long term goal is to keep the level the same without
> > hindering development.
> 
> Also, all quibbling aside, I don't want to hold you up on trivialities.
> 
> If you can think of a short, specific explanation for why this is
> preferable, I would appreciate you adding it.

I'll do that, thanks for the comments.