Re: [PATCH 10/15] xfs: disambiguate units for ftrace fields tagged "count"

From: Dave Chinner <david@fromorbit.com>
To: "Darrick J. Wong" <djwong@kernel.org>
Cc: sandeen@sandeen.net, linux-xfs@vger.kernel.org
Subject: Re: [PATCH 10/15] xfs: disambiguate units for ftrace fields tagged "count"
Date: Thu, 19 Aug 2021 13:11:59 +1000	[thread overview]
Message-ID: <20210819031159.GA3657114@dread.disaster.area> (raw)
In-Reply-To: <162924378705.761813.11309968953103960937.stgit@magnolia>

On Tue, Aug 17, 2021 at 04:43:07PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Some of our tracepoints have a field known as "count".  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
> when we're referring to blocks, extents, or IO operations.
> 
> "blockcount" are in units of fs blocks
> "bytecount" are in units of bytes
> "icount" are in units of inode records

This is where fsbcount and bbcount really look like a reasonable way
of encoding the unit into the description... :)

Also, having noted in the previous patch that the icreate record
trace point has a count of inodes as "count", perhaps this icount
would be better as "ireccount" so that icount can be used as a count
of inodes...

> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  fs/xfs/xfs_trace.h |   14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> 
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index 7ae654f7ae82..07da753588d5 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -346,7 +346,7 @@ DECLARE_EVENT_CLASS(xfs_bmap_class,
>  		__entry->caller_ip = caller_ip;
>  	),
>  	TP_printk("dev %d:%d ino 0x%llx state %s cur %p/%d "
> -		  "fileoff 0x%llx startblock 0x%llx count %lld flag %d caller %pS",
> +		  "fileoff 0x%llx startblock 0x%llx blockcount 0x%llx flag %d caller %pS",
>  		  MAJOR(__entry->dev), MINOR(__entry->dev),
>  		  __entry->ino,
>  		  __print_flags(__entry->bmap_state, "|", XFS_BMAP_EXT_FLAGS),
> @@ -806,7 +806,7 @@ DECLARE_EVENT_CLASS(xfs_iref_class,
>  		__entry->pincount = atomic_read(&ip->i_pincount);
>  		__entry->caller_ip = caller_ip;
>  	),
> -	TP_printk("dev %d:%d ino 0x%llx count %d pincount %d caller %pS",
> +	TP_printk("dev %d:%d ino 0x%llx icount %d pincount %d caller %pS",
>  		  MAJOR(__entry->dev), MINOR(__entry->dev),
>  		  __entry->ino,
>  		  __entry->count,

I don't think this is correct. This count is the current active
reference count of the inode, not a count of inode records...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com