[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[Cluster-devel] [RFC 3/3] GFS2 rename race - core changes



Upon failure of gfs2_unlink_ok(), we discard the old dentry, create a new dentry, and then fill in the correct inode contents. This is to avoid gfs_rename() returns ENOENT for destination file changes.


--- gfs2-rename2/fs/gfs2/inode.h	2006-12-11 01:50:23.000000000 -0500
+++ gfs2-kernel/fs/gfs2/inode.h	2006-12-11 01:55:54.000000000 -0500
@@ -47,7 +47,7 @@ struct inode *gfs2_createi(struct gfs2_h
 int gfs2_rmdiri(struct gfs2_inode *dip, const struct qstr *name,
 		struct gfs2_inode *ip);
 int gfs2_unlink_ok(struct gfs2_inode *dip, const struct qstr *name,
-		   struct gfs2_inode *ip);
+		   struct gfs2_inode *ip, struct gfs2_inum_host *inum);
 int gfs2_ok_to_move(struct gfs2_inode *this, struct gfs2_inode *to);
 int gfs2_readlinki(struct gfs2_inode *ip, char **buf, unsigned int *len);
 int gfs2_glock_nq_atime(struct gfs2_holder *gh);
--- gfs2-rename2/fs/gfs2/inode.c	2006-12-11 01:50:23.000000000 -0500
+++ gfs2-kernel/fs/gfs2/inode.c	2006-12-11 03:08:10.000000000 -0500
@@ -978,7 +978,8 @@ int gfs2_rmdiri(struct gfs2_inode *dip, 
  */
 
 int gfs2_unlink_ok(struct gfs2_inode *dip, const struct qstr *name,
-		   struct gfs2_inode *ip)
+		   struct gfs2_inode *ip,
+		   struct gfs2_inum_host *inump)
 {
 	struct gfs2_inum_host inum;
 	unsigned int type;
@@ -1003,8 +1004,17 @@ int gfs2_unlink_ok(struct gfs2_inode *di
 	if (error)
 		return error;
 
-	if (!gfs2_inum_equal(&inum, &ip->i_num))
+	if (!gfs2_inum_equal(&inum, &ip->i_num)) {
+
+		/*
+		 * Add for rename - bugzilla 204616
+		 *   return new inum such that gfs_rename can
+		 *   do its silly rename.
+		 */
+		if (inump) *inump = inum;
+
 		return -ENOENT;
+	}
 
 	if (IF2DT(ip->i_inode.i_mode) != type) {
 		gfs2_consist_inode(dip);
--- gfs2-rename2/fs/gfs2/ops_inode.c	2006-12-11 01:50:23.000000000 -0500
+++ gfs2-kernel/fs/gfs2/ops_inode.c	2006-12-11 10:23:06.000000000 -0500
@@ -274,7 +274,7 @@ static int gfs2_unlink(struct inode *dir
 	if (error)
 		goto out;
 
-	error = gfs2_unlink_ok(dip, &dentry->d_name, ip);
+	error = gfs2_unlink_ok(dip, &dentry->d_name, ip, NULL);
 	if (error)
 		goto out_gunlock;
 
@@ -459,7 +459,7 @@ static int gfs2_rmdir(struct inode *dir,
 	if (error)
 		goto out;
 
-	error = gfs2_unlink_ok(dip, &dentry->d_name, ip);
+	error = gfs2_unlink_ok(dip, &dentry->d_name, ip, NULL);
 	if (error)
 		goto out_gunlock;
 
@@ -529,6 +529,55 @@ static int gfs2_mknod(struct inode *dir,
 	return 0;
 }
 
+/*
+ * Obtain a clone dentry with correct inode:
+ *
+ * This is to work around the window between lock_rename() 
+ * and gfs2_rename() where another node may have renamed the 
+ * target file (bugzilla 204616). This bug most likely occurs
+ * in a mail server environment.
+ *
+ * Return true if the dentry has been successfully updated.
+ */
+static int
+gfs2_refresh_dentry(struct super_block *sb, struct dentry **f_dentry, struct gfs2_inum_host *inum_p) 
+{
+	struct inode *inode;
+	struct dentry *dentry, *sdentry=*f_dentry;
+
+	dentry = d_alloc(sdentry->d_parent,
+			 &sdentry->d_name);
+	if (!dentry) 
+		return 0;
+
+	/* Get the fully updated inode */
+	inode = NULL;
+	if (inum_p->no_formal_ino) {
+		inode = gfs2_refresh_iobj(sb, inum_p);
+		if (IS_ERR(inode)) { 
+			dput(dentry);
+			return 0;
+		}
+	} 
+
+	/* 
+	 * Remove the old dentry from cache to avoid 
+	 * another lookup.
+	 */
+	if (!d_unhashed(sdentry)) {
+		d_drop(sdentry);
+	}
+
+	/* Instantiate the new dentry */
+	dentry->d_op = &gfs2_dops;
+
+	d_splice_alias(inode, dentry);
+
+	/* Return with success */
+	*f_dentry = dentry;
+	return 1;
+}
+
 /**
  * gfs2_rename - Rename a file
  * @odir: Parent directory of old file name
@@ -547,12 +596,13 @@ static int gfs2_rename(struct inode *odi
 	struct gfs2_inode *ip = GFS2_I(odentry->d_inode);
 	struct gfs2_inode *nip = NULL;
 	struct gfs2_sbd *sdp = GFS2_SB(odir);
-	struct gfs2_holder ghs[4], r_gh;
+	struct gfs2_holder ghs[5], r_gh;
 	unsigned int num_gh=0;
 	int dir_rename = 0;
 	int alloc_required;
 	unsigned int x;
-	int error;
+	int error, refresh_cnt=0, handle_ndentry=0;
+	struct gfs2_inum_host inum;
 
 	if (ndentry->d_inode) {
 		nip = GFS2_I(ndentry->d_inode);
@@ -568,6 +618,7 @@ static int gfs2_rename(struct inode *odi
 		gfs2_holder_init(ndip->i_gl, LM_ST_EXCLUSIVE, 0, ghs);
 		num_gh++;
 	}
+	odir->i_sb->s_type->fs_flags &= ~FS_RENAME_DOES_D_MOVE;
 
 	/* Make sure we aren't trying to move a dirctory into it's subdir */
 	if (S_ISDIR(ip->i_inode.i_mode) && odip != ndip) {
@@ -579,7 +630,7 @@ static int gfs2_rename(struct inode *odi
 
 	gfs2_holder_init(odip->i_gl, LM_ST_EXCLUSIVE, 0, ghs + num_gh);
 	gfs2_holder_init(ip->i_gl, LM_ST_EXCLUSIVE, 0, ghs + num_gh);
-	num_gh += 2
+	num_gh += 2;
 
 	if (nip) {
 		gfs2_holder_init(nip->i_gl, LM_ST_EXCLUSIVE, 0, ghs + num_gh);
@@ -591,16 +642,38 @@ static int gfs2_rename(struct inode *odi
 		goto out_uninit;
 
 	/* Check out the old directory */
-	error = gfs2_unlink_ok(odip, &odentry->d_name, ip);
+	error = gfs2_unlink_ok(odip, &odentry->d_name, ip, NULL);
 	if (error)
 		goto out_gunlock;
 
 	/* Check out the new directory */
+gfs2_rename_restart:
 	if (nip) {
-		error = gfs2_unlink_ok(ndip, &ndentry->d_name, nip);
-		if (error)
+		inum.no_formal_ino = 0;
+		error = gfs2_unlink_ok(ndip, &ndentry->d_name, nip, &inum);
+		if (unlikely(error)) {
+			/* bugzilla 204616:
+			 * Handle a rare race condition that the new file
+			 * could have been renamed by another node - the
+			 * new inode number is passed into inum_p. We can
+			 * only go thru this logic once.
+			 */
+			if ((error == -ENOENT) && (!refresh_cnt)) {
+				refresh_cnt++;
+				if (gfs2_refresh_dentry(ndir->i_sb, &ndentry, &inum)){
+					odir->i_sb->s_type->fs_flags 
+						|= FS_RENAME_DOES_D_MOVE;
+					handle_ndentry = 1;
+					nip = GFS2_I(ndentry->d_inode);
+					if (nip)
+						gfs2_glock_nq_init(nip->i_gl,
+						LM_ST_EXCLUSIVE, 0,
+						&ghs[num_gh++]);
+					goto gfs2_rename_restart;
+				}
+			}
 			goto out_gunlock;
-
+		}
 		if (S_ISDIR(nip->i_inode.i_mode)) {
 			if (nip->i_di.di_entries < 2) {
 				if (gfs2_consist_inode(nip))
@@ -757,6 +830,15 @@ out_uninit:
 out_gunlock_r:
 	gfs2_glock_dq_uninit(&r_gh);
 out:
+	
+	/* We need to dput the new dentry since vfs layer
+	 * still use the old entry */
+	if (handle_ndentry) {
+		d_move(odentry, ndentry);
+		dput(ndentry);
+		dput(ndentry);
+	}
+
 	return error;
 }
 
@@ -1128,4 +1210,3 @@ struct inode_operations gfs2_symlink_iop
 	.listxattr = gfs2_listxattr,
 	.removexattr = gfs2_removexattr,
 };
-

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]