On Tue, Oct 15, 2024 at 05:33:39AM +0200, Christoph Hellwig wrote:
> On Tue, Oct 15, 2024 at 02:05:26PM +1100, Dave Chinner wrote:
> > > + cur->bc_ops->name, cur->bc_group->xg_index, fa);
> > ^^^^^^^^^^^^^^^^^^^^^^^
> >
> > Reading through this patch, I keep wanting to this to read as "group
> > number" as a replacement for AG number. i.e. pag_agno(pag) ->
> > group_num(grp) as the nice, short helper function.
> >
> > We're kinda used to this with terminology with agno, agbno, fsbno,
> > ino, agino, etc all refering to the "number" associated with an
> > object type. Hence it seems kinda natural to refer to these as
> > group numbers rather than an index into something....
> >
> > Just an observation, up to you whether you think it's worthwhile or
> > not.
>
> Or just rename xg_index to xg_gno. I though I was smart about the
> index when I did this a while ago, but in hinsight it might not have
> been the best choice.
It's probably easier to change it to xg_gno along with all the other
patch sedding.
--D
> > > STATIC struct xfs_btree_cur *
> > > @@ -36,29 +36,29 @@ xfs_cntbt_dup_cursor(
> > > struct xfs_btree_cur *cur)
> > > {
> > > return xfs_cntbt_init_cursor(cur->bc_mp, cur->bc_tp, cur->bc_ag.agbp,
> > > - cur->bc_ag.pag);
> > > + to_perag(cur->bc_group));
> > > }
> >
> > Huh. Less than ideal code will be generated for these (group on old
> > cursor -> perag -> back to group in new cursor) code, but converting
> > every single bit of the btree cursor code over to groups doesn't
> > need to be done here...
>
> We've actually done the cursor init cleanup for the RT rmap and reflink
> btrees (not in this patchbomb yet), and I though about doing it for
> the classic per-AG btrees as well. I can do that as a follow up.
>
>
