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

[Cluster-devel] [RFC 2/3] GFS2 rename race - lock reordering



Currently GFS2 only obtains its rename global locks if the "source" is a directory. This could result a deadlock if:

- Node 1 does "rename dir1/file-a dir2/file-b"
- Node 2 does "rename dir2/file-c dir1/file-a"

We'll unconditionally grab the global lock from now on unless the "source" and "destination" are from the same directory.



--- gfs2-2.6/fs/gfs2/ops_inode.c	2006-12-05 15:22:48.000000000 -0500
+++ gfs2-kernel/fs/gfs2/ops_inode.c	2006-12-10 23:35:42.000000000 -0500
@@ -548,7 +548,7 @@ static int gfs2_rename(struct inode *odi
 	struct gfs2_inode *nip = NULL;
 	struct gfs2_sbd *sdp = GFS2_SB(odir);
 	struct gfs2_holder ghs[4], r_gh;
-	unsigned int num_gh;
+	unsigned int num_gh=0;
 	int dir_rename = 0;
 	int alloc_required;
 	unsigned int x;
@@ -560,29 +560,26 @@ static int gfs2_rename(struct inode *odi
 			return 0;
 	}
 
-	/* Make sure we aren't trying to move a dirctory into it's subdir */
-
-	if (S_ISDIR(ip->i_inode.i_mode) && odip != ndip) {
-		dir_rename = 1;
-
-		error = gfs2_glock_nq_init(sdp->sd_rename_gl, LM_ST_EXCLUSIVE, 0,
-					   &r_gh);
+	if (odip != ndip) {
+		error = gfs2_glock_nq_init(sdp->sd_rename_gl, LM_ST_EXCLUSIVE, 
+					0, &r_gh);
 		if (error)
 			goto out;
+		gfs2_holder_init(ndip->i_gl, LM_ST_EXCLUSIVE, 0, ghs);
+		num_gh++;
+	}
 
+	/* Make sure we aren't trying to move a dirctory into it's subdir */
+	if (S_ISDIR(ip->i_inode.i_mode) && odip != ndip) {
+		dir_rename = 1;
 		error = gfs2_ok_to_move(ip, ndip);
 		if (error)
 			goto out_gunlock_r;
 	}
 
-	num_gh = 1;
-	gfs2_holder_init(odip->i_gl, LM_ST_EXCLUSIVE, 0, ghs);
-	if (odip != ndip) {
-		gfs2_holder_init(ndip->i_gl, LM_ST_EXCLUSIVE, 0, ghs + num_gh);
-		num_gh++;
-	}
+	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++;
+	num_gh += 2
 
 	if (nip) {
 		gfs2_holder_init(nip->i_gl, LM_ST_EXCLUSIVE, 0, ghs + num_gh);
@@ -594,13 +591,11 @@ static int gfs2_rename(struct inode *odi
 		goto out_uninit;
 
 	/* Check out the old directory */
-
 	error = gfs2_unlink_ok(odip, &odentry->d_name, ip);
 	if (error)
 		goto out_gunlock;
 
 	/* Check out the new directory */
-
 	if (nip) {
 		error = gfs2_unlink_ok(ndip, &ndentry->d_name, nip);
 		if (error)
@@ -760,8 +755,7 @@ out_uninit:
 	for (x = 0; x < num_gh; x++)
 		gfs2_holder_uninit(ghs + x);
 out_gunlock_r:
-	if (dir_rename)
-		gfs2_glock_dq_uninit(&r_gh);
+	gfs2_glock_dq_uninit(&r_gh);
 out:
 	return error;
 }

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