[RFC, PATCH] XFS_TRANS_DEBUG fixes
Lachlan McIlroy
lachlan at sgi.com
Fri Dec 5 01:10:24 CST 2008
XFS_TRANS_DEBUG is not enabled on a debug build and has suffered some bit rot.
Below is a set of fixes to get it working again. Whether it's of any use I don't
know but if we are going to carry the code we might as well make it work.
Some of the things I had to do to get it to work (and could be done some other
way) are:
- all buffers that are logged need to be mapped into kernel space so the
debugging code can make a copy of the buffer data and compare it later.
The easiest way to do that is to make all buffers mapped in xfs_bug_get_flags()
when XFS_TRANS_DEBUG is set.
- turning XFS_TRANS_DEBUG on does make the system run really slow (even slower
than a normal debug build) so we might want to keep it optional.
- Some bit setting functions (btst()/bset()/bfset()) appear to be missing so
I've coded up some trivial versions. There maybe some linux kernel functions
that do the same thing.
- the transaction debugging code will detect if we have modified more data in a
buffer than we have indicated to be logged. This picked up two places where
we modify a inode cluster buffer without logging it - firstly when we create
a new inode cluster we zero the entire buffer but only log the inode fields
(ie data after the xfs_dicore_t finishes is modified) and secondly the bulkstat
hack that zeroes the di_mode. I don't know if not logging these buffer changes
is actually a bug or not.
Is this debugging code worth hanging onto or should we just ditch the whole lot?
diff --git a/fs/xfs/linux-2.6/xfs_buf.c b/fs/xfs/linux-2.6/xfs_buf.c
index cd89c56..4ee182c 100644
--- a/fs/xfs/linux-2.6/xfs_buf.c
+++ b/fs/xfs/linux-2.6/xfs_buf.c
@@ -584,6 +584,10 @@ xfs_buf_get_flags(
xfs_buf_t *bp, *new_bp;
int error = 0, i;
+#ifdef XFS_TRANS_DEBUG
+ flags |= XBF_MAPPED;
+#endif
+
new_bp = xfs_buf_allocate(flags);
if (unlikely(!new_bp))
return NULL;
diff --git a/fs/xfs/xfs.h b/fs/xfs/xfs.h
index 17254b5..3df4a54 100644
--- a/fs/xfs/xfs.h
+++ b/fs/xfs/xfs.h
@@ -38,6 +38,7 @@
#define XFS_RW_TRACE 1
#define XFS_BUF_TRACE 1
#define XFS_INODE_TRACE 1
+#define XFS_TRANS_DEBUG 1
#define XFS_FILESTREAMS_TRACE 1
#endif
diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
index d245d04..bb019a0 100644
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -35,6 +35,37 @@ kmem_zone_t *xfs_buf_item_zone;
#ifdef XFS_TRANS_DEBUG
/*
+ * Test bit b in bitmap bp
+ */
+int
+btst(char *bp, int b)
+{
+ return (*(bp + (b >> 3)) & (0x80 >> (b & 0x7)));
+}
+
+/*
+ * Set bit b in bitmap bp
+ */
+void
+bset(char *bp, int b)
+{
+ *(bp + (b >> 3)) |= (0x80 >> (b & 0x7));
+}
+
+/*
+ * Set a bit field of length len in bitmap bp starting at b
+ */
+void
+bfset(char *bp, int b, int len)
+{
+ while (len) {
+ bset(bp, b);
+ len--;
+ b++;
+ }
+}
+
+/*
* This function uses an alternate strategy for tracking the bytes
* that the user requests to be logged. This can then be used
* in conjunction with the bli_orig array in the buf log item to
@@ -126,7 +157,7 @@ xfs_buf_item_log_check(
for (x = 0; x < XFS_BUF_COUNT(bp); x++) {
if (orig[x] != buffer[x] && !btst(bip->bli_logged, x))
cmn_err(CE_PANIC,
- "xfs_buf_item_log_check bip %x buffer %x orig %x index %d",
+ "xfs_buf_item_log_check bip %p buffer %p orig %p index %d",
bip, bp, orig, x);
}
}
diff --git a/fs/xfs/xfs_ialloc.c b/fs/xfs/xfs_ialloc.c
index e6ebbae..681d622 100644
--- a/fs/xfs/xfs_ialloc.c
+++ b/fs/xfs/xfs_ialloc.c
@@ -353,6 +353,8 @@ xfs_ialloc_ag_alloc(
* transactions causing a lot of log traffic.
*/
xfs_biozero(fbuf, 0, ninodes << args.mp->m_sb.sb_inodelog);
+ xfs_trans_log_buf(tp, fbuf, 0,
+ (ninodes << args.mp->m_sb.sb_inodelog) - 1);
for (i = 0; i < ninodes; i++) {
int ioffset = i << args.mp->m_sb.sb_inodelog;
uint isize = sizeof(struct xfs_dinode);
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 063da34..9d5df3a 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -2121,6 +2121,7 @@ xfs_ifree(
{
int error;
int delete;
+ int offset;
xfs_ino_t first_ino;
xfs_dinode_t *dip;
xfs_buf_t *ibp;
@@ -2179,6 +2180,8 @@ xfs_ifree(
* in the future.
*/
dip->di_mode = 0;
+ offset = ip->i_imap.im_boffset + offsetof(xfs_dinode_t, di_mode);
+ xfs_trans_log_buf(tp, ibp, offset, (offset + sizeof(__be16) - 1));
if (delete) {
xfs_ifree_cluster(ip, tp, first_ino);
@@ -2598,9 +2601,7 @@ xfs_iflush_fork(
char *cp;
xfs_ifork_t *ifp;
xfs_mount_t *mp;
-#ifdef XFS_TRANS_DEBUG
- int first;
-#endif
+
static const short brootflag[2] =
{ XFS_ILOG_DBROOT, XFS_ILOG_ABROOT };
static const short dataflag[2] =
@@ -3004,9 +3005,6 @@ xfs_iflush_int(
xfs_inode_log_item_t *iip;
xfs_dinode_t *dip;
xfs_mount_t *mp;
-#ifdef XFS_TRANS_DEBUG
- int first;
-#endif
ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL|XFS_ILOCK_SHARED));
ASSERT(!completion_done(&ip->i_flush));
--- a/fs/xfs/xfsidbg.c 2008-12-04 14:04:58.000000000 +1100
+++ b/fs/xfs/xfsidbg.c 2008-12-04 13:58:54.000000000 +1100
@@ -3231,7 +3208,7 @@ xfs_buf_item_print(xfs_buf_log_item_t *b
kdb_printf("blf flags: ");
printflags((uint)blip->bli_format.blf_flags, blf_flags, NULL);
#ifdef XFS_TRANS_DEBUG
- kdb_printf("orig 0x%x logged 0x%x",
+ kdb_printf("orig 0x%p logged 0x%p",
blip->bli_orig, blip->bli_logged);
#endif
kdb_printf("\n");
@@ -3599,7 +3576,7 @@ xfs_inode_item_print(xfs_inode_log_item_
ilip->ili_ilock_recur, ilip->ili_iolock_recur,
ilip->ili_extents_buf);
#ifdef XFS_TRANS_DEBUG
- kdb_printf("root bytes %d root orig 0x%x\n",
+ kdb_printf("root bytes %d root orig 0x%p\n",
ilip->ili_root_size, ilip->ili_orig_root);
#endif
kdb_printf("size %d ", ilip->ili_format.ilf_size);
More information about the xfs
mailing list