Skip to contentSkip to conversation

xfs: minor inode unlink debug enhancements

PATCH In review Brian Foster 2 participants 5 messages Apr 25, 2013 – Apr 26, 2013 XFS filesystem

lore
Share & tools
lore

Brian Foster

author
#0 lore raw

cover xfs: minor inode unlink debug enhancements

The following couple of patches provide a bit more debug info in the inode unlink code paths. This data proved useful for the investigation around a recent (downstream/distro) link/unlink race problem:

http://oss.sgi.com/archives/xfs/2013-04/msg00342.html

Brian

Brian Foster (1):
xfs: tracepoints for xfs_iunlink() and xfs_iunlink_remove()

Eric Sandeen (1):
xfs: enhance warning on xfs_imap() failure in xfs_iunlink_remove()

 fs/xfs/xfs_inode.c |   10 ++++++++--
 fs/xfs/xfs_trace.h |    2 ++
 2 files changed, 10 insertions(+), 2 deletions(-)
Signature · 6 lines
1.7.7.6

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
author
#1 lore raw

PATCH 1/2 xfs: tracepoints for xfs_iunlink() and xfs_iunlink_remove()

Add tracepoints for xfs_iunlink() and xfs_iunlink_remove(). These tracepoints are useful for debugging inode link (and unlinked list issues).

Signed-off-by Brian Foster <[email protected]>
---
 fs/xfs/xfs_inode.c |    4 ++++
 fs/xfs/xfs_trace.h |    2 ++
 2 files changed, 6 insertions(+), 0 deletions(-)
fs/xfs/xfs_inode.c +4 -0
index 4f20165..c22f980 100644
@@ -1519,6 +1519,8 @@ xfs_iunlink(
 
 	mp = tp->t_mountp;
 
+	trace_xfs_iunlink(ip);
+
 	/*
 	 * Get the agi buffer first.  It ensures lock ordering
 	 * on the list.

@@ -1598,6 +1600,8 @@ xfs_iunlink_remove(
 	mp = tp->t_mountp;
 	agno = XFS_INO_TO_AGNO(mp, ip->i_ino);
 
+	trace_xfs_iunlink_remove(ip);
+
 	/*
 	 * Get the agi buffer first.  It ensures lock ordering
 	 * on the list.
fs/xfs/xfs_trace.h +2 -0
index aa4db33..43bd572 100644
@@ -586,6 +586,8 @@ DEFINE_INODE_EVENT(xfs_file_fsync);
 DEFINE_INODE_EVENT(xfs_destroy_inode);
 DEFINE_INODE_EVENT(xfs_evict_inode);
 DEFINE_INODE_EVENT(xfs_update_time);
+DEFINE_INODE_EVENT(xfs_iunlink);
+DEFINE_INODE_EVENT(xfs_iunlink_remove);
 
 DEFINE_INODE_EVENT(xfs_dquot_dqalloc);
 DEFINE_INODE_EVENT(xfs_dquot_dqdetach);
Signature · 6 lines
1.7.7.6

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Reviewed-by on v1 1/2
#2 lore raw

Re: [PATCH 1/2] xfs: tracepoints for xfs_iunlink() and xfs_iunlink_remove()

Quoted 5 lines · Brian Foster

Add tracepoints for xfs_iunlink() and xfs_iunlink_remove(). These tracepoints are useful for debugging inode link (and unlinked list issues).

Signed-off-by Brian Foster <[email protected]>

Looks good.

Reviewed-by Dave Chinner <[email protected]>
Signature · 7 lines
Dave Chinner
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
author
#3 lore raw

PATCH 2/2 xfs: enhance warning on xfs_imap() failure in xfs_iunlink_remove()

In reply to Brian Foster

From: Eric Sandeen <[email protected]>

Include the global ino, ag number and agino in the warning message emitted should xfs_imap() fail in the xfs_iunlink_remove() code path.

Signed-off-by Brian Foster <[email protected]>
---
 fs/xfs/xfs_inode.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)
fs/xfs/xfs_inode.c +4 -2
index c22f980..a4fe989 100644
@@ -1678,8 +1678,10 @@ xfs_iunlink_remove(
 			error = xfs_imap(mp, tp, next_ino, &imap, 0);
 			if (error) {
 				xfs_warn(mp,
-	"%s: xfs_imap returned error %d.",
-					 __func__, error);
+					"%s: xfs_imap() returned error %d "
+					"for inode 0x%llx ag %d agino 0x%x",
+					__func__, error, ip->i_ino, agno, agino);
+
 				return error;
 			}
Signature · 6 lines
1.7.7.6

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
#4 lore raw

Re: [PATCH 2/2] xfs: enhance warning on xfs_imap() failure in xfs_iunlink_remove()

Quoted 16 lines · Brian Foster

From: Eric Sandeen <[email protected]>

Include the global ino, ag number and agino in the warning message emitted should xfs_imap() fail in the xfs_iunlink_remove() code path.

Signed-off-by Brian Foster <[email protected]>
---
 fs/xfs/xfs_inode.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)
fs/xfs/xfs_inode.c +0 -0
index c22f980..a4fe989 100644

fs/xfs/xfs_inode.c xfs_iunlink_remove( · line 1684

Quoted 9 lines
fs/xfs/xfs_inode.c +4 -2
@@ -1678,8 +1678,10 @@ xfs_iunlink_remove(
 			error = xfs_imap(mp, tp, next_ino, &imap, 0);
 			if (error) {
 				xfs_warn(mp,
-	"%s: xfs_imap returned error %d.",
-					 __func__, error);
+					"%s: xfs_imap() returned error %d "
+					"for inode 0x%llx ag %d agino 0x%x",
+					__func__, error, ip->i_ino, agno, agino);
+

The fact the format string is now split across 2 lines makes it hard to grep for. The indentation should be a clue that the warning code is intentionally formatted that way so that long format strings don't get wrapped. i.e:

xfs_warn(mp,
- "%s: xfs_imap returned error %d.",
- __func__, error);
+ "%s: xfs_imap() returned error %d for inode 0x%llx ag %d agino 0x%x",
+ __func__, error, ip->i_ino,
+ agno, agino);

fits into 80 columns and is much easier to read than the highly indented version split across 2 lines....

Cheers,

Dave.

Signature · 7 lines
Dave Chinner
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs