diff mbox series

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

Message ID 162924378705.761813.11309968953103960937.stgit@magnolia (mailing list archive)
State Superseded
Headers show
Series xfs: clean up ftrace field tags and formats | expand

Commit Message

Darrick J. Wong Aug. 17, 2021, 11:43 p.m. UTC 
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

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 fs/xfs/xfs_trace.h |   14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Comments

Dave Chinner Aug. 19, 2021, 3:11 a.m. UTC | #1 
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.
Darrick J. Wong Aug. 19, 2021, 3:19 a.m. UTC | #2 
On Thu, Aug 19, 2021 at 01:11:59PM +1000, Dave Chinner wrote:
> 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...

Ok, patch 9 updated.

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

Oops.  I guess I got carried away.  Fixed.

--D

> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com
diff mbox series

Patch

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,
@@ -1386,7 +1386,7 @@  DECLARE_EVENT_CLASS(xfs_file_class,
 		__entry->offset = iocb->ki_pos;
 		__entry->count = iov_iter_count(iter);
 	),
-	TP_printk("dev %d:%d ino 0x%llx size 0x%llx pos 0x%llx count 0x%zx",
+	TP_printk("dev %d:%d ino 0x%llx size 0x%llx pos 0x%llx bytecount 0x%zx",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
 		  __entry->ino,
 		  __entry->size,
@@ -1433,7 +1433,7 @@  DECLARE_EVENT_CLASS(xfs_imap_class,
 		__entry->startblock = irec ? irec->br_startblock : 0;
 		__entry->blockcount = irec ? irec->br_blockcount : 0;
 	),
-	TP_printk("dev %d:%d ino 0x%llx size 0x%llx pos 0x%llx count %zd "
+	TP_printk("dev %d:%d ino 0x%llx size 0x%llx pos 0x%llx bytecount 0x%zx "
 		  "fork %s startoff 0x%llx startblock 0x%llx blockcount 0x%llx",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
 		  __entry->ino,
@@ -1476,7 +1476,7 @@  DECLARE_EVENT_CLASS(xfs_simple_io_class,
 		__entry->count = count;
 	),
 	TP_printk("dev %d:%d ino 0x%llx isize 0x%llx disize 0x%llx "
-		  "pos 0x%llx count %zd",
+		  "pos 0x%llx bytecount 0x%zx",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
 		  __entry->ino,
 		  __entry->isize,
@@ -3217,7 +3217,7 @@  DECLARE_EVENT_CLASS(xfs_double_io_class,
 		__field(loff_t, src_isize)
 		__field(loff_t, src_disize)
 		__field(loff_t, src_offset)
-		__field(size_t, len)
+		__field(long long, len)
 		__field(xfs_ino_t, dest_ino)
 		__field(loff_t, dest_isize)
 		__field(loff_t, dest_disize)
@@ -3235,7 +3235,7 @@  DECLARE_EVENT_CLASS(xfs_double_io_class,
 		__entry->dest_disize = dest->i_disk_size;
 		__entry->dest_offset = doffset;
 	),
-	TP_printk("dev %d:%d count %zd "
+	TP_printk("dev %d:%d bytecount 0x%llx "
 		  "ino 0x%llx isize 0x%llx disize 0x%llx pos 0x%llx -> "
 		  "ino 0x%llx isize 0x%llx disize 0x%llx pos 0x%llx",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),