On Thu, Aug 19, 2021 at 01:01:47PM +1000, Dave Chinner wrote:
> On Tue, Aug 17, 2021 at 04:43:01PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@xxxxxxxxxx>
> >
> > Some of our tracepoints have a field known as "len". That name doesn't
> > describe any units, which makes the fields not very useful. Rename the
> > fields to capture units and ensure the format is hexadecimal.
> >
> > "blockcount" are in units of fs blocks
> > "daddrcount" are in units of 512b blocks
>
> Hmmm. This is the first set of units I'll consider suggesting a
> change in naming - "blockcount" seems ambiguous and easily mistaken,
> while "daddrcount" just seems a bit wierd. Perhaps:
>
> "fsbcount" is a length in units of fs blocks
> "bbcount" is a length in units of basic (512b) blocks
I like that suggestion. Will update.
--D
> .....
> > @@ -2363,7 +2363,7 @@ DECLARE_EVENT_CLASS(xfs_log_recover_icreate_item_class,
> > __entry->length = be32_to_cpu(in_f->icl_length);
> > __entry->gen = be32_to_cpu(in_f->icl_gen);
> > ),
> > - TP_printk("dev %d:%d agno 0x%x agbno 0x%x count %u isize %u length %u "
> > + TP_printk("dev %d:%d agno 0x%x agbno 0x%x count %u isize %u blockcount 0x%x "
> > "gen %u", MAJOR(__entry->dev), MINOR(__entry->dev),
> > __entry->agno, __entry->agbno, __entry->count, __entry->isize,
> > __entry->length, __entry->gen)
>
> THis one could probably do with a bit of help - count is the number
> of inodes, so the order of the tracepoint probably should be
> reworked to put the fsbcount directly after the agbno. i.e.
>
> TP_printk("dev %d:%d agno 0x%x agbno 0x%x fsbcount 0x%x isize %u count %u gen %u",
> ....
>
> The rest of the conversions look good, though.
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@xxxxxxxxxxxxx
