diff mbox series

[08/42] xfs: rework the perag trace points to be perag centric

Message ID 20230118224505.1964941-9-david@fromorbit.com (mailing list archive)
State Superseded
Headers show
Series xfs: per-ag centric allocation alogrithms | expand

Commit Message

Dave Chinner Jan. 18, 2023, 10:44 p.m. UTC 
From: Dave Chinner <dchinner@redhat.com>

So that they all output the same information in the traces to make
debugging refcount issues easier.

This means that all the lookup/drop functions no longer need to use
the full memory barrier atomic operations (atomic*_return()) so
will have less overhead when tracing is off. The set/clear tag
tracepoints no longer abuse the reference count to pass the tag -
the tag being cleared is obvious from the _RET_IP_ that is recorded
in the trace point.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/libxfs/xfs_ag.c | 25 +++++++++----------------
 fs/xfs/xfs_icache.c    |  4 ++--
 fs/xfs/xfs_trace.h     | 21 +++++++++++----------
 3 files changed, 22 insertions(+), 28 deletions(-)

Comments

Allison Henderson Jan. 21, 2023, 5:16 a.m. UTC | #1 
On Thu, 2023-01-19 at 09:44 +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> So that they all output the same information in the traces to make
> debugging refcount issues easier.
> 
> This means that all the lookup/drop functions no longer need to use
> the full memory barrier atomic operations (atomic*_return()) so
> will have less overhead when tracing is off. The set/clear tag
> tracepoints no longer abuse the reference count to pass the tag -
> the tag being cleared is obvious from the _RET_IP_ that is recorded
> in the trace point.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
Ok, makes sense
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>

>  fs/xfs/libxfs/xfs_ag.c | 25 +++++++++----------------
>  fs/xfs/xfs_icache.c    |  4 ++--
>  fs/xfs/xfs_trace.h     | 21 +++++++++++----------
>  3 files changed, 22 insertions(+), 28 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
> index 46e25c682bf4..7cff61875340 100644
> --- a/fs/xfs/libxfs/xfs_ag.c
> +++ b/fs/xfs/libxfs/xfs_ag.c
> @@ -44,16 +44,15 @@ xfs_perag_get(
>         xfs_agnumber_t          agno)
>  {
>         struct xfs_perag        *pag;
> -       int                     ref = 0;
>  
>         rcu_read_lock();
>         pag = radix_tree_lookup(&mp->m_perag_tree, agno);
>         if (pag) {
> +               trace_xfs_perag_get(pag, _RET_IP_);
>                 ASSERT(atomic_read(&pag->pag_ref) >= 0);
> -               ref = atomic_inc_return(&pag->pag_ref);
> +               atomic_inc(&pag->pag_ref);
>         }
>         rcu_read_unlock();
> -       trace_xfs_perag_get(mp, agno, ref, _RET_IP_);
>         return pag;
>  }
>  
> @@ -68,7 +67,6 @@ xfs_perag_get_tag(
>  {
>         struct xfs_perag        *pag;
>         int                     found;
> -       int                     ref;
>  
>         rcu_read_lock();
>         found = radix_tree_gang_lookup_tag(&mp->m_perag_tree,
> @@ -77,9 +75,9 @@ xfs_perag_get_tag(
>                 rcu_read_unlock();
>                 return NULL;
>         }
> -       ref = atomic_inc_return(&pag->pag_ref);
> +       trace_xfs_perag_get_tag(pag, _RET_IP_);
> +       atomic_inc(&pag->pag_ref);
>         rcu_read_unlock();
> -       trace_xfs_perag_get_tag(mp, pag->pag_agno, ref, _RET_IP_);
>         return pag;
>  }
>  
> @@ -87,11 +85,9 @@ void
>  xfs_perag_put(
>         struct xfs_perag        *pag)
>  {
> -       int     ref;
> -
> +       trace_xfs_perag_put(pag, _RET_IP_);
>         ASSERT(atomic_read(&pag->pag_ref) > 0);
> -       ref = atomic_dec_return(&pag->pag_ref);
> -       trace_xfs_perag_put(pag->pag_mount, pag->pag_agno, ref,
> _RET_IP_);
> +       atomic_dec(&pag->pag_ref);
>  }
>  
>  /*
> @@ -110,8 +106,7 @@ xfs_perag_grab(
>         rcu_read_lock();
>         pag = radix_tree_lookup(&mp->m_perag_tree, agno);
>         if (pag) {
> -               trace_xfs_perag_grab(mp, pag->pag_agno,
> -                               atomic_read(&pag->pag_active_ref),
> _RET_IP_);
> +               trace_xfs_perag_grab(pag, _RET_IP_);
>                 if (!atomic_inc_not_zero(&pag->pag_active_ref))
>                         pag = NULL;
>         }
> @@ -138,8 +133,7 @@ xfs_perag_grab_tag(
>                 rcu_read_unlock();
>                 return NULL;
>         }
> -       trace_xfs_perag_grab_tag(mp, pag->pag_agno,
> -                       atomic_read(&pag->pag_active_ref), _RET_IP_);
> +       trace_xfs_perag_grab_tag(pag, _RET_IP_);
>         if (!atomic_inc_not_zero(&pag->pag_active_ref))
>                 pag = NULL;
>         rcu_read_unlock();
> @@ -150,8 +144,7 @@ void
>  xfs_perag_rele(
>         struct xfs_perag        *pag)
>  {
> -       trace_xfs_perag_rele(pag->pag_mount, pag->pag_agno,
> -                       atomic_read(&pag->pag_active_ref), _RET_IP_);
> +       trace_xfs_perag_rele(pag, _RET_IP_);
>         if (atomic_dec_and_test(&pag->pag_active_ref))
>                 wake_up(&pag->pag_active_wq);
>  }
> diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> index 0f4a014dded3..8b2823d85a68 100644
> --- a/fs/xfs/xfs_icache.c
> +++ b/fs/xfs/xfs_icache.c
> @@ -255,7 +255,7 @@ xfs_perag_set_inode_tag(
>                 break;
>         }
>  
> -       trace_xfs_perag_set_inode_tag(mp, pag->pag_agno, tag,
> _RET_IP_);
> +       trace_xfs_perag_set_inode_tag(pag, _RET_IP_);
>  }
>  
>  /* Clear a tag on both the AG incore inode tree and the AG radix
> tree. */
> @@ -289,7 +289,7 @@ xfs_perag_clear_inode_tag(
>         radix_tree_tag_clear(&mp->m_perag_tree, pag->pag_agno, tag);
>         spin_unlock(&mp->m_perag_lock);
>  
> -       trace_xfs_perag_clear_inode_tag(mp, pag->pag_agno, tag,
> _RET_IP_);
> +       trace_xfs_perag_clear_inode_tag(pag, _RET_IP_);
>  }
>  
>  /*
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index f0b62054ea68..c921e9a5256d 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -159,33 +159,34 @@ TRACE_EVENT(xlog_intent_recovery_failed,
>  );
>  
>  DECLARE_EVENT_CLASS(xfs_perag_class,
> -       TP_PROTO(struct xfs_mount *mp, xfs_agnumber_t agno, int
> refcount,
> -                unsigned long caller_ip),
> -       TP_ARGS(mp, agno, refcount, caller_ip),
> +       TP_PROTO(struct xfs_perag *pag, unsigned long caller_ip),
> +       TP_ARGS(pag, caller_ip),
>         TP_STRUCT__entry(
>                 __field(dev_t, dev)
>                 __field(xfs_agnumber_t, agno)
>                 __field(int, refcount)
> +               __field(int, active_refcount)
>                 __field(unsigned long, caller_ip)
>         ),
>         TP_fast_assign(
> -               __entry->dev = mp->m_super->s_dev;
> -               __entry->agno = agno;
> -               __entry->refcount = refcount;
> +               __entry->dev = pag->pag_mount->m_super->s_dev;
> +               __entry->agno = pag->pag_agno;
> +               __entry->refcount = atomic_read(&pag->pag_ref);
> +               __entry->active_refcount = atomic_read(&pag-
> >pag_active_ref);
>                 __entry->caller_ip = caller_ip;
>         ),
> -       TP_printk("dev %d:%d agno 0x%x refcount %d caller %pS",
> +       TP_printk("dev %d:%d agno 0x%x passive refs %d active refs %d
> caller %pS",
>                   MAJOR(__entry->dev), MINOR(__entry->dev),
>                   __entry->agno,
>                   __entry->refcount,
> +                 __entry->active_refcount,
>                   (char *)__entry->caller_ip)
>  );
>  
>  #define DEFINE_PERAG_REF_EVENT(name)   \
>  DEFINE_EVENT(xfs_perag_class, name,    \
> -       TP_PROTO(struct xfs_mount *mp, xfs_agnumber_t agno, int
> refcount,       \
> -                unsigned long
> caller_ip),                                      \
> -       TP_ARGS(mp, agno, refcount, caller_ip))
> +       TP_PROTO(struct xfs_perag *pag, unsigned long caller_ip), \
> +       TP_ARGS(pag, caller_ip))
>  DEFINE_PERAG_REF_EVENT(xfs_perag_get);
>  DEFINE_PERAG_REF_EVENT(xfs_perag_get_tag);
>  DEFINE_PERAG_REF_EVENT(xfs_perag_put);
diff mbox series

Patch

diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
index 46e25c682bf4..7cff61875340 100644
--- a/fs/xfs/libxfs/xfs_ag.c
+++ b/fs/xfs/libxfs/xfs_ag.c
@@ -44,16 +44,15 @@  xfs_perag_get(
 	xfs_agnumber_t		agno)
 {
 	struct xfs_perag	*pag;
-	int			ref = 0;
 
 	rcu_read_lock();
 	pag = radix_tree_lookup(&mp->m_perag_tree, agno);
 	if (pag) {
+		trace_xfs_perag_get(pag, _RET_IP_);
 		ASSERT(atomic_read(&pag->pag_ref) >= 0);
-		ref = atomic_inc_return(&pag->pag_ref);
+		atomic_inc(&pag->pag_ref);
 	}
 	rcu_read_unlock();
-	trace_xfs_perag_get(mp, agno, ref, _RET_IP_);
 	return pag;
 }
 
@@ -68,7 +67,6 @@  xfs_perag_get_tag(
 {
 	struct xfs_perag	*pag;
 	int			found;
-	int			ref;
 
 	rcu_read_lock();
 	found = radix_tree_gang_lookup_tag(&mp->m_perag_tree,
@@ -77,9 +75,9 @@  xfs_perag_get_tag(
 		rcu_read_unlock();
 		return NULL;
 	}
-	ref = atomic_inc_return(&pag->pag_ref);
+	trace_xfs_perag_get_tag(pag, _RET_IP_);
+	atomic_inc(&pag->pag_ref);
 	rcu_read_unlock();
-	trace_xfs_perag_get_tag(mp, pag->pag_agno, ref, _RET_IP_);
 	return pag;
 }
 
@@ -87,11 +85,9 @@  void
 xfs_perag_put(
 	struct xfs_perag	*pag)
 {
-	int	ref;
-
+	trace_xfs_perag_put(pag, _RET_IP_);
 	ASSERT(atomic_read(&pag->pag_ref) > 0);
-	ref = atomic_dec_return(&pag->pag_ref);
-	trace_xfs_perag_put(pag->pag_mount, pag->pag_agno, ref, _RET_IP_);
+	atomic_dec(&pag->pag_ref);
 }
 
 /*
@@ -110,8 +106,7 @@  xfs_perag_grab(
 	rcu_read_lock();
 	pag = radix_tree_lookup(&mp->m_perag_tree, agno);
 	if (pag) {
-		trace_xfs_perag_grab(mp, pag->pag_agno,
-				atomic_read(&pag->pag_active_ref), _RET_IP_);
+		trace_xfs_perag_grab(pag, _RET_IP_);
 		if (!atomic_inc_not_zero(&pag->pag_active_ref))
 			pag = NULL;
 	}
@@ -138,8 +133,7 @@  xfs_perag_grab_tag(
 		rcu_read_unlock();
 		return NULL;
 	}
-	trace_xfs_perag_grab_tag(mp, pag->pag_agno,
-			atomic_read(&pag->pag_active_ref), _RET_IP_);
+	trace_xfs_perag_grab_tag(pag, _RET_IP_);
 	if (!atomic_inc_not_zero(&pag->pag_active_ref))
 		pag = NULL;
 	rcu_read_unlock();
@@ -150,8 +144,7 @@  void
 xfs_perag_rele(
 	struct xfs_perag	*pag)
 {
-	trace_xfs_perag_rele(pag->pag_mount, pag->pag_agno,
-			atomic_read(&pag->pag_active_ref), _RET_IP_);
+	trace_xfs_perag_rele(pag, _RET_IP_);
 	if (atomic_dec_and_test(&pag->pag_active_ref))
 		wake_up(&pag->pag_active_wq);
 }
diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index 0f4a014dded3..8b2823d85a68 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -255,7 +255,7 @@  xfs_perag_set_inode_tag(
 		break;
 	}
 
-	trace_xfs_perag_set_inode_tag(mp, pag->pag_agno, tag, _RET_IP_);
+	trace_xfs_perag_set_inode_tag(pag, _RET_IP_);
 }
 
 /* Clear a tag on both the AG incore inode tree and the AG radix tree. */
@@ -289,7 +289,7 @@  xfs_perag_clear_inode_tag(
 	radix_tree_tag_clear(&mp->m_perag_tree, pag->pag_agno, tag);
 	spin_unlock(&mp->m_perag_lock);
 
-	trace_xfs_perag_clear_inode_tag(mp, pag->pag_agno, tag, _RET_IP_);
+	trace_xfs_perag_clear_inode_tag(pag, _RET_IP_);
 }
 
 /*
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index f0b62054ea68..c921e9a5256d 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -159,33 +159,34 @@  TRACE_EVENT(xlog_intent_recovery_failed,
 );
 
 DECLARE_EVENT_CLASS(xfs_perag_class,
-	TP_PROTO(struct xfs_mount *mp, xfs_agnumber_t agno, int refcount,
-		 unsigned long caller_ip),
-	TP_ARGS(mp, agno, refcount, caller_ip),
+	TP_PROTO(struct xfs_perag *pag, unsigned long caller_ip),
+	TP_ARGS(pag, caller_ip),
 	TP_STRUCT__entry(
 		__field(dev_t, dev)
 		__field(xfs_agnumber_t, agno)
 		__field(int, refcount)
+		__field(int, active_refcount)
 		__field(unsigned long, caller_ip)
 	),
 	TP_fast_assign(
-		__entry->dev = mp->m_super->s_dev;
-		__entry->agno = agno;
-		__entry->refcount = refcount;
+		__entry->dev = pag->pag_mount->m_super->s_dev;
+		__entry->agno = pag->pag_agno;
+		__entry->refcount = atomic_read(&pag->pag_ref);
+		__entry->active_refcount = atomic_read(&pag->pag_active_ref);
 		__entry->caller_ip = caller_ip;
 	),
-	TP_printk("dev %d:%d agno 0x%x refcount %d caller %pS",
+	TP_printk("dev %d:%d agno 0x%x passive refs %d active refs %d caller %pS",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
 		  __entry->agno,
 		  __entry->refcount,
+		  __entry->active_refcount,
 		  (char *)__entry->caller_ip)
 );
 
 #define DEFINE_PERAG_REF_EVENT(name)	\
 DEFINE_EVENT(xfs_perag_class, name,	\
-	TP_PROTO(struct xfs_mount *mp, xfs_agnumber_t agno, int refcount,	\
-		 unsigned long caller_ip),					\
-	TP_ARGS(mp, agno, refcount, caller_ip))
+	TP_PROTO(struct xfs_perag *pag, unsigned long caller_ip), \
+	TP_ARGS(pag, caller_ip))
 DEFINE_PERAG_REF_EVENT(xfs_perag_get);
 DEFINE_PERAG_REF_EVENT(xfs_perag_get_tag);
 DEFINE_PERAG_REF_EVENT(xfs_perag_put);