Re: [PATCH 5/5] xfs: embedd struct xfbtree into the owning structure

[Christoph Hellwig <hch@lst.de>]

On Wed, Jan 03, 2024 at 05:21:33PM -0800, Darrick J. Wong wrote:
> > -int xfbtree_create(struct xfs_mount *mp, const struct xfbtree_config *cfg,
> > -		struct xfbtree **xfbtreep);
> > +int xfbtree_init(struct xfs_mount *mp, struct xfbtree *xfbt,
> > +		const struct xfs_btree_ops *btree_ops);
> 
> Why not pass the xfs_buftarg and the owner into the init function?  It
> feels a little funny that the callsites are:
> 
> 	xfbt->target = buftarg;
> 	xfbt->owner = agno;
> 	return xfbtree_init(mp, &xfbt, btree_ops);
> 
> vs:
> 
> 	return xfbtree_init(mp, &xfbt, buftarg, agno, btree_ops);

Yes, but..

The owner assignment should really just move into the caller of the
helpers, which would clean things up.

And the target one I need to fully understand, but maybe let's bring
this up here and ask the question I was going to ask elsewhere after
doing a bit more research.

The way the in-memory buftargs work right now look weird to me.

Why do we keep the target as a separate concept from the xfbtree?
My logical assumption would be that the xfbtree creates the target
internally and the caller shouldn't have to bother with it.

This also goes further and makes me wonder why the
xfs_buf_cache is embdded in the xfile and not just allocated when
allocating a file-backed buftarg?

Btw, once you start touching the xfbtree can we think a bit about
the naming?  Right now we have xfbtree but also a xfs_btree_mem.h,
which seems very confusing.  I think just doing a xfs_btree_mem
naming and moving it out of scrub/ would make sense as the concept
isn't really scrub/repair specific.  But if we want to stick with
it I'd prefer to not also have _mem-based naming.