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

[David Sterba <dsterba@suse.cz>]

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.