cluster: RHEL6 - fsck.gfs2 truncates directories with more than 100, 000 entries

Bob Peterson rpeterso at fedoraproject.org
Fri Sep 17 18:44:43 UTC 2010


Gitweb:        http://git.fedorahosted.org/git/cluster.git?p=cluster.git;a=commitdiff;h=f230a400c7b99d9cc95e19ab6fdca9fcd64811ee
Commit:        f230a400c7b99d9cc95e19ab6fdca9fcd64811ee
Parent:        87fdbcaeea44fc14264695a4c2164f8c3cec9a84
Author:        Bob Peterson <rpeterso at redhat.com>
AuthorDate:    Fri Sep 17 13:42:50 2010 -0500
Committer:     Bob Peterson <rpeterso at redhat.com>
CommitterDate: Fri Sep 17 13:45:46 2010 -0500

fsck.gfs2 truncates directories with more than 100,000 entries

This patch allows fsck.gfs2 to properly process "continuation
leaf blocks" in directories.  It also fixes a memory leak.

rhbz#628013
---
 gfs2/fsck/link.c         |    8 ++
 gfs2/fsck/lost_n_found.c |   18 ++++-
 gfs2/fsck/metawalk.c     |  172 ++++++++++++++++++++++------------------------
 gfs2/fsck/pass1.c        |   15 +++-
 gfs2/libgfs2/fs_ops.c    |   12 ++-
 5 files changed, 123 insertions(+), 102 deletions(-)

diff --git a/gfs2/fsck/link.c b/gfs2/fsck/link.c
index 5bb3ae2..ce8eb52 100644
--- a/gfs2/fsck/link.c
+++ b/gfs2/fsck/link.c
@@ -68,6 +68,14 @@ int decrement_link(uint64_t inode_no, uint64_t referenced_from,
 	/* If the list has entries, look for one that matches
 	 * inode_no */
 	if(ii) {
+		if (!ii->counted_links) {
+			log_debug( _("Directory %lld (0x%llx)'s link to "
+			     " %"PRIu64" (0x%" PRIx64 ") via %s is zero!\n"),
+			   (unsigned long long)referenced_from,
+			   (unsigned long long)referenced_from,
+			   inode_no, inode_no, why);
+			return 0;
+		}
 		ii->counted_links--;
 		log_debug( _("Directory %lld (0x%llx) decremented counted "
 			     "links to %u for %"PRIu64" (0x%" PRIx64 ") "
diff --git a/gfs2/fsck/lost_n_found.c b/gfs2/fsck/lost_n_found.c
index 4a3a926..3256bdc 100644
--- a/gfs2/fsck/lost_n_found.c
+++ b/gfs2/fsck/lost_n_found.c
@@ -108,10 +108,20 @@ int add_inode_to_lf(struct gfs2_inode *ip){
 				       ip->i_di.di_num.no_addr,
 				       _(".. unlinked, moving to lost+found"));
 			dip = fsck_load_inode(sdp, di->dotdot_parent);
-			dip->i_di.di_nlink--;
-			log_debug(_("Decrementing its links to %d\n"),
-				  dip->i_di.di_nlink);
-			bmodified(dip->i_bh);
+			if (dip->i_di.di_nlink > 0) {
+				dip->i_di.di_nlink--;
+				log_debug(_("Decrementing its links to %d\n"),
+					  dip->i_di.di_nlink);
+				bmodified(dip->i_bh);
+			} else if (!dip->i_di.di_nlink) {
+				log_debug(_("Its link count is zero.\n"));
+			} else {
+				log_debug(_("Its link count is %d!  "
+					    "Changing it to 0.\n"),
+					  dip->i_di.di_nlink);
+				dip->i_di.di_nlink = 0;
+				bmodified(dip->i_bh);
+			}
 			fsck_inode_put(&dip);
 			di = NULL;
 		} else
diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c
index ef1b6b5..2151ced 100644
--- a/gfs2/fsck/metawalk.c
+++ b/gfs2/fsck/metawalk.c
@@ -377,7 +377,6 @@ static int check_entries(struct gfs2_inode *ip, struct gfs2_buffer_head *bh,
 				log_debug( _("First dirent is a sentinel (place holder).\n"));
 				first = 0;
 			} else {
-				/* FIXME: Do something about this */
 				log_err( _("Directory entry with inode number of "
 					"zero in leaf %llu (0x%llx) of "
 					"directory %llu (0x%llx)!\n"),
@@ -526,6 +525,68 @@ static int fix_leaf_pointers(struct gfs2_inode *dip, int *lindex,
 	return 0;
 }
 
+/* check_num_ptrs - check a previously processed leaf's pointer count */
+static int check_num_ptrs(struct gfs2_inode *ip, uint64_t old_leaf,
+			  int *ref_count, int *exp_count, int *lindex,
+			  struct gfs2_leaf *oldleaf)
+{
+	int factor = 0, divisor = *ref_count, multiple = 1, error = 0;
+	struct gfs2_buffer_head *lbh;
+
+	/* Check to see if the number of pointers we found is a power of 2.
+	   It needs to be and if it's not we need to fix it.*/
+	while (divisor > 1) {
+		factor++;
+		divisor /= 2;
+		multiple = multiple << 1;
+	}
+	if (*ref_count != multiple) {
+		log_err( _("Directory #%llu (0x%llx) has an "
+			   "invalid number of pointers to "
+			   "leaf #%llu (0x%llx)\n\tFound: %u, "
+			   "which is not a factor of 2.\n"),
+			 (unsigned long long)ip->i_di.di_num.no_addr,
+			 (unsigned long long)ip->i_di.di_num.no_addr,
+			 (unsigned long long)old_leaf,
+			 (unsigned long long)old_leaf, *ref_count);
+		if (!query( _("Attempt to fix it? (y/n) "))) {
+			log_err( _("Directory inode was not fixed.\n"));
+			return 1;
+		}
+		error = fix_leaf_pointers(ip, lindex, *ref_count, multiple);
+		if (error)
+			return error;
+		*ref_count = multiple;
+		log_err( _("Directory inode was fixed.\n"));
+	}
+	/* Check to see if the counted number of leaf pointers is what we
+	   expect. */
+	if (*ref_count != *exp_count) {
+		log_err( _("Directory #%llu (0x%llx) has an "
+			   "incorrect number of pointers to "
+			   "leaf #%llu (0x%llx)\n\tFound: "
+			   "%u,  Expected: %u\n"),
+			 (unsigned long long)ip->i_di.di_num.no_addr,
+			 (unsigned long long)ip->i_di.di_num.no_addr,
+			 (unsigned long long)old_leaf,
+			 (unsigned long long)old_leaf, *ref_count, *exp_count);
+		if (!query( _("Attempt to fix it? (y/n) "))) {
+			log_err( _("Directory leaf was not fixed.\n"));
+			return 1;
+		}
+		lbh = bread(ip->i_sbd, old_leaf);
+		gfs2_leaf_in(oldleaf, lbh);
+		log_err( _("Leaf depth was %d, changed to %d\n"),
+			 oldleaf->lf_depth, ip->i_di.di_depth - factor);
+		oldleaf->lf_depth = ip->i_di.di_depth - factor;
+		gfs2_leaf_out(oldleaf, lbh);
+		brelse(lbh);
+		*exp_count = *ref_count;
+		log_err( _("Directory leaf was fixed.\n"));
+	}
+	return 0;
+}
+
 /* Checks exhash directory entries */
 static int check_leaf_blks(struct gfs2_inode *ip, struct metawalk_fxns *pass)
 {
@@ -582,76 +643,17 @@ static int check_leaf_blks(struct gfs2_inode *ip, struct metawalk_fxns *pass)
 			ref_count++;
 			continue;
 		}
-		if (gfs2_check_range(ip->i_sbd, old_leaf) == 0) {
-			int factor = 0, divisor = ref_count, multiple = 1;
-
-			/* Check to see if the number of pointers we found is
-			   a power of 2.  It needs to be and if it's not we
-			   need to fix it.*/
-			while (divisor > 1) {
-				factor++;
-				divisor /= 2;
-				multiple = multiple << 1;
-			}
-			if (ref_count != multiple) {
-				log_err( _("Directory #%llu (0x%llx) has an "
-					   "invalid number of pointers to "
-					   "leaf #%llu (0x%llx)\n\tFound: %u, "
-					   "which is not a factor of 2.\n"),
-					 (unsigned long long)
-					 ip->i_di.di_num.no_addr,
-					 (unsigned long long)
-					 ip->i_di.di_num.no_addr,
-					 (unsigned long long)old_leaf,
-					 (unsigned long long)old_leaf,
-					 ref_count);
-				if (!query( _("Attempt to fix it? (y/n) "))) {
-					log_err( _("Directory inode was not "
-						   "fixed.\n"));
-					return 1;
-				}
-				error = fix_leaf_pointers(ip, &lindex,
-							  ref_count, multiple);
+
+		do {
+			if (gfs2_check_range(ip->i_sbd, old_leaf) == 0) {
+				error = check_num_ptrs(ip, old_leaf,
+						       &ref_count, &exp_count,
+						       &lindex, &oldleaf);
 				if (error)
 					return error;
-				ref_count = multiple;
-				log_err( _("Directory inode was fixed.\n"));
-			}
-			/* Check to see if the counted number of leaf pointers
-			   is what we expect. */
-			if (ref_count != exp_count) {
-				log_err( _("Directory #%llu (0x%llx) has an "
-					   "incorrect number of pointers to "
-					   "leaf #%llu (0x%llx)\n\tFound: "
-					   "%u,  Expected: %u\n"),
-					 (unsigned long long)
-					 ip->i_di.di_num.no_addr,
-					 (unsigned long long)
-					 ip->i_di.di_num.no_addr,
-					 (unsigned long long)old_leaf,
-					 (unsigned long long)old_leaf,
-					 ref_count, exp_count);
-				if (!query( _("Attempt to fix it? (y/n) "))) {
-					log_err( _("Directory leaf was not "
-						   "fixed.\n"));
-					return 1;
-				}
-				lbh = bread(sbp, old_leaf);
-				gfs2_leaf_in(&oldleaf, lbh);
-				log_err( _("Leaf depth was %d, changed to "
-					   "%d\n"), oldleaf.lf_depth,
-					 ip->i_di.di_depth - factor);
-				oldleaf.lf_depth = ip->i_di.di_depth - factor;
-				gfs2_leaf_out(&oldleaf, lbh);
-				brelse(lbh);
-				exp_count = ref_count;
-				log_err( _("Directory leaf was fixed.\n"));
 			}
-		}
-		ref_count = 1;
-
-		count = 0;
-		do {
+			ref_count = 1;
+			count = 0;
 			if (fsck_abort)
 				break;
 			/* Make sure the block number is in range. */
@@ -684,10 +686,9 @@ static int check_leaf_blks(struct gfs2_inode *ip, struct metawalk_fxns *pass)
 				break;
 			}
 			gfs2_leaf_in(&leaf, lbh);
-			if(pass->check_leaf) {
+			if(pass->check_leaf)
 				error = pass->check_leaf(ip, leaf_no, lbh,
 							 pass->private);
-			}
 
 			/*
 			 * Early versions of GFS2 had an endianess bug in the
@@ -722,22 +723,17 @@ static int check_leaf_blks(struct gfs2_inode *ip, struct metawalk_fxns *pass)
 			  leaf depth %u\n"),
 			  exp_count, ip->i_di.di_depth, leaf.lf_depth);*/
 
-			if(pass->check_dentry && S_ISDIR(ip->i_di.di_mode)) {
+			if (pass->check_dentry && S_ISDIR(ip->i_di.di_mode)) {
 				error = check_entries(ip, lbh, DIR_EXHASH,
 						      &count, pass);
-
-				/* Since the buffer possibly got
-				 * updated directly, release it now,
-				 * and grab it again later if we need it. */
-
-				brelse(lbh);
-
 				if(error < 0) {
 					stack;
+					brelse(lbh);
 					return -1;
 				}
 
 				if(count != leaf.lf_entries) {
+					brelse(lbh);
 					lbh = bread(sbp, leaf_no);
 					gfs2_leaf_in(&leaf, lbh);
 
@@ -759,21 +755,17 @@ static int check_leaf_blks(struct gfs2_inode *ip, struct metawalk_fxns *pass)
 						log_warn( _("Leaf entry count updated\n"));
 					} else
 						log_err( _("Leaf entry count left in inconsistant state\n"));
-					brelse(lbh);
 				}
-				/* FIXME: Need to get entry count and
-				 * compare it against leaf->lf_entries */
-				break; /* not a chain; go back to outer loop */
-			} else {
-				brelse(lbh);
-				if(!leaf.lf_next)
-					break;
-				leaf_no = leaf.lf_next;
-				log_debug( _("Leaf chain detected.\n"));
 			}
+			brelse(lbh);
+			old_leaf = leaf_no;
+			memcpy(&oldleaf, &leaf, sizeof(oldleaf));
+			if (!leaf.lf_next)
+				break;
+			leaf_no = leaf.lf_next;
+			log_debug( _("Leaf chain 0x%llx detected.\n"),
+				   (unsigned long long)leaf_no);
 		} while(1); /* while we have chained leaf blocks */
-		old_leaf = leaf_no;
-		memcpy(&oldleaf, &leaf, sizeof(oldleaf));
 	} /* for every leaf block */
 	return 0;
 }
diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c
index 17b23df..bc500fd 100644
--- a/gfs2/fsck/pass1.c
+++ b/gfs2/fsck/pass1.c
@@ -265,13 +265,16 @@ static int check_metalist(struct gfs2_inode *ip, uint64_t block,
 			return 1;
 		}
 		brelse(nbh);
+		nbh = NULL;
 	} else /* blk check ok */
 		*bh = nbh;
 
 	bc->indir_count++;
-	if (found_dup)
+	if (found_dup) {
+		if (nbh)
+			brelse(nbh);
 		return 1; /* don't process the metadata again */
-	else
+	} else
 		fsck_blockmap_set(ip, block, _("indirect"),
 				  gfs2_indir_blk);
 
@@ -323,13 +326,17 @@ static int undo_check_metalist(struct gfs2_inode *ip, uint64_t block,
 			return 1;
 		}
 		brelse(nbh);
+		nbh = NULL;
 	} else /* blk check ok */
 		*bh = nbh;
 
 	bc->indir_count--;
-	if (found_dup)
+	if (found_dup) {
+		if (nbh)
+			brelse(nbh);
+		*bh = NULL;
 		return 1; /* don't process the metadata again */
-	else
+	} else
 		fsck_blockmap_set(ip, block, _("bad indirect"),
 				  gfs2_block_free);
 	return 0;
diff --git a/gfs2/libgfs2/fs_ops.c b/gfs2/libgfs2/fs_ops.c
index e0a1681..4019894 100644
--- a/gfs2/libgfs2/fs_ops.c
+++ b/gfs2/libgfs2/fs_ops.c
@@ -790,6 +790,13 @@ void dirent2_del(struct gfs2_inode *dip, struct gfs2_buffer_head *bh,
 	uint16_t cur_rec_len, prev_rec_len;
 
 	bmodified(bh);
+	if (gfs2_check_meta(bh, GFS2_METATYPE_LF) == 0) {
+		struct gfs2_leaf *lf = (struct gfs2_leaf *)bh->b_data;
+
+		lf->lf_entries = be16_to_cpu(lf->lf_entries) - 1;
+		lf->lf_entries = cpu_to_be16(lf->lf_entries);
+	}
+
 	if (dip->i_di.di_entries) {
 		bmodified(dip->i_bh);
 		dip->i_di.di_entries--;
@@ -908,9 +915,6 @@ static void dir_split_leaf(struct gfs2_inode *dip, uint32_t lindex,
 
 			dirent2_del(dip, obh, prev, dent);
 
-			oleaf->lf_entries = be16_to_cpu(oleaf->lf_entries) - 1;
-			oleaf->lf_entries = cpu_to_be16(oleaf->lf_entries);
-
 			if (!prev)
 				prev = dent;
 
@@ -1726,7 +1730,7 @@ int gfs2_freedi(struct gfs2_sbd *sdp, uint64_t diblock)
 	/* Set the bitmap type for inode to free space: */
 	gfs2_set_bitmap(sdp, ip->i_di.di_num.no_addr, GFS2_BLKST_FREE);
 	inode_put(&ip);
-	/* inode_put deallocated the extra block used by the dist inode, */
+	/* inode_put deallocated the extra block used by the disk inode, */
 	/* so adjust it in the superblock struct */
 	sdp->blks_alloced--;
 	/* Now we have to adjust the rg freespace count and inode count: */


More information about the cluster-commits mailing list