On Mon, Oct 14, 2024 at 05:02:16PM -0700, Darrick J. Wong wrote:
> On Mon, Oct 14, 2024 at 01:04:01AM -0700, Christoph Hellwig wrote:
> > The actual intent code looks good:
> >
> > Reviewed-by: Christoph Hellwig <hch@xxxxxx>
> >
> > but while re-reviewing I noticed a minor thing in the tracing code:
> >
> > > + __entry->dev = mp->m_super->s_dev;
> > > + __entry->type = bi->bi_group->xg_type;
> > > + __entry->agno = bi->bi_group->xg_index;
> > > + switch (__entry->type) {
> > > + case XG_TYPE_RTG:
> > > + /*
> > > + * Use the 64-bit version of xfs_rtb_to_rgbno because
> > > + * legacy rt filesystems can have group block numbers
> > > + * that exceed the size of an xfs_rgblock_t.
> > > + */
> > > + __entry->gbno = __xfs_rtb_to_rgbno(mp,
> > > bi->bi_bmap.br_startblock);
> > > + break;
> > > + case XG_TYPE_AG:
> > > + __entry->gbno = XFS_FSB_TO_AGBNO(mp,
> > > bi->bi_bmap.br_startblock);
> > > + break;
> > > + default:
> > > + /* should never happen */
> > > + __entry->gbno = -1ULL;
> > > + break;
> >
> > Maybe just make this an
> >
> > if (type == XG_TYPE_RTG)
> > __xfs_rtb_to_rgbno()
> > else
> > xfs_fsb_to_gbno()
> >
> > ?
>
> Hmmm that *would* get rid of that __entry->gbno = -1ULL ugliness above.
>
> Ok let's do it.
>
> Until we get to patch, the helper looks like:
I meant to write:
Until we get to "xfs: move the group geometry into struct xfs_groups",
the helper will look like:
--D
> xfs_agblock_t
> xfs_fsb_to_gbno(
> struct xfs_mount *mp,
> xfs_fsblock_t fsbno,
> enum xfs_group_type type)
> {
> if (type == XG_TYPE_RTG)
> return xfs_rtb_to_rgbno(mp, fsbno);
> return XFS_FSB_TO_AGBNO(mp, fsbno);
> }
>
> and the tracepoint code become:
>
> __entry->type = bi->bi_group->xg_type;
> __entry->agno = bi->bi_group->xg_index;
> if (bi->bi_group->xg_type == XG_TYPE_RTG &&
> !xfs_has_rtgroups(mp)) {
> /*
> * Legacy rt filesystems do not have allocation
> * groups ondisk. We emulate this incore with
> * one gigantic rtgroup whose size can exceed a
> * 32-bit block number. For this tracepoint, we
> * report group 0 and a 64-bit group block
> * number.
> */
> __entry->gbno = bi->bi_bmap.br_startblock;
> } else {
> __entry->gbno = xfs_fsb_to_gbno(mp,
> bi->bi_bmap.br_startblock,
> bi->bi_group->xg_type);
> }
> __entry->ino = ip->i_ino;
>
> --D
>
> > > __entry->l_len,
> > >
> > >
> > ---end quoted text---
> >
>
