bcachefs: fs-gc, fsck improvements

Make some checks tighter

2 files changed, 110 insertions, 65 deletions

diff --git a/drivers/md/bcache/fs-gc.c b/drivers/md/bcache/fs-gc.c
index e68d35026051..53c7ec0283a9 100644
--- a/drivers/md/bcache/fs-gc.c
+++ b/drivers/md/bcache/fs-gc.c
@@ -20,7 +20,7 @@ DECLARE_GENRADIX_TYPE(nlinks, struct nlink);
 
 static void inc_link(struct cache_set *c, struct nlinks *links,
 		     u64 range_start, u64 *range_end,
-		     u64 inum, unsigned count, bool dir)
+		     u64 inum, bool dir)
 {
 	struct nlink *link;
 
@@ -35,9 +35,9 @@ static void inc_link(struct cache_set *c, struct nlinks *links,
 	}
 
 	if (dir)
-		link->dir_count += count;
+		link->dir_count++;
 	else
-		link->count += count;
+		link->count++;
 }
 
 /*
@@ -55,7 +55,7 @@ static int bch_gc_walk_dirents(struct cache_set *c, struct nlinks *links,
 	u64 d_inum;
 	int ret;
 
-	inc_link(c, links, range_start, range_end, BCACHE_ROOT_INO, 2, false);
+	inc_link(c, links, range_start, range_end, BCACHE_ROOT_INO, false);
 
 	for_each_btree_key(&iter, c, BTREE_ID_DIRENTS, POS_MIN, k) {
 		switch (k.k->type) {
@@ -63,15 +63,12 @@ static int bch_gc_walk_dirents(struct cache_set *c, struct nlinks *links,
 			d = bkey_s_c_to_dirent(k);
 			d_inum = le64_to_cpu(d.v->d_inum);
 
-			if (d.v->d_type == DT_DIR) {
+			if (d.v->d_type == DT_DIR)
 				inc_link(c, links, range_start, range_end,
-					 d_inum, 2, false);
-				inc_link(c, links, range_start, range_end,
-					 d.k->p.inode, 1, true);
-			} else {
-				inc_link(c, links, range_start, range_end,
-					 d_inum, 1, false);
-			}
+					 d.k->p.inode, true);
+
+			inc_link(c, links, range_start, range_end,
+				 d_inum, false);
 
 			break;
 		}
@@ -111,11 +108,26 @@ static int bch_gc_do_inode(struct cache_set *c, struct btree_iter *iter,
 	u64 i_size  = le64_to_cpu(inode.v->i_size);
 	s64 i_sectors = 0;
 	int ret = 0;
+	u32 real_i_nlink;
 
 	cache_set_inconsistent_on(i_nlink < link.count, c,
-			 "i_link too small (%u < %u, type %i)",
-			 i_nlink, link.count + link.dir_count,
-			 mode_to_type(i_mode));
+			 "inode %llu i_link too small (%u < %u, type %i)",
+			 inode.k->p.inode, i_nlink,
+			 link.count, mode_to_type(i_mode));
+
+	if (S_ISDIR(i_mode)) {
+		cache_set_inconsistent_on(link.count > 1, c,
+			"directory %llu with multiple hardlinks: %u",
+			inode.k->p.inode, link.count);
+
+		real_i_nlink = link.count * 2 + link.dir_count;
+	} else {
+		cache_set_inconsistent_on(link.dir_count, c,
+			"found dirents for non directory %llu",
+			inode.k->p.inode);
+
+		real_i_nlink = link.count + link.dir_count;
+	}
 
 	if (!link.count) {
 		cache_set_inconsistent_on(CACHE_SET_CLEAN(&c->disk_sb), c,
@@ -129,11 +141,12 @@ static int bch_gc_do_inode(struct cache_set *c, struct btree_iter *iter,
 			"inode nlink %u, dir links found %u",
 			i_nlink, link.dir_count);
 
-		bch_verbose(c, "deleting inum %llu", inode.k->p.inode);
+		bch_verbose(c, "deleting inode %llu", inode.k->p.inode);
 
 		ret = bch_inode_rm(c, inode.k->p.inode);
 		if (ret)
-			bch_err(c, "error in fs gc: error %i while deleting inode", ret);
+			bch_err(c, "error in fs gc: error %i "
+				"while deleting inode", ret);
 		return ret;
 	}
 
@@ -172,7 +185,8 @@ static int bch_gc_do_inode(struct cache_set *c, struct btree_iter *iter,
 				"but inode %llu has i_sectors dirty",
 				inode.k->p.inode);
 
-		bch_verbose(c, "recounting sectors for inode %llu", inode.k->p.inode);
+		bch_verbose(c, "recounting sectors for inode %llu",
+			    inode.k->p.inode);
 
 		i_sectors = bch_count_inode_sectors(c, inode.k->p.inode);
 		if (i_sectors < 0) {
@@ -183,29 +197,28 @@ static int bch_gc_do_inode(struct cache_set *c, struct btree_iter *iter,
 		}
 	}
 
-	if (i_nlink != link.count + link.dir_count) {
+	if (i_nlink != real_i_nlink) {
 		cache_set_inconsistent_on(CACHE_SET_CLEAN(&c->disk_sb), c,
 				"filesystem marked clean, "
 				"but inode %llu has wrong i_nlink "
 				"(type %u i_nlink %u, should be %u)",
-				inode.k->p.inode,
-				mode_to_type(i_mode), i_nlink,
-				link.count + link.dir_count);
+				inode.k->p.inode, mode_to_type(i_mode),
+				i_nlink, real_i_nlink);
 
-		bch_verbose(c, "setting inum %llu nlinks from %u to %u",
-			    inode.k->p.inode, i_nlink,
-			    link.count + link.dir_count);
+		bch_verbose(c, "setting inode %llu nlinks from %u to %u",
+			    inode.k->p.inode, i_nlink, real_i_nlink);
 	}
 
-	if (i_nlink != link.count + link.dir_count ||
+	if (i_nlink != real_i_nlink||
 	    i_flags & BCH_INODE_I_SECTORS_DIRTY ||
 	    i_flags & BCH_INODE_I_SIZE_DIRTY) {
 		struct bkey_i_inode update;
 
 		bkey_reassemble(&update.k_i, inode.s_c);
-		update.v.i_nlink = cpu_to_le32(link.count + link.dir_count);
+		update.v.i_nlink = cpu_to_le32(real_i_nlink);
 		update.v.i_flags = cpu_to_le32(i_flags &
-				~(BCH_INODE_I_SIZE_DIRTY|BCH_INODE_I_SECTORS_DIRTY));
+				~(BCH_INODE_I_SIZE_DIRTY|
+				  BCH_INODE_I_SECTORS_DIRTY));
 
 		if (i_flags & BCH_INODE_I_SECTORS_DIRTY)
 			update.v.i_sectors = cpu_to_le64(i_sectors);
@@ -228,27 +241,33 @@ static int bch_gc_walk_inodes(struct cache_set *c, struct nlinks *links,
 	struct btree_iter iter;
 	struct bkey_s_c k;
 	struct nlink *link, zero_links = { 0, 0 };
+	struct genradix_iter nlinks_iter;
 	int ret = 0, ret2 = 0;
-	u64 i = 0;
+	u64 nlinks_pos;
 
 	bch_btree_iter_init(&iter, c, BTREE_ID_INODES, POS(range_start, 0));
+	genradix_iter_init(&nlinks_iter);
 
-	while ((k = bch_btree_iter_peek(&iter)).k) {
-		if (k.k->p.inode >= range_end)
-			break;
+	while (1) {
+		k = bch_btree_iter_peek(&iter);
+peek_nlinks:	link = genradix_iter_peek(&nlinks_iter, links);
 
-		link = genradix_ptr(links, i) ?: &zero_links;
+		if (!link && (!k.k || iter.pos.inode >= range_end))
+			break;
 
-		while (i < k.k->p.inode - range_start) {
-			cache_set_inconsistent_on(link->count, c,
-					 "missing inode %llu",
-					 range_start + i);
-			i++;
-			link = genradix_ptr(links, i) ?: &zero_links;
+		nlinks_pos = range_start + nlinks_iter.pos;
+		if (iter.pos.inode > nlinks_pos) {
+			cache_set_inconsistent_on(link && link->count, c,
+					 "missing inode %llu (nlink %u)",
+					 nlinks_pos, link->count);
+			genradix_iter_advance(&nlinks_iter, links);
+			goto peek_nlinks;
 		}
 
-		switch (k.k->type) {
-		case BCH_INODE_FS:
+		if (iter.pos.inode < nlinks_pos || !link)
+			link = &zero_links;
+
+		if (k.k && k.k->type == BCH_INODE_FS) {
 			/*
 			 * Avoid potential deadlocks with iter for
 			 * truncate/rm/etc.:
@@ -261,24 +280,23 @@ static int bch_gc_walk_inodes(struct cache_set *c, struct nlinks *links,
 			if (ret == -EINTR)
 				continue;
 			if (ret)
-				goto out;
+				break;
 
-			break;
-		default:
+			if (link->count)
+				atomic_long_inc(&c->nr_inodes);
+		} else {
 			cache_set_inconsistent_on(link->count, c,
-					 "missing inode %llu",
-					 range_start + i);
-			break;
+					 "missing inode %llu (nlink %u)",
+					 nlinks_pos, link->count);
 		}
 
-		if (link->count)
-			atomic_long_inc(&c->nr_inodes);
+		if (nlinks_pos == iter.pos.inode)
+			genradix_iter_advance(&nlinks_iter, links);
 
 		bch_btree_iter_advance_pos(&iter);
-		i++;
 		bch_btree_iter_cond_resched(&iter);
 	}
-out:
+
 	ret2 = bch_btree_iter_unlock(&iter);
 	if (ret2)
 		bch_err(c, "error in fs gc: btree error %i while walking inodes", ret2);
@@ -318,16 +336,16 @@ int bch_gc_inode_nlinks(struct cache_set *c)
 	return ret;
 }
 
-static inline bool next_inode(struct cache_set *c, struct bkey_s_c k,
+static inline bool next_inode(struct cache_set *c, u64 inum,
 			      u64 *cur_inum,
 			      struct bkey_i_inode *inode,
 			      struct bch_inode **bi,
 			      u64 *i_size, u16 *i_mode)
 {
-	if (k.k->p.inode == *cur_inum)
+	if (inum == *cur_inum)
 		return false;
 
-	if (!bch_inode_find_by_inum(c, k.k->p.inode, inode)) {
+	if (!bch_inode_find_by_inum(c, inum, inode)) {
 		*i_mode = le16_to_cpu(inode->v.i_mode);
 		*i_size = le64_to_cpu(inode->v.i_size);
 		*bi = &inode->v;
@@ -335,14 +353,11 @@ static inline bool next_inode(struct cache_set *c, struct bkey_s_c k,
 		*bi = NULL;
 	}
 
-	*cur_inum = k.k->p.inode;
+	*cur_inum = inum;
 	return true;
 }
 
-#define fsck_err(c, fmt, ...)					\
-do {								\
-	bch_err(c, fmt,  ##__VA_ARGS__);			\
-} while (0)
+#define fsck_err(c, ...)	cache_set_inconsistent(c, ##__VA_ARGS__)
 
 /*
  * Checks for inconsistencies that shouldn't happen, unless we have a bug.
@@ -354,9 +369,11 @@ void bch_fsck(struct cache_set *c)
 	struct bkey_s_c k;
 	struct bkey_i_inode inode;
 	struct bch_inode *bi = NULL;
+	struct bkey_s_c_dirent d;
 	u64 i_size = 0;
 	u16 i_mode = 0;
 	u64 cur_inum;
+	u64 d_inum;
 	char buf[100];
 
 	cur_inum = -1;
@@ -365,8 +382,8 @@ void bch_fsck(struct cache_set *c)
 		if (k.k->type == KEY_TYPE_DISCARD)
 			continue;
 
-		if (next_inode(c, k, &cur_inum, &inode, &bi,
-			       &i_size, &i_mode) &&
+		if (next_inode(c, k.k->p.inode, &cur_inum, &inode,
+			       &bi, &i_size, &i_mode) &&
 		    bi &&
 		    !(le32_to_cpu(bi->i_flags) & BCH_INODE_I_SECTORS_DIRTY)) {
 			u64 i_sectors = bch_count_inode_sectors(c, cur_inum);
@@ -397,22 +414,48 @@ void bch_fsck(struct cache_set *c)
 	cur_inum = -1;
 	for_each_btree_key(&iter, c, BTREE_ID_DIRENTS,
 			   POS(BCACHE_ROOT_INO, 0), k) {
-		next_inode(c, k, &cur_inum, &inode, &bi, &i_size, &i_mode);
+		switch (k.k->type) {
+		case BCH_DIRENT:
+			d = bkey_s_c_to_dirent(k);
+			d_inum = le64_to_cpu(d.v->d_inum);
+
+			if (d_inum == d.k->p.inode)
+				fsck_err(c, "dirent points to own directory");
+
+			next_inode(c, d_inum, &cur_inum, &inode,
+				   &bi, &i_size, &i_mode);
+
+			if (d.v->d_type != mode_to_type(i_mode))
+				fsck_err(c, "incorrect d_type: got %u should be %u, filename %s",
+					 d.v->d_type, mode_to_type(i_mode),
+					 d.v->d_name);
+			break;
+		}
+	}
+	bch_btree_iter_unlock(&iter);
+
+	cur_inum = -1;
+	for_each_btree_key(&iter, c, BTREE_ID_DIRENTS,
+			   POS(BCACHE_ROOT_INO, 0), k) {
+		next_inode(c, k.k->p.inode, &cur_inum, &inode,
+			   &bi, &i_size, &i_mode);
 
 		if (!bi)
-			fsck_err(c, "dirent for missing inode %llu", k.k->p.inode);
+			fsck_err(c, "dirent in nonexisting directory %llu", k.k->p.inode);
 
 		if (!S_ISDIR(i_mode))
 			fsck_err(c,
-				 "dirent for non directory, inode %llu mode %o",
+				 "dirent in non directory inode %llu mode %o",
 				 k.k->p.inode, i_mode);
+
 	}
 	bch_btree_iter_unlock(&iter);
 
 	cur_inum = -1;
 	for_each_btree_key(&iter, c, BTREE_ID_XATTRS,
 			   POS(BCACHE_ROOT_INO, 0), k) {
-		next_inode(c, k, &cur_inum, &inode, &bi, &i_size, &i_mode);
+		next_inode(c, k.k->p.inode, &cur_inum, &inode,
+			   &bi, &i_size, &i_mode);
 
 		if (!bi)
 			fsck_err(c, "xattr for missing inode %llu",
diff --git a/drivers/md/bcache/journal.c b/drivers/md/bcache/journal.c
index a382cc5cf84a..fd93cbfe2a19 100644
--- a/drivers/md/bcache/journal.c
+++ b/drivers/md/bcache/journal.c
@@ -334,6 +334,8 @@ int bch_journal_seq_should_ignore(struct cache_set *c, u64 seq, struct btree *b)
 			 "bset journal seq too far in the future: %llu > %llu",
 			 seq, j->seq);
 
+	bch_verbose(c, "blacklisting journal sequence number %llu", seq);
+
 	/*
 	 * When we start the journal, bch_journal_start() will skip over @seq:
 	 */