diff mbox series

[10/10] xfs: trace log intent item recovery failures

Message ID 160704435695.734470.320027217185016602.stgit@magnolia (mailing list archive)
State Superseded
Headers show
Series xfs: strengthen log intent validation | expand

Commit Message

Darrick J. Wong Dec. 4, 2020, 1:12 a.m. UTC 
From: Darrick J. Wong <darrick.wong@oracle.com>

Add a trace point so that we can capture when a recovered log intent
item fails to recover.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_log_recover.c |    5 ++++-
 fs/xfs/xfs_trace.h       |   19 +++++++++++++++++++
 2 files changed, 23 insertions(+), 1 deletion(-)

Comments

Brian Foster Dec. 4, 2020, 2 p.m. UTC | #1 
On Thu, Dec 03, 2020 at 05:12:37PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Add a trace point so that we can capture when a recovered log intent
> item fails to recover.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_log_recover.c |    5 ++++-
>  fs/xfs/xfs_trace.h       |   19 +++++++++++++++++++
>  2 files changed, 23 insertions(+), 1 deletion(-)
> 
> 
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index 87886b7f77da..ed92c72976c9 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -2559,8 +2559,11 @@ xlog_recover_process_intents(
>  		spin_unlock(&ailp->ail_lock);
>  		error = lip->li_ops->iop_recover(lip, &capture_list);
>  		spin_lock(&ailp->ail_lock);
> -		if (error)
> +		if (error) {
> +			trace_xfs_error_return(log->l_mp, error,
> +					lip->li_ops->iop_recover);
>  			break;
> +		}
>  	}
>  
>  	xfs_trans_ail_cursor_done(&cur);
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index 86951652d3ed..99383b1acd49 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -103,6 +103,25 @@ DEFINE_ATTR_LIST_EVENT(xfs_attr_list_notfound);
>  DEFINE_ATTR_LIST_EVENT(xfs_attr_leaf_list);
>  DEFINE_ATTR_LIST_EVENT(xfs_attr_node_list);
>  
> +TRACE_EVENT(xfs_error_return,

xfs_error_return seems rather vague of a name given the current use.

> +	TP_PROTO(struct xfs_mount *mp, int error, void *caller_ip),
> +	TP_ARGS(mp, error, caller_ip),
> +	TP_STRUCT__entry(
> +		__field(dev_t, dev)
> +		__field(int, error)
> +		__field(void *, caller_ip)
> +	),
> +	TP_fast_assign(
> +		__entry->dev = mp->m_super->s_dev;
> +		__entry->error = error;
> +		__entry->caller_ip = caller_ip;
> +	),
> +	TP_printk("dev %d:%d error %d caller %pS",
> +		  MAJOR(__entry->dev), MINOR(__entry->dev),
> +		  __entry->error, __entry->caller_ip)
> +

Extra whitespace. Also, using the text "caller" here is a bit misleading
IMO. I'd suggest just calling it "function" or some such, but not that
big of a deal.

Brian

> +);
> +
>  DECLARE_EVENT_CLASS(xfs_perag_class,
>  	TP_PROTO(struct xfs_mount *mp, xfs_agnumber_t agno, int refcount,
>  		 unsigned long caller_ip),
>
Darrick J. Wong Dec. 4, 2020, 7:57 p.m. UTC | #2 
On Fri, Dec 04, 2020 at 09:00:52AM -0500, Brian Foster wrote:
> On Thu, Dec 03, 2020 at 05:12:37PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Add a trace point so that we can capture when a recovered log intent
> > item fails to recover.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > ---
> >  fs/xfs/xfs_log_recover.c |    5 ++++-
> >  fs/xfs/xfs_trace.h       |   19 +++++++++++++++++++
> >  2 files changed, 23 insertions(+), 1 deletion(-)
> > 
> > 
> > diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> > index 87886b7f77da..ed92c72976c9 100644
> > --- a/fs/xfs/xfs_log_recover.c
> > +++ b/fs/xfs/xfs_log_recover.c
> > @@ -2559,8 +2559,11 @@ xlog_recover_process_intents(
> >  		spin_unlock(&ailp->ail_lock);
> >  		error = lip->li_ops->iop_recover(lip, &capture_list);
> >  		spin_lock(&ailp->ail_lock);
> > -		if (error)
> > +		if (error) {
> > +			trace_xfs_error_return(log->l_mp, error,
> > +					lip->li_ops->iop_recover);
> >  			break;
> > +		}
> >  	}
> >  
> >  	xfs_trans_ail_cursor_done(&cur);
> > diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> > index 86951652d3ed..99383b1acd49 100644
> > --- a/fs/xfs/xfs_trace.h
> > +++ b/fs/xfs/xfs_trace.h
> > @@ -103,6 +103,25 @@ DEFINE_ATTR_LIST_EVENT(xfs_attr_list_notfound);
> >  DEFINE_ATTR_LIST_EVENT(xfs_attr_leaf_list);
> >  DEFINE_ATTR_LIST_EVENT(xfs_attr_node_list);
> >  
> > +TRACE_EVENT(xfs_error_return,
> 
> xfs_error_return seems rather vague of a name given the current use.

Yeah, I'll change it to xlog_intent_recovery_failed.

> > +	TP_PROTO(struct xfs_mount *mp, int error, void *caller_ip),
> > +	TP_ARGS(mp, error, caller_ip),
> > +	TP_STRUCT__entry(
> > +		__field(dev_t, dev)
> > +		__field(int, error)
> > +		__field(void *, caller_ip)
> > +	),
> > +	TP_fast_assign(
> > +		__entry->dev = mp->m_super->s_dev;
> > +		__entry->error = error;
> > +		__entry->caller_ip = caller_ip;
> > +	),
> > +	TP_printk("dev %d:%d error %d caller %pS",
> > +		  MAJOR(__entry->dev), MINOR(__entry->dev),
> > +		  __entry->error, __entry->caller_ip)
> > +
> 
> Extra whitespace. Also, using the text "caller" here is a bit misleading
> IMO. I'd suggest just calling it "function" or some such, but not that
> big of a deal.

Fixed, thanks.

--D

> Brian
> 
> > +);
> > +
> >  DECLARE_EVENT_CLASS(xfs_perag_class,
> >  	TP_PROTO(struct xfs_mount *mp, xfs_agnumber_t agno, int refcount,
> >  		 unsigned long caller_ip),
> > 
>
diff mbox series

Patch

diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 87886b7f77da..ed92c72976c9 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -2559,8 +2559,11 @@  xlog_recover_process_intents(
 		spin_unlock(&ailp->ail_lock);
 		error = lip->li_ops->iop_recover(lip, &capture_list);
 		spin_lock(&ailp->ail_lock);
-		if (error)
+		if (error) {
+			trace_xfs_error_return(log->l_mp, error,
+					lip->li_ops->iop_recover);
 			break;
+		}
 	}
 
 	xfs_trans_ail_cursor_done(&cur);
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index 86951652d3ed..99383b1acd49 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -103,6 +103,25 @@  DEFINE_ATTR_LIST_EVENT(xfs_attr_list_notfound);
 DEFINE_ATTR_LIST_EVENT(xfs_attr_leaf_list);
 DEFINE_ATTR_LIST_EVENT(xfs_attr_node_list);
 
+TRACE_EVENT(xfs_error_return,
+	TP_PROTO(struct xfs_mount *mp, int error, void *caller_ip),
+	TP_ARGS(mp, error, caller_ip),
+	TP_STRUCT__entry(
+		__field(dev_t, dev)
+		__field(int, error)
+		__field(void *, caller_ip)
+	),
+	TP_fast_assign(
+		__entry->dev = mp->m_super->s_dev;
+		__entry->error = error;
+		__entry->caller_ip = caller_ip;
+	),
+	TP_printk("dev %d:%d error %d caller %pS",
+		  MAJOR(__entry->dev), MINOR(__entry->dev),
+		  __entry->error, __entry->caller_ip)
+
+);
+
 DECLARE_EVENT_CLASS(xfs_perag_class,
 	TP_PROTO(struct xfs_mount *mp, xfs_agnumber_t agno, int refcount,
 		 unsigned long caller_ip),