[PATCH 0/7] btrfs: check: Check and repair invalid free space cahce inode mode

[PATCH 0/7] btrfs: check: Check and repair invalid free space cahce inode mode

[Qu Wenruo]

This patchset can be fetched from github:
https://github.com/adam900710/btrfs-progs/tree/free_space_cache_imode_fix
Which is based on v4.20.2 tag.

There is a minor problem reported in mail list where invalid inode mode
(0) can sneak into free space cache inodes:
        item 9 key (256 INODE_ITEM 0) itemoff 13702 itemsize 160
                generation 30 transid 30 size 65536 nbytes 1507328
                block group 0 mode 0 links 1 uid 0 gid 0 rdev 0
                sequence 23 flags 0x1b(NODATASUM|NODATACOW|NOCOMPRESS|PREALLOC)
                atime 0.0 (1970-01-01 08:00:00)
                ctime 1553491158.189771625 (2019-03-25 13:19:18)
                mtime 0.0 (1970-01-01 08:00:00)
                otime 0.0 (1970-01-01 08:00:00)

Kernel free space cache code doesn't realy care about the inode mode,
but tree-checker inode item checker cares and will report this as big
problem, rejecting the image.

I didn't see any reason to make inode item checker any less strict, as
there is already fuzzed image can break kernel using invalid inode mode.

So here I just enhance btrfs-progs before strict inode item checker
lands.

Next time when users report kernel rejecting previously valid fs, they
should have a way to fix it in btrfs-progs.

Qu Wenruo (7):
  btrfs-progs: check/lowmem: Add inode mode check
  btrfs-progs: check/original: Add inode mode check
  btrfs-progs: check/lowmem: Repair invalid inode mode in root tree
  btrfs-progs: check/original: Repair invalid inode mode in root tree
  btrfs: check/lowmem: Check and repair free space cache inode mode
  btrfs: check/original: Check and repair free space cache inode item
  btrfs: tests/fsck: Add test image for free space cache mode repair

 check/main.c                                  |  36 +++++++
 check/mode-common.c                           |  48 +++++++++
 check/mode-common.h                           |  27 +++++
 check/mode-lowmem.c                           |  98 ++++++++++++++++++
 check/mode-lowmem.h                           |   3 +
 check/mode-original.h                         |   1 +
 .../test.raw.xz                               | Bin 0 -> 159128 bytes
 7 files changed, 213 insertions(+)
 create mode 100644 tests/fsck-tests/038-bad-free-space-cache-inode-mode/test.raw.xz

-- 
2.21.0

[PATCH 1/7] btrfs-progs: check/lowmem: Add inode mode check

[Qu Wenruo]

There is one report about invalid free space cache inode mode.
Normally free space cache inode should have mode 100600 (regular file,
no uid/gid/sticky bit, rw------ bit).

But in that report, we have free space cache inode mode as 0.

So at least btrfs check should report invalid inode mode.

This patch will at least make btrfs check lowmem mode to detect this
problem.

Please note that, this check only applies to inodes in fs/subvol trees.
It doesn't apply to free space cache inodes.

Reported-by: Thorsten Hirsch <t.hirsch@web.de>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/mode-common.h | 24 ++++++++++++++++++++++++
 check/mode-lowmem.c |  5 +++++
 check/mode-lowmem.h |  1 +
 3 files changed, 30 insertions(+)

diff --git a/check/mode-common.h b/check/mode-common.h
index 1fb3cda7d89d..4c88365abbcc 100644
--- a/check/mode-common.h
+++ b/check/mode-common.h
@@ -126,4 +126,28 @@ int delete_corrupted_dir_item(struct btrfs_trans_handle *trans,
 			      struct btrfs_key *di_key, char *namebuf,
 			      u32 namelen);
 
+/*
+ * Check if the inode mode @imode is valid
+ *
+ * This check focuses on S_FTMT bits and unused bits.
+ * Sticky/setuid/setgid and regular owner/group/other bits won't cause
+ * any problem.
+ */
+static inline bool is_valid_imode(u32 imode)
+{
+	if (imode & ~(S_IFMT | 07777))
+		return false;
+
+	/*
+	 * S_IFMT is not bitmap, nor pure numbering sequence. Need per valid
+	 * number check.
+	 */
+	imode &= S_IFMT;
+	if (imode != S_IFDIR && imode != S_IFCHR && imode != S_IFBLK &&
+	    imode != S_IFREG && imode != S_IFIFO && imode != S_IFLNK &&
+	    imode != S_IFSOCK)
+		return false;
+	return true;
+}
+
 #endif
diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index fc6228a05a1b..1553a4a5d2c1 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -2452,6 +2452,11 @@ static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path)
 	nlink = btrfs_inode_nlink(node, ii);
 	nodatasum = btrfs_inode_flags(node, ii) & BTRFS_INODE_NODATASUM;
 
+	if (!is_valid_imode(mode)) {
+		error("invalid imode mode bits: 0%o", mode);
+		err |= INODE_MODE_ERROR;
+	}
+
 	if (S_ISLNK(mode) &&
 	    flags & (BTRFS_INODE_IMMUTABLE | BTRFS_INODE_APPEND)) {
 		err |= INODE_FLAGS_ERROR;
diff --git a/check/mode-lowmem.h b/check/mode-lowmem.h
index 46b9b191ce24..e0ab30b770d5 100644
--- a/check/mode-lowmem.h
+++ b/check/mode-lowmem.h
@@ -46,6 +46,7 @@
 #define FATAL_ERROR             (1<<22) /* Fatal bit for errno */
 #define INODE_FLAGS_ERROR	(1<<23) /* Invalid inode flags */
 #define DIR_ITEM_HASH_MISMATCH	(1<<24) /* Dir item hash mismatch */
+#define INODE_MODE_ERROR	(1<<25) /* Bad inode mode */
 
 /*
  * Error bit for low memory mode check.
-- 
2.21.0

[PATCH 2/7] btrfs-progs: check/original: Add inode mode check

[Qu Wenruo]

Just like lowmem mode, check inode mode, specially for S_IFMT bits and
beyond.

Please note that, this check only applies to inodes in fs/subvol trees.
It doesn't apply to free space cache inodes.

Reported-by: Thorsten Hirsch <t.hirsch@web.de>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/main.c          | 5 +++++
 check/mode-original.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/check/main.c b/check/main.c
index 7547209c5604..553c93caa2c9 100644
--- a/check/main.c
+++ b/check/main.c
@@ -616,6 +616,9 @@ static void print_inode_error(struct btrfs_root *root, struct inode_record *rec)
 		fprintf(stderr, ", odd inode flags");
 	if (errors & I_ERR_INLINE_RAM_BYTES_WRONG)
 		fprintf(stderr, ", invalid inline ram bytes");
+	if (errors & I_ERR_INVALID_IMODE)
+		fprintf(stderr, ", invalid inode mode bit 0%o",
+			rec->imode & ~07777);
 	fprintf(stderr, "\n");
 	/* Print the orphan extents if needed */
 	if (errors & I_ERR_FILE_EXTENT_ORPHAN)
@@ -811,6 +814,8 @@ static void maybe_free_inode_rec(struct cache_tree *inode_cache,
 	if (!rec->checked || rec->merging)
 		return;
 
+	if (!is_valid_imode(rec->imode))
+		rec->errors |= I_ERR_INVALID_IMODE;
 	if (S_ISDIR(rec->imode)) {
 		if (rec->found_size != rec->isize)
 			rec->errors |= I_ERR_DIR_ISIZE_WRONG;
diff --git a/check/mode-original.h b/check/mode-original.h
index 25ca274118a7..e40a12930a6f 100644
--- a/check/mode-original.h
+++ b/check/mode-original.h
@@ -189,6 +189,7 @@ struct file_extent_hole {
 #define I_ERR_ODD_INODE_FLAGS		(1 << 16)
 #define I_ERR_INLINE_RAM_BYTES_WRONG	(1 << 17)
 #define I_ERR_MISMATCH_DIR_HASH		(1 << 18)
+#define I_ERR_INVALID_IMODE		(1 << 19)
 
 struct inode_record {
 	struct list_head backrefs;
-- 
2.21.0

[PATCH 3/7] btrfs-progs: check/lowmem: Repair invalid inode mode in root tree

[Qu Wenruo]

In root tree, we only have 2 types of inodes:
- ROOT_TREE_DIR inode
  Its mode is fixed to 40755
- free space cache inodes
  Its mode is fixed to 100600

This patch will add the ability to repair such inodes to lowmem mode.
For fs/subvolume tree error, at least we haven't see such corruption
yet, so we don't need to rush to fix corruption in fs trees yet.

The repair function, repair_imode() can be reused by later original mode
patch, so it's placed in check/mode-common.c.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/mode-common.c | 48 +++++++++++++++++++++++++++++++++++++++++++
 check/mode-common.h |  2 ++
 check/mode-lowmem.c | 50 ++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 99 insertions(+), 1 deletion(-)

diff --git a/check/mode-common.c b/check/mode-common.c
index fed102b0ce7a..1c150a3d5a7c 100644
--- a/check/mode-common.c
+++ b/check/mode-common.c
@@ -795,3 +795,51 @@ out:
 	btrfs_release_path(&path);
 	return ret;
 }
+
+/*
+ * Reset inode mode with invalid mode to its known correct value.
+ *
+ * Only support free space cache inode and root tree dir inode of root tree.
+ * Caller should ensure such condition.
+ *
+ * Return 0 if repair is done, @path will point to the correct inode item.
+ * Return <0 for errors.
+ */
+int repair_imode(struct btrfs_trans_handle *trans, struct btrfs_root *root,
+		 struct btrfs_path *path, u64 ino)
+{
+	struct btrfs_inode_item *iitem;
+	struct extent_buffer *leaf;
+	struct btrfs_key key;
+	u32 correct_mode;
+	int slot;
+	int ret;
+
+	ASSERT(root->root_key.objectid == BTRFS_ROOT_TREE_OBJECTID);
+	ASSERT(ino == BTRFS_ROOT_TREE_DIR_OBJECTID || is_fstree(ino));
+
+	if (ino == BTRFS_ROOT_TREE_DIR_OBJECTID)
+		correct_mode = 040755;	/* Directory, 0755 */
+	else
+		correct_mode = 0100600;	/* Regular file, 0600 */
+
+	key.objectid = ino;
+	key.type = BTRFS_INODE_ITEM_KEY;
+	key.offset = 0;
+
+	ret = btrfs_search_slot(trans, root, &key, path, 0, 1);
+	if (ret > 0)
+		ret = -ENOENT;
+	if (ret < 0) {
+		errno = -ret;
+		error("failed to search tree %llu: %m",
+		      root->root_key.objectid);
+		return ret;
+	}
+	leaf = path->nodes[0];
+	slot = path->slots[0];
+	iitem = btrfs_item_ptr(leaf, slot, struct btrfs_inode_item);
+	btrfs_set_inode_mode(leaf, iitem, correct_mode);
+	btrfs_mark_buffer_dirty(leaf);
+	return ret;
+}
diff --git a/check/mode-common.h b/check/mode-common.h
index 4c88365abbcc..8895747adf1d 100644
--- a/check/mode-common.h
+++ b/check/mode-common.h
@@ -125,6 +125,8 @@ int delete_corrupted_dir_item(struct btrfs_trans_handle *trans,
 			      struct btrfs_root *root,
 			      struct btrfs_key *di_key, char *namebuf,
 			      u32 namelen);
+int repair_imode(struct btrfs_trans_handle *trans, struct btrfs_root *root,
+		 struct btrfs_path *path, u64 ino);
 
 /*
  * Check if the inode mode @imode is valid
diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 1553a4a5d2c1..f37c3b42e6b6 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -2396,6 +2396,48 @@ static bool has_orphan_item(struct btrfs_root *root, u64 ino)
 	return false;
 }
 
+static int repair_imode_lowmem(struct btrfs_root *root,
+			       struct btrfs_path *path)
+{
+	struct btrfs_trans_handle *trans;
+	struct btrfs_key key;
+	int ret;
+
+	if (root->root_key.objectid != BTRFS_ROOT_TREE_OBJECTID) {
+		error(
+		"repair inode mode outside of root tree is not supported yet");
+		return -ENOTTY;
+	}
+	btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
+	ASSERT(key.type == BTRFS_INODE_ITEM_KEY);
+	if (key.objectid != BTRFS_ROOT_TREE_DIR_OBJECTID &&
+	    !is_fstree(key.objectid)) {
+		error("unsupported ino %llu", key.objectid);
+		return -ENOTTY;
+	}
+
+	trans = btrfs_start_transaction(root, 1);
+	if (IS_ERR(trans)) {
+		ret = PTR_ERR(trans);
+		errno = -ret;
+		error("failed to start transaction: %m");
+		return ret;
+	}
+	btrfs_release_path(path);
+
+	ret = repair_imode(trans, root, path, key.objectid);
+	if (ret < 0)
+		goto abort;
+	ret = btrfs_commit_transaction(trans, root);
+	if (!ret)
+		printf("reset mode for inode %llu root %llu\n",
+			key.objectid, root->root_key.objectid);
+	return ret;
+abort:
+	btrfs_abort_transaction(trans, ret);
+ 	return ret;
+}
+
 /*
  * Check INODE_ITEM and related ITEMs (the same inode number)
  * 1. check link count
@@ -2454,7 +2496,13 @@ static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path)
 
 	if (!is_valid_imode(mode)) {
 		error("invalid imode mode bits: 0%o", mode);
-		err |= INODE_MODE_ERROR;
+		if (repair) {
+			ret = repair_imode_lowmem(root, path);
+			if (ret < 0)
+				err |= INODE_MODE_ERROR;
+		} else {
+			err |= INODE_MODE_ERROR;
+		}
 	}
 
 	if (S_ISLNK(mode) &&
-- 
2.21.0

[PATCH 4/7] btrfs-progs: check/original: Repair invalid inode mode in root tree

[Qu Wenruo]

This patch will reuse the mode independent repair_imode() function, to
repair invalid inode mode.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/main.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/check/main.c b/check/main.c
index 553c93caa2c9..5db0716d23a1 100644
--- a/check/main.c
+++ b/check/main.c
@@ -2698,6 +2698,24 @@ static int repair_mismatch_dir_hash(struct btrfs_trans_handle *trans,
 	return ret;
 }
 
+static int repair_imode_original(struct btrfs_trans_handle *trans,
+				 struct btrfs_root *root,
+				 struct btrfs_path *path,
+				 struct inode_record *rec)
+{
+	int ret;
+
+	if (root->root_key.objectid != BTRFS_ROOT_TREE_OBJECTID)
+		return 0;
+	if (rec->ino != BTRFS_ROOT_TREE_DIR_OBJECTID || !is_fstree(rec->ino))
+		return 0;
+	ret = repair_imode(trans, root, path, rec->ino);
+	if (ret < 0)
+		return ret;
+	rec->errors &= ~I_ERR_INVALID_IMODE;
+	return ret;
+}
+
 static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec)
 {
 	struct btrfs_trans_handle *trans;
@@ -2745,6 +2763,8 @@ static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec)
 		ret = repair_inode_nbytes(trans, root, &path, rec);
 	if (!ret && rec->errors & I_ERR_INLINE_RAM_BYTES_WRONG)
 		ret = repair_inline_ram_bytes(trans, root, &path, rec);
+	if (!ret && rec->errors & I_ERR_INVALID_IMODE)
+		ret = repair_imode_original(trans, root, &path, rec);
 	btrfs_commit_transaction(trans, root);
 	btrfs_release_path(&path);
 	return ret;
-- 
2.21.0

[PATCH 5/7] btrfs: check/lowmem: Check and repair free space cache inode mode

[Qu Wenruo]

Unlike inodes in fs roots, we don't really check the inode items in root
tree, in fact we just skip everything other than ROOT_ITEM and ROOT_REF.

This makes invalid inode items sneak into root tree.
For example:
        item 9 key (256 INODE_ITEM 0) itemoff 13702 itemsize 160
                generation 30 transid 30 size 65536 nbytes 1507328
                block group 0 mode 0 links 1 uid 0 gid 0 rdev 0
				   ^ Should be 100600
                sequence 23 flags 0x1b(NODATASUM|NODATACOW|NOCOMPRESS|PREALLOC)
                atime 0.0 (1970-01-01 08:00:00)
                ctime 1553491158.189771625 (2019-03-25 13:19:18)
                mtime 0.0 (1970-01-01 08:00:00)
                otime 0.0 (1970-01-01 08:00:00)

There is a report of such problem in the mail list.

This patch will check and repair inode items of free space cache inodes in
lowmem mode.

Since free space cache inodes doesn't have INODE_REF but still has 1
link, we can't use check_inode_item() directly.
Instead we only check the inode mode, as that's the important part.

The check and repair function: check_repair_free_space_inode() is also
exported for original mode.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/mode-common.h |  1 +
 check/mode-lowmem.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 check/mode-lowmem.h |  2 ++
 3 files changed, 48 insertions(+)

diff --git a/check/mode-common.h b/check/mode-common.h
index 8895747adf1d..855f83617854 100644
--- a/check/mode-common.h
+++ b/check/mode-common.h
@@ -24,6 +24,7 @@
 #include <sys/stat.h>
 #include "ctree.h"
 
+#define FREE_SPACE_CACHE_INODE_MODE	(0100600)
 /*
  * Use for tree walk to walk through trees whose leaves/nodes can be shared
  * between different trees. (Namely subvolume/fs trees)
diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index f37c3b42e6b6..cdb9b66a63a3 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -5091,6 +5091,40 @@ out:
 	return err;
 }
 
+/*
+ * For free space inodes, we can't call check_inode_item() as free space
+ * cache inode doesn't have INODE_REF.
+ * We just check its inode mode.
+ */
+int check_repair_free_space_inode(struct btrfs_fs_info *fs_info,
+				  struct btrfs_path *path)
+{
+	struct btrfs_inode_item *iitem;
+	struct btrfs_key key;
+	u32 mode;
+	int ret = 0;
+
+	btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
+	ASSERT(key.type == BTRFS_INODE_ITEM_KEY && is_fstree(key.objectid));
+	iitem = btrfs_item_ptr(path->nodes[0], path->slots[0],
+			       struct btrfs_inode_item);
+	mode = btrfs_inode_mode(path->nodes[0], iitem);
+	if (mode != FREE_SPACE_CACHE_INODE_MODE) {
+		error(
+	"free space cache inode %llu has invalid mode: has 0%o expect 0%o",
+			key.objectid, mode, FREE_SPACE_CACHE_INODE_MODE);
+		ret = -EUCLEAN;
+		if (repair) {
+			ret = repair_imode_lowmem(fs_info->tree_root,
+						  path);
+			if (ret < 0)
+				return ret;
+			return ret;
+		}
+	}
+	return ret;
+}
+
 /*
  * Check all fs/file tree in low_memory mode.
  *
@@ -5130,6 +5164,17 @@ int check_fs_roots_lowmem(struct btrfs_fs_info *fs_info)
 		btrfs_item_key_to_cpu(node, &key, slot);
 		if (key.objectid > BTRFS_LAST_FREE_OBJECTID)
 			goto out;
+		if (key.type == BTRFS_INODE_ITEM_KEY &&
+		    is_fstree(key.objectid)) {
+			ret = check_repair_free_space_inode(fs_info, &path);
+			/* Check if we still have a valid path to continue */
+			if (ret < 0 && path.nodes[0]) {
+				err |= ret;
+				goto next;
+			}
+			if (ret < 0 && !path.nodes[0])
+				goto out;
+		}
 		if (key.type == BTRFS_ROOT_ITEM_KEY &&
 		    fs_root_objectid(key.objectid)) {
 			if (key.objectid == BTRFS_TREE_RELOC_OBJECTID) {
diff --git a/check/mode-lowmem.h b/check/mode-lowmem.h
index e0ab30b770d5..d2983fd12eb4 100644
--- a/check/mode-lowmem.h
+++ b/check/mode-lowmem.h
@@ -67,5 +67,7 @@
 
 int check_fs_roots_lowmem(struct btrfs_fs_info *fs_info);
 int check_chunks_and_extents_lowmem(struct btrfs_fs_info *fs_info);
+int check_repair_free_space_inode(struct btrfs_fs_info *fs_info,
+				  struct btrfs_path *path);
 
 #endif
-- 
2.21.0

[PATCH 6/7] btrfs: check/original: Check and repair free space cache inode item

[Qu Wenruo]

Just like lowmem mode, also check and repair free space cache inode
item.

And since we don't really have a good timing/function to check free
space chace inodes, we use the same lowmem mode
check_repair_free_space_inode() when iterating root tree.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/main.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/check/main.c b/check/main.c
index 5db0716d23a1..b920165e3d0d 100644
--- a/check/main.c
+++ b/check/main.c
@@ -3620,6 +3620,17 @@ again:
 			   key.type == BTRFS_ROOT_BACKREF_KEY) {
 			process_root_ref(leaf, path.slots[0], &key,
 					 root_cache);
+		} else if (key.type == BTRFS_INODE_ITEM_KEY &&
+			   is_fstree(key.objectid)) {
+			ret = check_repair_free_space_inode(fs_info, &path);
+			if (ret < 0 && !path.nodes[0]) {
+				err = 1;
+				goto out;
+			}
+			if (ret < 0 && path.nodes[0]) {
+				err = 1;
+				goto next;
+			}
 		}
 next:
 		path.slots[0]++;
-- 
2.21.0

[PATCH 7/7] btrfs: tests/fsck: Add test image for free space cache mode repair

[Qu Wenruo]

The image has one free space cache inode with invalid mode (0).
        item 9 key (256 INODE_ITEM 0) itemoff 13702 itemsize 160
                generation 30 transid 30 size 65536 nbytes 1507328
                block group 0 mode 0 links 1 uid 0 gid 0 rdev 0
                sequence 23 flags 0x1b(NODATASUM|NODATACOW|NOCOMPRESS|PREALLOC)
                atime 0.0 (1970-01-01 08:00:00)
                ctime 1553491158.189771625 (2019-03-25 13:19:18)
                mtime 0.0 (1970-01-01 08:00:00)
                otime 0.0 (1970-01-01 08:00:00)

Both lowmem and original mode should be able to detect and fix it.

The extracted test image is pretty big (1G extracted), as kernel won't
cache small chunks.
Even with SSD, such test may still take some seconds just extracting the
image.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 .../test.raw.xz                                | Bin 0 -> 159128 bytes
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 tests/fsck-tests/038-bad-free-space-cache-inode-mode/test.raw.xz

diff --git a/tests/fsck-tests/038-bad-free-space-cache-inode-mode/test.raw.xz b/tests/fsck-tests/038-bad-free-space-cache-inode-mode/test.raw.xz
new file mode 100644
index 0000000000000000000000000000000000000000..c2603c024098992bece235d27187eefa696b3fe0
GIT binary patch
literal 159128
zcmeI5c{JB+*T%_|kTGN`Ny-=vGN%zgWGHizsHl+npiGgu5Q$8cF+#=^DI`;7GNzO$
zgow_MywCHlXRYU5?{m(2b55%FoXg*>)mr<vzx%#E*WUNO=OtbA3`j`G2ix=ZEhFJ2
z<su;=VQA2+TUel9W^tH=#Cm>V;b!5&etD%;wo*qmi^4Vz-t?vVJocK$t3|9*T9%ZM
zbG|wtKuzo>U8j5UKoB3JcE3J*r9;3gr`dul?zEO=efG*`M)73ifu^El?BhqbrsO8(
z72Ab4UpKX6a$Q+}+d+M~;$hPu?LA4Ug(siCc^SH=?FnbgdhTS0{-MTAJBC*rRu3<x
zx<7h7BFE%;vDw6Z`RSBB{io>;5RaHd%7^9midOP+J?cD3^^v?(yQ&~q^^vk=?~SGy
z>b+Ml?_OCPv87?Bsw2UsglR1D<Ke2F{MzGIX#(5Z8M(S|vDdWJGIiG#^R!;!IUJST
zpv6(lQs%O~dWE6U+ooKiPI$iOZ1J}4w^;$R?xbOP`^*~FoRbJL60sDverq)r_{i&1
za8|sSBionpPli#=L)N)BxQVKxQ*n;x^CbwZ>UvQe=V?q<D@PAF$yXgcb-v2_SEjC+
zIWqDhS^v9QNkccO1xL~s3^rug(z1r7luBJ%|I#Kdh40A7##*1Ih^T<`f}xWF4l}FE
zD5KP(=AUP^U8b+#*PfjY-7Zd%ls#ts$VP2HZBr;?dZr0?@v|HOb|Zb?;p19*rNfRH
z`mf&@QY`;m)s;C%)Dp6<=P_Kn>vONR-0UVZY8j7O!P77DWyUK?M31D?%M*_8y3D+C
zfgybMth4?sP5nZ|#(nf@b8daa=*^-!WtV(DdV7|cFWG<1^dZb5IevkGRET86PCI43
z^p2Z2M|@$hxYge=8ej86Xwoa(MZgx3xd_-IKXn8Q0bAO`1Q~`5Lxz1fkp)??1cDef
z3^fcj3^nX$C2JIY6#b?3ZjfQfFl5+IWvAkk#3zYQ5})M%=avv^7-|@57-|@5*xzUl
z07V}~A4MNU|L;-sl}V*Z-oHAf+OvJ4x~@O>*1h(lC*OOow9%>-&#jzpyJVU-QuDng
zu)e19{ycMM6k;Mqn0~_a)6xw(qOXg-F8aFY>!Ppw7a1}C8i9kNkD`yFkD`yFkD`yO
zN56Lm45uVcNt}{6C2>mPl*B2CQ}P>KEr+attbnY5tbnY5tbnY5tbnZeF)Mzc!=S^y
zj}8;YvZw!R>(+!SwbQROm6r}gdSTuP^G=v|`Y!9}Fe`vD^q*x6J(bk!uX{{Bq%#uQ
z%{hOlG@UF=Vb?0#*uE>HcYEUNM)j^+jtBJ#r})gPHh5l$mm7IrM}9!Ami+m+_CRBZ
zxn*lse8R?oSJkuCr&LN5)6}L}9|RS5ZsA({OOHppuI(P{z^jU*w>ju<O^6WL_iWpE
z+~HZy+YNlFvy_3I)9&qboC@chZhN!vS~az~e3WO=HvRo|nKRxkG|ur00Tgn7{CCLw
zu%@=Tos4^eWw~#AtaR<8?M^Xug!uS@P($<n5au{`yO&!k4vHFU*dM;VrqegbgTv_I
z6f>pqi6iRqjH1_6Lew_q8?$aX9#EIP{7k)<likJ8%P~)+h7WX!eQCH%uvFMlS37>h
z)%t8<afdC%-5|D(hpBbf2g|tjK5=8%C%NaWZgYzI1i@W4+_-!4(S>FYRgtvTu_hkJ
zqWdflYbh%(`zEtRI*3J)+{sWCby&{AA!K!=$BpQaaD(b_bco-AQ&tCQS7^2ib>{y0
z6?Tpn%WT$vCMyfQV{39~%}v`E#~jt@$`5?J9kWi3nT3}^#b(QNlYFE}&7_9xOODFZ
z(#nBpOfoqy(xdh%o4XUkynN^Lr1qYtwJ*=PSMQKIA9?A(OrD9w%Kh05xfAPVPsf*f
zD7h+koPFRcTP5Jyer#3Ots=^T9TVB&6dGC#7n-L=meoD&yeTZ;Xpkw$f0Mn<oW1yC
z!^*qS0RlA*vg_6O8p3=>4HC|LzC95@M>Sm4_wY-}#5Vetw#Q777w$y|pB1)lzb;eP
zCs0@INu2a8H!Z2FTv>96gSfq}P-#<^onouM1@D%%1x1P7BFrYy9{YCd$|(fT1#I9M
ziv8ujrbYe8#kDno+?(Q+mhC-e_%v>v@$`Pb2a3`(t?mh3>GaZFsymu$PF}v$YV7_d
zws$UH)yjq<m`YDDy;4yl?G&ppm8*x9=)f;l(e)QgF3Ryacx%eW)@(M}JXbeO&O`Me
z{ca84Dmw;sM(H=4H->LZ$?US2DS3QKd7gy5^h{3XdTuVFR<_=j+6S8k>^9^~9xrG~
z(suOlYH0ql<M^rmqy*B)sPq{3vuwi>qa(#xjkiWMn>(Ut?lpbsyXTSQr}tLsNT!>1
zc~C}M)(Yy$N|DWWVf8eAPab+-Zj}z^o>jbZ&-_fx2m4_0{Oell>1M8)1z6a55boE|
ztM#u`%5HSAwkkQevr#LU@$#C=DL<Qi9yG=JJ!$FMT?X0oMDcy%?v70IS}WF=on70)
zK_CVu@j6a$SNrq1(|O8fN#0@?(N1rhUM{G#zN*rYboklpoKqWvDL+N;-Id0A&^cx7
zIcuqIZ^46<wB(NSS81&+x?S7u&es}D*IeA~d90k<%)D+V(_?R0(Xwq4b@%xv+!)?_
zl@J`x9TJ(T5%KruFE4+n*&ORr#PhCHG`!;sgH)c-6PBE$l*7yqF0}d`AkE^5I6tEP
zakqPD;-TxB`2)kdJ?l4^nHiC`W>A}(M2Ilycy_NKvk!EPp^pr!$s*tJt1t~aA;iId
z`R9y+4Ntea6ur_BRg+=gn=xOk`Ydg@?8Lfl%H1z7HA=JbI#Tl99o#54+!EwOr7~FI
zctBBsYOp^!lR9yWZffah3`hRwp-_SuF)GJAB==5UZ)np{m6wq35V3N5U`B6>=0MpC
z2Ev}iH>;lb(_Up^YH{B`-0!+0eqGDUm23w(@=dlU@W_)0^}Suk)XCDzy|X=o@@~La
z)UTgm-EVium3i<B_yzm|e)$>qj|2Dsd_N2DVF~_Mzea#o1zHu~r&WP>DZcW75PXvO
zB=JdNiVIU*nBu||7pAx{#f2%ZAO5@?>>b!Uuy?Rl?(eZy4hA9&#HD)w74{D79oRdt
zcVO?p-hsUXdk6Lo>>b!UdP^m@f8fS#D19h>D19h>D19h>D19h>D19h>D19jXGRi2m
zsQKqvZI|gQ_<tau{J{SjXaqC@8Uc-fMnEH=5zq)|^dr&e4ypSeU(Av{X8n`bFs;#a
zN`;sFvSdn%xxKL)*hMF(1Pcmiv)qpMIngOSH5QhC`G&m9E_UXG^{nC1gS#9wV-j6@
zZNkr-xXL5(@mwO=v}@pk1m{}iyKPpYr?ztiN_M{!s&b!9rZ)_c2sF(ypxytKSG#^&
zadS%F_x<N~9Ev~lBb;oY%p5eWe9W8GG<&g!C+hwCtn;<qHy6V6^DDex*+|ROM^YX#
z;!WKkLS;SSaO}|)vrm4)GUi3?p6MU@XvDNGla1LBSnj00lhRRBS$-@if{xBHQj+Y%
zPRqT^OlPDN7lx(GsfJQ4J+!8jM`wyZGp{=+Q6qTe8FPi%YYoYFLh+#=SL<4oCfzmL
zA#OWPcJXkR{szBQ>S1iEYb{f#>q;)2S+#oEvfS2Rja4L%B-KZcnJ?TX`QJ-}G%wmF
z_G#xAYI)8C8tBhxdwKa^9i6gGF4~=N;!xIGvMhCx;fzNO_kEi?S|5MDc<!l@HFIX4
z$rBgnYdc+^1RW(9MJHBUUw9fTKz%_@h^0K~`VNvh=O!sJ4r?!^jilvqw~RBqiZg=f
zGD+V=wJ>e7c$n^OZ@pS=N8B^Zh-)M1k~2NJ@(QNU&p*A!cBaTZ<Wu<k1_cWByW{aU
zLuu}bI_R)^F5I7`mzpvUY1h74P_xY0kmw|QZ|rCf|FSr@_2tB!v#CmwxnXt>I$A>l
zD2XzcwYZkO&Z-@vVO?=;I5{l-nyKf>j2CNci6Nm|oqWiH*laYeH3>iGN@@)Kpr$y&
zzC(Pp<<;SmC`bJmvzXo0S?5=n$h?Ylp!AVr-b&bdX+~C{O8-Xds?;^a$>!_hiqTEx
ztD=cCgPl(6?fjj2lSHUnw=j%Fm$;{qyYp^&@K7?`Q<TP?)HA_rk~OY<d9S=^vEkPF
z6|$P)g`A-xIv#p4yG070Hplqgs#Ueh()Kc9o75etx1k)96o}Gxaq|y3S8%l=!S%GM
zu(npglV@TM+nd*1$Yw|ovD~iKUwlV#+m|SUT$dM>ph`?&9A|(<v*9hrSn^(<kqL?k
z_T<47dpWL_*<%C)VQL4*nQQ)667@ICDeL{xL|3t&$2rmUoMm*zZsrx|?o6$4(QfZ#
z*xlSu{47=$T-MONw&OtdyNji5{z7>TR4V=4gQvqUh<UCqcKF12HF1D4+e6Rt7~wuK
zFZ+GJ0W*P*>qX?+mmNOubE!D2FWfLV&+Dy`X|r~ixa!<qRw)AYvP3Q=t<A4J__K$Y
zds^9BB@=5m&ii*#2^6oJE?;lx8l5oHTd&U46Ir6ixo=K|%}Zr9WvX}X3%}9V@}pHY
zua7l8RIE+QDW5U7Z&P*_6l3)Ju<d=N)!RI;5h1$4SiiwHS2!!H8uh}(sI6-cH}X7~
zbgeZ$`6=q)ahG?S3UW^ks<x9@U%MsEsxcX!>{D?zNn4v)s`_l$9tMVn{5DB@8b^x;
z-A2OErxHxcQk;7^`<Q&4l^;3GNtQl0EviuHOiuR_m^YkK(baQSV=5obq3<~HV8Byv
zD*J7xgzmb$4s&Ayci8Uy6H5E0YxT<v8*lrbOw{B`cx$wmJL2>HU)A4gy$Yk$Ssk}0
z?p5>aeV-1F`#A`2?tZLsFm&BjO0px>HZm+_5|U9#t4NcEZc+=5q%Rn3$grhl4NEDN
zy0reKO<W4!k&%tHK1~r(0p|rnCj}g4RxgQ*(fC*`0$Xmd<px`Bu;m6@ZfKiA8PhXO
zaOYyl&z%?fGUJPMWPe-Bf0y41?)f5rD+CMyL%RX(2DBS~a;vT@bB?G5Gx$fE!MOX1
zyRS<%j1L_K9R?i+9R?i+9fq+FjD0}qL+N889ux6P(o2S?pCiC-w6t;?O^`(b9Rh}c
z;gejt0U{KA6#b<gQA36y!;oQRmwf)U4j7*PA{7k+hJY=t1Vn}*!;oQUv!Ts~HXGV(
zXtSZshBh18Y-qEg&BhNj0vZ90fJQ*0za1LIlXm`@1uH-2mK!7S;Ij)K_Od!p?@=+n
zBA|LA?5J?mV2S!+;)vo-QlHHegq|z52{(H6WJ&U9?FDTD>SF`_KNLOodC^fk*InJ-
zB$620;koW<B*~$~^&Fcf+r=rL`;k1m{$zr_=)>c(6Ir=aeDCIu3vX6ek|v9p9_M&J
z;-uSFk}%m|{fN_$nnaiF(cZJ+sVvIe>VZd88n#$T*fEzncZJ;&l2%)_$^YHL7vXp3
zN;emnc*pfiP8+%DbUQ}$ahKjO)ZXg0Z%*Tztf5f(<rzPgMqp_KmPTM{1eQi%X~g$h
zR)k3qOoCt%1d|}tG9G{IVQ839Fr#2b!Hj|#^|v#luw`SBTQ=xPCH{Kuor5St7rpgn
zscSbR2?mT3;vDgX!QX6P!@eu*F>Eo|Vz9+vi@_FyEe2bRKJ7nQ0}KuZ2ZR6TcY%xQ
zi~KGSFa&IA3kfm|8HNnQ?g($sGV_XmO)|%m*8lZvF~?{f#*}r^GH1e1d*93SVyZI^
zsT=3l<lO2=n|zc};b_5tf6cDVUJ8@HL=z<#d&Ai{a+h6t#UC_gt2HR|NyqjB_eApW
zOoWZ~Y?D;eo|!aJ4vDDDofhV<-sc9U+^6P_J&sKZPo|(07bqOMqiecTZsOvm)1P&_
zMpg`}xVf=iu#C13D&)Fp#Two-NUU<YSE*voZ>5u^M3W&Jyers%??lUuZkK%#1Rkqv
zp<#iau1+P^X~=CfaeK%~%jxif(XA&vyGo%_BTItsY0q+vA!fP40gqz7)LF{F&T02{
zI!=XiPPe^Tc&(b+Tt3RP{QD8KO6`|>;n=~kgJTEB4vrlhJ2-Z5><rib>FEGioQvc}
z1PlRNTF^#@A;XYi=sNuAvmKuBA`1xuhJY<?<wb@e!;oQU)BMji4aU6}Iqr>sAz*lg
zQdH;Pw<{qlAS)m%AS)m%1W*5g%?of`7g;_LFa&IA&l_YIG7K4pCJ34!Sby-%ZoNBZ
z=)ZnrNU{8LRafR5QR^G6z6X3S@~A?<5U`~^+mT_&Fk~3~F!*8c!-}6R{U%bl6^lG?
z5HJL6X%A>*7%~hQ1_KcWA`C<rh<NAp8=1Y36_6E>6_6E>6_6E>6_6E>6?i@!&!^-0
zbUdGq=hMf5MnEH=5zq)|1T+E~0gZr0KqH_L&<JP*G%^Aj0gZr0KqH_L&<JP*Gy)m{
zjetf#BcKt`=r`UT#ues8c2f~B1Z-(N7GxMQ3>k*40@x~mtpeC8@Ov&EPDz}SI3;mP
z;*`WGiBl4%<TvVdK~_LkKvqCjKvqCjKvqCjKvw*i5gO<)=rHIo=rHIo=rHIo=rHIo
z=rA{Nj`+f0Tx|HgYJjDYi`2{r7y`Dmkbw+Ch9SdnBNsPvaU&PP6~gr!VJu_?WCdge
zWCdgeWCdgeWCdi!k8#JL!=S^U!=S^U!=S^U!=S^U!=S^ka19IBuy73v*RXI63)ir4
zZ7t9UXaqC@8Uc-fMnEH=5zq)|1T+E~0gZr0)<7em5zq)|1T+E~0gZr0KqH_L&<JP*
zGy)o7(MLVd2xtT}0vZ90fJQ(gpb^jrXaqC@8Uc-fMp{54pb^jrXaqC@8Uc-fMnEH=
z5zq)|1T+E~Z3h|wjetf#BcKt`2xtT}0vZ90fJQ(gpb^k09cTnJ0vZ90fJQ(gpb^jr
zXaqC@8Uc-fMnEGipb^jrXaqC@8Uc-fMnEH=5zq)|1T+E~0gYAzjetf#BcKt`2xtT}
z0vZ90fJQ(gpb^jrXjB0-0vZ90fJQ(gpb^jrXaqC@8Uc-fMnEH=kqpoXXaqC@8Uc-f
zMnEH=5zq)|1T+E~0gZr0>_8)+5zq)|1T+E~0gZr0KqH_L&<JP*Gy)pY1C4-2KqH_L
z&<JP*Gy)m{jetf#BcRduKqJpC4-!_A5j*Xa`O-UX;vDgX|E<Hs1AHV%{_$7TAo-m1
iabF_|=~x%F$3OnaNk~d>QI@f=$!sS3=RY9VH~24oN>cy;

literal 0
HcmV?d00001

-- 
2.21.0

Re: [PATCH 5/7] btrfs: check/lowmem: Check and repair free space cache inode mode

[Su Yue]

On 2019/3/25 4:22 PM, Qu Wenruo wrote:
> Unlike inodes in fs roots, we don't really check the inode items in root
> tree, in fact we just skip everything other than ROOT_ITEM and ROOT_REF.
>
> This makes invalid inode items sneak into root tree.
> For example:
>          item 9 key (256 INODE_ITEM 0) itemoff 13702 itemsize 160
>                  generation 30 transid 30 size 65536 nbytes 1507328
>                  block group 0 mode 0 links 1 uid 0 gid 0 rdev 0
> 				   ^ Should be 100600
>                  sequence 23 flags 0x1b(NODATASUM|NODATACOW|NOCOMPRESS|PREALLOC)
>                  atime 0.0 (1970-01-01 08:00:00)
>                  ctime 1553491158.189771625 (2019-03-25 13:19:18)
>                  mtime 0.0 (1970-01-01 08:00:00)
>                  otime 0.0 (1970-01-01 08:00:00)
>
> There is a report of such problem in the mail list.
>
> This patch will check and repair inode items of free space cache inodes in
> lowmem mode.
>
> Since free space cache inodes doesn't have INODE_REF but still has 1
> link, we can't use check_inode_item() directly.
> Instead we only check the inode mode, as that's the important part.
>
> The check and repair function: check_repair_free_space_inode() is also
> exported for original mode.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>   check/mode-common.h |  1 +
>   check/mode-lowmem.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>   check/mode-lowmem.h |  2 ++
>   3 files changed, 48 insertions(+)
>
> diff --git a/check/mode-common.h b/check/mode-common.h
> index 8895747adf1d..855f83617854 100644
> --- a/check/mode-common.h
> +++ b/check/mode-common.h
> @@ -24,6 +24,7 @@
>   #include <sys/stat.h>
>   #include "ctree.h"
>
> +#define FREE_SPACE_CACHE_INODE_MODE	(0100600)
>   /*
>    * Use for tree walk to walk through trees whose leaves/nodes can be shared
>    * between different trees. (Namely subvolume/fs trees)
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index f37c3b42e6b6..cdb9b66a63a3 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -5091,6 +5091,40 @@ out:
>   	return err;
>   }
>
> +/*
> + * For free space inodes, we can't call check_inode_item() as free space
> + * cache inode doesn't have INODE_REF.
> + * We just check its inode mode.
> + */
> +int check_repair_free_space_inode(struct btrfs_fs_info *fs_info,
> +				  struct btrfs_path *path)
> +{


In personal taste, I prefer check_free_space_inode() to
follow existed code.

> +	struct btrfs_inode_item *iitem;
> +	struct btrfs_key key;
> +	u32 mode;
> +	int ret = 0;
> +
> +	btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
> +	ASSERT(key.type == BTRFS_INODE_ITEM_KEY && is_fstree(key.objectid));
> +	iitem = btrfs_item_ptr(path->nodes[0], path->slots[0],
> +			       struct btrfs_inode_item);
> +	mode = btrfs_inode_mode(path->nodes[0], iitem);
> +	if (mode != FREE_SPACE_CACHE_INODE_MODE) {
> +		error(
> +	"free space cache inode %llu has invalid mode: has 0%o expect 0%o",
> +			key.objectid, mode, FREE_SPACE_CACHE_INODE_MODE);
> +		ret = -EUCLEAN;
> +		if (repair) {
> +			ret = repair_imode_lowmem(fs_info->tree_root,
> +						  path);
> +			if (ret < 0)
> +				return ret;
> +			return ret;
> +		}
> +	}
> +	return ret;
> +}
> +
>   /*
>    * Check all fs/file tree in low_memory mode.
>    *
> @@ -5130,6 +5164,17 @@ int check_fs_roots_lowmem(struct btrfs_fs_info *fs_info)
>   		btrfs_item_key_to_cpu(node, &key, slot);
>   		if (key.objectid > BTRFS_LAST_FREE_OBJECTID)
>   			goto out;
> +		if (key.type == BTRFS_INODE_ITEM_KEY &&
> +		    is_fstree(key.objectid)) {
> +			ret = check_repair_free_space_inode(fs_info, &path);
> +			/* Check if we still have a valid path to continue */

'path.nodes[0]' is quite interesting :).
What about the situation btrfs_search_slot() succeed but failed to
commit a transaction?


---
Su
> +			if (ret < 0 && path.nodes[0]) {
> +				err |= ret;
> +				goto next;
> +			}
> +			if (ret < 0 && !path.nodes[0])
> +				goto out;
> +		}
>   		if (key.type == BTRFS_ROOT_ITEM_KEY &&
>   		    fs_root_objectid(key.objectid)) {
>   			if (key.objectid == BTRFS_TREE_RELOC_OBJECTID) {
> diff --git a/check/mode-lowmem.h b/check/mode-lowmem.h
> index e0ab30b770d5..d2983fd12eb4 100644
> --- a/check/mode-lowmem.h
> +++ b/check/mode-lowmem.h
> @@ -67,5 +67,7 @@
>
>   int check_fs_roots_lowmem(struct btrfs_fs_info *fs_info);
>   int check_chunks_and_extents_lowmem(struct btrfs_fs_info *fs_info);
> +int check_repair_free_space_inode(struct btrfs_fs_info *fs_info,
> +				  struct btrfs_path *path);
>
>   #endif
>

Re: [PATCH 5/7] btrfs: check/lowmem: Check and repair free space cache inode mode

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 6091 bytes --]



On 2019/3/25 下午10:36, Su Yue wrote:
> 
> 
> On 2019/3/25 4:22 PM, Qu Wenruo wrote:
>> Unlike inodes in fs roots, we don't really check the inode items in root
>> tree, in fact we just skip everything other than ROOT_ITEM and ROOT_REF.
>>
>> This makes invalid inode items sneak into root tree.
>> For example:
>>          item 9 key (256 INODE_ITEM 0) itemoff 13702 itemsize 160
>>                  generation 30 transid 30 size 65536 nbytes 1507328
>>                  block group 0 mode 0 links 1 uid 0 gid 0 rdev 0
>>                    ^ Should be 100600
>>                  sequence 23 flags
>> 0x1b(NODATASUM|NODATACOW|NOCOMPRESS|PREALLOC)
>>                  atime 0.0 (1970-01-01 08:00:00)
>>                  ctime 1553491158.189771625 (2019-03-25 13:19:18)
>>                  mtime 0.0 (1970-01-01 08:00:00)
>>                  otime 0.0 (1970-01-01 08:00:00)
>>
>> There is a report of such problem in the mail list.
>>
>> This patch will check and repair inode items of free space cache
>> inodes in
>> lowmem mode.
>>
>> Since free space cache inodes doesn't have INODE_REF but still has 1
>> link, we can't use check_inode_item() directly.
>> Instead we only check the inode mode, as that's the important part.
>>
>> The check and repair function: check_repair_free_space_inode() is also
>> exported for original mode.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>   check/mode-common.h |  1 +
>>   check/mode-lowmem.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>>   check/mode-lowmem.h |  2 ++
>>   3 files changed, 48 insertions(+)
>>
>> diff --git a/check/mode-common.h b/check/mode-common.h
>> index 8895747adf1d..855f83617854 100644
>> --- a/check/mode-common.h
>> +++ b/check/mode-common.h
>> @@ -24,6 +24,7 @@
>>   #include <sys/stat.h>
>>   #include "ctree.h"
>>
>> +#define FREE_SPACE_CACHE_INODE_MODE    (0100600)
>>   /*
>>    * Use for tree walk to walk through trees whose leaves/nodes can be
>> shared
>>    * between different trees. (Namely subvolume/fs trees)
>> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
>> index f37c3b42e6b6..cdb9b66a63a3 100644
>> --- a/check/mode-lowmem.c
>> +++ b/check/mode-lowmem.c
>> @@ -5091,6 +5091,40 @@ out:
>>       return err;
>>   }
>>
>> +/*
>> + * For free space inodes, we can't call check_inode_item() as free space
>> + * cache inode doesn't have INODE_REF.
>> + * We just check its inode mode.
>> + */
>> +int check_repair_free_space_inode(struct btrfs_fs_info *fs_info,
>> +                  struct btrfs_path *path)
>> +{
> 
> 
> In personal taste, I prefer check_free_space_inode() to
> follow existed code.
> 
>> +    struct btrfs_inode_item *iitem;
>> +    struct btrfs_key key;
>> +    u32 mode;
>> +    int ret = 0;
>> +
>> +    btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
>> +    ASSERT(key.type == BTRFS_INODE_ITEM_KEY && is_fstree(key.objectid));
>> +    iitem = btrfs_item_ptr(path->nodes[0], path->slots[0],
>> +                   struct btrfs_inode_item);
>> +    mode = btrfs_inode_mode(path->nodes[0], iitem);
>> +    if (mode != FREE_SPACE_CACHE_INODE_MODE) {
>> +        error(
>> +    "free space cache inode %llu has invalid mode: has 0%o expect 0%o",
>> +            key.objectid, mode, FREE_SPACE_CACHE_INODE_MODE);
>> +        ret = -EUCLEAN;
>> +        if (repair) {
>> +            ret = repair_imode_lowmem(fs_info->tree_root,
>> +                          path);
>> +            if (ret < 0)
>> +                return ret;
>> +            return ret;
>> +        }
>> +    }
>> +    return ret;
>> +}
>> +
>>   /*
>>    * Check all fs/file tree in low_memory mode.
>>    *
>> @@ -5130,6 +5164,17 @@ int check_fs_roots_lowmem(struct btrfs_fs_info
>> *fs_info)
>>           btrfs_item_key_to_cpu(node, &key, slot);
>>           if (key.objectid > BTRFS_LAST_FREE_OBJECTID)
>>               goto out;
>> +        if (key.type == BTRFS_INODE_ITEM_KEY &&
>> +            is_fstree(key.objectid)) {
>> +            ret = check_repair_free_space_inode(fs_info, &path);
>> +            /* Check if we still have a valid path to continue */
> 
> 'path.nodes[0]' is quite interesting :).
> What about the situation btrfs_search_slot() succeed but failed to
> commit a transaction?

As long as path.nodes[0] exists, we still have a valid path of root
tree, check can continue going.

For that valid path but failed commit case, I believe the most possible
problem would be a corrupted extent tree. And in that case, we can still
continue checking fs tree.

Thanks,
Qu

> 
> 
> ---
> Su
>> +            if (ret < 0 && path.nodes[0]) {
>> +                err |= ret;
>> +                goto next;
>> +            }
>> +            if (ret < 0 && !path.nodes[0])
>> +                goto out;
>> +        }
>>           if (key.type == BTRFS_ROOT_ITEM_KEY &&
>>               fs_root_objectid(key.objectid)) {
>>               if (key.objectid == BTRFS_TREE_RELOC_OBJECTID) {
>> diff --git a/check/mode-lowmem.h b/check/mode-lowmem.h
>> index e0ab30b770d5..d2983fd12eb4 100644
>> --- a/check/mode-lowmem.h
>> +++ b/check/mode-lowmem.h
>> @@ -67,5 +67,7 @@
>>
>>   int check_fs_roots_lowmem(struct btrfs_fs_info *fs_info);
>>   int check_chunks_and_extents_lowmem(struct btrfs_fs_info *fs_info);
>> +int check_repair_free_space_inode(struct btrfs_fs_info *fs_info,
>> +                  struct btrfs_path *path);
>>
>>   #endif
>>
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/7] btrfs-progs: check/lowmem: Add inode mode check

[Nikolay Borisov]

On 25.03.19 г. 10:22 ч., Qu Wenruo wrote:
> There is one report about invalid free space cache inode mode.
> Normally free space cache inode should have mode 100600 (regular file,
> no uid/gid/sticky bit, rw------ bit).
> 
> But in that report, we have free space cache inode mode as 0.
> 
> So at least btrfs check should report invalid inode mode.
> 
> This patch will at least make btrfs check lowmem mode to detect this
> problem.
> 
> Please note that, this check only applies to inodes in fs/subvol trees.
> It doesn't apply to free space cache inodes.
> 
> Reported-by: Thorsten Hirsch <t.hirsch@web.de>
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
>  check/mode-common.h | 24 ++++++++++++++++++++++++
>  check/mode-lowmem.c |  5 +++++
>  check/mode-lowmem.h |  1 +
>  3 files changed, 30 insertions(+)
> 
> diff --git a/check/mode-common.h b/check/mode-common.h
> index 1fb3cda7d89d..4c88365abbcc 100644
> --- a/check/mode-common.h
> +++ b/check/mode-common.h
> @@ -126,4 +126,28 @@ int delete_corrupted_dir_item(struct btrfs_trans_handle *trans,
>  			      struct btrfs_key *di_key, char *namebuf,
>  			      u32 namelen);
>  
> +/*
> + * Check if the inode mode @imode is valid
> + *
> + * This check focuses on S_FTMT bits and unused bits.
> + * Sticky/setuid/setgid and regular owner/group/other bits won't cause
> + * any problem.
> + */
> +static inline bool is_valid_imode(u32 imode)
> +{
> +	if (imode & ~(S_IFMT | 07777))
> +		return false;
> +
> +	/*
> +	 * S_IFMT is not bitmap, nor pure numbering sequence. Need per valid
> +	 * number check.
> +	 */
> +	imode &= S_IFMT;
> +	if (imode != S_IFDIR && imode != S_IFCHR && imode != S_IFBLK &&
> +	    imode != S_IFREG && imode != S_IFIFO && imode != S_IFLNK &&
> +	    imode != S_IFSOCK)
> +		return false;
> +	return true;
> +}
> +
>  #endif
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index fc6228a05a1b..1553a4a5d2c1 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -2452,6 +2452,11 @@ static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path)
>  	nlink = btrfs_inode_nlink(node, ii);
>  	nodatasum = btrfs_inode_flags(node, ii) & BTRFS_INODE_NODATASUM;
>  
> +	if (!is_valid_imode(mode)) {
> +		error("invalid imode mode bits: 0%o", mode);
> +		err |= INODE_MODE_ERROR;
> +	}
> +
>  	if (S_ISLNK(mode) &&
>  	    flags & (BTRFS_INODE_IMMUTABLE | BTRFS_INODE_APPEND)) {
>  		err |= INODE_FLAGS_ERROR;
> diff --git a/check/mode-lowmem.h b/check/mode-lowmem.h
> index 46b9b191ce24..e0ab30b770d5 100644
> --- a/check/mode-lowmem.h
> +++ b/check/mode-lowmem.h
> @@ -46,6 +46,7 @@
>  #define FATAL_ERROR             (1<<22) /* Fatal bit for errno */
>  #define INODE_FLAGS_ERROR	(1<<23) /* Invalid inode flags */
>  #define DIR_ITEM_HASH_MISMATCH	(1<<24) /* Dir item hash mismatch */
> +#define INODE_MODE_ERROR	(1<<25) /* Bad inode mode */
>  
>  /*
>   * Error bit for low memory mode check.
> 

Re: [PATCH 2/7] btrfs-progs: check/original: Add inode mode check

[Nikolay Borisov]

On 25.03.19 г. 10:22 ч., Qu Wenruo wrote:
> Just like lowmem mode, check inode mode, specially for S_IFMT bits and
> beyond.
> 
> Please note that, this check only applies to inodes in fs/subvol trees.
> It doesn't apply to free space cache inodes.
> 
> Reported-by: Thorsten Hirsch <t.hirsch@web.de>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  check/main.c          | 5 +++++
>  check/mode-original.h | 1 +
>  2 files changed, 6 insertions(+)
> 
> diff --git a/check/main.c b/check/main.c
> index 7547209c5604..553c93caa2c9 100644
> --- a/check/main.c
> +++ b/check/main.c
> @@ -616,6 +616,9 @@ static void print_inode_error(struct btrfs_root *root, struct inode_record *rec)
>  		fprintf(stderr, ", odd inode flags");
>  	if (errors & I_ERR_INLINE_RAM_BYTES_WRONG)
>  		fprintf(stderr, ", invalid inline ram bytes");
> +	if (errors & I_ERR_INVALID_IMODE)
> +		fprintf(stderr, ", invalid inode mode bit 0%o",
> +			rec->imode & ~07777);
>  	fprintf(stderr, "\n");
>  	/* Print the orphan extents if needed */
>  	if (errors & I_ERR_FILE_EXTENT_ORPHAN)
> @@ -811,6 +814,8 @@ static void maybe_free_inode_rec(struct cache_tree *inode_cache,
>  	if (!rec->checked || rec->merging)
>  		return;
>  
> +	if (!is_valid_imode(rec->imode))
> +		rec->errors |= I_ERR_INVALID_IMODE;

should this check actually be moved before the call to imode_to_type.
Because if the mode is busted this means we could potentially get
REF_ERR_FILETYPE_UNMATCH error?  I.e this check should really fail
before anything else ?

>  	if (S_ISDIR(rec->imode)) {
>  		if (rec->found_size != rec->isize)
>  			rec->errors |= I_ERR_DIR_ISIZE_WRONG;
> diff --git a/check/mode-original.h b/check/mode-original.h
> index 25ca274118a7..e40a12930a6f 100644
> --- a/check/mode-original.h
> +++ b/check/mode-original.h
> @@ -189,6 +189,7 @@ struct file_extent_hole {
>  #define I_ERR_ODD_INODE_FLAGS		(1 << 16)
>  #define I_ERR_INLINE_RAM_BYTES_WRONG	(1 << 17)
>  #define I_ERR_MISMATCH_DIR_HASH		(1 << 18)
> +#define I_ERR_INVALID_IMODE		(1 << 19)
>  
>  struct inode_record {
>  	struct list_head backrefs;
> 

Re: [PATCH 2/7] btrfs-progs: check/original: Add inode mode check

[Qu Wenruo]

On 2019/3/25 下午11:43, Nikolay Borisov wrote:
>
>
> On 25.03.19 г. 10:22 ч., Qu Wenruo wrote:
>> Just like lowmem mode, check inode mode, specially for S_IFMT bits and
>> beyond.
>>
>> Please note that, this check only applies to inodes in fs/subvol trees.
>> It doesn't apply to free space cache inodes.
>>
>> Reported-by: Thorsten Hirsch <t.hirsch@web.de>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>  check/main.c          | 5 +++++
>>  check/mode-original.h | 1 +
>>  2 files changed, 6 insertions(+)
>>
>> diff --git a/check/main.c b/check/main.c
>> index 7547209c5604..553c93caa2c9 100644
>> --- a/check/main.c
>> +++ b/check/main.c
>> @@ -616,6 +616,9 @@ static void print_inode_error(struct btrfs_root *root, struct inode_record *rec)
>>  		fprintf(stderr, ", odd inode flags");
>>  	if (errors & I_ERR_INLINE_RAM_BYTES_WRONG)
>>  		fprintf(stderr, ", invalid inline ram bytes");
>> +	if (errors & I_ERR_INVALID_IMODE)
>> +		fprintf(stderr, ", invalid inode mode bit 0%o",
>> +			rec->imode & ~07777);
>>  	fprintf(stderr, "\n");
>>  	/* Print the orphan extents if needed */
>>  	if (errors & I_ERR_FILE_EXTENT_ORPHAN)
>> @@ -811,6 +814,8 @@ static void maybe_free_inode_rec(struct cache_tree *inode_cache,
>>  	if (!rec->checked || rec->merging)
>>  		return;
>>
>> +	if (!is_valid_imode(rec->imode))
>> +		rec->errors |= I_ERR_INVALID_IMODE;
>
> should this check actually be moved before the call to imode_to_type.
> Because if the mode is busted this means we could potentially get
> REF_ERR_FILETYPE_UNMATCH error?  I.e this check should really fail
> before anything else ?

Yes, that should be the case.

I'll update the patch to do this check before using imode.

Thanks,
Qu

>
>>  	if (S_ISDIR(rec->imode)) {
>>  		if (rec->found_size != rec->isize)
>>  			rec->errors |= I_ERR_DIR_ISIZE_WRONG;
>> diff --git a/check/mode-original.h b/check/mode-original.h
>> index 25ca274118a7..e40a12930a6f 100644
>> --- a/check/mode-original.h
>> +++ b/check/mode-original.h
>> @@ -189,6 +189,7 @@ struct file_extent_hole {
>>  #define I_ERR_ODD_INODE_FLAGS		(1 << 16)
>>  #define I_ERR_INLINE_RAM_BYTES_WRONG	(1 << 17)
>>  #define I_ERR_MISMATCH_DIR_HASH		(1 << 18)
>> +#define I_ERR_INVALID_IMODE		(1 << 19)
>>
>>  struct inode_record {
>>  	struct list_head backrefs;
>>

Re: [PATCH 3/7] btrfs-progs: check/lowmem: Repair invalid inode mode in root tree

[Nikolay Borisov]

On 25.03.19 г. 10:22 ч., Qu Wenruo wrote:
> In root tree, we only have 2 types of inodes:
> - ROOT_TREE_DIR inode
>   Its mode is fixed to 40755
> - free space cache inodes
>   Its mode is fixed to 100600
> 
> This patch will add the ability to repair such inodes to lowmem mode.
> For fs/subvolume tree error, at least we haven't see such corruption
> yet, so we don't need to rush to fix corruption in fs trees yet.
> 
> The repair function, repair_imode() can be reused by later original mode
> patch, so it's placed in check/mode-common.c.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  check/mode-common.c | 48 +++++++++++++++++++++++++++++++++++++++++++
>  check/mode-common.h |  2 ++
>  check/mode-lowmem.c | 50 ++++++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 99 insertions(+), 1 deletion(-)
> 
> diff --git a/check/mode-common.c b/check/mode-common.c
> index fed102b0ce7a..1c150a3d5a7c 100644
> --- a/check/mode-common.c
> +++ b/check/mode-common.c
> @@ -795,3 +795,51 @@ out:
>  	btrfs_release_path(&path);
>  	return ret;
>  }
> +
> +/*
> + * Reset inode mode with invalid mode to its known correct value.
> + *
> + * Only support free space cache inode and root tree dir inode of root tree.
> + * Caller should ensure such condition.
> + *
> + * Return 0 if repair is done, @path will point to the correct inode item.
> + * Return <0 for errors.
> + */
> +int repair_imode(struct btrfs_trans_handle *trans, struct btrfs_root *root,
> +		 struct btrfs_path *path, u64 ino)
> +{

How about renaming this function to reset_imode/set_imode:

reset_imode and make it take a u32 parameter for the imode.


> +	struct btrfs_inode_item *iitem;
> +	struct extent_buffer *leaf;
> +	struct btrfs_key key;
> +	u32 correct_mode;
> +	int slot;
> +	int ret;
> +
> +	ASSERT(root->root_key.objectid == BTRFS_ROOT_TREE_OBJECTID);
> +	ASSERT(ino == BTRFS_ROOT_TREE_DIR_OBJECTID || is_fstree(ino));
> +
> +	if (ino == BTRFS_ROOT_TREE_DIR_OBJECTID)
> +		correct_mode = 040755;	/* Directory, 0755 */
> +	else
> +		correct_mode = 0100600;	/* Regular file, 0600 */
> +
> +	key.objectid = ino;
> +	key.type = BTRFS_INODE_ITEM_KEY;
> +	key.offset = 0;
> +
> +	ret = btrfs_search_slot(trans, root, &key, path, 0, 1);
> +	if (ret > 0)
> +		ret = -ENOENT;
> +	if (ret < 0) {
> +		errno = -ret;
> +		error("failed to search tree %llu: %m",
> +		      root->root_key.objectid);
> +		return ret;
> +	}
> +	leaf = path->nodes[0];
> +	slot = path->slots[0];
> +	iitem = btrfs_item_ptr(leaf, slot, struct btrfs_inode_item);
> +	btrfs_set_inode_mode(leaf, iitem, correct_mode);
> +	btrfs_mark_buffer_dirty(leaf);
> +	return ret;
> +}
> diff --git a/check/mode-common.h b/check/mode-common.h
> index 4c88365abbcc..8895747adf1d 100644
> --- a/check/mode-common.h
> +++ b/check/mode-common.h
> @@ -125,6 +125,8 @@ int delete_corrupted_dir_item(struct btrfs_trans_handle *trans,
>  			      struct btrfs_root *root,
>  			      struct btrfs_key *di_key, char *namebuf,
>  			      u32 namelen);
> +int repair_imode(struct btrfs_trans_handle *trans, struct btrfs_root *root,
> +		 struct btrfs_path *path, u64 ino);
>  
>  /*
>   * Check if the inode mode @imode is valid
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index 1553a4a5d2c1..f37c3b42e6b6 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -2396,6 +2396,48 @@ static bool has_orphan_item(struct btrfs_root *root, u64 ino)
>  	return false;
>  }
>  
> +static int repair_imode_lowmem(struct btrfs_root *root,
> +			       struct btrfs_path *path)
> +{

> +	struct btrfs_trans_handle *trans;
> +	struct btrfs_key key;
> +	int ret;
> +
> +	if (root->root_key.objectid != BTRFS_ROOT_TREE_OBJECTID) {
> +		error(
> +		"repair inode mode outside of root tree is not supported yet");
> +		return -ENOTTY;
> +	}
> +	btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
> +	ASSERT(key.type == BTRFS_INODE_ITEM_KEY);
> +	if (key.objectid != BTRFS_ROOT_TREE_DIR_OBJECTID &&
> +	    !is_fstree(key.objectid)) {
> +		error("unsupported ino %llu", key.objectid);
> +		return -ENOTTY;
> +	}
> +
> +	trans = btrfs_start_transaction(root, 1);
> +	if (IS_ERR(trans)) {
> +		ret = PTR_ERR(trans);
> +		errno = -ret;
> +		error("failed to start transaction: %m");
> +		return ret;
> +	}
> +	btrfs_release_path(path);
> +
> +	ret = repair_imode(trans, root, path, key.objectid);

And calling it here with the correct parameter based on whether ino is
BTRFS_ROOT_TREE_DIR_OBJECTID. Also move the asserts from reset_imode to
this function.

Otherwise you are separating the checks/asserts across 2 function and
this is somewhat cumbersome to follow.

> +	if (ret < 0)
> +		goto abort;
> +	ret = btrfs_commit_transaction(trans, root);
> +	if (!ret)
> +		printf("reset mode for inode %llu root %llu\n",
> +			key.objectid, root->root_key.objectid);
> +	return ret;
> +abort:
> +	btrfs_abort_transaction(trans, ret);
> + 	return ret;
> +}
> +
>  /*
>   * Check INODE_ITEM and related ITEMs (the same inode number)
>   * 1. check link count
> @@ -2454,7 +2496,13 @@ static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path)
>  
>  	if (!is_valid_imode(mode)) {
>  		error("invalid imode mode bits: 0%o", mode);
> -		err |= INODE_MODE_ERROR;
> +		if (repair) {
> +			ret = repair_imode_lowmem(root, path);
> +			if (ret < 0)
> +				err |= INODE_MODE_ERROR;
> +		} else {
> +			err |= INODE_MODE_ERROR;
> +		}
>  	}
>  
>  	if (S_ISLNK(mode) &&
> 

Re: [PATCH 5/7] btrfs: check/lowmem: Check and repair free space cache inode mode

[Nikolay Borisov]

On 25.03.19 г. 10:22 ч., Qu Wenruo wrote:
> Unlike inodes in fs roots, we don't really check the inode items in root
> tree, in fact we just skip everything other than ROOT_ITEM and ROOT_REF.
> 
> This makes invalid inode items sneak into root tree.
> For example:
>         item 9 key (256 INODE_ITEM 0) itemoff 13702 itemsize 160
>                 generation 30 transid 30 size 65536 nbytes 1507328
>                 block group 0 mode 0 links 1 uid 0 gid 0 rdev 0
> 				   ^ Should be 100600
>                 sequence 23 flags 0x1b(NODATASUM|NODATACOW|NOCOMPRESS|PREALLOC)
>                 atime 0.0 (1970-01-01 08:00:00)
>                 ctime 1553491158.189771625 (2019-03-25 13:19:18)
>                 mtime 0.0 (1970-01-01 08:00:00)
>                 otime 0.0 (1970-01-01 08:00:00)
> 
> There is a report of such problem in the mail list.
> 
> This patch will check and repair inode items of free space cache inodes in
> lowmem mode.
> 
> Since free space cache inodes doesn't have INODE_REF but still has 1
> link, we can't use check_inode_item() directly.
> Instead we only check the inode mode, as that's the important part.
> 
> The check and repair function: check_repair_free_space_inode() is also
> exported for original mode.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  check/mode-common.h |  1 +
>  check/mode-lowmem.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  check/mode-lowmem.h |  2 ++
>  3 files changed, 48 insertions(+)
> 
> diff --git a/check/mode-common.h b/check/mode-common.h
> index 8895747adf1d..855f83617854 100644
> --- a/check/mode-common.h
> +++ b/check/mode-common.h
> @@ -24,6 +24,7 @@
>  #include <sys/stat.h>
>  #include "ctree.h"
>  
> +#define FREE_SPACE_CACHE_INODE_MODE	(0100600)
>  /*
>   * Use for tree walk to walk through trees whose leaves/nodes can be shared
>   * between different trees. (Namely subvolume/fs trees)
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index f37c3b42e6b6..cdb9b66a63a3 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -5091,6 +5091,40 @@ out:
>  	return err;
>  }
>  
> +/*
> + * For free space inodes, we can't call check_inode_item() as free space
> + * cache inode doesn't have INODE_REF.
> + * We just check its inode mode.
> + */
> +int check_repair_free_space_inode(struct btrfs_fs_info *fs_info,
> +				  struct btrfs_path *path)
> +{
> +	struct btrfs_inode_item *iitem;
> +	struct btrfs_key key;
> +	u32 mode;
> +	int ret = 0;
> +
> +	btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
> +	ASSERT(key.type == BTRFS_INODE_ITEM_KEY && is_fstree(key.objectid));
> +	iitem = btrfs_item_ptr(path->nodes[0], path->slots[0],
> +			       struct btrfs_inode_item);
> +	mode = btrfs_inode_mode(path->nodes[0], iitem);
> +	if (mode != FREE_SPACE_CACHE_INODE_MODE) {
> +		error(
> +	"free space cache inode %llu has invalid mode: has 0%o expect 0%o",
> +			key.objectid, mode, FREE_SPACE_CACHE_INODE_MODE);
> +		ret = -EUCLEAN;
> +		if (repair) {
> +			ret = repair_imode_lowmem(fs_info->tree_root,
> +						  path);
> +			if (ret < 0)
> +				return ret;
> +			return ret;
> +		}
> +	}
> +	return ret;
> +}

You put this function in mode-lowmem, yet it's also used in the next
patch to implement check/repair for original mode. Since you are oging
to re-use function across the 2 modes it makes no sense to separate them
in their names. Perhaps put them in mode-common.c

> +
>  /*
>   * Check all fs/file tree in low_memory mode.
>   *
> @@ -5130,6 +5164,17 @@ int check_fs_roots_lowmem(struct btrfs_fs_info *fs_info)
>  		btrfs_item_key_to_cpu(node, &key, slot);
>  		if (key.objectid > BTRFS_LAST_FREE_OBJECTID)
>  			goto out;
> +		if (key.type == BTRFS_INODE_ITEM_KEY &&
> +		    is_fstree(key.objectid)) {
> +			ret = check_repair_free_space_inode(fs_info, &path);
> +			/* Check if we still have a valid path to continue */
> +			if (ret < 0 && path.nodes[0]) {
> +				err |= ret;
> +				goto next;
> +			}
> +			if (ret < 0 && !path.nodes[0])
> +				goto out;
> +		}
>  		if (key.type == BTRFS_ROOT_ITEM_KEY &&
>  		    fs_root_objectid(key.objectid)) {
>  			if (key.objectid == BTRFS_TREE_RELOC_OBJECTID) {
> diff --git a/check/mode-lowmem.h b/check/mode-lowmem.h
> index e0ab30b770d5..d2983fd12eb4 100644
> --- a/check/mode-lowmem.h
> +++ b/check/mode-lowmem.h
> @@ -67,5 +67,7 @@
>  
>  int check_fs_roots_lowmem(struct btrfs_fs_info *fs_info);
>  int check_chunks_and_extents_lowmem(struct btrfs_fs_info *fs_info);
> +int check_repair_free_space_inode(struct btrfs_fs_info *fs_info,
> +				  struct btrfs_path *path);
>  
>  #endif
> 

Re: [PATCH 3/7] btrfs-progs: check/lowmem: Repair invalid inode mode in root tree

[Qu Wenruo]

On 2019/3/29 下午5:36, Nikolay Borisov wrote:
> 
> 
> On 25.03.19 г. 10:22 ч., Qu Wenruo wrote:
>> In root tree, we only have 2 types of inodes:
>> - ROOT_TREE_DIR inode
>>   Its mode is fixed to 40755
>> - free space cache inodes
>>   Its mode is fixed to 100600
>>
>> This patch will add the ability to repair such inodes to lowmem mode.
>> For fs/subvolume tree error, at least we haven't see such corruption
>> yet, so we don't need to rush to fix corruption in fs trees yet.
>>
>> The repair function, repair_imode() can be reused by later original mode
>> patch, so it's placed in check/mode-common.c.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>  check/mode-common.c | 48 +++++++++++++++++++++++++++++++++++++++++++
>>  check/mode-common.h |  2 ++
>>  check/mode-lowmem.c | 50 ++++++++++++++++++++++++++++++++++++++++++++-
>>  3 files changed, 99 insertions(+), 1 deletion(-)
>>
>> diff --git a/check/mode-common.c b/check/mode-common.c
>> index fed102b0ce7a..1c150a3d5a7c 100644
>> --- a/check/mode-common.c
>> +++ b/check/mode-common.c
>> @@ -795,3 +795,51 @@ out:
>>  	btrfs_release_path(&path);
>>  	return ret;
>>  }
>> +
>> +/*
>> + * Reset inode mode with invalid mode to its known correct value.
>> + *
>> + * Only support free space cache inode and root tree dir inode of root tree.
>> + * Caller should ensure such condition.
>> + *
>> + * Return 0 if repair is done, @path will point to the correct inode item.
>> + * Return <0 for errors.
>> + */
>> +int repair_imode(struct btrfs_trans_handle *trans, struct btrfs_root *root,
>> +		 struct btrfs_path *path, u64 ino)
>> +{
> 
> How about renaming this function to reset_imode/set_imode:
> 
> reset_imode and make it take a u32 parameter for the imode.
[snip]
>> +	btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
>> +	ASSERT(key.type == BTRFS_INODE_ITEM_KEY);
>> +	if (key.objectid != BTRFS_ROOT_TREE_DIR_OBJECTID &&
>> +	    !is_fstree(key.objectid)) {
>> +		error("unsupported ino %llu", key.objectid);
>> +		return -ENOTTY;
>> +	}
>> +
>> +	trans = btrfs_start_transaction(root, 1);
>> +	if (IS_ERR(trans)) {
>> +		ret = PTR_ERR(trans);
>> +		errno = -ret;
>> +		error("failed to start transaction: %m");
>> +		return ret;
>> +	}
>> +	btrfs_release_path(path);
>> +
>> +	ret = repair_imode(trans, root, path, key.objectid);
> 
> And calling it here with the correct parameter based on whether ino is
> BTRFS_ROOT_TREE_DIR_OBJECTID. Also move the asserts from reset_imode to
> this function.

Makes sense.

I'll go that direction.

Thanks,
Qu

> 
> Otherwise you are separating the checks/asserts across 2 function and
> this is somewhat cumbersome to follow.
> 

Re: [PATCH 5/7] btrfs: check/lowmem: Check and repair free space cache inode mode

[Qu Wenruo]

[snip]
>> +/*
>> + * For free space inodes, we can't call check_inode_item() as free space
>> + * cache inode doesn't have INODE_REF.
>> + * We just check its inode mode.
>> + */
>> +int check_repair_free_space_inode(struct btrfs_fs_info *fs_info,
>> +				  struct btrfs_path *path)
>> +{
>> +	struct btrfs_inode_item *iitem;
>> +	struct btrfs_key key;
>> +	u32 mode;
>> +	int ret = 0;
>> +
>> +	btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
>> +	ASSERT(key.type == BTRFS_INODE_ITEM_KEY && is_fstree(key.objectid));
>> +	iitem = btrfs_item_ptr(path->nodes[0], path->slots[0],
>> +			       struct btrfs_inode_item);
>> +	mode = btrfs_inode_mode(path->nodes[0], iitem);
>> +	if (mode != FREE_SPACE_CACHE_INODE_MODE) {
>> +		error(
>> +	"free space cache inode %llu has invalid mode: has 0%o expect 0%o",
>> +			key.objectid, mode, FREE_SPACE_CACHE_INODE_MODE);
>> +		ret = -EUCLEAN;
>> +		if (repair) {
>> +			ret = repair_imode_lowmem(fs_info->tree_root,
>> +						  path);
>> +			if (ret < 0)
>> +				return ret;
>> +			return ret;
>> +		}
>> +	}
>> +	return ret;
>> +}
> 
> You put this function in mode-lowmem, yet it's also used in the next
> patch to implement check/repair for original mode. Since you are oging
> to re-use function across the 2 modes it makes no sense to separate them
> in their names. Perhaps put them in mode-common.c
> 

The limitation is due to repair_imode_lowmem().

Thanks,
Qu

>> +
>>  /*
>>   * Check all fs/file tree in low_memory mode.
>>   *
>> @@ -5130,6 +5164,17 @@ int check_fs_roots_lowmem(struct btrfs_fs_info *fs_info)
>>  		btrfs_item_key_to_cpu(node, &key, slot);
>>  		if (key.objectid > BTRFS_LAST_FREE_OBJECTID)
>>  			goto out;
>> +		if (key.type == BTRFS_INODE_ITEM_KEY &&
>> +		    is_fstree(key.objectid)) {
>> +			ret = check_repair_free_space_inode(fs_info, &path);
>> +			/* Check if we still have a valid path to continue */
>> +			if (ret < 0 && path.nodes[0]) {
>> +				err |= ret;
>> +				goto next;
>> +			}
>> +			if (ret < 0 && !path.nodes[0])
>> +				goto out;
>> +		}
>>  		if (key.type == BTRFS_ROOT_ITEM_KEY &&
>>  		    fs_root_objectid(key.objectid)) {
>>  			if (key.objectid == BTRFS_TREE_RELOC_OBJECTID) {
>> diff --git a/check/mode-lowmem.h b/check/mode-lowmem.h
>> index e0ab30b770d5..d2983fd12eb4 100644
>> --- a/check/mode-lowmem.h
>> +++ b/check/mode-lowmem.h
>> @@ -67,5 +67,7 @@
>>  
>>  int check_fs_roots_lowmem(struct btrfs_fs_info *fs_info);
>>  int check_chunks_and_extents_lowmem(struct btrfs_fs_info *fs_info);
>> +int check_repair_free_space_inode(struct btrfs_fs_info *fs_info,
>> +				  struct btrfs_path *path);
>>  
>>  #endif
>>

Re: [PATCH 5/7] btrfs: check/lowmem: Check and repair free space cache inode mode

[Nikolay Borisov]

On 29.03.19 г. 13:02 ч., Qu Wenruo wrote:
> [snip]
>>> +/*
>>> + * For free space inodes, we can't call check_inode_item() as free space
>>> + * cache inode doesn't have INODE_REF.
>>> + * We just check its inode mode.
>>> + */
>>> +int check_repair_free_space_inode(struct btrfs_fs_info *fs_info,
>>> +				  struct btrfs_path *path)
>>> +{
>>> +	struct btrfs_inode_item *iitem;
>>> +	struct btrfs_key key;
>>> +	u32 mode;
>>> +	int ret = 0;
>>> +
>>> +	btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
>>> +	ASSERT(key.type == BTRFS_INODE_ITEM_KEY && is_fstree(key.objectid));
>>> +	iitem = btrfs_item_ptr(path->nodes[0], path->slots[0],
>>> +			       struct btrfs_inode_item);
>>> +	mode = btrfs_inode_mode(path->nodes[0], iitem);
>>> +	if (mode != FREE_SPACE_CACHE_INODE_MODE) {
>>> +		error(
>>> +	"free space cache inode %llu has invalid mode: has 0%o expect 0%o",
>>> +			key.objectid, mode, FREE_SPACE_CACHE_INODE_MODE);
>>> +		ret = -EUCLEAN;
>>> +		if (repair) {
>>> +			ret = repair_imode_lowmem(fs_info->tree_root,
>>> +						  path);
>>> +			if (ret < 0)
>>> +				return ret;
>>> +			return ret;
>>> +		}
>>> +	}
>>> +	return ret;
>>> +}
>>
>> You put this function in mode-lowmem, yet it's also used in the next
>> patch to implement check/repair for original mode. Since you are oging
>> to re-use function across the 2 modes it makes no sense to separate them
>> in their names. Perhaps put them in mode-common.c
>>
> 
> The limitation is due to repair_imode_lowmem().

move it to the common file as well ?

> 
> Thanks,
> Qu
> 
>>> +
>>>  /*
>>>   * Check all fs/file tree in low_memory mode.
>>>   *
>>> @@ -5130,6 +5164,17 @@ int check_fs_roots_lowmem(struct btrfs_fs_info *fs_info)
>>>  		btrfs_item_key_to_cpu(node, &key, slot);
>>>  		if (key.objectid > BTRFS_LAST_FREE_OBJECTID)
>>>  			goto out;
>>> +		if (key.type == BTRFS_INODE_ITEM_KEY &&
>>> +		    is_fstree(key.objectid)) {
>>> +			ret = check_repair_free_space_inode(fs_info, &path);
>>> +			/* Check if we still have a valid path to continue */
>>> +			if (ret < 0 && path.nodes[0]) {
>>> +				err |= ret;
>>> +				goto next;
>>> +			}
>>> +			if (ret < 0 && !path.nodes[0])
>>> +				goto out;
>>> +		}
>>>  		if (key.type == BTRFS_ROOT_ITEM_KEY &&
>>>  		    fs_root_objectid(key.objectid)) {
>>>  			if (key.objectid == BTRFS_TREE_RELOC_OBJECTID) {
>>> diff --git a/check/mode-lowmem.h b/check/mode-lowmem.h
>>> index e0ab30b770d5..d2983fd12eb4 100644
>>> --- a/check/mode-lowmem.h
>>> +++ b/check/mode-lowmem.h
>>> @@ -67,5 +67,7 @@
>>>  
>>>  int check_fs_roots_lowmem(struct btrfs_fs_info *fs_info);
>>>  int check_chunks_and_extents_lowmem(struct btrfs_fs_info *fs_info);
>>> +int check_repair_free_space_inode(struct btrfs_fs_info *fs_info,
>>> +				  struct btrfs_path *path);
>>>  
>>>  #endif
>>>

Re: [PATCH 7/7] btrfs: tests/fsck: Add test image for free space cache mode repair

[Nikolay Borisov]

On 25.03.19 г. 10:22 ч., Qu Wenruo wrote:
> The image has one free space cache inode with invalid mode (0).
>         item 9 key (256 INODE_ITEM 0) itemoff 13702 itemsize 160
>                 generation 30 transid 30 size 65536 nbytes 1507328
>                 block group 0 mode 0 links 1 uid 0 gid 0 rdev 0
>                 sequence 23 flags 0x1b(NODATASUM|NODATACOW|NOCOMPRESS|PREALLOC)
>                 atime 0.0 (1970-01-01 08:00:00)
>                 ctime 1553491158.189771625 (2019-03-25 13:19:18)
>                 mtime 0.0 (1970-01-01 08:00:00)
>                 otime 0.0 (1970-01-01 08:00:00)
> 
> Both lowmem and original mode should be able to detect and fix it.
> 
> The extracted test image is pretty big (1G extracted), as kernel won't
> cache small chunks.
> Even with SSD, such test may still take some seconds just extracting the
> image.


Should this image contain also a non-freespace but root inode (for the
root dir case) so it has full coverage of the code you introduced?

> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  .../test.raw.xz                                | Bin 0 -> 159128 bytes
>  1 file changed, 0 insertions(+), 0 deletions(-)
>  create mode 100644 tests/fsck-tests/038-bad-free-space-cache-inode-mode/test.raw.xz
> 
> diff --git a/tests/fsck-tests/038-bad-free-space-cache-inode-mode/test.raw.xz b/tests/fsck-tests/038-bad-free-space-cache-inode-mode/test.raw.xz
> new file mode 100644
> index 0000000000000000000000000000000000000000..c2603c024098992bece235d27187eefa696b3fe0
> GIT binary patch
> literal 159128
> zcmeI5c{JB+*T%_|kTGN`Ny-=vGN%zgWGHizsHl+npiGgu5Q$8cF+#=^DI`;7GNzO$
> zgow_MywCHlXRYU5?{m(2b55%FoXg*>)mr<vzx%#E*WUNO=OtbA3`j`G2ix=ZEhFJ2
> z<su;=VQA2+TUel9W^tH=#Cm>V;b!5&etD%;wo*qmi^4Vz-t?vVJocK$t3|9*T9%ZM
> zbG|wtKuzo>U8j5UKoB3JcE3J*r9;3gr`dul?zEO=efG*`M)73ifu^El?BhqbrsO8(
> z72Ab4UpKX6a$Q+}+d+M~;$hPu?LA4Ug(siCc^SH=?FnbgdhTS0{-MTAJBC*rRu3<x
> zx<7h7BFE%;vDw6Z`RSBB{io>;5RaHd%7^9midOP+J?cD3^^v?(yQ&~q^^vk=?~SGy
> z>b+Ml?_OCPv87?Bsw2UsglR1D<Ke2F{MzGIX#(5Z8M(S|vDdWJGIiG#^R!;!IUJST
> zpv6(lQs%O~dWE6U+ooKiPI$iOZ1J}4w^;$R?xbOP`^*~FoRbJL60sDverq)r_{i&1
> za8|sSBionpPli#=L)N)BxQVKxQ*n;x^CbwZ>UvQe=V?q<D@PAF$yXgcb-v2_SEjC+
> zIWqDhS^v9QNkccO1xL~s3^rug(z1r7luBJ%|I#Kdh40A7##*1Ih^T<`f}xWF4l}FE
> zD5KP(=AUP^U8b+#*PfjY-7Zd%ls#ts$VP2HZBr;?dZr0?@v|HOb|Zb?;p19*rNfRH
> z`mf&@QY`;m)s;C%)Dp6<=P_Kn>vONR-0UVZY8j7O!P77DWyUK?M31D?%M*_8y3D+C
> zfgybMth4?sP5nZ|#(nf@b8daa=*^-!WtV(DdV7|cFWG<1^dZb5IevkGRET86PCI43
> z^p2Z2M|@$hxYge=8ej86Xwoa(MZgx3xd_-IKXn8Q0bAO`1Q~`5Lxz1fkp)??1cDef
> z3^fcj3^nX$C2JIY6#b?3ZjfQfFl5+IWvAkk#3zYQ5})M%=avv^7-|@57-|@5*xzUl
> z07V}~A4MNU|L;-sl}V*Z-oHAf+OvJ4x~@O>*1h(lC*OOow9%>-&#jzpyJVU-QuDng
> zu)e19{ycMM6k;Mqn0~_a)6xw(qOXg-F8aFY>!Ppw7a1}C8i9kNkD`yFkD`yFkD`yO
> zN56Lm45uVcNt}{6C2>mPl*B2CQ}P>KEr+attbnY5tbnY5tbnY5tbnZeF)Mzc!=S^y
> zj}8;YvZw!R>(+!SwbQROm6r}gdSTuP^G=v|`Y!9}Fe`vD^q*x6J(bk!uX{{Bq%#uQ
> z%{hOlG@UF=Vb?0#*uE>HcYEUNM)j^+jtBJ#r})gPHh5l$mm7IrM}9!Ami+m+_CRBZ
> zxn*lse8R?oSJkuCr&LN5)6}L}9|RS5ZsA({OOHppuI(P{z^jU*w>ju<O^6WL_iWpE
> z+~HZy+YNlFvy_3I)9&qboC@chZhN!vS~az~e3WO=HvRo|nKRxkG|ur00Tgn7{CCLw
> zu%@=Tos4^eWw~#AtaR<8?M^Xug!uS@P($<n5au{`yO&!k4vHFU*dM;VrqegbgTv_I
> z6f>pqi6iRqjH1_6Lew_q8?$aX9#EIP{7k)<likJ8%P~)+h7WX!eQCH%uvFMlS37>h
> z)%t8<afdC%-5|D(hpBbf2g|tjK5=8%C%NaWZgYzI1i@W4+_-!4(S>FYRgtvTu_hkJ
> zqWdflYbh%(`zEtRI*3J)+{sWCby&{AA!K!=$BpQaaD(b_bco-AQ&tCQS7^2ib>{y0
> z6?Tpn%WT$vCMyfQV{39~%}v`E#~jt@$`5?J9kWi3nT3}^#b(QNlYFE}&7_9xOODFZ
> z(#nBpOfoqy(xdh%o4XUkynN^Lr1qYtwJ*=PSMQKIA9?A(OrD9w%Kh05xfAPVPsf*f
> zD7h+koPFRcTP5Jyer#3Ots=^T9TVB&6dGC#7n-L=meoD&yeTZ;Xpkw$f0Mn<oW1yC
> z!^*qS0RlA*vg_6O8p3=>4HC|LzC95@M>Sm4_wY-}#5Vetw#Q777w$y|pB1)lzb;eP
> zCs0@INu2a8H!Z2FTv>96gSfq}P-#<^onouM1@D%%1x1P7BFrYy9{YCd$|(fT1#I9M
> ziv8ujrbYe8#kDno+?(Q+mhC-e_%v>v@$`Pb2a3`(t?mh3>GaZFsymu$PF}v$YV7_d
> zws$UH)yjq<m`YDDy;4yl?G&ppm8*x9=)f;l(e)QgF3Ryacx%eW)@(M}JXbeO&O`Me
> z{ca84Dmw;sM(H=4H->LZ$?US2DS3QKd7gy5^h{3XdTuVFR<_=j+6S8k>^9^~9xrG~
> z(suOlYH0ql<M^rmqy*B)sPq{3vuwi>qa(#xjkiWMn>(Ut?lpbsyXTSQr}tLsNT!>1
> zc~C}M)(Yy$N|DWWVf8eAPab+-Zj}z^o>jbZ&-_fx2m4_0{Oell>1M8)1z6a55boE|
> ztM#u`%5HSAwkkQevr#LU@$#C=DL<Qi9yG=JJ!$FMT?X0oMDcy%?v70IS}WF=on70)
> zK_CVu@j6a$SNrq1(|O8fN#0@?(N1rhUM{G#zN*rYboklpoKqWvDL+N;-Id0A&^cx7
> zIcuqIZ^46<wB(NSS81&+x?S7u&es}D*IeA~d90k<%)D+V(_?R0(Xwq4b@%xv+!)?_
> zl@J`x9TJ(T5%KruFE4+n*&ORr#PhCHG`!;sgH)c-6PBE$l*7yqF0}d`AkE^5I6tEP
> zakqPD;-TxB`2)kdJ?l4^nHiC`W>A}(M2Ilycy_NKvk!EPp^pr!$s*tJt1t~aA;iId
> z`R9y+4Ntea6ur_BRg+=gn=xOk`Ydg@?8Lfl%H1z7HA=JbI#Tl99o#54+!EwOr7~FI
> zctBBsYOp^!lR9yWZffah3`hRwp-_SuF)GJAB==5UZ)np{m6wq35V3N5U`B6>=0MpC
> z2Ev}iH>;lb(_Up^YH{B`-0!+0eqGDUm23w(@=dlU@W_)0^}Suk)XCDzy|X=o@@~La
> z)UTgm-EVium3i<B_yzm|e)$>qj|2Dsd_N2DVF~_Mzea#o1zHu~r&WP>DZcW75PXvO
> zB=JdNiVIU*nBu||7pAx{#f2%ZAO5@?>>b!Uuy?Rl?(eZy4hA9&#HD)w74{D79oRdt
> zcVO?p-hsUXdk6Lo>>b!UdP^m@f8fS#D19h>D19h>D19h>D19h>D19h>D19jXGRi2m
> zsQKqvZI|gQ_<tau{J{SjXaqC@8Uc-fMnEH=5zq)|^dr&e4ypSeU(Av{X8n`bFs;#a
> zN`;sFvSdn%xxKL)*hMF(1Pcmiv)qpMIngOSH5QhC`G&m9E_UXG^{nC1gS#9wV-j6@
> zZNkr-xXL5(@mwO=v}@pk1m{}iyKPpYr?ztiN_M{!s&b!9rZ)_c2sF(ypxytKSG#^&
> zadS%F_x<N~9Ev~lBb;oY%p5eWe9W8GG<&g!C+hwCtn;<qHy6V6^DDex*+|ROM^YX#
> z;!WKkLS;SSaO}|)vrm4)GUi3?p6MU@XvDNGla1LBSnj00lhRRBS$-@if{xBHQj+Y%
> zPRqT^OlPDN7lx(GsfJQ4J+!8jM`wyZGp{=+Q6qTe8FPi%YYoYFLh+#=SL<4oCfzmL
> zA#OWPcJXkR{szBQ>S1iEYb{f#>q;)2S+#oEvfS2Rja4L%B-KZcnJ?TX`QJ-}G%wmF
> z_G#xAYI)8C8tBhxdwKa^9i6gGF4~=N;!xIGvMhCx;fzNO_kEi?S|5MDc<!l@HFIX4
> z$rBgnYdc+^1RW(9MJHBUUw9fTKz%_@h^0K~`VNvh=O!sJ4r?!^jilvqw~RBqiZg=f
> zGD+V=wJ>e7c$n^OZ@pS=N8B^Zh-)M1k~2NJ@(QNU&p*A!cBaTZ<Wu<k1_cWByW{aU
> zLuu}bI_R)^F5I7`mzpvUY1h74P_xY0kmw|QZ|rCf|FSr@_2tB!v#CmwxnXt>I$A>l
> zD2XzcwYZkO&Z-@vVO?=;I5{l-nyKf>j2CNci6Nm|oqWiH*laYeH3>iGN@@)Kpr$y&
> zzC(Pp<<;SmC`bJmvzXo0S?5=n$h?Ylp!AVr-b&bdX+~C{O8-Xds?;^a$>!_hiqTEx
> ztD=cCgPl(6?fjj2lSHUnw=j%Fm$;{qyYp^&@K7?`Q<TP?)HA_rk~OY<d9S=^vEkPF
> z6|$P)g`A-xIv#p4yG070Hplqgs#Ueh()Kc9o75etx1k)96o}Gxaq|y3S8%l=!S%GM
> zu(npglV@TM+nd*1$Yw|ovD~iKUwlV#+m|SUT$dM>ph`?&9A|(<v*9hrSn^(<kqL?k
> z_T<47dpWL_*<%C)VQL4*nQQ)667@ICDeL{xL|3t&$2rmUoMm*zZsrx|?o6$4(QfZ#
> z*xlSu{47=$T-MONw&OtdyNji5{z7>TR4V=4gQvqUh<UCqcKF12HF1D4+e6Rt7~wuK
> zFZ+GJ0W*P*>qX?+mmNOubE!D2FWfLV&+Dy`X|r~ixa!<qRw)AYvP3Q=t<A4J__K$Y
> zds^9BB@=5m&ii*#2^6oJE?;lx8l5oHTd&U46Ir6ixo=K|%}Zr9WvX}X3%}9V@}pHY
> zua7l8RIE+QDW5U7Z&P*_6l3)Ju<d=N)!RI;5h1$4SiiwHS2!!H8uh}(sI6-cH}X7~
> zbgeZ$`6=q)ahG?S3UW^ks<x9@U%MsEsxcX!>{D?zNn4v)s`_l$9tMVn{5DB@8b^x;
> z-A2OErxHxcQk;7^`<Q&4l^;3GNtQl0EviuHOiuR_m^YkK(baQSV=5obq3<~HV8Byv
> zD*J7xgzmb$4s&Ayci8Uy6H5E0YxT<v8*lrbOw{B`cx$wmJL2>HU)A4gy$Yk$Ssk}0
> z?p5>aeV-1F`#A`2?tZLsFm&BjO0px>HZm+_5|U9#t4NcEZc+=5q%Rn3$grhl4NEDN
> zy0reKO<W4!k&%tHK1~r(0p|rnCj}g4RxgQ*(fC*`0$Xmd<px`Bu;m6@ZfKiA8PhXO
> zaOYyl&z%?fGUJPMWPe-Bf0y41?)f5rD+CMyL%RX(2DBS~a;vT@bB?G5Gx$fE!MOX1
> zyRS<%j1L_K9R?i+9R?i+9fq+FjD0}qL+N889ux6P(o2S?pCiC-w6t;?O^`(b9Rh}c
> z;gejt0U{KA6#b<gQA36y!;oQRmwf)U4j7*PA{7k+hJY=t1Vn}*!;oQUv!Ts~HXGV(
> zXtSZshBh18Y-qEg&BhNj0vZ90fJQ*0za1LIlXm`@1uH-2mK!7S;Ij)K_Od!p?@=+n
> zBA|LA?5J?mV2S!+;)vo-QlHHegq|z52{(H6WJ&U9?FDTD>SF`_KNLOodC^fk*InJ-
> zB$620;koW<B*~$~^&Fcf+r=rL`;k1m{$zr_=)>c(6Ir=aeDCIu3vX6ek|v9p9_M&J
> z;-uSFk}%m|{fN_$nnaiF(cZJ+sVvIe>VZd88n#$T*fEzncZJ;&l2%)_$^YHL7vXp3
> zN;emnc*pfiP8+%DbUQ}$ahKjO)ZXg0Z%*Tztf5f(<rzPgMqp_KmPTM{1eQi%X~g$h
> zR)k3qOoCt%1d|}tG9G{IVQ839Fr#2b!Hj|#^|v#luw`SBTQ=xPCH{Kuor5St7rpgn
> zscSbR2?mT3;vDgX!QX6P!@eu*F>Eo|Vz9+vi@_FyEe2bRKJ7nQ0}KuZ2ZR6TcY%xQ
> zi~KGSFa&IA3kfm|8HNnQ?g($sGV_XmO)|%m*8lZvF~?{f#*}r^GH1e1d*93SVyZI^
> zsT=3l<lO2=n|zc};b_5tf6cDVUJ8@HL=z<#d&Ai{a+h6t#UC_gt2HR|NyqjB_eApW
> zOoWZ~Y?D;eo|!aJ4vDDDofhV<-sc9U+^6P_J&sKZPo|(07bqOMqiecTZsOvm)1P&_
> zMpg`}xVf=iu#C13D&)Fp#Two-NUU<YSE*voZ>5u^M3W&Jyers%??lUuZkK%#1Rkqv
> zp<#iau1+P^X~=CfaeK%~%jxif(XA&vyGo%_BTItsY0q+vA!fP40gqz7)LF{F&T02{
> zI!=XiPPe^Tc&(b+Tt3RP{QD8KO6`|>;n=~kgJTEB4vrlhJ2-Z5><rib>FEGioQvc}
> z1PlRNTF^#@A;XYi=sNuAvmKuBA`1xuhJY<?<wb@e!;oQU)BMji4aU6}Iqr>sAz*lg
> zQdH;Pw<{qlAS)m%AS)m%1W*5g%?of`7g;_LFa&IA&l_YIG7K4pCJ34!Sby-%ZoNBZ
> z=)ZnrNU{8LRafR5QR^G6z6X3S@~A?<5U`~^+mT_&Fk~3~F!*8c!-}6R{U%bl6^lG?
> z5HJL6X%A>*7%~hQ1_KcWA`C<rh<NAp8=1Y36_6E>6_6E>6_6E>6_6E>6?i@!&!^-0
> zbUdGq=hMf5MnEH=5zq)|1T+E~0gZr0KqH_L&<JP*G%^Aj0gZr0KqH_L&<JP*Gy)m{
> zjetf#BcKt`=r`UT#ues8c2f~B1Z-(N7GxMQ3>k*40@x~mtpeC8@Ov&EPDz}SI3;mP
> z;*`WGiBl4%<TvVdK~_LkKvqCjKvqCjKvqCjKvw*i5gO<)=rHIo=rHIo=rHIo=rHIo
> z=rA{Nj`+f0Tx|HgYJjDYi`2{r7y`Dmkbw+Ch9SdnBNsPvaU&PP6~gr!VJu_?WCdge
> zWCdgeWCdgeWCdi!k8#JL!=S^U!=S^U!=S^U!=S^U!=S^ka19IBuy73v*RXI63)ir4
> zZ7t9UXaqC@8Uc-fMnEH=5zq)|1T+E~0gZr0)<7em5zq)|1T+E~0gZr0KqH_L&<JP*
> zGy)o7(MLVd2xtT}0vZ90fJQ(gpb^jrXaqC@8Uc-fMp{54pb^jrXaqC@8Uc-fMnEH=
> z5zq)|1T+E~Z3h|wjetf#BcKt`2xtT}0vZ90fJQ(gpb^k09cTnJ0vZ90fJQ(gpb^jr
> zXaqC@8Uc-fMnEGipb^jrXaqC@8Uc-fMnEH=5zq)|1T+E~0gYAzjetf#BcKt`2xtT}
> z0vZ90fJQ(gpb^jrXjB0-0vZ90fJQ(gpb^jrXaqC@8Uc-fMnEH=kqpoXXaqC@8Uc-f
> zMnEH=5zq)|1T+E~0gZr0>_8)+5zq)|1T+E~0gZr0KqH_L&<JP*Gy)pY1C4-2KqH_L
> z&<JP*Gy)m{jetf#BcRduKqJpC4-!_A5j*Xa`O-UX;vDgX|E<Hs1AHV%{_$7TAo-m1
> iabF_|=~x%F$3OnaNk~d>QI@f=$!sS3=RY9VH~24oN>cy;
> 
> literal 0
> HcmV?d00001
> 

Re: [PATCH 7/7] btrfs: tests/fsck: Add test image for free space cache mode repair

[Qu Wenruo]

On 2019/3/29 下午8:05, Nikolay Borisov wrote:
> 
> 
> On 25.03.19 г. 10:22 ч., Qu Wenruo wrote:
>> The image has one free space cache inode with invalid mode (0).
>>         item 9 key (256 INODE_ITEM 0) itemoff 13702 itemsize 160
>>                 generation 30 transid 30 size 65536 nbytes 1507328
>>                 block group 0 mode 0 links 1 uid 0 gid 0 rdev 0
>>                 sequence 23 flags 0x1b(NODATASUM|NODATACOW|NOCOMPRESS|PREALLOC)
>>                 atime 0.0 (1970-01-01 08:00:00)
>>                 ctime 1553491158.189771625 (2019-03-25 13:19:18)
>>                 mtime 0.0 (1970-01-01 08:00:00)
>>                 otime 0.0 (1970-01-01 08:00:00)
>>
>> Both lowmem and original mode should be able to detect and fix it.
>>
>> The extracted test image is pretty big (1G extracted), as kernel won't
>> cache small chunks.
>> Even with SSD, such test may still take some seconds just extracting the
>> image.
> 
> 
> Should this image contain also a non-freespace but root inode (for the
> root dir case) so it has full coverage of the code you introduced?

OK, another image will be added for ROOT_TREE_DIR.

Thanks,
Qu

> 
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>  .../test.raw.xz                                | Bin 0 -> 159128 bytes
>>  1 file changed, 0 insertions(+), 0 deletions(-)
>>  create mode 100644 tests/fsck-tests/038-bad-free-space-cache-inode-mode/test.raw.xz
>>
>> diff --git a/tests/fsck-tests/038-bad-free-space-cache-inode-mode/test.raw.xz b/tests/fsck-tests/038-bad-free-space-cache-inode-mode/test.raw.xz
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..c2603c024098992bece235d27187eefa696b3fe0
>> GIT binary patch
>> literal 159128
>> zcmeI5c{JB+*T%_|kTGN`Ny-=vGN%zgWGHizsHl+npiGgu5Q$8cF+#=^DI`;7GNzO$
>> zgow_MywCHlXRYU5?{m(2b55%FoXg*>)mr<vzx%#E*WUNO=OtbA3`j`G2ix=ZEhFJ2
>> z<su;=VQA2+TUel9W^tH=#Cm>V;b!5&etD%;wo*qmi^4Vz-t?vVJocK$t3|9*T9%ZM
>> zbG|wtKuzo>U8j5UKoB3JcE3J*r9;3gr`dul?zEO=efG*`M)73ifu^El?BhqbrsO8(
>> z72Ab4UpKX6a$Q+}+d+M~;$hPu?LA4Ug(siCc^SH=?FnbgdhTS0{-MTAJBC*rRu3<x
>> zx<7h7BFE%;vDw6Z`RSBB{io>;5RaHd%7^9midOP+J?cD3^^v?(yQ&~q^^vk=?~SGy
>> z>b+Ml?_OCPv87?Bsw2UsglR1D<Ke2F{MzGIX#(5Z8M(S|vDdWJGIiG#^R!;!IUJST
>> zpv6(lQs%O~dWE6U+ooKiPI$iOZ1J}4w^;$R?xbOP`^*~FoRbJL60sDverq)r_{i&1
>> za8|sSBionpPli#=L)N)BxQVKxQ*n;x^CbwZ>UvQe=V?q<D@PAF$yXgcb-v2_SEjC+
>> zIWqDhS^v9QNkccO1xL~s3^rug(z1r7luBJ%|I#Kdh40A7##*1Ih^T<`f}xWF4l}FE
>> zD5KP(=AUP^U8b+#*PfjY-7Zd%ls#ts$VP2HZBr;?dZr0?@v|HOb|Zb?;p19*rNfRH
>> z`mf&@QY`;m)s;C%)Dp6<=P_Kn>vONR-0UVZY8j7O!P77DWyUK?M31D?%M*_8y3D+C
>> zfgybMth4?sP5nZ|#(nf@b8daa=*^-!WtV(DdV7|cFWG<1^dZb5IevkGRET86PCI43
>> z^p2Z2M|@$hxYge=8ej86Xwoa(MZgx3xd_-IKXn8Q0bAO`1Q~`5Lxz1fkp)??1cDef
>> z3^fcj3^nX$C2JIY6#b?3ZjfQfFl5+IWvAkk#3zYQ5})M%=avv^7-|@57-|@5*xzUl
>> z07V}~A4MNU|L;-sl}V*Z-oHAf+OvJ4x~@O>*1h(lC*OOow9%>-&#jzpyJVU-QuDng
>> zu)e19{ycMM6k;Mqn0~_a)6xw(qOXg-F8aFY>!Ppw7a1}C8i9kNkD`yFkD`yFkD`yO
>> zN56Lm45uVcNt}{6C2>mPl*B2CQ}P>KEr+attbnY5tbnY5tbnY5tbnZeF)Mzc!=S^y
>> zj}8;YvZw!R>(+!SwbQROm6r}gdSTuP^G=v|`Y!9}Fe`vD^q*x6J(bk!uX{{Bq%#uQ
>> z%{hOlG@UF=Vb?0#*uE>HcYEUNM)j^+jtBJ#r})gPHh5l$mm7IrM}9!Ami+m+_CRBZ
>> zxn*lse8R?oSJkuCr&LN5)6}L}9|RS5ZsA({OOHppuI(P{z^jU*w>ju<O^6WL_iWpE
>> z+~HZy+YNlFvy_3I)9&qboC@chZhN!vS~az~e3WO=HvRo|nKRxkG|ur00Tgn7{CCLw
>> zu%@=Tos4^eWw~#AtaR<8?M^Xug!uS@P($<n5au{`yO&!k4vHFU*dM;VrqegbgTv_I
>> z6f>pqi6iRqjH1_6Lew_q8?$aX9#EIP{7k)<likJ8%P~)+h7WX!eQCH%uvFMlS37>h
>> z)%t8<afdC%-5|D(hpBbf2g|tjK5=8%C%NaWZgYzI1i@W4+_-!4(S>FYRgtvTu_hkJ
>> zqWdflYbh%(`zEtRI*3J)+{sWCby&{AA!K!=$BpQaaD(b_bco-AQ&tCQS7^2ib>{y0
>> z6?Tpn%WT$vCMyfQV{39~%}v`E#~jt@$`5?J9kWi3nT3}^#b(QNlYFE}&7_9xOODFZ
>> z(#nBpOfoqy(xdh%o4XUkynN^Lr1qYtwJ*=PSMQKIA9?A(OrD9w%Kh05xfAPVPsf*f
>> zD7h+koPFRcTP5Jyer#3Ots=^T9TVB&6dGC#7n-L=meoD&yeTZ;Xpkw$f0Mn<oW1yC
>> z!^*qS0RlA*vg_6O8p3=>4HC|LzC95@M>Sm4_wY-}#5Vetw#Q777w$y|pB1)lzb;eP
>> zCs0@INu2a8H!Z2FTv>96gSfq}P-#<^onouM1@D%%1x1P7BFrYy9{YCd$|(fT1#I9M
>> ziv8ujrbYe8#kDno+?(Q+mhC-e_%v>v@$`Pb2a3`(t?mh3>GaZFsymu$PF}v$YV7_d
>> zws$UH)yjq<m`YDDy;4yl?G&ppm8*x9=)f;l(e)QgF3Ryacx%eW)@(M}JXbeO&O`Me
>> z{ca84Dmw;sM(H=4H->LZ$?US2DS3QKd7gy5^h{3XdTuVFR<_=j+6S8k>^9^~9xrG~
>> z(suOlYH0ql<M^rmqy*B)sPq{3vuwi>qa(#xjkiWMn>(Ut?lpbsyXTSQr}tLsNT!>1
>> zc~C}M)(Yy$N|DWWVf8eAPab+-Zj}z^o>jbZ&-_fx2m4_0{Oell>1M8)1z6a55boE|
>> ztM#u`%5HSAwkkQevr#LU@$#C=DL<Qi9yG=JJ!$FMT?X0oMDcy%?v70IS}WF=on70)
>> zK_CVu@j6a$SNrq1(|O8fN#0@?(N1rhUM{G#zN*rYboklpoKqWvDL+N;-Id0A&^cx7
>> zIcuqIZ^46<wB(NSS81&+x?S7u&es}D*IeA~d90k<%)D+V(_?R0(Xwq4b@%xv+!)?_
>> zl@J`x9TJ(T5%KruFE4+n*&ORr#PhCHG`!;sgH)c-6PBE$l*7yqF0}d`AkE^5I6tEP
>> zakqPD;-TxB`2)kdJ?l4^nHiC`W>A}(M2Ilycy_NKvk!EPp^pr!$s*tJt1t~aA;iId
>> z`R9y+4Ntea6ur_BRg+=gn=xOk`Ydg@?8Lfl%H1z7HA=JbI#Tl99o#54+!EwOr7~FI
>> zctBBsYOp^!lR9yWZffah3`hRwp-_SuF)GJAB==5UZ)np{m6wq35V3N5U`B6>=0MpC
>> z2Ev}iH>;lb(_Up^YH{B`-0!+0eqGDUm23w(@=dlU@W_)0^}Suk)XCDzy|X=o@@~La
>> z)UTgm-EVium3i<B_yzm|e)$>qj|2Dsd_N2DVF~_Mzea#o1zHu~r&WP>DZcW75PXvO
>> zB=JdNiVIU*nBu||7pAx{#f2%ZAO5@?>>b!Uuy?Rl?(eZy4hA9&#HD)w74{D79oRdt
>> zcVO?p-hsUXdk6Lo>>b!UdP^m@f8fS#D19h>D19h>D19h>D19h>D19h>D19jXGRi2m
>> zsQKqvZI|gQ_<tau{J{SjXaqC@8Uc-fMnEH=5zq)|^dr&e4ypSeU(Av{X8n`bFs;#a
>> zN`;sFvSdn%xxKL)*hMF(1Pcmiv)qpMIngOSH5QhC`G&m9E_UXG^{nC1gS#9wV-j6@
>> zZNkr-xXL5(@mwO=v}@pk1m{}iyKPpYr?ztiN_M{!s&b!9rZ)_c2sF(ypxytKSG#^&
>> zadS%F_x<N~9Ev~lBb;oY%p5eWe9W8GG<&g!C+hwCtn;<qHy6V6^DDex*+|ROM^YX#
>> z;!WKkLS;SSaO}|)vrm4)GUi3?p6MU@XvDNGla1LBSnj00lhRRBS$-@if{xBHQj+Y%
>> zPRqT^OlPDN7lx(GsfJQ4J+!8jM`wyZGp{=+Q6qTe8FPi%YYoYFLh+#=SL<4oCfzmL
>> zA#OWPcJXkR{szBQ>S1iEYb{f#>q;)2S+#oEvfS2Rja4L%B-KZcnJ?TX`QJ-}G%wmF
>> z_G#xAYI)8C8tBhxdwKa^9i6gGF4~=N;!xIGvMhCx;fzNO_kEi?S|5MDc<!l@HFIX4
>> z$rBgnYdc+^1RW(9MJHBUUw9fTKz%_@h^0K~`VNvh=O!sJ4r?!^jilvqw~RBqiZg=f
>> zGD+V=wJ>e7c$n^OZ@pS=N8B^Zh-)M1k~2NJ@(QNU&p*A!cBaTZ<Wu<k1_cWByW{aU
>> zLuu}bI_R)^F5I7`mzpvUY1h74P_xY0kmw|QZ|rCf|FSr@_2tB!v#CmwxnXt>I$A>l
>> zD2XzcwYZkO&Z-@vVO?=;I5{l-nyKf>j2CNci6Nm|oqWiH*laYeH3>iGN@@)Kpr$y&
>> zzC(Pp<<;SmC`bJmvzXo0S?5=n$h?Ylp!AVr-b&bdX+~C{O8-Xds?;^a$>!_hiqTEx
>> ztD=cCgPl(6?fjj2lSHUnw=j%Fm$;{qyYp^&@K7?`Q<TP?)HA_rk~OY<d9S=^vEkPF
>> z6|$P)g`A-xIv#p4yG070Hplqgs#Ueh()Kc9o75etx1k)96o}Gxaq|y3S8%l=!S%GM
>> zu(npglV@TM+nd*1$Yw|ovD~iKUwlV#+m|SUT$dM>ph`?&9A|(<v*9hrSn^(<kqL?k
>> z_T<47dpWL_*<%C)VQL4*nQQ)667@ICDeL{xL|3t&$2rmUoMm*zZsrx|?o6$4(QfZ#
>> z*xlSu{47=$T-MONw&OtdyNji5{z7>TR4V=4gQvqUh<UCqcKF12HF1D4+e6Rt7~wuK
>> zFZ+GJ0W*P*>qX?+mmNOubE!D2FWfLV&+Dy`X|r~ixa!<qRw)AYvP3Q=t<A4J__K$Y
>> zds^9BB@=5m&ii*#2^6oJE?;lx8l5oHTd&U46Ir6ixo=K|%}Zr9WvX}X3%}9V@}pHY
>> zua7l8RIE+QDW5U7Z&P*_6l3)Ju<d=N)!RI;5h1$4SiiwHS2!!H8uh}(sI6-cH}X7~
>> zbgeZ$`6=q)ahG?S3UW^ks<x9@U%MsEsxcX!>{D?zNn4v)s`_l$9tMVn{5DB@8b^x;
>> z-A2OErxHxcQk;7^`<Q&4l^;3GNtQl0EviuHOiuR_m^YkK(baQSV=5obq3<~HV8Byv
>> zD*J7xgzmb$4s&Ayci8Uy6H5E0YxT<v8*lrbOw{B`cx$wmJL2>HU)A4gy$Yk$Ssk}0
>> z?p5>aeV-1F`#A`2?tZLsFm&BjO0px>HZm+_5|U9#t4NcEZc+=5q%Rn3$grhl4NEDN
>> zy0reKO<W4!k&%tHK1~r(0p|rnCj}g4RxgQ*(fC*`0$Xmd<px`Bu;m6@ZfKiA8PhXO
>> zaOYyl&z%?fGUJPMWPe-Bf0y41?)f5rD+CMyL%RX(2DBS~a;vT@bB?G5Gx$fE!MOX1
>> zyRS<%j1L_K9R?i+9R?i+9fq+FjD0}qL+N889ux6P(o2S?pCiC-w6t;?O^`(b9Rh}c
>> z;gejt0U{KA6#b<gQA36y!;oQRmwf)U4j7*PA{7k+hJY=t1Vn}*!;oQUv!Ts~HXGV(
>> zXtSZshBh18Y-qEg&BhNj0vZ90fJQ*0za1LIlXm`@1uH-2mK!7S;Ij)K_Od!p?@=+n
>> zBA|LA?5J?mV2S!+;)vo-QlHHegq|z52{(H6WJ&U9?FDTD>SF`_KNLOodC^fk*InJ-
>> zB$620;koW<B*~$~^&Fcf+r=rL`;k1m{$zr_=)>c(6Ir=aeDCIu3vX6ek|v9p9_M&J
>> z;-uSFk}%m|{fN_$nnaiF(cZJ+sVvIe>VZd88n#$T*fEzncZJ;&l2%)_$^YHL7vXp3
>> zN;emnc*pfiP8+%DbUQ}$ahKjO)ZXg0Z%*Tztf5f(<rzPgMqp_KmPTM{1eQi%X~g$h
>> zR)k3qOoCt%1d|}tG9G{IVQ839Fr#2b!Hj|#^|v#luw`SBTQ=xPCH{Kuor5St7rpgn
>> zscSbR2?mT3;vDgX!QX6P!@eu*F>Eo|Vz9+vi@_FyEe2bRKJ7nQ0}KuZ2ZR6TcY%xQ
>> zi~KGSFa&IA3kfm|8HNnQ?g($sGV_XmO)|%m*8lZvF~?{f#*}r^GH1e1d*93SVyZI^
>> zsT=3l<lO2=n|zc};b_5tf6cDVUJ8@HL=z<#d&Ai{a+h6t#UC_gt2HR|NyqjB_eApW
>> zOoWZ~Y?D;eo|!aJ4vDDDofhV<-sc9U+^6P_J&sKZPo|(07bqOMqiecTZsOvm)1P&_
>> zMpg`}xVf=iu#C13D&)Fp#Two-NUU<YSE*voZ>5u^M3W&Jyers%??lUuZkK%#1Rkqv
>> zp<#iau1+P^X~=CfaeK%~%jxif(XA&vyGo%_BTItsY0q+vA!fP40gqz7)LF{F&T02{
>> zI!=XiPPe^Tc&(b+Tt3RP{QD8KO6`|>;n=~kgJTEB4vrlhJ2-Z5><rib>FEGioQvc}
>> z1PlRNTF^#@A;XYi=sNuAvmKuBA`1xuhJY<?<wb@e!;oQU)BMji4aU6}Iqr>sAz*lg
>> zQdH;Pw<{qlAS)m%AS)m%1W*5g%?of`7g;_LFa&IA&l_YIG7K4pCJ34!Sby-%ZoNBZ
>> z=)ZnrNU{8LRafR5QR^G6z6X3S@~A?<5U`~^+mT_&Fk~3~F!*8c!-}6R{U%bl6^lG?
>> z5HJL6X%A>*7%~hQ1_KcWA`C<rh<NAp8=1Y36_6E>6_6E>6_6E>6_6E>6?i@!&!^-0
>> zbUdGq=hMf5MnEH=5zq)|1T+E~0gZr0KqH_L&<JP*G%^Aj0gZr0KqH_L&<JP*Gy)m{
>> zjetf#BcKt`=r`UT#ues8c2f~B1Z-(N7GxMQ3>k*40@x~mtpeC8@Ov&EPDz}SI3;mP
>> z;*`WGiBl4%<TvVdK~_LkKvqCjKvqCjKvqCjKvw*i5gO<)=rHIo=rHIo=rHIo=rHIo
>> z=rA{Nj`+f0Tx|HgYJjDYi`2{r7y`Dmkbw+Ch9SdnBNsPvaU&PP6~gr!VJu_?WCdge
>> zWCdgeWCdgeWCdi!k8#JL!=S^U!=S^U!=S^U!=S^U!=S^ka19IBuy73v*RXI63)ir4
>> zZ7t9UXaqC@8Uc-fMnEH=5zq)|1T+E~0gZr0)<7em5zq)|1T+E~0gZr0KqH_L&<JP*
>> zGy)o7(MLVd2xtT}0vZ90fJQ(gpb^jrXaqC@8Uc-fMp{54pb^jrXaqC@8Uc-fMnEH=
>> z5zq)|1T+E~Z3h|wjetf#BcKt`2xtT}0vZ90fJQ(gpb^k09cTnJ0vZ90fJQ(gpb^jr
>> zXaqC@8Uc-fMnEGipb^jrXaqC@8Uc-fMnEH=5zq)|1T+E~0gYAzjetf#BcKt`2xtT}
>> z0vZ90fJQ(gpb^jrXjB0-0vZ90fJQ(gpb^jrXaqC@8Uc-fMnEH=kqpoXXaqC@8Uc-f
>> zMnEH=5zq)|1T+E~0gZr0>_8)+5zq)|1T+E~0gZr0KqH_L&<JP*Gy)pY1C4-2KqH_L
>> z&<JP*Gy)m{jetf#BcRduKqJpC4-!_A5j*Xa`O-UX;vDgX|E<Hs1AHV%{_$7TAo-m1
>> iabF_|=~x%F$3OnaNk~d>QI@f=$!sS3=RY9VH~24oN>cy;
>>
>> literal 0
>> HcmV?d00001
>>

Re: [PATCH 7/7] btrfs: tests/fsck: Add test image for free space cache mode repair

[Qu Wenruo]

On 2019/3/29 下午8:05, Nikolay Borisov wrote:
>
>
> On 25.03.19 г. 10:22 ч., Qu Wenruo wrote:
>> The image has one free space cache inode with invalid mode (0).
>>         item 9 key (256 INODE_ITEM 0) itemoff 13702 itemsize 160
>>                 generation 30 transid 30 size 65536 nbytes 1507328
>>                 block group 0 mode 0 links 1 uid 0 gid 0 rdev 0
>>                 sequence 23 flags 0x1b(NODATASUM|NODATACOW|NOCOMPRESS|PREALLOC)
>>                 atime 0.0 (1970-01-01 08:00:00)
>>                 ctime 1553491158.189771625 (2019-03-25 13:19:18)
>>                 mtime 0.0 (1970-01-01 08:00:00)
>>                 otime 0.0 (1970-01-01 08:00:00)
>>
>> Both lowmem and original mode should be able to detect and fix it.
>>
>> The extracted test image is pretty big (1G extracted), as kernel won't
>> cache small chunks.
>> Even with SSD, such test may still take some seconds just extracting the
>> image.
>
>
> Should this image contain also a non-freespace but root inode (for the
> root dir case) so it has full coverage of the code you introduced?

In fact the only possible repair is for free space cache.
ROOT_TREE_DIR repair is just a common base, and there is no entry for it.

Thanks,
Qu

>
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>  .../test.raw.xz                                | Bin 0 -> 159128 bytes
>>  1 file changed, 0 insertions(+), 0 deletions(-)
>>  create mode 100644 tests/fsck-tests/038-bad-free-space-cache-inode-mode/test.raw.xz
>>
>> diff --git a/tests/fsck-tests/038-bad-free-space-cache-inode-mode/test.raw.xz b/tests/fsck-tests/038-bad-free-space-cache-inode-mode/test.raw.xz
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..c2603c024098992bece235d27187eefa696b3fe0
>> GIT binary patch
>> literal 159128
>> zcmeI5c{JB+*T%_|kTGN`Ny-=vGN%zgWGHizsHl+npiGgu5Q$8cF+#=^DI`;7GNzO$
>> zgow_MywCHlXRYU5?{m(2b55%FoXg*>)mr<vzx%#E*WUNO=OtbA3`j`G2ix=ZEhFJ2
>> z<su;=VQA2+TUel9W^tH=#Cm>V;b!5&etD%;wo*qmi^4Vz-t?vVJocK$t3|9*T9%ZM
>> zbG|wtKuzo>U8j5UKoB3JcE3J*r9;3gr`dul?zEO=efG*`M)73ifu^El?BhqbrsO8(
>> z72Ab4UpKX6a$Q+}+d+M~;$hPu?LA4Ug(siCc^SH=?FnbgdhTS0{-MTAJBC*rRu3<x
>> zx<7h7BFE%;vDw6Z`RSBB{io>;5RaHd%7^9midOP+J?cD3^^v?(yQ&~q^^vk=?~SGy
>> z>b+Ml?_OCPv87?Bsw2UsglR1D<Ke2F{MzGIX#(5Z8M(S|vDdWJGIiG#^R!;!IUJST
>> zpv6(lQs%O~dWE6U+ooKiPI$iOZ1J}4w^;$R?xbOP`^*~FoRbJL60sDverq)r_{i&1
>> za8|sSBionpPli#=L)N)BxQVKxQ*n;x^CbwZ>UvQe=V?q<D@PAF$yXgcb-v2_SEjC+
>> zIWqDhS^v9QNkccO1xL~s3^rug(z1r7luBJ%|I#Kdh40A7##*1Ih^T<`f}xWF4l}FE
>> zD5KP(=AUP^U8b+#*PfjY-7Zd%ls#ts$VP2HZBr;?dZr0?@v|HOb|Zb?;p19*rNfRH
>> z`mf&@QY`;m)s;C%)Dp6<=P_Kn>vONR-0UVZY8j7O!P77DWyUK?M31D?%M*_8y3D+C
>> zfgybMth4?sP5nZ|#(nf@b8daa=*^-!WtV(DdV7|cFWG<1^dZb5IevkGRET86PCI43
>> z^p2Z2M|@$hxYge=8ej86Xwoa(MZgx3xd_-IKXn8Q0bAO`1Q~`5Lxz1fkp)??1cDef
>> z3^fcj3^nX$C2JIY6#b?3ZjfQfFl5+IWvAkk#3zYQ5})M%=avv^7-|@57-|@5*xzUl
>> z07V}~A4MNU|L;-sl}V*Z-oHAf+OvJ4x~@O>*1h(lC*OOow9%>-&#jzpyJVU-QuDng
>> zu)e19{ycMM6k;Mqn0~_a)6xw(qOXg-F8aFY>!Ppw7a1}C8i9kNkD`yFkD`yFkD`yO
>> zN56Lm45uVcNt}{6C2>mPl*B2CQ}P>KEr+attbnY5tbnY5tbnY5tbnZeF)Mzc!=S^y
>> zj}8;YvZw!R>(+!SwbQROm6r}gdSTuP^G=v|`Y!9}Fe`vD^q*x6J(bk!uX{{Bq%#uQ
>> z%{hOlG@UF=Vb?0#*uE>HcYEUNM)j^+jtBJ#r})gPHh5l$mm7IrM}9!Ami+m+_CRBZ
>> zxn*lse8R?oSJkuCr&LN5)6}L}9|RS5ZsA({OOHppuI(P{z^jU*w>ju<O^6WL_iWpE
>> z+~HZy+YNlFvy_3I)9&qboC@chZhN!vS~az~e3WO=HvRo|nKRxkG|ur00Tgn7{CCLw
>> zu%@=Tos4^eWw~#AtaR<8?M^Xug!uS@P($<n5au{`yO&!k4vHFU*dM;VrqegbgTv_I
>> z6f>pqi6iRqjH1_6Lew_q8?$aX9#EIP{7k)<likJ8%P~)+h7WX!eQCH%uvFMlS37>h
>> z)%t8<afdC%-5|D(hpBbf2g|tjK5=8%C%NaWZgYzI1i@W4+_-!4(S>FYRgtvTu_hkJ
>> zqWdflYbh%(`zEtRI*3J)+{sWCby&{AA!K!=$BpQaaD(b_bco-AQ&tCQS7^2ib>{y0
>> z6?Tpn%WT$vCMyfQV{39~%}v`E#~jt@$`5?J9kWi3nT3}^#b(QNlYFE}&7_9xOODFZ
>> z(#nBpOfoqy(xdh%o4XUkynN^Lr1qYtwJ*=PSMQKIA9?A(OrD9w%Kh05xfAPVPsf*f
>> zD7h+koPFRcTP5Jyer#3Ots=^T9TVB&6dGC#7n-L=meoD&yeTZ;Xpkw$f0Mn<oW1yC
>> z!^*qS0RlA*vg_6O8p3=>4HC|LzC95@M>Sm4_wY-}#5Vetw#Q777w$y|pB1)lzb;eP
>> zCs0@INu2a8H!Z2FTv>96gSfq}P-#<^onouM1@D%%1x1P7BFrYy9{YCd$|(fT1#I9M
>> ziv8ujrbYe8#kDno+?(Q+mhC-e_%v>v@$`Pb2a3`(t?mh3>GaZFsymu$PF}v$YV7_d
>> zws$UH)yjq<m`YDDy;4yl?G&ppm8*x9=)f;l(e)QgF3Ryacx%eW)@(M}JXbeO&O`Me
>> z{ca84Dmw;sM(H=4H->LZ$?US2DS3QKd7gy5^h{3XdTuVFR<_=j+6S8k>^9^~9xrG~
>> z(suOlYH0ql<M^rmqy*B)sPq{3vuwi>qa(#xjkiWMn>(Ut?lpbsyXTSQr}tLsNT!>1
>> zc~C}M)(Yy$N|DWWVf8eAPab+-Zj}z^o>jbZ&-_fx2m4_0{Oell>1M8)1z6a55boE|
>> ztM#u`%5HSAwkkQevr#LU@$#C=DL<Qi9yG=JJ!$FMT?X0oMDcy%?v70IS}WF=on70)
>> zK_CVu@j6a$SNrq1(|O8fN#0@?(N1rhUM{G#zN*rYboklpoKqWvDL+N;-Id0A&^cx7
>> zIcuqIZ^46<wB(NSS81&+x?S7u&es}D*IeA~d90k<%)D+V(_?R0(Xwq4b@%xv+!)?_
>> zl@J`x9TJ(T5%KruFE4+n*&ORr#PhCHG`!;sgH)c-6PBE$l*7yqF0}d`AkE^5I6tEP
>> zakqPD;-TxB`2)kdJ?l4^nHiC`W>A}(M2Ilycy_NKvk!EPp^pr!$s*tJt1t~aA;iId
>> z`R9y+4Ntea6ur_BRg+=gn=xOk`Ydg@?8Lfl%H1z7HA=JbI#Tl99o#54+!EwOr7~FI
>> zctBBsYOp^!lR9yWZffah3`hRwp-_SuF)GJAB==5UZ)np{m6wq35V3N5U`B6>=0MpC
>> z2Ev}iH>;lb(_Up^YH{B`-0!+0eqGDUm23w(@=dlU@W_)0^}Suk)XCDzy|X=o@@~La
>> z)UTgm-EVium3i<B_yzm|e)$>qj|2Dsd_N2DVF~_Mzea#o1zHu~r&WP>DZcW75PXvO
>> zB=JdNiVIU*nBu||7pAx{#f2%ZAO5@?>>b!Uuy?Rl?(eZy4hA9&#HD)w74{D79oRdt
>> zcVO?p-hsUXdk6Lo>>b!UdP^m@f8fS#D19h>D19h>D19h>D19h>D19h>D19jXGRi2m
>> zsQKqvZI|gQ_<tau{J{SjXaqC@8Uc-fMnEH=5zq)|^dr&e4ypSeU(Av{X8n`bFs;#a
>> zN`;sFvSdn%xxKL)*hMF(1Pcmiv)qpMIngOSH5QhC`G&m9E_UXG^{nC1gS#9wV-j6@
>> zZNkr-xXL5(@mwO=v}@pk1m{}iyKPpYr?ztiN_M{!s&b!9rZ)_c2sF(ypxytKSG#^&
>> zadS%F_x<N~9Ev~lBb;oY%p5eWe9W8GG<&g!C+hwCtn;<qHy6V6^DDex*+|ROM^YX#
>> z;!WKkLS;SSaO}|)vrm4)GUi3?p6MU@XvDNGla1LBSnj00lhRRBS$-@if{xBHQj+Y%
>> zPRqT^OlPDN7lx(GsfJQ4J+!8jM`wyZGp{=+Q6qTe8FPi%YYoYFLh+#=SL<4oCfzmL
>> zA#OWPcJXkR{szBQ>S1iEYb{f#>q;)2S+#oEvfS2Rja4L%B-KZcnJ?TX`QJ-}G%wmF
>> z_G#xAYI)8C8tBhxdwKa^9i6gGF4~=N;!xIGvMhCx;fzNO_kEi?S|5MDc<!l@HFIX4
>> z$rBgnYdc+^1RW(9MJHBUUw9fTKz%_@h^0K~`VNvh=O!sJ4r?!^jilvqw~RBqiZg=f
>> zGD+V=wJ>e7c$n^OZ@pS=N8B^Zh-)M1k~2NJ@(QNU&p*A!cBaTZ<Wu<k1_cWByW{aU
>> zLuu}bI_R)^F5I7`mzpvUY1h74P_xY0kmw|QZ|rCf|FSr@_2tB!v#CmwxnXt>I$A>l
>> zD2XzcwYZkO&Z-@vVO?=;I5{l-nyKf>j2CNci6Nm|oqWiH*laYeH3>iGN@@)Kpr$y&
>> zzC(Pp<<;SmC`bJmvzXo0S?5=n$h?Ylp!AVr-b&bdX+~C{O8-Xds?;^a$>!_hiqTEx
>> ztD=cCgPl(6?fjj2lSHUnw=j%Fm$;{qyYp^&@K7?`Q<TP?)HA_rk~OY<d9S=^vEkPF
>> z6|$P)g`A-xIv#p4yG070Hplqgs#Ueh()Kc9o75etx1k)96o}Gxaq|y3S8%l=!S%GM
>> zu(npglV@TM+nd*1$Yw|ovD~iKUwlV#+m|SUT$dM>ph`?&9A|(<v*9hrSn^(<kqL?k
>> z_T<47dpWL_*<%C)VQL4*nQQ)667@ICDeL{xL|3t&$2rmUoMm*zZsrx|?o6$4(QfZ#
>> z*xlSu{47=$T-MONw&OtdyNji5{z7>TR4V=4gQvqUh<UCqcKF12HF1D4+e6Rt7~wuK
>> zFZ+GJ0W*P*>qX?+mmNOubE!D2FWfLV&+Dy`X|r~ixa!<qRw)AYvP3Q=t<A4J__K$Y
>> zds^9BB@=5m&ii*#2^6oJE?;lx8l5oHTd&U46Ir6ixo=K|%}Zr9WvX}X3%}9V@}pHY
>> zua7l8RIE+QDW5U7Z&P*_6l3)Ju<d=N)!RI;5h1$4SiiwHS2!!H8uh}(sI6-cH}X7~
>> zbgeZ$`6=q)ahG?S3UW^ks<x9@U%MsEsxcX!>{D?zNn4v)s`_l$9tMVn{5DB@8b^x;
>> z-A2OErxHxcQk;7^`<Q&4l^;3GNtQl0EviuHOiuR_m^YkK(baQSV=5obq3<~HV8Byv
>> zD*J7xgzmb$4s&Ayci8Uy6H5E0YxT<v8*lrbOw{B`cx$wmJL2>HU)A4gy$Yk$Ssk}0
>> z?p5>aeV-1F`#A`2?tZLsFm&BjO0px>HZm+_5|U9#t4NcEZc+=5q%Rn3$grhl4NEDN
>> zy0reKO<W4!k&%tHK1~r(0p|rnCj}g4RxgQ*(fC*`0$Xmd<px`Bu;m6@ZfKiA8PhXO
>> zaOYyl&z%?fGUJPMWPe-Bf0y41?)f5rD+CMyL%RX(2DBS~a;vT@bB?G5Gx$fE!MOX1
>> zyRS<%j1L_K9R?i+9R?i+9fq+FjD0}qL+N889ux6P(o2S?pCiC-w6t;?O^`(b9Rh}c
>> z;gejt0U{KA6#b<gQA36y!;oQRmwf)U4j7*PA{7k+hJY=t1Vn}*!;oQUv!Ts~HXGV(
>> zXtSZshBh18Y-qEg&BhNj0vZ90fJQ*0za1LIlXm`@1uH-2mK!7S;Ij)K_Od!p?@=+n
>> zBA|LA?5J?mV2S!+;)vo-QlHHegq|z52{(H6WJ&U9?FDTD>SF`_KNLOodC^fk*InJ-
>> zB$620;koW<B*~$~^&Fcf+r=rL`;k1m{$zr_=)>c(6Ir=aeDCIu3vX6ek|v9p9_M&J
>> z;-uSFk}%m|{fN_$nnaiF(cZJ+sVvIe>VZd88n#$T*fEzncZJ;&l2%)_$^YHL7vXp3
>> zN;emnc*pfiP8+%DbUQ}$ahKjO)ZXg0Z%*Tztf5f(<rzPgMqp_KmPTM{1eQi%X~g$h
>> zR)k3qOoCt%1d|}tG9G{IVQ839Fr#2b!Hj|#^|v#luw`SBTQ=xPCH{Kuor5St7rpgn
>> zscSbR2?mT3;vDgX!QX6P!@eu*F>Eo|Vz9+vi@_FyEe2bRKJ7nQ0}KuZ2ZR6TcY%xQ
>> zi~KGSFa&IA3kfm|8HNnQ?g($sGV_XmO)|%m*8lZvF~?{f#*}r^GH1e1d*93SVyZI^
>> zsT=3l<lO2=n|zc};b_5tf6cDVUJ8@HL=z<#d&Ai{a+h6t#UC_gt2HR|NyqjB_eApW
>> zOoWZ~Y?D;eo|!aJ4vDDDofhV<-sc9U+^6P_J&sKZPo|(07bqOMqiecTZsOvm)1P&_
>> zMpg`}xVf=iu#C13D&)Fp#Two-NUU<YSE*voZ>5u^M3W&Jyers%??lUuZkK%#1Rkqv
>> zp<#iau1+P^X~=CfaeK%~%jxif(XA&vyGo%_BTItsY0q+vA!fP40gqz7)LF{F&T02{
>> zI!=XiPPe^Tc&(b+Tt3RP{QD8KO6`|>;n=~kgJTEB4vrlhJ2-Z5><rib>FEGioQvc}
>> z1PlRNTF^#@A;XYi=sNuAvmKuBA`1xuhJY<?<wb@e!;oQU)BMji4aU6}Iqr>sAz*lg
>> zQdH;Pw<{qlAS)m%AS)m%1W*5g%?of`7g;_LFa&IA&l_YIG7K4pCJ34!Sby-%ZoNBZ
>> z=)ZnrNU{8LRafR5QR^G6z6X3S@~A?<5U`~^+mT_&Fk~3~F!*8c!-}6R{U%bl6^lG?
>> z5HJL6X%A>*7%~hQ1_KcWA`C<rh<NAp8=1Y36_6E>6_6E>6_6E>6_6E>6?i@!&!^-0
>> zbUdGq=hMf5MnEH=5zq)|1T+E~0gZr0KqH_L&<JP*G%^Aj0gZr0KqH_L&<JP*Gy)m{
>> zjetf#BcKt`=r`UT#ues8c2f~B1Z-(N7GxMQ3>k*40@x~mtpeC8@Ov&EPDz}SI3;mP
>> z;*`WGiBl4%<TvVdK~_LkKvqCjKvqCjKvqCjKvw*i5gO<)=rHIo=rHIo=rHIo=rHIo
>> z=rA{Nj`+f0Tx|HgYJjDYi`2{r7y`Dmkbw+Ch9SdnBNsPvaU&PP6~gr!VJu_?WCdge
>> zWCdgeWCdgeWCdi!k8#JL!=S^U!=S^U!=S^U!=S^U!=S^ka19IBuy73v*RXI63)ir4
>> zZ7t9UXaqC@8Uc-fMnEH=5zq)|1T+E~0gZr0)<7em5zq)|1T+E~0gZr0KqH_L&<JP*
>> zGy)o7(MLVd2xtT}0vZ90fJQ(gpb^jrXaqC@8Uc-fMp{54pb^jrXaqC@8Uc-fMnEH=
>> z5zq)|1T+E~Z3h|wjetf#BcKt`2xtT}0vZ90fJQ(gpb^k09cTnJ0vZ90fJQ(gpb^jr
>> zXaqC@8Uc-fMnEGipb^jrXaqC@8Uc-fMnEH=5zq)|1T+E~0gYAzjetf#BcKt`2xtT}
>> z0vZ90fJQ(gpb^jrXjB0-0vZ90fJQ(gpb^jrXaqC@8Uc-fMnEH=kqpoXXaqC@8Uc-f
>> zMnEH=5zq)|1T+E~0gZr0>_8)+5zq)|1T+E~0gZr0KqH_L&<JP*Gy)pY1C4-2KqH_L
>> z&<JP*Gy)m{jetf#BcRduKqJpC4-!_A5j*Xa`O-UX;vDgX|E<Hs1AHV%{_$7TAo-m1
>> iabF_|=~x%F$3OnaNk~d>QI@f=$!sS3=RY9VH~24oN>cy;
>>
>> literal 0
>> HcmV?d00001
>>