[PATCH 0/5] btrfs-progs: check: properly skip preallocated/nodatasum extents when re-calculating csum tree

[PATCH 0/5] btrfs-progs: check: properly skip preallocated/nodatasum extents when re-calculating csum tree

[Qu Wenruo]

Merry Christmas and happy new year!

[BUG]
Issue #420 mentions that --init-csum-tree creates extra csum for
preallocated extents.

Causing extra errors after --init-csum-tree.

[CAUSE]
Csum re-calculation code doesn't take the following factors into
consideration:

- NODATASUM inodes
- Preallocated extents
  No csum should be calculated for those data extents

- Partically written preallocated csum
  Csum should only be created for the referred part

So currently csum recalculation will just create csum for any data
extents it found, causing the mentioned errors.

[FIX]

- Bug fix for backref iteration
  Firstly fix a bug in backref code where indirect ref is not properly
  resolved.

  This allows us to use iterate_extent_inodes() to make our lives much
  easier checking the file extents.

- Code move for --init-csum-tree
  Move it to mode independent mode-common.[ch]

- Fix for --init-csum-tree
  Using iterate_extent_inodes() to do that.

- Fix for --init-csum-tree --init-extent-tree
  The same fix, just into a different context

- New test case

Qu Wenruo (5):
  btrfs-progs: backref: properly queue indirect refs
  btrfs-progs: check: move csum tree population into mode-common.[ch]
  btrfs-progs: check: don't calculate csum for preallocated file extents
  btrfs-progs: check: skip NODATASUM inodes for `--init-csum-tree
    --init-extent-tree`
  btrfs-progs: fsck-tests: add test case for init-csum-tree

 check/main.c                                | 244 -------------
 check/mode-common.c                         | 358 ++++++++++++++++++++
 check/mode-common.h                         |   1 +
 kernel-shared/backref.c                     |  10 +-
 tests/fsck-tests/052-init-csum-tree/test.sh |  54 +++
 5 files changed, 421 insertions(+), 246 deletions(-)
 create mode 100755 tests/fsck-tests/052-init-csum-tree/test.sh

-- 
2.34.1

[PATCH 1/5] btrfs-progs: backref: properly queue indirect refs

[Qu Wenruo]

[BUG]
When calling iterate_extent_inodes() on data extents with indirect ref
(with inline or keyed EXTENT_DATA_REF_KEY), it will fail to execute the
call back function at all.

[CAUSE]
In fucntion find_parent_nodes(), we only add the target tree block if a
backref has @parent populated.

For indirect backref like EXTENT_DATA_REF_KEY, we rely on
__resolve_indirect_ref() to get the parent leaves.

However __resolve_indirect_ref() only grabs backrefs from
&prefstate->pending_indirect_refs.

Meaning callers should queue any indrect backref to
pending_indirect_refs.

But unfortunately in __add_prelim_ref() and __add_missing_keys(), none
of them properly queue the indirect backrefs to pending_indirect_refs,
but directly to pending.

Making all indirect backrefs never got resolved, thus no callback
function executed

[FIX]
Fix __add_prelim_ref() and __add_missing_keys() to properly queue
indirect backrefs to the correct list.

Currently there is no such direct user in btrfs-progs, but later csum
tree re-initialization code will rely this to do proper csum
re-calculate (to avoid preallocated/nodatasum extents).

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 kernel-shared/backref.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/kernel-shared/backref.c b/kernel-shared/backref.c
index 42832c481cae..f1a638ededa8 100644
--- a/kernel-shared/backref.c
+++ b/kernel-shared/backref.c
@@ -192,7 +192,10 @@ static int __add_prelim_ref(struct pref_state *prefstate, u64 root_id,
 	ref->root_id = root_id;
 	if (key) {
 		ref->key_for_search = *key;
-		head = &prefstate->pending;
+		if (parent)
+			head = &prefstate->pending;
+		else
+			head = &prefstate->pending_indirect_refs;
 	} else if (parent) {
 		memset(&ref->key_for_search, 0, sizeof(ref->key_for_search));
 		head = &prefstate->pending;
@@ -467,7 +470,10 @@ static int __add_missing_keys(struct btrfs_fs_info *fs_info,
 		else
 			btrfs_node_key_to_cpu(eb, &ref->key_for_search, 0);
 		free_extent_buffer(eb);
-		list_move(&ref->list, &prefstate->pending);
+		if (ref->parent)
+			list_move(&ref->list, &prefstate->pending);
+		else
+			list_move(&ref->list, &prefstate->pending_indirect_refs);
 	}
 	return 0;
 }
-- 
2.34.1

[PATCH 2/5] btrfs-progs: check: move csum tree population into mode-common.[ch]

[Qu Wenruo]

This part has no mode specific operations, just move them into
mode-common.[ch].

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/main.c        | 244 --------------------------------------------
 check/mode-common.c | 244 ++++++++++++++++++++++++++++++++++++++++++++
 check/mode-common.h |   1 +
 3 files changed, 245 insertions(+), 244 deletions(-)

diff --git a/check/main.c b/check/main.c
index 540130b8e223..ea2f2d7b16b8 100644
--- a/check/main.c
+++ b/check/main.c
@@ -9557,250 +9557,6 @@ static int zero_log_tree(struct btrfs_root *root)
 	return ret;
 }
 
-static int populate_csum(struct btrfs_trans_handle *trans,
-			 struct btrfs_root *csum_root, char *buf, u64 start,
-			 u64 len)
-{
-	u64 offset = 0;
-	u64 sectorsize;
-	int ret = 0;
-
-	while (offset < len) {
-		sectorsize = gfs_info->sectorsize;
-		ret = read_extent_data(gfs_info, buf, start + offset,
-				       &sectorsize, 0);
-		if (ret)
-			break;
-		ret = btrfs_csum_file_block(trans, start + len, start + offset,
-					    buf, sectorsize);
-		if (ret)
-			break;
-		offset += sectorsize;
-	}
-	return ret;
-}
-
-static int fill_csum_tree_from_one_fs_root(struct btrfs_trans_handle *trans,
-					   struct btrfs_root *cur_root)
-{
-	struct btrfs_root *csum_root;
-	struct btrfs_path path;
-	struct btrfs_key key;
-	struct extent_buffer *node;
-	struct btrfs_file_extent_item *fi;
-	char *buf = NULL;
-	u64 start = 0;
-	u64 len = 0;
-	int slot = 0;
-	int ret = 0;
-
-	buf = malloc(gfs_info->sectorsize);
-	if (!buf)
-		return -ENOMEM;
-
-	btrfs_init_path(&path);
-	key.objectid = 0;
-	key.offset = 0;
-	key.type = 0;
-	ret = btrfs_search_slot(NULL, cur_root, &key, &path, 0, 0);
-	if (ret < 0)
-		goto out;
-	/* Iterate all regular file extents and fill its csum */
-	while (1) {
-		btrfs_item_key_to_cpu(path.nodes[0], &key, path.slots[0]);
-
-		if (key.type != BTRFS_EXTENT_DATA_KEY)
-			goto next;
-		node = path.nodes[0];
-		slot = path.slots[0];
-		fi = btrfs_item_ptr(node, slot, struct btrfs_file_extent_item);
-		if (btrfs_file_extent_type(node, fi) != BTRFS_FILE_EXTENT_REG)
-			goto next;
-		start = btrfs_file_extent_disk_bytenr(node, fi);
-		len = btrfs_file_extent_disk_num_bytes(node, fi);
-
-		csum_root = btrfs_csum_root(gfs_info, start);
-		ret = populate_csum(trans, csum_root, buf, start, len);
-		if (ret == -EEXIST)
-			ret = 0;
-		if (ret < 0)
-			goto out;
-next:
-		/*
-		 * TODO: if next leaf is corrupted, jump to nearest next valid
-		 * leaf.
-		 */
-		ret = btrfs_next_item(cur_root, &path);
-		if (ret < 0)
-			goto out;
-		if (ret > 0) {
-			ret = 0;
-			goto out;
-		}
-	}
-
-out:
-	btrfs_release_path(&path);
-	free(buf);
-	return ret;
-}
-
-static int fill_csum_tree_from_fs(struct btrfs_trans_handle *trans)
-{
-	struct btrfs_path path;
-	struct btrfs_root *tree_root = gfs_info->tree_root;
-	struct btrfs_root *cur_root;
-	struct extent_buffer *node;
-	struct btrfs_key key;
-	int slot = 0;
-	int ret = 0;
-
-	btrfs_init_path(&path);
-	key.objectid = BTRFS_FS_TREE_OBJECTID;
-	key.offset = 0;
-	key.type = BTRFS_ROOT_ITEM_KEY;
-	ret = btrfs_search_slot(NULL, tree_root, &key, &path, 0, 0);
-	if (ret < 0)
-		goto out;
-	if (ret > 0) {
-		ret = -ENOENT;
-		goto out;
-	}
-
-	while (1) {
-		node = path.nodes[0];
-		slot = path.slots[0];
-		btrfs_item_key_to_cpu(node, &key, slot);
-		if (key.objectid > BTRFS_LAST_FREE_OBJECTID)
-			goto out;
-		if (key.type != BTRFS_ROOT_ITEM_KEY)
-			goto next;
-		if (!is_fstree(key.objectid))
-			goto next;
-		key.offset = (u64)-1;
-
-		cur_root = btrfs_read_fs_root(gfs_info, &key);
-		if (IS_ERR(cur_root) || !cur_root) {
-			fprintf(stderr, "Fail to read fs/subvol tree: %lld\n",
-				key.objectid);
-			goto out;
-		}
-		ret = fill_csum_tree_from_one_fs_root(trans, cur_root);
-		if (ret < 0)
-			goto out;
-next:
-		ret = btrfs_next_item(tree_root, &path);
-		if (ret > 0) {
-			ret = 0;
-			goto out;
-		}
-		if (ret < 0)
-			goto out;
-	}
-
-out:
-	btrfs_release_path(&path);
-	return ret;
-}
-
-static int fill_csum_tree_from_extent(struct btrfs_trans_handle *trans,
-				      struct btrfs_root *extent_root)
-{
-	struct btrfs_root *csum_root;
-	struct btrfs_path path;
-	struct btrfs_extent_item *ei;
-	struct extent_buffer *leaf;
-	char *buf;
-	struct btrfs_key key;
-	int ret;
-
-	btrfs_init_path(&path);
-	key.objectid = 0;
-	key.type = BTRFS_EXTENT_ITEM_KEY;
-	key.offset = 0;
-	ret = btrfs_search_slot(NULL, extent_root, &key, &path, 0, 0);
-	if (ret < 0) {
-		btrfs_release_path(&path);
-		return ret;
-	}
-
-	buf = malloc(gfs_info->sectorsize);
-	if (!buf) {
-		btrfs_release_path(&path);
-		return -ENOMEM;
-	}
-
-	while (1) {
-		if (path.slots[0] >= btrfs_header_nritems(path.nodes[0])) {
-			ret = btrfs_next_leaf(extent_root, &path);
-			if (ret < 0)
-				break;
-			if (ret) {
-				ret = 0;
-				break;
-			}
-		}
-		leaf = path.nodes[0];
-
-		btrfs_item_key_to_cpu(leaf, &key, path.slots[0]);
-		if (key.type != BTRFS_EXTENT_ITEM_KEY) {
-			path.slots[0]++;
-			continue;
-		}
-
-		ei = btrfs_item_ptr(leaf, path.slots[0],
-				    struct btrfs_extent_item);
-		if (!(btrfs_extent_flags(leaf, ei) &
-		      BTRFS_EXTENT_FLAG_DATA)) {
-			path.slots[0]++;
-			continue;
-		}
-
-		csum_root = btrfs_csum_root(gfs_info, key.objectid);
-		ret = populate_csum(trans, csum_root, buf, key.objectid,
-				    key.offset);
-		if (ret)
-			break;
-		path.slots[0]++;
-	}
-
-	btrfs_release_path(&path);
-	free(buf);
-	return ret;
-}
-
-/*
- * Recalculate the csum and put it into the csum tree.
- *
- * Extent tree init will wipe out all the extent info, so in that case, we
- * can't depend on extent tree, but use fs tree.  If search_fs_tree is set, we
- * will use fs/subvol trees to init the csum tree.
- */
-static int fill_csum_tree(struct btrfs_trans_handle *trans,
-			  int search_fs_tree)
-{
-	struct btrfs_root *root;
-	struct rb_node *n;
-	int ret;
-
-	if (search_fs_tree)
-		return fill_csum_tree_from_fs(trans);
-
-	root = btrfs_extent_root(gfs_info, 0);
-	while (1) {
-		ret = fill_csum_tree_from_extent(trans, root);
-		if (ret)
-			break;
-		n = rb_next(&root->rb_node);
-		if (!n)
-			break;
-		root = rb_entry(n, struct btrfs_root, rb_node);
-		if (root->root_key.objectid != BTRFS_EXTENT_TREE_OBJECTID)
-			break;
-	}
-	return ret;
-}
-
 static void free_roots_info_cache(void)
 {
 	if (!roots_info_cache)
diff --git a/check/mode-common.c b/check/mode-common.c
index 62a9837bde5e..299d66d644e7 100644
--- a/check/mode-common.c
+++ b/check/mode-common.c
@@ -1191,3 +1191,247 @@ error:
 	btrfs_abort_transaction(trans, ret);
 	return ret;
 }
+
+static int populate_csum(struct btrfs_trans_handle *trans,
+			 struct btrfs_root *csum_root, char *buf, u64 start,
+			 u64 len)
+{
+	u64 offset = 0;
+	u64 sectorsize;
+	int ret = 0;
+
+	while (offset < len) {
+		sectorsize = gfs_info->sectorsize;
+		ret = read_extent_data(gfs_info, buf, start + offset,
+				       &sectorsize, 0);
+		if (ret)
+			break;
+		ret = btrfs_csum_file_block(trans, start + len, start + offset,
+					    buf, sectorsize);
+		if (ret)
+			break;
+		offset += sectorsize;
+	}
+	return ret;
+}
+
+static int fill_csum_tree_from_one_fs_root(struct btrfs_trans_handle *trans,
+					   struct btrfs_root *cur_root)
+{
+	struct btrfs_root *csum_root;
+	struct btrfs_path path;
+	struct btrfs_key key;
+	struct extent_buffer *node;
+	struct btrfs_file_extent_item *fi;
+	char *buf = NULL;
+	u64 start = 0;
+	u64 len = 0;
+	int slot = 0;
+	int ret = 0;
+
+	buf = malloc(gfs_info->sectorsize);
+	if (!buf)
+		return -ENOMEM;
+
+	btrfs_init_path(&path);
+	key.objectid = 0;
+	key.offset = 0;
+	key.type = 0;
+	ret = btrfs_search_slot(NULL, cur_root, &key, &path, 0, 0);
+	if (ret < 0)
+		goto out;
+	/* Iterate all regular file extents and fill its csum */
+	while (1) {
+		btrfs_item_key_to_cpu(path.nodes[0], &key, path.slots[0]);
+
+		if (key.type != BTRFS_EXTENT_DATA_KEY)
+			goto next;
+		node = path.nodes[0];
+		slot = path.slots[0];
+		fi = btrfs_item_ptr(node, slot, struct btrfs_file_extent_item);
+		if (btrfs_file_extent_type(node, fi) != BTRFS_FILE_EXTENT_REG)
+			goto next;
+		start = btrfs_file_extent_disk_bytenr(node, fi);
+		len = btrfs_file_extent_disk_num_bytes(node, fi);
+
+		csum_root = btrfs_csum_root(gfs_info, start);
+		ret = populate_csum(trans, csum_root, buf, start, len);
+		if (ret == -EEXIST)
+			ret = 0;
+		if (ret < 0)
+			goto out;
+next:
+		/*
+		 * TODO: if next leaf is corrupted, jump to nearest next valid
+		 * leaf.
+		 */
+		ret = btrfs_next_item(cur_root, &path);
+		if (ret < 0)
+			goto out;
+		if (ret > 0) {
+			ret = 0;
+			goto out;
+		}
+	}
+
+out:
+	btrfs_release_path(&path);
+	free(buf);
+	return ret;
+}
+
+static int fill_csum_tree_from_fs(struct btrfs_trans_handle *trans)
+{
+	struct btrfs_path path;
+	struct btrfs_root *tree_root = gfs_info->tree_root;
+	struct btrfs_root *cur_root;
+	struct extent_buffer *node;
+	struct btrfs_key key;
+	int slot = 0;
+	int ret = 0;
+
+	btrfs_init_path(&path);
+	key.objectid = BTRFS_FS_TREE_OBJECTID;
+	key.offset = 0;
+	key.type = BTRFS_ROOT_ITEM_KEY;
+	ret = btrfs_search_slot(NULL, tree_root, &key, &path, 0, 0);
+	if (ret < 0)
+		goto out;
+	if (ret > 0) {
+		ret = -ENOENT;
+		goto out;
+	}
+
+	while (1) {
+		node = path.nodes[0];
+		slot = path.slots[0];
+		btrfs_item_key_to_cpu(node, &key, slot);
+		if (key.objectid > BTRFS_LAST_FREE_OBJECTID)
+			goto out;
+		if (key.type != BTRFS_ROOT_ITEM_KEY)
+			goto next;
+		if (!is_fstree(key.objectid))
+			goto next;
+		key.offset = (u64)-1;
+
+		cur_root = btrfs_read_fs_root(gfs_info, &key);
+		if (IS_ERR(cur_root) || !cur_root) {
+			fprintf(stderr, "Fail to read fs/subvol tree: %lld\n",
+				key.objectid);
+			goto out;
+		}
+		ret = fill_csum_tree_from_one_fs_root(trans, cur_root);
+		if (ret < 0)
+			goto out;
+next:
+		ret = btrfs_next_item(tree_root, &path);
+		if (ret > 0) {
+			ret = 0;
+			goto out;
+		}
+		if (ret < 0)
+			goto out;
+	}
+
+out:
+	btrfs_release_path(&path);
+	return ret;
+}
+
+static int fill_csum_tree_from_extent(struct btrfs_trans_handle *trans,
+				      struct btrfs_root *extent_root)
+{
+	struct btrfs_root *csum_root;
+	struct btrfs_path path;
+	struct btrfs_extent_item *ei;
+	struct extent_buffer *leaf;
+	char *buf;
+	struct btrfs_key key;
+	int ret;
+
+	btrfs_init_path(&path);
+	key.objectid = 0;
+	key.type = BTRFS_EXTENT_ITEM_KEY;
+	key.offset = 0;
+	ret = btrfs_search_slot(NULL, extent_root, &key, &path, 0, 0);
+	if (ret < 0) {
+		btrfs_release_path(&path);
+		return ret;
+	}
+
+	buf = malloc(gfs_info->sectorsize);
+	if (!buf) {
+		btrfs_release_path(&path);
+		return -ENOMEM;
+	}
+
+	while (1) {
+		if (path.slots[0] >= btrfs_header_nritems(path.nodes[0])) {
+			ret = btrfs_next_leaf(extent_root, &path);
+			if (ret < 0)
+				break;
+			if (ret) {
+				ret = 0;
+				break;
+			}
+		}
+		leaf = path.nodes[0];
+
+		btrfs_item_key_to_cpu(leaf, &key, path.slots[0]);
+		if (key.type != BTRFS_EXTENT_ITEM_KEY) {
+			path.slots[0]++;
+			continue;
+		}
+
+		ei = btrfs_item_ptr(leaf, path.slots[0],
+				    struct btrfs_extent_item);
+		if (!(btrfs_extent_flags(leaf, ei) &
+		      BTRFS_EXTENT_FLAG_DATA)) {
+			path.slots[0]++;
+			continue;
+		}
+
+		csum_root = btrfs_csum_root(gfs_info, key.objectid);
+		ret = populate_csum(trans, csum_root, buf, key.objectid,
+				    key.offset);
+		if (ret)
+			break;
+		path.slots[0]++;
+	}
+
+	btrfs_release_path(&path);
+	free(buf);
+	return ret;
+}
+
+/*
+ * Recalculate the csum and put it into the csum tree.
+ *
+ * @search_fs_tree:	How to get the data extent item.
+ * 			If true, iterate all fs roots to get all
+ * 			extent data (which can be slow).
+ * 			Otherwise, search extent tree for extent data.
+ */
+int fill_csum_tree(struct btrfs_trans_handle *trans, bool search_fs_tree)
+{
+	struct btrfs_root *root;
+	struct rb_node *n;
+	int ret;
+
+	if (search_fs_tree)
+		return fill_csum_tree_from_fs(trans);
+
+	root = btrfs_extent_root(gfs_info, 0);
+	while (1) {
+		ret = fill_csum_tree_from_extent(trans, root);
+		if (ret)
+			break;
+		n = rb_next(&root->rb_node);
+		if (!n)
+			break;
+		root = rb_entry(n, struct btrfs_root, rb_node);
+		if (root->root_key.objectid != BTRFS_EXTENT_TREE_OBJECTID)
+			break;
+	}
+	return ret;
+}
diff --git a/check/mode-common.h b/check/mode-common.h
index 8cd135b9bfc2..b894c2be4f97 100644
--- a/check/mode-common.h
+++ b/check/mode-common.h
@@ -199,4 +199,5 @@ static inline void btrfs_check_subpage_eb_alignment(struct btrfs_fs_info *info,
 int repair_dev_item_bytes_used(struct btrfs_fs_info *fs_info,
 			       u64 devid, u64 bytes_used_expected);
 
+int fill_csum_tree(struct btrfs_trans_handle *trans, bool search_fs_tree);
 #endif
-- 
2.34.1

[PATCH 3/5] btrfs-progs: check: don't calculate csum for preallocated file extents

[Qu Wenruo]

[BUG]
If a btrfs filesystem has preallocated file extents, `btrfs check
--init-csum-tree` will create csum item for preallocated extents, and
cause error:

  # mkfs.btrfs -f test.img
  # mount test.img /mnt/btrfs
  # fallocate -l 32K /mnt/btrfs/file
  # umount /mnt/btrfs
  # btrfs check --init-csum-tree --force test.img
  ...
  [4/7] checking fs roots
  root 5 inode 257 errors 800, odd csum item
  ERROR: errors found in fs roots
  found 376832 bytes used, error(s) found

And the csum tree is not empty, containing csum for the preallocated
extent:

  $ btrfs ins dump-tree -t csum test.img
  btrfs-progs v5.15.1
  checksum tree key (CSUM_TREE ROOT_ITEM 0)
  leaf 30408704 items 1 free space 16226 generation 9 owner CSUM_TREE
  leaf 30408704 flags 0x1(WRITTEN) backref revision 1
  fs uuid ecc79835-5611-4609-b985-e4ccd6f15b54
  chunk uuid b1c75553-5b82-4aa6-bbbe-e7f50643b1a8
  	item 0 key (EXTENT_CSUM EXTENT_CSUM 13631488) itemoff 16251 itemsize 32
  		range start 13631488 end 13664256 length 32768

[CAUSE]
For `--init-csum-tree` alone, we will use extent tree to iterate each
data extent, and calcluate csum for them.

But extent items alone can not tell us if the file extent belongs to a
NODATASUM inode, nor if it's preallocated.

Thus we create csums for those data extents, and cause the problem.

[FIX]
Fix it by iterate all inode items and file extent items to skip the
following cases:

- Inodes with NODATASUM flags
- Preallocated/hole file extents

Issue: #430
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/mode-common.c | 103 ++++++++++++++++++++++++++++++++++++++------
 1 file changed, 90 insertions(+), 13 deletions(-)

diff --git a/check/mode-common.c b/check/mode-common.c
index 299d66d644e7..dd0b1d695bfa 100644
--- a/check/mode-common.c
+++ b/check/mode-common.c
@@ -22,6 +22,7 @@
 #include "common/utils.h"
 #include "kernel-shared/disk-io.h"
 #include "kernel-shared/volumes.h"
+#include "kernel-shared/backref.h"
 #include "common/repair.h"
 #include "check/mode-common.h"
 
@@ -1338,14 +1339,99 @@ out:
 	return ret;
 }
 
+static int fill_csum_for_file_extent(u64 ino, u64 offset, u64 rootid, void *ctx)
+{
+	struct btrfs_trans_handle *trans = (struct btrfs_trans_handle *)ctx;
+	struct btrfs_fs_info *fs_info = trans->fs_info;
+	struct btrfs_file_extent_item *fi;
+	struct btrfs_inode_item *ii;
+	struct btrfs_path path = {};
+	struct btrfs_key key;
+	struct btrfs_root *root;
+	struct btrfs_root *csum_root;
+	char *buf;
+	u64 disk_bytenr;
+	u64 disk_len;
+	int ret = 0;
+
+	key.objectid = rootid;
+	key.type = BTRFS_ROOT_ITEM_KEY;
+	key.offset = (u64)-1;
+	root = btrfs_read_fs_root(fs_info, &key);
+	if (IS_ERR(root)) {
+		ret = PTR_ERR(root);
+		goto out;
+	}
+
+	/* Check if the inode has NODATASUM flag */
+	key.objectid = ino;
+	key.type = BTRFS_INODE_ITEM_KEY;
+	key.offset = 0;
+	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
+	if (ret > 0)
+		ret = -ENOENT;
+	if (ret < 0)
+		goto out;
+
+	ii = btrfs_item_ptr(path.nodes[0], path.slots[0],
+			    struct btrfs_inode_item);
+	if (btrfs_inode_flags(path.nodes[0], ii) & BTRFS_INODE_NODATASUM) {
+		/* The inode has NODATASUM flag, not need to calculate csum */
+		ret = 0;
+		goto out;
+	}
+
+	btrfs_release_path(&path);
+
+	/* Check the file extent item to make sure it's not preallocated */
+	key.objectid = ino;
+	key.type = BTRFS_EXTENT_DATA_KEY;
+	key.offset = offset;
+	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
+	if (ret > 0)
+		ret = -ENOENT;
+	if (ret < 0)
+		goto out;
+	fi = btrfs_item_ptr(path.nodes[0], path.slots[0],
+			    struct btrfs_file_extent_item);
+	if (btrfs_file_extent_type(path.nodes[0], fi) != BTRFS_FILE_EXTENT_REG)
+		goto out;
+	disk_bytenr = btrfs_file_extent_disk_bytenr(path.nodes[0], fi);
+	disk_len = btrfs_file_extent_disk_num_bytes(path.nodes[0], fi);
+	if (!disk_bytenr)
+		goto out;
+
+	/*
+	 * Now it's non-hole regular file extents, get its real on-disk bytenr
+	 * for csum population.
+	 */
+	if (btrfs_file_extent_compression(path.nodes[0], fi) ==
+	    BTRFS_COMPRESS_NONE) {
+		disk_bytenr += btrfs_file_extent_offset(path.nodes[0], fi);
+		disk_len = btrfs_file_extent_num_bytes(path.nodes[0], fi);
+	}
+	btrfs_release_path(&path);
+
+	csum_root = btrfs_csum_root(fs_info, disk_bytenr);
+	buf = malloc(fs_info->sectorsize);
+	if (!buf) {
+		ret = -ENOMEM;
+		goto out;
+	}
+	/* Insert the csum */
+	ret = populate_csum(trans, csum_root, buf, disk_bytenr, disk_len);
+	free(buf);
+out:
+	btrfs_release_path(&path);
+	return ret;
+}
+
 static int fill_csum_tree_from_extent(struct btrfs_trans_handle *trans,
 				      struct btrfs_root *extent_root)
 {
-	struct btrfs_root *csum_root;
 	struct btrfs_path path;
 	struct btrfs_extent_item *ei;
 	struct extent_buffer *leaf;
-	char *buf;
 	struct btrfs_key key;
 	int ret;
 
@@ -1359,12 +1445,6 @@ static int fill_csum_tree_from_extent(struct btrfs_trans_handle *trans,
 		return ret;
 	}
 
-	buf = malloc(gfs_info->sectorsize);
-	if (!buf) {
-		btrfs_release_path(&path);
-		return -ENOMEM;
-	}
-
 	while (1) {
 		if (path.slots[0] >= btrfs_header_nritems(path.nodes[0])) {
 			ret = btrfs_next_leaf(extent_root, &path);
@@ -1390,17 +1470,14 @@ static int fill_csum_tree_from_extent(struct btrfs_trans_handle *trans,
 			path.slots[0]++;
 			continue;
 		}
-
-		csum_root = btrfs_csum_root(gfs_info, key.objectid);
-		ret = populate_csum(trans, csum_root, buf, key.objectid,
-				    key.offset);
+		ret = iterate_extent_inodes(trans->fs_info, key.objectid, 0, 0,
+					    fill_csum_for_file_extent, trans);
 		if (ret)
 			break;
 		path.slots[0]++;
 	}
 
 	btrfs_release_path(&path);
-	free(buf);
 	return ret;
 }
 
-- 
2.34.1

[PATCH 4/5] btrfs-progs: check: skip NODATASUM inodes for `--init-csum-tree --init-extent-tree`

[Qu Wenruo]

When using `--init-csum-tree` with `--init-extent-tree`, csum tree
population will be done by iterating all file extent items.

This allow us to skip preallocated extents, but it still has the
folllwing problems:

- Inodes with NODATASUM

- Hole file extents

- Written preallocated extents
  We will generate csum for the whole extent, while other part may still
  be unwritten.

Make it to follow the same behavior of recently introduced
fill_csum_for_file_extent(), so we can generate correct csum.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/mode-common.c | 43 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 40 insertions(+), 3 deletions(-)

diff --git a/check/mode-common.c b/check/mode-common.c
index dd0b1d695bfa..1c1374bae70b 100644
--- a/check/mode-common.c
+++ b/check/mode-common.c
@@ -1225,6 +1225,7 @@ static int fill_csum_tree_from_one_fs_root(struct btrfs_trans_handle *trans,
 	struct extent_buffer *node;
 	struct btrfs_file_extent_item *fi;
 	char *buf = NULL;
+	u64 skip_ino = 0;
 	u64 start = 0;
 	u64 len = 0;
 	int slot = 0;
@@ -1245,15 +1246,51 @@ static int fill_csum_tree_from_one_fs_root(struct btrfs_trans_handle *trans,
 	while (1) {
 		btrfs_item_key_to_cpu(path.nodes[0], &key, path.slots[0]);
 
-		if (key.type != BTRFS_EXTENT_DATA_KEY)
+		if (key.type != BTRFS_EXTENT_DATA_KEY &&
+		    key.type != BTRFS_INODE_ITEM_KEY)
+			goto next;
+
+		/* This item belongs to an inode with NODATASUM, skip it */
+		if (key.objectid == skip_ino)
+			goto next;
+
+		if (key.type == BTRFS_INODE_ITEM_KEY) {
+			struct btrfs_inode_item *ii;
+
+			ii = btrfs_item_ptr(path.nodes[0], path.slots[0],
+					    struct btrfs_inode_item);
+			/* Check if the inode has NODATASUM flag */
+			if (btrfs_inode_flags(path.nodes[0], ii) &
+			    BTRFS_INODE_NODATASUM)
+				skip_ino = key.objectid;
 			goto next;
+		}
 		node = path.nodes[0];
 		slot = path.slots[0];
 		fi = btrfs_item_ptr(node, slot, struct btrfs_file_extent_item);
+
+		/* Skip preallocated/inline extents */
 		if (btrfs_file_extent_type(node, fi) != BTRFS_FILE_EXTENT_REG)
 			goto next;
-		start = btrfs_file_extent_disk_bytenr(node, fi);
-		len = btrfs_file_extent_disk_num_bytes(node, fi);
+
+		/* Skip hole extents */
+		if (btrfs_file_extent_disk_bytenr(node, fi) == 0)
+			goto next;
+
+		if (btrfs_file_extent_compression(node, fi) ==
+		    BTRFS_COMPRESS_NONE) {
+			/*
+			 * Non-compressed extent, only calculate csum for the
+			 * referred part, as the original larger extent can
+			 * be preallocated.
+			 */
+			start = btrfs_file_extent_disk_bytenr(node, fi) +
+				btrfs_file_extent_offset(node, fi);
+			len = btrfs_file_extent_num_bytes(node, fi);
+		} else {
+			start = btrfs_file_extent_disk_bytenr(node, fi);
+			len = btrfs_file_extent_disk_num_bytes(node, fi);
+		}
 
 		csum_root = btrfs_csum_root(gfs_info, start);
 		ret = populate_csum(trans, csum_root, buf, start, len);
-- 
2.34.1

[PATCH 5/5] btrfs-progs: fsck-tests: add test case for init-csum-tree

[Qu Wenruo]

This new test script will create a fs with the following situations:

- Preallocated extents (no data csum)
- Nodatasum inodes (no data csum)
- Partially written preallocated extents (no data csum for part of the
  extent)
- Regular data extents (with data csum)

And make sure after --init-csum-tree (with or without
--init-extent-tree) the result fs can still pass fsck.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 tests/fsck-tests/052-init-csum-tree/test.sh | 54 +++++++++++++++++++++
 1 file changed, 54 insertions(+)
 create mode 100755 tests/fsck-tests/052-init-csum-tree/test.sh

diff --git a/tests/fsck-tests/052-init-csum-tree/test.sh b/tests/fsck-tests/052-init-csum-tree/test.sh
new file mode 100755
index 000000000000..d3bf03fab491
--- /dev/null
+++ b/tests/fsck-tests/052-init-csum-tree/test.sh
@@ -0,0 +1,54 @@
+#!/bin/bash
+#
+# Verify that `btrfs check --init-csum-tree` can handle various nodatasum
+# cases.
+
+source "$TEST_TOP/common"
+
+check_prereq btrfs
+check_global_prereq fallocate
+check_global_prereq dd
+setup_root_helper
+prepare_test_dev
+
+run_check_mkfs_test_dev
+
+# Create an inode with nodatasum and some content
+run_check_mount_test_dev -o nodatasum
+
+run_check $SUDO_HELPER dd if=/dev/urandom of="$TEST_MNT/nodatasum_file" \
+	bs=16k count=1 status=noxfer > /dev/null 2>&1
+
+# Revert to default datasum
+run_check $SUDO_HELPER mount -o remount,datasum "$TEST_MNT"
+
+# Then create an inode with datasum, but all preallocated extents
+run_check fallocate -l 32k "$TEST_MNT/prealloc1"
+
+# Create preallocated extent but partially written
+run_check fallocate -l 32k "$TEST_MNT/prealloc2"
+run_check $SUDO_HELPER dd if=/dev/urandom of="$TEST_MNT/prealloc2" \
+	bs=16k count=1 conv=notrunc status=noxfer> /dev/null 2>&1
+
+# Then some regular files
+run_check $SUDO_HELPER dd if=/dev/urandom of="$TEST_MNT/regular" \
+	bs=16k count=1 status=noxfer > /dev/null 2>&1
+
+# And create a snapshot, every data extent is at least shared twice
+run_check $SUDO_HELPER "$TOP/btrfs" subvolume snapshot "$TEST_MNT" \
+	"$TEST_MNT/snapshot"
+run_check_umount_test_dev
+
+# --init-csum-tree should not fail
+run_check $SUDO_HELPER "$TOP/btrfs" check --force \
+	--init-csum-tree "$TEST_DEV"
+
+# No error should be found
+run_check $SUDO_HELPER "$TOP/btrfs" check "$TEST_DEV"
+
+# --init-csum-tree with --init-extent-tree should not fail
+run_check $SUDO_HELPER "$TOP/btrfs" check --force \
+	--init-csum-tree --init-extent-tree "$TEST_DEV"
+
+# No error should be found
+run_check $SUDO_HELPER "$TOP/btrfs" check "$TEST_DEV"
-- 
2.34.1

Re: [PATCH 0/5] btrfs-progs: check: properly skip preallocated/nodatasum extents when re-calculating csum tree

[Josef Bacik]

On Fri, Dec 24, 2021 at 01:50:14PM +0800, Qu Wenruo wrote:
> Merry Christmas and happy new year!
> 
> [BUG]
> Issue #420 mentions that --init-csum-tree creates extra csum for
> preallocated extents.
> 
> Causing extra errors after --init-csum-tree.
> 
> [CAUSE]
> Csum re-calculation code doesn't take the following factors into
> consideration:
> 
> - NODATASUM inodes
> - Preallocated extents
>   No csum should be calculated for those data extents
> 
> - Partically written preallocated csum
>   Csum should only be created for the referred part
> 
> So currently csum recalculation will just create csum for any data
> extents it found, causing the mentioned errors.
> 
> [FIX]
> 
> - Bug fix for backref iteration
>   Firstly fix a bug in backref code where indirect ref is not properly
>   resolved.
> 
>   This allows us to use iterate_extent_inodes() to make our lives much
>   easier checking the file extents.
> 
> - Code move for --init-csum-tree
>   Move it to mode independent mode-common.[ch]
> 
> - Fix for --init-csum-tree
>   Using iterate_extent_inodes() to do that.
> 
> - Fix for --init-csum-tree --init-extent-tree
>   The same fix, just into a different context
> 
> - New test case
> 

Eesh that's not great.  Thanks for fixing this up,

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef