From: Fan Yong <fan.yong@intel.com>
Date: Sat, 3 Oct 2015 07:49:46 +0000 (+0800)
Subject: LU-7447 lfsck: correct nlink attr for new created dir
X-Git-Tag: 2.7.64~14
X-Git-Url: https://git.whamcloud.com/?p=fs%2Flustre-release.git;a=commitdiff_plain;h=5434d9402f6deb46f25bcf4c92726711b9d77d00

LU-7447 lfsck: correct nlink attr for new created dir

For new created directory object, there are three main operations:
1) insert dot entry
2) insert dotdot entry
3) ref_add on the directory object.

Sometimes, the 3rd step maybe ahead of 1st, sometimes maybe between
1st and 2nd. Usually, the developers would think that such order is
not important. But in fact, such assumption isn't true, because the
ldiskfs will set the new created directory object's nlink attr as 2
when inserting dotdot entry. Then if ref_add() is called after ".."
inserted, the empty directory object's nlink attr is 3.

To avoid above trouble and make the OSD API to be order-independent,
we will make the osd-ldiskfs to backup the new created dir object's
nlink attr before calling ldiskfs_add_dot_dotdot(), then restore it
after ldiskfs_add_dot_dotdot() called.

On the other hand, this patch also adds some missed logic for lfsck
declaring to insert dot/dotdot entries, remove some redundant logic.

Add e2fsck check for on-disk consistency verification after LFSCK.

Signed-off-by: Fan Yong <fan.yong@intel.com>
Change-Id: I065f683388417f3e23d47471c065419bd9bfcb19
Reviewed-on: http://review.whamcloud.com/17269
Tested-by: Jenkins
Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>
Reviewed-by: Alex Zhuravlev <alexey.zhuravlev@intel.com>
Tested-by: Maloo <hpdd-maloo@intel.com>
---

diff --git a/lustre/lfsck/lfsck_lib.c b/lustre/lfsck/lfsck_lib.c
index b7bb529..9f340e8 100644
--- a/lustre/lfsck/lfsck_lib.c
+++ b/lustre/lfsck/lfsck_lib.c
@@ -663,12 +663,30 @@ static int lfsck_create_lpf_local(const struct lu_env *env,
 	if (rc != 0)
 		GOTO(stop, rc);
 
+	if (!dt_try_as_dir(env, child))
+		GOTO(stop, rc = -ENOTDIR);
+
 	/* 2a. increase child nlink */
 	rc = dt_declare_ref_add(env, child, th);
 	if (rc != 0)
 		GOTO(stop, rc);
 
-	/* 3a. insert linkEA for child */
+	/* 3a. insert dot into child dir */
+	rec->rec_type = S_IFDIR;
+	rec->rec_fid = cfid;
+	rc = dt_declare_insert(env, child, (const struct dt_rec *)rec,
+			       (const struct dt_key *)dot, th);
+	if (rc != 0)
+		GOTO(stop, rc);
+
+	/* 4a. insert dotdot into child dir */
+	rec->rec_fid = &LU_LPF_FID;
+	rc = dt_declare_insert(env, child, (const struct dt_rec *)rec,
+			       (const struct dt_key *)dotdot, th);
+	if (rc != 0)
+		GOTO(stop, rc);
+
+	/* 5a. insert linkEA for child */
 	lfsck_buf_init(&linkea_buf, ldata.ld_buf->lb_buf,
 		       ldata.ld_leh->leh_len);
 	rc = dt_declare_xattr_set(env, child, &linkea_buf,
@@ -676,7 +694,7 @@ static int lfsck_create_lpf_local(const struct lu_env *env,
 	if (rc != 0)
 		GOTO(stop, rc);
 
-	/* 4a. insert name into parent dir */
+	/* 6a. insert name into parent dir */
 	rec->rec_type = S_IFDIR;
 	rec->rec_fid = cfid;
 	rc = dt_declare_insert(env, parent, (const struct dt_rec *)rec,
@@ -684,12 +702,12 @@ static int lfsck_create_lpf_local(const struct lu_env *env,
 	if (rc != 0)
 		GOTO(stop, rc);
 
-	/* 5a. increase parent nlink */
+	/* 7a. increase parent nlink */
 	rc = dt_declare_ref_add(env, parent, th);
 	if (rc != 0)
 		GOTO(stop, rc);
 
-	/* 6a. update bookmark */
+	/* 8a. update bookmark */
 	rc = dt_declare_record_write(env, bk_obj,
 				     lfsck_buf_get(env, bk, len), 0, th);
 	if (rc != 0)
@@ -700,41 +718,38 @@ static int lfsck_create_lpf_local(const struct lu_env *env,
 		GOTO(stop, rc);
 
 	dt_write_lock(env, child, 0);
-	/* 1b.1. create child */
+	/* 1b. create child */
 	rc = dt_create(env, child, la, NULL, dof, th);
 	if (rc != 0)
 		GOTO(unlock, rc);
 
-	if (unlikely(!dt_try_as_dir(env, child)))
-		GOTO(unlock, rc = -ENOTDIR);
+	/* 2b. increase child nlink */
+	rc = dt_ref_add(env, child, th);
+	if (rc != 0)
+		GOTO(unlock, rc);
 
-	/* 1b.2. insert dot into child dir */
+	/* 3b. insert dot into child dir */
 	rec->rec_fid = cfid;
 	rc = dt_insert(env, child, (const struct dt_rec *)rec,
 		       (const struct dt_key *)dot, th, 1);
 	if (rc != 0)
 		GOTO(unlock, rc);
 
-	/* 1b.3. insert dotdot into child dir */
+	/* 4b. insert dotdot into child dir */
 	rec->rec_fid = &LU_LPF_FID;
 	rc = dt_insert(env, child, (const struct dt_rec *)rec,
 		       (const struct dt_key *)dotdot, th, 1);
 	if (rc != 0)
 		GOTO(unlock, rc);
 
-	/* 2b. increase child nlink */
-	rc = dt_ref_add(env, child, th);
-	if (rc != 0)
-		GOTO(unlock, rc);
-
-	/* 3b. insert linkEA for child. */
+	/* 5b. insert linkEA for child. */
 	rc = dt_xattr_set(env, child, &linkea_buf,
 			  XATTR_NAME_LINK, 0, th);
 	dt_write_unlock(env, child);
 	if (rc != 0)
 		GOTO(stop, rc);
 
-	/* 4b. insert name into parent dir */
+	/* 6b. insert name into parent dir */
 	rec->rec_fid = cfid;
 	rc = dt_insert(env, parent, (const struct dt_rec *)rec,
 		       (const struct dt_key *)name, th, 1);
@@ -742,7 +757,7 @@ static int lfsck_create_lpf_local(const struct lu_env *env,
 		GOTO(stop, rc);
 
 	dt_write_lock(env, parent, 0);
-	/* 5b. increase parent nlink */
+	/* 7b. increase parent nlink */
 	rc = dt_ref_add(env, parent, th);
 	dt_write_unlock(env, parent);
 	if (rc != 0)
@@ -751,7 +766,7 @@ static int lfsck_create_lpf_local(const struct lu_env *env,
 	bk->lb_lpf_fid = *cfid;
 	lfsck_bookmark_cpu_to_le(&lfsck->li_bookmark_disk, bk);
 
-	/* 6b. update bookmark */
+	/* 8b. update bookmark */
 	rc = dt_record_write(env, bk_obj,
 			     lfsck_buf_get(env, bk, len), &pos, th);
 
@@ -830,12 +845,30 @@ static int lfsck_create_lpf_remote(const struct lu_env *env,
 	if (rc != 0)
 		GOTO(stop, rc);
 
+	if (!dt_try_as_dir(env, child))
+		GOTO(stop, rc = -ENOTDIR);
+
 	/* 2a. increase child nlink */
 	rc = dt_declare_ref_add(env, child, th);
 	if (rc != 0)
 		GOTO(stop, rc);
 
-	/* 3a. insert linkEA for child */
+	/* 3a. insert dot into child dir */
+	rec->rec_type = S_IFDIR;
+	rec->rec_fid = cfid;
+	rc = dt_declare_insert(env, child, (const struct dt_rec *)rec,
+			       (const struct dt_key *)dot, th);
+	if (rc != 0)
+		GOTO(stop, rc);
+
+	/* 4a. insert dotdot into child dir */
+	rec->rec_fid = &LU_LPF_FID;
+	rc = dt_declare_insert(env, child, (const struct dt_rec *)rec,
+			       (const struct dt_key *)dotdot, th);
+	if (rc != 0)
+		GOTO(stop, rc);
+
+	/* 5a. insert linkEA for child */
 	lfsck_buf_init(&linkea_buf, ldata.ld_buf->lb_buf,
 		       ldata.ld_leh->leh_len);
 	rc = dt_declare_xattr_set(env, child, &linkea_buf,
@@ -843,7 +876,7 @@ static int lfsck_create_lpf_remote(const struct lu_env *env,
 	if (rc != 0)
 		GOTO(stop, rc);
 
-	/* 4a. update bookmark */
+	/* 6a. update bookmark */
 	rc = dt_declare_record_write(env, bk_obj,
 				     lfsck_buf_get(env, bk, len), 0, th);
 	if (rc != 0)
@@ -854,15 +887,17 @@ static int lfsck_create_lpf_remote(const struct lu_env *env,
 		GOTO(stop, rc);
 
 	dt_write_lock(env, child, 0);
-	/* 1b.1. create child */
+	/* 1b. create child */
 	rc = dt_create(env, child, la, NULL, dof, th);
 	if (rc != 0)
 		GOTO(unlock, rc);
 
-	if (unlikely(!dt_try_as_dir(env, child)))
-		GOTO(unlock, rc = -ENOTDIR);
+	/* 2b. increase child nlink */
+	rc = dt_ref_add(env, child, th);
+	if (rc != 0)
+		GOTO(unlock, rc);
 
-	/* 1b.2. insert dot into child dir */
+	/* 3b. insert dot into child dir */
 	rec->rec_type = S_IFDIR;
 	rec->rec_fid = cfid;
 	rc = dt_insert(env, child, (const struct dt_rec *)rec,
@@ -870,19 +905,14 @@ static int lfsck_create_lpf_remote(const struct lu_env *env,
 	if (rc != 0)
 		GOTO(unlock, rc);
 
-	/* 1b.3. insert dotdot into child dir */
+	/* 4b. insert dotdot into child dir */
 	rec->rec_fid = &LU_LPF_FID;
 	rc = dt_insert(env, child, (const struct dt_rec *)rec,
 		       (const struct dt_key *)dotdot, th, 1);
 	if (rc != 0)
 		GOTO(unlock, rc);
 
-	/* 2b. increase child nlink */
-	rc = dt_ref_add(env, child, th);
-	if (rc != 0)
-		GOTO(unlock, rc);
-
-	/* 3b. insert linkEA for child */
+	/* 5b. insert linkEA for child */
 	rc = dt_xattr_set(env, child, &linkea_buf,
 			  XATTR_NAME_LINK, 0, th);
 	if (rc != 0)
@@ -891,7 +921,7 @@ static int lfsck_create_lpf_remote(const struct lu_env *env,
 	bk->lb_lpf_fid = *cfid;
 	lfsck_bookmark_cpu_to_le(&lfsck->li_bookmark_disk, bk);
 
-	/* 4b. update bookmark */
+	/* 6b. update bookmark */
 	rc = dt_record_write(env, bk_obj,
 			     lfsck_buf_get(env, bk, len), &pos, th);
 
diff --git a/lustre/lfsck/lfsck_namespace.c b/lustre/lfsck/lfsck_namespace.c
index fa3b851..bfbed00 100644
--- a/lustre/lfsck/lfsck_namespace.c
+++ b/lustre/lfsck/lfsck_namespace.c
@@ -1395,6 +1395,10 @@ again:
 	if (unlikely(!dt_try_as_dir(env, orphan)))
 		GOTO(stop, rc = -ENOTDIR);
 
+	rc = dt_declare_ref_add(env, orphan, th);
+	if (rc != 0)
+		GOTO(stop, rc);
+
 	rec->rec_type = S_IFDIR;
 	rec->rec_fid = cfid;
 	rc = dt_declare_insert(env, orphan, (const struct dt_rec *)rec,
@@ -1405,13 +1409,6 @@ again:
 	rec->rec_fid = lfsck_dto2fid(parent);
 	rc = dt_declare_insert(env, orphan, (const struct dt_rec *)rec,
 			       (const struct dt_key *)dotdot, th);
-	if (rc == 0)
-		rc = dt_declare_ref_add(env, orphan, th);
-
-	if (rc != 0)
-		GOTO(stop, rc);
-
-	rc = dt_declare_ref_add(env, orphan, th);
 	if (rc != 0)
 		GOTO(stop, rc);
 
@@ -1451,6 +1448,10 @@ again:
 	if (rc != 0)
 		GOTO(unlock2, rc);
 
+	rc = dt_ref_add(env, orphan, th);
+	if (rc != 0)
+		GOTO(unlock2, rc);
+
 	rec->rec_fid = cfid;
 	rc = dt_insert(env, orphan, (const struct dt_rec *)rec,
 		       (const struct dt_key *)dot, th, 1);
@@ -1463,10 +1464,6 @@ again:
 	if (rc != 0)
 		GOTO(unlock2, rc);
 
-	rc = dt_ref_add(env, orphan, th);
-	if (rc != 0)
-		GOTO(unlock2, rc);
-
 	if (lmv != NULL) {
 		rc = dt_xattr_set(env, orphan, &lmv_buf, XATTR_NAME_LMV, 0, th);
 		if (rc != 0)
@@ -4880,7 +4877,12 @@ int lfsck_namespace_repair_dangling(const struct lu_env *env,
 		if (unlikely(!dt_try_as_dir(env, child)))
 			GOTO(stop, rc = -ENOTDIR);
 
-		/* 2a. insert dot into child dir */
+		/* 2a. increase child nlink */
+		rc = dt_declare_ref_add(env, child, th);
+		if (rc != 0)
+			GOTO(stop, rc);
+
+		/* 3a. insert dot into child dir */
 		rec->rec_type = S_IFDIR;
 		rec->rec_fid = cfid;
 		rc = dt_declare_insert(env, child,
@@ -4889,7 +4891,7 @@ int lfsck_namespace_repair_dangling(const struct lu_env *env,
 		if (rc != 0)
 			GOTO(stop, rc);
 
-		/* 3a. insert dotdot into child dir */
+		/* 4a. insert dotdot into child dir */
 		rec->rec_fid = pfid;
 		rc = dt_declare_insert(env, child,
 				       (const struct dt_rec *)rec,
@@ -4897,11 +4899,6 @@ int lfsck_namespace_repair_dangling(const struct lu_env *env,
 		if (rc != 0)
 			GOTO(stop, rc);
 
-		/* 4a. increase child nlink */
-		rc = dt_declare_ref_add(env, child, th);
-		if (rc != 0)
-			GOTO(stop, rc);
-
 		/* 5a. generate slave LMV EA. */
 		if (lnr->lnr_lmv != NULL && lnr->lnr_lmv->ll_lmv_master) {
 			int idx;
@@ -4944,7 +4941,12 @@ int lfsck_namespace_repair_dangling(const struct lu_env *env,
 		GOTO(unlock, rc = (rc == -EEXIST ? 1 : rc));
 
 	if (S_ISDIR(type)) {
-		/* 2b. insert dot into child dir */
+		/* 2b. increase child nlink */
+		rc = dt_ref_add(env, child, th);
+		if (rc != 0)
+			GOTO(unlock, rc);
+
+		/* 3b. insert dot into child dir */
 		rec->rec_type = S_IFDIR;
 		rec->rec_fid = cfid;
 		rc = dt_insert(env, child, (const struct dt_rec *)rec,
@@ -4952,18 +4954,13 @@ int lfsck_namespace_repair_dangling(const struct lu_env *env,
 		if (rc != 0)
 			GOTO(unlock, rc);
 
-		/* 3b. insert dotdot into child dir */
+		/* 4b. insert dotdot into child dir */
 		rec->rec_fid = pfid;
 		rc = dt_insert(env, child, (const struct dt_rec *)rec,
 			       (const struct dt_key *)dotdot, th, 1);
 		if (rc != 0)
 			GOTO(unlock, rc);
 
-		/* 4b. increase child nlink */
-		rc = dt_ref_add(env, child, th);
-		if (rc != 0)
-			GOTO(unlock, rc);
-
 		/* 5b. generate slave LMV EA. */
 		if (lnr->lnr_lmv != NULL && lnr->lnr_lmv->ll_lmv_master) {
 			rc = dt_xattr_set(env, child, &lmv_buf, XATTR_NAME_LMV,
diff --git a/lustre/osd-ldiskfs/osd_handler.c b/lustre/osd-ldiskfs/osd_handler.c
index f2850a4..facf412 100644
--- a/lustre/osd-ldiskfs/osd_handler.c
+++ b/lustre/osd-ldiskfs/osd_handler.c
@@ -2654,6 +2654,8 @@ static int osd_add_dot_dotdot_internal(struct osd_thread_info *info,
 {
 	struct ldiskfs_dentry_param *dot_ldp;
 	struct ldiskfs_dentry_param *dot_dot_ldp;
+	__u32 saved_nlink = dir->i_nlink;
+	int rc;
 
 	dot_dot_ldp = (struct ldiskfs_dentry_param *)info->oti_ldp2;
 	osd_get_ldiskfs_dirent_param(dot_dot_ldp, dot_dot_fid);
@@ -2661,8 +2663,23 @@ static int osd_add_dot_dotdot_internal(struct osd_thread_info *info,
 	dot_ldp = (struct ldiskfs_dentry_param *)info->oti_ldp;
 	dot_ldp->edp_magic = 0;
 
-	return ldiskfs_add_dot_dotdot(oth->ot_handle, parent_dir,
-				      dir, dot_ldp, dot_dot_ldp);
+	rc = ldiskfs_add_dot_dotdot(oth->ot_handle, parent_dir,
+				    dir, dot_ldp, dot_dot_ldp);
+	/* The ldiskfs_add_dot_dotdot() may dir->i_nlink as 2, then
+	 * the subseqent ref_add() will increase the dir->i_nlink
+	 * as 3. That is incorrect for new created directory.
+	 *
+	 * It looks like hack, because we want to make the OSD API
+	 * to be order-independent for new created directory object
+	 * between dt_insert(..) and ref_add() operations.
+	 *
+	 * Here, we only restore the in-RAM dir-inode's nlink attr,
+	 * becuase if the nlink attr is not 2, then there will be
+	 * ref_add() called following the dt_insert(..), such call
+	 * will make both the in-RAM and on-disk dir-inode's nlink
+	 * attr to be set as 2. LU-7447 */
+	set_nlink(dir, saved_nlink);
+	return rc;
 }
 
 /**
diff --git a/lustre/tests/sanity-lfsck.sh b/lustre/tests/sanity-lfsck.sh
index fa5f59d..8a65275 100644
--- a/lustre/tests/sanity-lfsck.sh
+++ b/lustre/tests/sanity-lfsck.sh
@@ -109,6 +109,19 @@ lfsck_prep() {
 	echo "prepared $(date)."
 }
 
+run_e2fsck_on_mdt0() {
+	[ $(facet_fstype $SINGLEMDS) != ldiskfs ] && return
+
+	stop $SINGLEMDS > /dev/null || error "(0) Fail to the stop MDT0"
+	run_e2fsck $(facet_active_host $SINGLEMDS) $(mdsdevname 1) "-n" |
+		grep "Fix? no" && {
+		run_e2fsck $(facet_active_host $SINGLEMDS) $(mdsdevname 1) "-n"
+		error "(2) Detected inconsistency on MDT0"
+	}
+	start $SINGLEMDS $MDT_DEVNAME $MOUNT_OPTS_NOSCRUB > /dev/null ||
+		error "(3) Fail to start MDT0"
+}
+
 test_0() {
 	lfsck_prep 3 3
 
@@ -195,6 +208,8 @@ test_1a() {
 	[ $repaired -eq 1 ] ||
 		error "(5) Fail to repair crashed FID-in-dirent: $repaired"
 
+	run_e2fsck_on_mdt0
+
 	mount_client $MOUNT || error "(6) Fail to start client!"
 
 	#define OBD_FAIL_FID_LOOKUP	0x1505
@@ -239,6 +254,8 @@ test_1b()
 		error "(5) Fail to repair the missing FID-in-LMA: $repaired"
 
 	do_facet $SINGLEMDS $LCTL set_param fail_loc=0
+	run_e2fsck_on_mdt0
+
 	mount_client $MOUNT || error "(6) Fail to start client!"
 
 	#define OBD_FAIL_FID_LOOKUP	0x1505
@@ -276,6 +293,8 @@ test_2a() {
 	[ $repaired -eq 1 ] ||
 		error "(5) Fail to repair crashed linkEA: $repaired"
 
+	run_e2fsck_on_mdt0
+
 	mount_client $MOUNT || error "(6) Fail to start client!"
 
 	stat $DIR/$tdir/dummy | grep "Links: 1" > /dev/null ||
@@ -311,6 +330,8 @@ test_2b()
 	[ $repaired -eq 1 ] ||
 		error "(5) Fail to repair crashed linkEA: $repaired"
 
+	run_e2fsck_on_mdt0
+
 	mount_client $MOUNT || error "(6) Fail to start client!"
 
 	stat $DIR/$tdir/dummy | grep "Links: 1" > /dev/null ||
@@ -346,6 +367,8 @@ test_2c()
 	[ $repaired -eq 1 ] ||
 		error "(5) Fail to repair crashed linkEA: $repaired"
 
+	run_e2fsck_on_mdt0
+
 	mount_client $MOUNT || error "(6) Fail to start client!"
 
 	stat $DIR/$tdir/dummy | grep "Links: 1" > /dev/null ||
@@ -381,6 +404,8 @@ test_2d()
 	[ $repaired -eq 1 ] ||
 		error "(5) Fail to repair crashed linkEA: $repaired"
 
+	run_e2fsck_on_mdt0
+
 	mount_client $MOUNT || error "(6) Fail to start client!"
 
 	stat $DIR/$tdir/dummy | grep "Links: 1" > /dev/null ||
@@ -518,6 +543,8 @@ test_4()
 	[ $repaired -ge 9 ] ||
 		error "(9) Fail to re-generate FID-in-dirent: $repaired"
 
+	run_e2fsck_on_mdt0
+
 	mount_client $MOUNT || error "(10) Fail to start client!"
 
 	#define OBD_FAIL_FID_LOOKUP	0x1505
@@ -576,6 +603,8 @@ test_5()
 	[ $repaired -ge 2 ] ||
 		error "(9) Fail to generate FID-in-dirent for IGIF: $repaired"
 
+	run_e2fsck_on_mdt0
+
 	mount_client $MOUNT || error "(10) Fail to start client!"
 
 	#define OBD_FAIL_FID_LOOKUP	0x1505