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.
