diff mbox series

[6/9] xfs: consolidate btree block freeing tracepoints

Message ID 170404829675.1748854.18135934618780501542.stgit@frogsfrogsfrogs (mailing list archive)
State Superseded
Headers show
Series [1/9] xfs: dump xfiles for debugging purposes | expand

Commit Message

Darrick J. Wong Dec. 31, 2023, 8:15 p.m. UTC 
From: Darrick J. Wong <djwong@kernel.org>

Don't waste tracepoint segment memory on per-btree block freeing
tracepoints when we can do it from the generic btree code.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 fs/xfs/libxfs/xfs_btree.c          |    2 ++
 fs/xfs/libxfs/xfs_refcount_btree.c |    2 --
 fs/xfs/libxfs/xfs_rmap_btree.c     |    2 --
 fs/xfs/xfs_trace.h                 |   32 ++++++++++++++++++++++++++++++--
 4 files changed, 32 insertions(+), 6 deletions(-)

Comments

Christoph Hellwig Jan. 3, 2024, 8:53 a.m. UTC | #1 
On Sun, Dec 31, 2023 at 12:15:07PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Don't waste tracepoint segment memory on per-btree block freeing
> tracepoints when we can do it from the generic btree code.

The patch looks good, but what is "tracepoint segment memory"?
Darrick J. Wong Jan. 3, 2024, 7:37 p.m. UTC | #2 
On Wed, Jan 03, 2024 at 12:53:17AM -0800, Christoph Hellwig wrote:
> On Sun, Dec 31, 2023 at 12:15:07PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Don't waste tracepoint segment memory on per-btree block freeing
> > tracepoints when we can do it from the generic btree code.
> 
> The patch looks good, but what is "tracepoint segment memory"?

The size of the ELF segments where the ftrace strings/code/etc are
stored.  With this and the next patch applied, the output of:

$ objdump -x fs/xfs/xfs.ko | grep tracepoint

Before:

 10 __tracepoints_ptrs 00000b38  0000000000000000  0000000000000000  001418b0  2**2
 14 __tracepoints_strings 00005433  0000000000000000  0000000000000000  00168f60  2**5
 29 __tracepoints 00010d30  0000000000000000  0000000000000000  00240080  2**5

After:

 10 __tracepoints_ptrs 00000b30  0000000000000000  0000000000000000  00142170  2**2
 14 __tracepoints_strings 000053f3  0000000000000000  0000000000000000  00169860  2**5
 29 __tracepoints 00010c70  0000000000000000  0000000000000000  00241180  2**5

Removing these two tracepoints reduces the size of the ELF segments by
264 bytes.  I'll add this note to the commit message.

--D
Christoph Hellwig Jan. 4, 2024, 6:19 a.m. UTC | #3 
On Wed, Jan 03, 2024 at 11:37:05AM -0800, Darrick J. Wong wrote:
> Removing these two tracepoints reduces the size of the ELF segments by
> 264 bytes.  I'll add this note to the commit message.

Yeah.  Maybe just say memory usage - segment size feels awfully specific
to an implementation detail.
Darrick J. Wong Jan. 4, 2024, 7:15 a.m. UTC | #4 
On Wed, Jan 03, 2024 at 10:19:29PM -0800, Christoph Hellwig wrote:
> On Wed, Jan 03, 2024 at 11:37:05AM -0800, Darrick J. Wong wrote:
> > Removing these two tracepoints reduces the size of the ELF segments by
> > 264 bytes.  I'll add this note to the commit message.
> 
> Yeah.  Maybe just say memory usage - segment size feels awfully specific
> to an implementation detail.

Done.

--D
diff mbox series

Patch

diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
index 28ba528086888..3e966182b90a9 100644
--- a/fs/xfs/libxfs/xfs_btree.c
+++ b/fs/xfs/libxfs/xfs_btree.c
@@ -414,6 +414,8 @@  xfs_btree_free_block(
 {
 	int			error;
 
+	trace_xfs_btree_free_block(cur, bp);
+
 	error = cur->bc_ops->free_block(cur, bp);
 	if (!error) {
 		xfs_trans_binval(cur->bc_tp, bp);
diff --git a/fs/xfs/libxfs/xfs_refcount_btree.c b/fs/xfs/libxfs/xfs_refcount_btree.c
index 0d80bd99147cc..a346e49981ac3 100644
--- a/fs/xfs/libxfs/xfs_refcount_btree.c
+++ b/fs/xfs/libxfs/xfs_refcount_btree.c
@@ -107,8 +107,6 @@  xfs_refcountbt_free_block(
 	struct xfs_agf		*agf = agbp->b_addr;
 	xfs_fsblock_t		fsbno = XFS_DADDR_TO_FSB(mp, xfs_buf_daddr(bp));
 
-	trace_xfs_refcountbt_free_block(cur->bc_mp, cur->bc_ag.pag->pag_agno,
-			XFS_FSB_TO_AGBNO(cur->bc_mp, fsbno), 1);
 	be32_add_cpu(&agf->agf_refcount_blocks, -1);
 	xfs_alloc_log_agf(cur->bc_tp, agbp, XFS_AGF_REFCOUNT_BLOCKS);
 	return xfs_free_extent_later(cur->bc_tp, fsbno, 1,
diff --git a/fs/xfs/libxfs/xfs_rmap_btree.c b/fs/xfs/libxfs/xfs_rmap_btree.c
index 6c81b20e97d21..0dc086bc528f7 100644
--- a/fs/xfs/libxfs/xfs_rmap_btree.c
+++ b/fs/xfs/libxfs/xfs_rmap_btree.c
@@ -125,8 +125,6 @@  xfs_rmapbt_free_block(
 	int			error;
 
 	bno = xfs_daddr_to_agbno(cur->bc_mp, xfs_buf_daddr(bp));
-	trace_xfs_rmapbt_free_block(cur->bc_mp, pag->pag_agno,
-			bno, 1);
 	be32_add_cpu(&agf->agf_rmap_blocks, -1);
 	xfs_alloc_log_agf(cur->bc_tp, agbp, XFS_AGF_RMAP_BLOCKS);
 	error = xfs_alloc_put_freelist(pag, cur->bc_tp, agbp, NULL, bno, 1);
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index 7d075e426c5d0..5076770d9b000 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -2493,6 +2493,36 @@  DEFINE_EVENT(xfs_btree_cur_class, name, \
 DEFINE_BTREE_CUR_EVENT(xfs_btree_updkeys);
 DEFINE_BTREE_CUR_EVENT(xfs_btree_overlapped_query_range);
 
+TRACE_EVENT(xfs_btree_free_block,
+	TP_PROTO(struct xfs_btree_cur *cur, struct xfs_buf *bp),
+	TP_ARGS(cur, bp),
+	TP_STRUCT__entry(
+		__field(dev_t, dev)
+		__field(xfs_agnumber_t, agno)
+		__field(xfs_ino_t, ino)
+		__field(xfs_btnum_t, btnum)
+		__field(xfs_agblock_t, agbno)
+	),
+	TP_fast_assign(
+		__entry->dev = cur->bc_mp->m_super->s_dev;
+		__entry->agno = xfs_daddr_to_agno(cur->bc_mp,
+							xfs_buf_daddr(bp));
+		if (cur->bc_flags & XFS_BTREE_ROOT_IN_INODE)
+			__entry->ino = cur->bc_ino.ip->i_ino;
+		else
+			__entry->ino = 0;
+		__entry->btnum = cur->bc_btnum;
+		__entry->agbno = xfs_daddr_to_agbno(cur->bc_mp,
+							xfs_buf_daddr(bp));
+	),
+	TP_printk("dev %d:%d btree %s agno 0x%x ino 0x%llx agbno 0x%x",
+		  MAJOR(__entry->dev), MINOR(__entry->dev),
+		  __print_symbolic(__entry->btnum, XFS_BTNUM_STRINGS),
+		  __entry->agno,
+		  __entry->ino,
+		  __entry->agbno)
+);
+
 /* deferred ops */
 struct xfs_defer_pending;
 
@@ -2856,7 +2886,6 @@  DEFINE_RMAP_DEFERRED_EVENT(xfs_rmap_defer);
 DEFINE_RMAP_DEFERRED_EVENT(xfs_rmap_deferred);
 
 DEFINE_BUSY_EVENT(xfs_rmapbt_alloc_block);
-DEFINE_BUSY_EVENT(xfs_rmapbt_free_block);
 DEFINE_RMAPBT_EVENT(xfs_rmap_update);
 DEFINE_RMAPBT_EVENT(xfs_rmap_insert);
 DEFINE_RMAPBT_EVENT(xfs_rmap_delete);
@@ -3215,7 +3244,6 @@  DEFINE_EVENT(xfs_refcount_triple_extent_class, name, \
 
 /* refcount btree tracepoints */
 DEFINE_BUSY_EVENT(xfs_refcountbt_alloc_block);
-DEFINE_BUSY_EVENT(xfs_refcountbt_free_block);
 DEFINE_AG_BTREE_LOOKUP_EVENT(xfs_refcount_lookup);
 DEFINE_REFCOUNT_EXTENT_EVENT(xfs_refcount_get);
 DEFINE_REFCOUNT_EXTENT_EVENT(xfs_refcount_update);