Re: [PATCH] btrfs: implement ref_tracker for delayed_nodes

[David Sterba <dsterba@suse.cz>]

On Thu, Jul 10, 2025 at 12:54:34PM +0100, Filipe Manana wrote:
> > @@ -63,10 +109,18 @@ struct btrfs_delayed_node {
> >         struct rb_root_cached del_root;
> >         struct mutex mutex;
> >         struct btrfs_inode_item inode_item;
> > +
> >         refcount_t refs;
> > -       int count;
> > +       /* Used to track all references to this delayed node. */
> > +       btrfs_delayed_node_ref_tracker_dir ref_dir;
> > +       /* Used to track delayed node reference stored in node list. */
> > +       btrfs_delayed_node_ref_tracker node_list_tracker;
> > +       /* Used to track delayed node reference stored in inode cache. */
> > +       btrfs_delayed_node_ref_tracker inode_cache_tracker;
> > +
> >         u64 index_cnt;
> >         unsigned long flags;
> > +       int count;
> 
> Why are you moving the count field?
> This increases the size of the struct even without the new config
> option enabled and as a result we get less objects per page.
> You are essentially reverting this commit:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a20725e1e7014642fc8ba4c7dd2c4ef6d4ec56a9
> 
> Without any explanation about why.
> 
> I'm also not a big fan of the typedefs and would prefer to have
> struct ref_tracker used directly in the structure surrounded by an
> #ifdef block.

Agreed, we use typedefs only for function callbacks, for variable types
not anymore.

> I also don't like adding a new Kconfig option just for this... How
> much slower would a build with CONFIG_BTRFS_DEBUG=y be?
> 
> If that's really a lot slower, then perhaps a more generic config
> option name in case we add similar tracking to other data structures
> one day.

Right, we want to keep the config options at minimum and for debugging
features it should be fine under BTRFS_DEBUG. If this needs to be
configured for performance reasons then it can be a mount option, same
as we have ref-verify or fragment already.