[PATCH 1/1] xfs: create a function to duplicate an active perag reference

Darrick J. Wong
2022-12-30T22:36:37Z

There a few object constructor functions throughout XFS where a caller
provides an active perag reference and the constructor wants to give the
new object its own active reference. Replace the open-coded logic with
a common function to do this instead of open-coding atomic_inc logic.

This new function adds a few safeguards -- it checks that there's at
least one active reference to the perag structure passed in, and it
records the refcount bump in the ftrace information. This makes it much
easier to debug refcounting problems.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
fs/xfs/libxfs/xfs_ag.c | 15 +++++++++++++++
fs/xfs/libxfs/xfs_ag.h | 1 +
fs/xfs/libxfs/xfs_alloc_btree.c | 4 +---
fs/xfs/libxfs/xfs_ialloc_btree.c | 4 +---
fs/xfs/libxfs/xfs_refcount_btree.c | 5 +----
fs/xfs/libxfs/xfs_rmap_btree.c | 5 +----
fs/xfs/xfs_iunlink_item.c | 4 +---
fs/xfs/xfs_iwalk.c | 3 +--
fs/xfs/xfs_trace.h | 1 +
9 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
index 8de4143a5899..fed965831f2d 100644
--- a/fs/xfs/libxfs/xfs_ag.c
+++ b/fs/xfs/libxfs/xfs_ag.c
@@ -57,6 +57,21 @@ xfs_perag_get(
return pag;
}

+/* Get our own reference to a perag, given an existing active reference. */
+struct xfs_perag *
+xfs_perag_bump(
+ struct xfs_perag *pag)
+{
+ if (!atomic_inc_not_zero(&pag->pag_ref)) {
+ ASSERT(0);
+ return NULL;
+ }
+
+ trace_xfs_perag_bump(pag->pag_mount, pag->pag_agno,
+ atomic_read(&pag->pag_ref), _RET_IP_);
+ return pag;
+}
+
/*
* search from @first to find the next perag with the given tag set.
*/
diff --git a/fs/xfs/libxfs/xfs_ag.h b/fs/xfs/libxfs/xfs_ag.h
index 191b22b9a35b..d61b07e60802 100644
--- a/fs/xfs/libxfs/xfs_ag.h
+++ b/fs/xfs/libxfs/xfs_ag.h
@@ -112,6 +112,7 @@ int xfs_initialize_perag_data(struct xfs_mount *mp, xfs_agnumber_t agno);
void xfs_free_perag(struct xfs_mount *mp);

struct xfs_perag *xfs_perag_get(struct xfs_mount *mp, xfs_agnumber_t agno);
+struct xfs_perag *xfs_perag_bump(struct xfs_perag *pag);
struct xfs_perag *xfs_perag_get_tag(struct xfs_mount *mp, xfs_agnumber_t agno,
unsigned int tag);
void xfs_perag_put(struct xfs_perag *pag);
diff --git a/fs/xfs/libxfs/xfs_alloc_btree.c b/fs/xfs/libxfs/xfs_alloc_btree.c
index 549a3cba0234..0e78e00e02f9 100644
--- a/fs/xfs/libxfs/xfs_alloc_btree.c
+++ b/fs/xfs/libxfs/xfs_alloc_btree.c
@@ -492,9 +492,7 @@ xfs_allocbt_init_common(
cur->bc_statoff = XFS_STATS_CALC_INDEX(xs_abtb_2);
}

- /* take a reference for the cursor */
- atomic_inc(&pag->pag_ref);
- cur->bc_ag.pag = pag;
+ cur->bc_ag.pag = xfs_perag_bump(pag);

if (xfs_has_crc(mp))
cur->bc_flags |= XFS_BTREE_CRC_BLOCKS;
diff --git a/fs/xfs/libxfs/xfs_ialloc_btree.c b/fs/xfs/libxfs/xfs_ialloc_btree.c
index 2dbe553d87fb..fb10760fd686 100644
--- a/fs/xfs/libxfs/xfs_ialloc_btree.c
+++ b/fs/xfs/libxfs/xfs_ialloc_btree.c
@@ -450,9 +450,7 @@ xfs_inobt_init_common(
if (xfs_has_crc(mp))
cur->bc_flags |= XFS_BTREE_CRC_BLOCKS;

- /* take a reference for the cursor */
- atomic_inc(&pag->pag_ref);
- cur->bc_ag.pag = pag;
+ cur->bc_ag.pag = xfs_perag_bump(pag);
return cur;
}

diff --git a/fs/xfs/libxfs/xfs_refcount_btree.c b/fs/xfs/libxfs/xfs_refcount_btree.c
index 3d8e62da2ccc..f5bdac3cf19f 100644
--- a/fs/xfs/libxfs/xfs_refcount_btree.c
+++ b/fs/xfs/libxfs/xfs_refcount_btree.c
@@ -340,10 +340,7 @@ xfs_refcountbt_init_common(

cur->bc_flags |= XFS_BTREE_CRC_BLOCKS;

- /* take a reference for the cursor */
- atomic_inc(&pag->pag_ref);
- cur->bc_ag.pag = pag;
-
+ cur->bc_ag.pag = xfs_perag_bump(pag);
cur->bc_ag.refc.nr_ops = 0;
cur->bc_ag.refc.shape_changes = 0;
cur->bc_ops = &xfs_refcountbt_ops;
diff --git a/fs/xfs/libxfs/xfs_rmap_btree.c b/fs/xfs/libxfs/xfs_rmap_btree.c
index 7f83f62e51e0..12c26c42c162 100644
--- a/fs/xfs/libxfs/xfs_rmap_btree.c
+++ b/fs/xfs/libxfs/xfs_rmap_btree.c
@@ -460,10 +460,7 @@ xfs_rmapbt_init_common(
cur->bc_statoff = XFS_STATS_CALC_INDEX(xs_rmap_2);
cur->bc_ops = &xfs_rmapbt_ops;

- /* take a reference for the cursor */
- atomic_inc(&pag->pag_ref);
- cur->bc_ag.pag = pag;
-
+ cur->bc_ag.pag = xfs_perag_bump(pag);
return cur;
}

diff --git a/fs/xfs/xfs_iunlink_item.c b/fs/xfs/xfs_iunlink_item.c
index 43005ce8bd48..5024a59f0c75 100644
--- a/fs/xfs/xfs_iunlink_item.c
+++ b/fs/xfs/xfs_iunlink_item.c
@@ -168,9 +168,7 @@ xfs_iunlink_log_inode(
iup->ip = ip;
iup->next_agino = next_agino;
iup->old_agino = ip->i_next_unlinked;
-
- atomic_inc(&pag->pag_ref);
- iup->pag = pag;
+ iup->pag = xfs_perag_bump(pag);

xfs_trans_add_item(tp, &iup->item);
tp->t_flags |= XFS_TRANS_DIRTY;
diff --git a/fs/xfs/xfs_iwalk.c b/fs/xfs/xfs_iwalk.c
index 7558486f4937..594ccadb729f 100644
--- a/fs/xfs/xfs_iwalk.c
+++ b/fs/xfs/xfs_iwalk.c
@@ -670,8 +670,7 @@ xfs_iwalk_threaded(
* perag is being handed off to async work, so take another
* reference for the async work to release.
*/
- atomic_inc(&pag->pag_ref);
- iwag->pag = pag;
+ iwag->pag = xfs_perag_bump(pag);
iwag->iwalk_fn = iwalk_fn;
iwag->data = data;
iwag->startino = startino;
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index 6b0e9ae7c513..0448b992a561 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -187,6 +187,7 @@ DEFINE_EVENT(xfs_perag_class, name, \
unsigned long caller_ip), \
TP_ARGS(mp, agno, refcount, caller_ip))
DEFINE_PERAG_REF_EVENT(xfs_perag_get);
+DEFINE_PERAG_REF_EVENT(xfs_perag_bump);
DEFINE_PERAG_REF_EVENT(xfs_perag_get_tag);
DEFINE_PERAG_REF_EVENT(xfs_perag_put);
DEFINE_PERAG_REF_EVENT(xfs_perag_set_inode_tag);