Re: [PATCH 08/15] xfs: disambiguate units for ftrace fields tagged "offset"

[Carlos Maiolino <cmaiolino@redhat.com>]

On Thu, Aug 19, 2021 at 12:51:17PM +1000, Dave Chinner wrote:
> On Tue, Aug 17, 2021 at 04:42:56PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Some of our tracepoints describe fields as "offset".  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.
> > 
> > "fileoff" means file offset, in units of fs blocks
> > "pos" means file offset, in bytes
> > "forkoff" means inode fork offset, in bytes
> > 
> > The one remaining "offset" value is for iclogs, since that's the byte
> > offset of the end of where we've written into the current iclog.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---

Just realized Dave already spotted it, sorry the redundancy :(

> >  fs/xfs/scrub/trace.h |    6 +++---
> >  fs/xfs/xfs_trace.h   |   29 ++++++++++++++---------------
> >  2 files changed, 17 insertions(+), 18 deletions(-)
> 
> ....
> 
> > @@ -2145,7 +2145,7 @@ DECLARE_EVENT_CLASS(xfs_swap_extent_class,
> >  		__entry->fork_off = XFS_IFORK_BOFF(ip);
> >  	),
> >  	TP_printk("dev %d:%d ino 0x%llx (%s), %s format, num_extents %d, "
> > -		  "broot size %d, fork offset %d",
> > +		  "broot size %d, forkoff %d",
> >  		  MAJOR(__entry->dev), MINOR(__entry->dev),
> >  		  __entry->ino,
> >  		  __print_symbolic(__entry->which, XFS_SWAPEXT_INODES),
> 
> Format should be 0x%x?
> 
> Otherwise looks fine. With that fixed,
> 
> Reviewed-by: Dave Chinner <dchinner@redhat.com>
> -- 
> Dave Chinner
> david@fromorbit.com
> 

-- 
Carlos