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.
> > 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.
