diff mbox

[004/119] xfs: enable buffer deadlock postmortem diagnosis via ftrace

Message ID 146612629822.12839.7938642541078923297.stgit@birch.djwong.org (mailing list archive)
State New, archived
Headers show

Commit Message

Darrick J. Wong June 17, 2016, 1:18 a.m. UTC
Create a second buf_trylock tracepoint so that we can distinguish
between a successful and a failed trylock.  With this piece, we can
use a script to look at the ftrace output to detect buffer deadlocks.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_buf.c   |    3 ++-
 fs/xfs/xfs_trace.h |    1 +
 2 files changed, 3 insertions(+), 1 deletion(-)



--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Christoph Hellwig June 17, 2016, 11:34 a.m. UTC | #1
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index efa2a73..2333db7 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -947,7 +947,8 @@ xfs_buf_trylock(
>  	if (locked)
>  		XB_SET_OWNER(bp);
>  
> -	trace_xfs_buf_trylock(bp, _RET_IP_);
> +	locked ? trace_xfs_buf_trylock(bp, _RET_IP_) :
> +		 trace_xfs_buf_trylock_fail(bp, _RET_IP_);
>  	return locked;

I think this should be something like:

	if (locked) {
		XB_SET_OWNER(bp);
		trace_xfs_buf_trylock(bp, _RET_IP_);
	} else {
		trace_xfs_buf_trylock_fail(bp, _RET_IP_);
	}

otherwise this looks good and can go in without the rest of the series.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dave Chinner June 21, 2016, 12:47 a.m. UTC | #2
On Fri, Jun 17, 2016 at 04:34:23AM -0700, Christoph Hellwig wrote:
> > diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> > index efa2a73..2333db7 100644
> > --- a/fs/xfs/xfs_buf.c
> > +++ b/fs/xfs/xfs_buf.c
> > @@ -947,7 +947,8 @@ xfs_buf_trylock(
> >  	if (locked)
> >  		XB_SET_OWNER(bp);
> >  
> > -	trace_xfs_buf_trylock(bp, _RET_IP_);
> > +	locked ? trace_xfs_buf_trylock(bp, _RET_IP_) :
> > +		 trace_xfs_buf_trylock_fail(bp, _RET_IP_);
> >  	return locked;
> 
> I think this should be something like:
> 
> 	if (locked) {
> 		XB_SET_OWNER(bp);
> 		trace_xfs_buf_trylock(bp, _RET_IP_);
> 	} else {
> 		trace_xfs_buf_trylock_fail(bp, _RET_IP_);
> 	}
> 
> otherwise this looks good and can go in without the rest of the series.

I'll fix that up on commit.

Cheers,

Dave.
diff mbox

Patch

diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index efa2a73..2333db7 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -947,7 +947,8 @@  xfs_buf_trylock(
 	if (locked)
 		XB_SET_OWNER(bp);
 
-	trace_xfs_buf_trylock(bp, _RET_IP_);
+	locked ? trace_xfs_buf_trylock(bp, _RET_IP_) :
+		 trace_xfs_buf_trylock_fail(bp, _RET_IP_);
 	return locked;
 }
 
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index ea94ee0..68f27f7 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -354,6 +354,7 @@  DEFINE_BUF_EVENT(xfs_buf_submit_wait);
 DEFINE_BUF_EVENT(xfs_buf_bawrite);
 DEFINE_BUF_EVENT(xfs_buf_lock);
 DEFINE_BUF_EVENT(xfs_buf_lock_done);
+DEFINE_BUF_EVENT(xfs_buf_trylock_fail);
 DEFINE_BUF_EVENT(xfs_buf_trylock);
 DEFINE_BUF_EVENT(xfs_buf_unlock);
 DEFINE_BUF_EVENT(xfs_buf_iowait);