Re: oops with dual xeon 2.8ghz 4gb ram +smp, software raid, lvm, and xfs

[Christoph Hellwig <hch@infradead.org>]

On Tue, Nov 23, 2004 at 06:02:23PM +0100, Jakob Oestergaard wrote:
> With SMP, what I see is that sometimes a directory might decide that
> it's a file - but I can't delete it, becuase it isn't 'empty' (it's
> still somehow a directory).  Waiting a day or two, the system will
> change its mind back to letting the directory be a directory. Sometimes
> modes will be fscked up as well - a regular file can change owner, or it
> can change modes from '-rw-rw---' to '?---------'.    Weird stuff, no
> way to reproduce it reliably.

Actually I can reproduce it reliably by running nfs_fsstress.sh for a
looong time.  The problem is that in the current XFS code the inode
generation counter starts at 0, but higher level code uses that as
a wildcard for any possible generation, so you may get a newly created
file for a stale nfs file handler of an deleted file with the same inode
number.

The patch below fixes it for me:


Index: fs/xfs/xfs_inode.c
===================================================================
RCS file: /cvs/linux-2.6-xfs/fs/xfs/xfs_inode.c,v
retrieving revision 1.406
diff -u -p -r1.406 xfs_inode.c
--- fs/xfs/xfs_inode.c	27 Oct 2004 12:06:24 -0000	1.406
+++ fs/xfs/xfs_inode.c	23 Nov 2004 20:40:56 -0000
@@ -1224,9 +1224,16 @@ xfs_ialloc(
 	ip->i_d.di_nextents = 0;
 	ASSERT(ip->i_d.di_nblocks == 0);
 	xfs_ichgtime(ip, XFS_ICHGTIME_CHG|XFS_ICHGTIME_ACC|XFS_ICHGTIME_MOD);
+
 	/*
-	 * di_gen will have been taken care of in xfs_iread.
+	 * Bump the generation count so no one will confuse us with an
+	 * earlier incarnations of this inode.
+	 *
+	 * Done early to skip generation 0, which is used as a wildcard
+	 * by higher level code.
 	 */
+	ip->i_d.di_gen++;
+
 	ip->i_d.di_extsize = 0;
 	ip->i_d.di_dmevmask = 0;
 	ip->i_d.di_dmstate = 0;
@@ -2370,11 +2377,6 @@ xfs_ifree(
 		XFS_IFORK_DSIZE(ip) / (uint)sizeof(xfs_bmbt_rec_t);
 	ip->i_d.di_format = XFS_DINODE_FMT_EXTENTS;
 	ip->i_d.di_aformat = XFS_DINODE_FMT_EXTENTS;
-	/*
-	 * Bump the generation count so no one will be confused
-	 * by reincarnations of this inode.
-	 */
-	ip->i_d.di_gen++;
 	xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
 
 	if (delete) {