Skip to contentSkip to conversation

fsck.gfs2: Make pass2 go by directory rbtree for performance

PATCH New Bob Peterson 2 participants 3 messages Jul 13, 2016 – Mar 20, 2017 1 message not in the archive

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

#1 lore raw

PATCH 1/1 fsck.gfs2: Make pass2 go by directory rbtree for performance

Hi,

I posted this fsck.gfs2 performance patch previously on 22 June as part of the same multi-patch set.

Pass2 went from 21m53.031s to 17m10.214s with the previous patch.

With this patch pass2 went from 17m10.214s to 12m38.876s, a savings of 4.52 minutes: a 26 percent speedup.

Regards,

Bob Peterson
Red Hat File Systems
---
This patch speeds up pass2 of fsck.gfs2 for large file systems. Before, pass2 was checking the bitmap for every block in the file system. If the file system is 15TB, that means looping 15 trillion times. With this patch, it merely traverses the inode tree, since it needs that information anyway, and already ignores any blocks that don't have it. Traversing an empty 15TB file system goes from 13.5 minutes to 0.00 seconds for pass2.

---

gfs2/fsck/pass2.c +8 -13
index 808cf21..abc2b96 100644
@@ -2203,8 +2203,11 @@ static int pass2_check_dir(struct gfs2_sbd *sdp, struct gfs2_inode *ip)
  */
 int pass2(struct gfs2_sbd *sdp)
 {
+	struct osi_node *tmp, *next = NULL;
+	struct gfs2_inode *ip;
+	struct dir_info *dt;
 	uint64_t dirblk;
-	int q;
+	int error;
 
 	/* Check all the system directory inodes. */
 	if (!sdp->gfs1 &&
@@ -2236,11 +2239,11 @@ int pass2(struct gfs2_sbd *sdp)
 		return FSCK_OK;
 	log_info( _("Checking directory inodes.\n"));
 	/* Grab each directory inode, and run checks on it */
-	for (dirblk = 0; dirblk < last_fs_block; dirblk++) {
-		struct gfs2_inode *ip;
-		struct dir_info *dt;
-		int error;
+	for (tmp = osi_first(&dirtree); tmp; tmp = next) {
+		next = osi_next(tmp);
 
+		dt = (struct dir_info *)tmp;
+		dirblk = dt->dinode.no_addr;
 		warm_fuzzy_stuff(dirblk);
 		if (skip_this_pass || fsck_abort) /* if asked to skip the rest */
 			return FSCK_OK;
@@ -2249,14 +2252,6 @@ int pass2(struct gfs2_sbd *sdp)
 		if (is_system_dir(sdp, dirblk))
 			continue;
 
-		q = bitmap_type(sdp, dirblk);
-		if (q != GFS2_BLKST_DINODE)
-			continue;
-
-		dt = dirtree_find(dirblk);
-		if (dt == NULL)
-			continue;
-
 		/* If we created lost+found, its links should have been
 		   properly adjusted, so don't check it. */
 		if (lf_was_created &&
#2 lore raw

PATCH 1/1 fsck.gfs2: Make pass2 go by directory rbtree for performance

Quoted 9 lines · Bob Peterson

Hi,

I posted this fsck.gfs2 performance patch previously on 22 June as part of the same multi-patch set.

Pass2 went from 21m53.031s to 17m10.214s with the previous patch.

With this patch pass2 went from 17m10.214s to 12m38.876s, a savings of 4.52 minutes: a 26 percent speedup.

Another nice improvement...

Quoted 34 lines

Regards,

Bob Peterson
Red Hat File Systems
---
This patch speeds up pass2 of fsck.gfs2 for large file systems. Before, pass2 was checking the bitmap for every block in the file system. If the file system is 15TB, that means looping 15 trillion times. With this patch, it merely traverses the inode tree, since it needs that information anyway, and already ignores any blocks that don't have it. Traversing an empty 15TB file system goes from 13.5 minutes to 0.00 seconds for pass2.

---

gfs2/fsck/pass2.c +4 -1
index 808cf21..abc2b96 100644
@@ -2203,8 +2203,11 @@ static int pass2_check_dir(struct gfs2_sbd *sdp, struct gfs2_inode *ip)
  */
 int pass2(struct gfs2_sbd *sdp)
 {
+	struct osi_node *tmp, *next = NULL;
+	struct gfs2_inode *ip;
+	struct dir_info *dt;
 	uint64_t dirblk;
-	int q;
+	int error;
 
 	/* Check all the system directory inodes. */
 	if (!sdp->gfs1 &&

gfs2/fsck/pass2.c int pass2(struct gfs2_sbd *sdp) · line 2243

Quoted 9 lines
gfs2/fsck/pass2.c +2 -4
@@ -2236,11 +2239,11 @@ int pass2(struct gfs2_sbd *sdp)
 		return FSCK_OK;
 	log_info( _("Checking directory inodes.\n"));
 	/* Grab each directory inode, and run checks on it */
-	for (dirblk = 0; dirblk < last_fs_block; dirblk++) {
-		struct gfs2_inode *ip;
-		struct dir_info *dt;
-		int error;
+	for (tmp = osi_first(&dirtree); tmp; tmp = next) {
+		next = osi_next(tmp);

Why not 'for (next = osi_first(&dirtree); next; next = osi_next(next)) {' ?

ACK

Andy

Quoted 22 lines
gfs2/fsck/pass2.c +2 -8
+		dt = (struct dir_info *)tmp;
+		dirblk = dt->dinode.no_addr;
 		warm_fuzzy_stuff(dirblk);
 		if (skip_this_pass || fsck_abort) /* if asked to skip the rest */
 			return FSCK_OK;
@@ -2249,14 +2252,6 @@ int pass2(struct gfs2_sbd *sdp)
 		if (is_system_dir(sdp, dirblk))
 			continue;
 
-		q = bitmap_type(sdp, dirblk);
-		if (q != GFS2_BLKST_DINODE)
-			continue;
-
-		dt = dirtree_find(dirblk);
-		if (dt == NULL)
-			continue;
-
 		/* If we created lost+found, its links should have been
 		   properly adjusted, so don't check it. */
 		if (lf_was_created &&
#3 lore raw

PATCH 1/1 fsck.gfs2: Make pass2 go by directory rbtree for performance

Hi Andy,

Sorry for the delay. I guess I've been caught up in kernel work too long.

----- Original Message -----

| > +	for (tmp = osi_first(&dirtree); tmp; tmp = next) {
| > +		next = osi_next(tmp);
| 
| Why not 'for (next = osi_first(&dirtree); next; next = osi_next(next)) {' ?

The reason is: Any corruption caught might take the item off the list, so it needs to be a "safe" version.

| ACK
| 
| Andy

Thanks for the review and the ACK. I'm under the gun on this (due to my own fault, but nonetheless), so I'm going to push it to master.

Regards,

Bob Peterson
Red Hat File Systems