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