[PATCH 00/15] btrfs-progs: fix filetype mismatch in check

[PATCH 00/15] btrfs-progs: fix filetype mismatch in check

[Su Yue]

This patchset is based on Qu Wenruo's
"[PATCH v2 0/3] Lowmem fsck repair to fix filetype mismatch".
It can be fetched from my github:
https://github.com/Damenly/btrfs-progs/tree/mismatch_filetype

Above Qu's patchset fixes mismatched filetype already. But both
original and lowmem can't handle more complex cases like image in the
last of this patchset:
Both filetypes of dir_item/index are corrupted and inode item is
missing/mismatched.

This patch contains fixes of lowmem check and original check.
For lowmem:
Patch[1-5] fix the complex cases by this way:
  Check filetypes of couple (dir_item,dir_index) and inode mode.
  If two of three have same filetype, choose it as right filetype.
  Handle it in repair_dir_item().
  
Patch[6-9] fix minor bugs of lowmem repair.

Patch[10-12] fix minor bugs about error bit and return value of
	     original repair.
	     
Patch[13-14] fix the complex cases by another way:
  Since original mode store one filetype from dir_item/dir_index and
  inode mode, if backref has mismatched filetype, we think inode mode
  is trusted. If inode item is missing, get filetype from a normal
  couple (dir_item and dir_index);
  
Patch[15] provides a test image.

Su Yue (15):
  btrfs-progs: lowmem check: introduce repair_inode_item_mismatch()
  btrfs-progs: lowmem check: find and guess inode filetype
  btrfs-progs: lowmem check: find filetype in repair_inode_missing()
  btrfs-progs: lowmem check: repair complex cases in repair_dir_item()
  btrfs-progs: lowmem check: let check_dir_item() continue if find wrong
    inode_item
  btrfs-progs: lowmem check: let check_dir_item() return if repaired
  btrfs-progs: lowmem check: find_dir_item by di_key in check_dir_item()
  btrfs-progs: lowmem check: call get_dir_isize() after repair
  btrfs-progs: lowmem check: change logic of leaf process if repair
  btrfs-progs: check: clear I_ERR_FILE_EXTENT_DISCOUNT after repair
  btrfs-progs: check: modify indoe_rec and backref after repair
  btrfs-progs: check: increase counter error in check_inode_recs()
  btrfs-progs: check: find inode filetype in create_inode_item()
  btrfs-progs: check: handle mismatched filetype in repair_inode_backref
  btrfs-progs: fsck-tests: add image for original and lowmem check

 cmds-check.c                                       | 585 +++++++++++++++++++--
 .../default_case.img                               | Bin 0 -> 3072 bytes
 2 files changed, 528 insertions(+), 57 deletions(-)
 create mode 100644 tests/fsck-tests/029-mismatched-filetype-no-inode/default_case.img

-- 
2.16.1

[PATCH 01/15] btrfs-progs: lowmem check: introduce repair_inode_item_mismatch()

[Su Yue]

New function repair_inode_item_mismatch() only changes inode item'
filetype.

Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
 cmds-check.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/cmds-check.c b/cmds-check.c
index f302724dd840..e3505a7f9d6b 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -4913,6 +4913,58 @@ static void print_inode_ref_err(struct btrfs_root *root, struct btrfs_key *key,
 		      namebuf, filetype);
 }
 
+/*
+ * Change inode mode.
+ *
+ * Returns 0 means success.
+ * Returns <0 means error.
+ */
+static int repair_inode_item_mismatch(struct btrfs_root *root, u64 ino,
+				      u8 filetype)
+{
+	struct btrfs_key key;
+	struct btrfs_trans_handle *trans;
+	struct btrfs_path path;
+	struct btrfs_inode_item *ii;
+	u32 mode;
+	int ret;
+
+	key.objectid = ino;
+	key.type = BTRFS_INODE_ITEM_KEY;
+	key.offset = 0;
+
+	btrfs_init_path(&path);
+	trans = btrfs_start_transaction(root, 1);
+	if (IS_ERR(trans)) {
+		ret = -EIO;
+		goto out;
+	}
+
+	ret = btrfs_search_slot(trans, root, &key, &path, 0, 1);
+	if (ret > 0)
+		ret = -ENOENT;
+	if (ret)
+		goto fail;
+
+	ii = btrfs_item_ptr(path.nodes[0], path.slots[0],
+			    struct btrfs_inode_item);
+	mode = btrfs_inode_mode(path.nodes[0], ii);
+	mode &= ~S_IFMT;
+	mode |= btrfs_type_to_imode(filetype);
+
+	btrfs_set_inode_mode(path.nodes[0], ii, mode);
+	btrfs_mark_buffer_dirty(path.nodes[0]);
+	ret = 0;
+fail:
+	btrfs_commit_transaction(trans, root);
+out:
+	btrfs_release_path(&path);
+	if (ret)
+		error("failed to repair root %llu INODE ITEM[%llu] MISMATCH",
+		      root->objectid, ino);
+	return ret;
+}
+
 /*
  * Insert the missing inode item.
  *
-- 
2.16.1

[PATCH 02/15] btrfs-progs: lowmem check: find and guess inode filetype

[Su Yue]

Introduce find_file_type_lowmem() and guess_file_type_lowmem().

find_file_type_lowmem() gets filetypes from inode_item, dir_item and
dir_index. If two of three's filetype are same and valid, it thinks
the value is correct.

guess_file_type_lowmem() searches file_extent and dir_item/index then
returns with filetype.

Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
 cmds-check.c | 193 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 193 insertions(+)

diff --git a/cmds-check.c b/cmds-check.c
index e3505a7f9d6b..b200fdccf0e5 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -3306,6 +3306,199 @@ static int find_file_type(struct inode_record *rec, u8 *type)
 	return -ENOENT;
 }
 
+/*
+ * Fetch filetype from exited completed dir_item, dir_index and inode_item.
+ * If two of tree items'filetype are same, we think the type is trusted.
+ *
+ * Return 0 if file type is found and BTRFS_FT_* is stored into type.
+ * Return <0 if file type is not found.
+ */
+static int find_file_type_lowmem(struct btrfs_root *root, u64 ino, u8 *type)
+{
+	struct btrfs_key key;
+	struct btrfs_path path;
+	struct btrfs_path path2;
+	struct btrfs_inode_ref *iref;
+	struct btrfs_dir_item *dir_item;
+	struct btrfs_dir_item *dir_index;
+	struct extent_buffer *eb;
+	u64 dir;
+	u64 index;
+	char namebuf[BTRFS_NAME_LEN] = {0};
+	u32 namelen;
+	u8 inode_filetype = BTRFS_FT_UNKNOWN;
+	u8 dir_item_filetype;
+	u8 dir_index_filetype;
+	u8 true_file_type;
+	int slot;
+	int ret;
+
+	key.objectid = ino;
+	key.type = BTRFS_INODE_ITEM_KEY;
+	key.offset = 0;
+
+	btrfs_init_path(&path);
+	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
+	if (ret < 0)
+		goto out;
+	if (!ret) {
+		struct btrfs_inode_item *ii;
+
+		ii = btrfs_item_ptr(path.nodes[0], path.slots[0],
+				    struct btrfs_inode_item);
+		inode_filetype = imode_to_type(btrfs_inode_mode(path.nodes[0],
+								ii));
+	}
+
+	key.objectid = ino;
+	key.type = BTRFS_INODE_REF_KEY;
+	key.offset = (u64)-1;
+
+	btrfs_release_path(&path);
+	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
+	if (ret < 0)
+		goto out;
+	if (!ret) {
+		ret = -EIO;
+		goto out;
+	}
+
+	btrfs_init_path(&path2);
+next:
+	btrfs_release_path(&path2);
+	ret = btrfs_previous_item(root, &path, ino, BTRFS_INODE_REF_KEY);
+	if (ret) {
+		ret = -ENOENT;
+		goto out;
+	}
+
+	eb = path.nodes[0];
+	slot = path.slots[0];
+	btrfs_item_key_to_cpu(eb, &key, slot);
+	dir = key.offset;
+	iref = btrfs_item_ptr(eb, slot, struct btrfs_inode_ref);
+	index = btrfs_inode_ref_index(eb, iref);
+	namelen = btrfs_inode_ref_name_len(eb, iref);
+	read_extent_buffer(eb, namebuf, (unsigned long)(iref + 1), namelen);
+
+	dir_index = btrfs_lookup_dir_index(NULL, root, &path2, dir, namebuf,
+					   namelen, index, 0);
+	if (!dir_index)
+		goto next;
+	dir_index_filetype = btrfs_dir_type(path2.nodes[0], dir_index);
+	btrfs_release_path(&path2);
+	if (dir_index_filetype == inode_filetype) {
+		true_file_type = inode_filetype;
+		goto found;
+	}
+
+	dir_item = btrfs_lookup_dir_item(NULL, root, &path2, dir, namebuf,
+					 namelen, 0);
+	if (!dir_item)
+		goto next;
+	dir_item_filetype = btrfs_dir_type(path2.nodes[0], dir_item);
+	btrfs_release_path(&path2);
+	if (dir_item_filetype == inode_filetype) {
+		true_file_type = inode_filetype;
+		goto found;
+	}
+
+	if (dir_index_filetype == dir_item_filetype) {
+		true_file_type = dir_index_filetype;
+		goto found;
+	}
+	goto next;
+found:
+	/* rare case, two of three items are both corrupted */
+	if (true_file_type == BTRFS_FT_UNKNOWN ||
+	    true_file_type >= BTRFS_FT_MAX)
+		goto next;
+	*type = true_file_type;
+	ret = 0;
+out:
+	btrfs_release_path(&path);
+	return ret;
+}
+
+static int find_normal_file_extent(struct btrfs_root *root, u64 ino);
+/*
+ * Try to determine inode type if type not found.
+ *
+ * For found regular file extent, it must be FILE.
+ * For found dir_item/index, it must be DIR.
+ *
+ * Return 0 if file type is confirmed and BTRFS_FT_* is stored into type.
+ * Return <0 if file type is unknown.
+ */
+static int guess_file_type_lowmem(struct btrfs_root *root, u64 ino, u8 *type)
+{
+	struct btrfs_key key;
+	struct btrfs_path *path = NULL;
+	bool is_dir = false;
+	bool is_file = false;
+	int ret;
+
+	if (find_normal_file_extent(root, ino)) {
+		is_file = true;
+		goto out;
+	}
+
+	key.objectid = ino;
+	key.type = BTRFS_DIR_ITEM_KEY;
+	key.offset = (u64)-1;
+
+	path = btrfs_alloc_path();
+	if (!path)
+		goto out;
+	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
+	if (ret < 0)
+		goto out;
+	/*
+	 * (u64)-1 may hit the hashed value in offset.
+	 */
+	if (!ret) {
+		is_dir = true;
+		goto out;
+	}
+
+	ret = btrfs_previous_item(root, path, ino, BTRFS_DIR_ITEM_KEY);
+	if (!ret) {
+		is_dir = true;
+		goto out;
+	}
+
+	key.type = BTRFS_DIR_INDEX_KEY;
+	key.offset = (u64)-1;
+
+	btrfs_release_path(path);
+	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
+	if (ret < 0)
+		goto out;
+	if (!ret) {
+		is_dir = true;
+		goto out;
+	}
+	ret = btrfs_previous_item(root, path, ino, BTRFS_DIR_INDEX_KEY);
+	if (!ret) {
+		is_dir = true;
+		goto out;
+	}
+out:
+	if (path)
+		btrfs_release_path(path);
+
+	if (is_dir) {
+		*type = BTRFS_FT_DIR;
+		ret = 0;
+	} else if (is_file) {
+		*type = BTRFS_FT_REG_FILE;
+		ret = 0;
+	} else {
+		ret = -ENOENT;
+	}
+	return ret;
+}
+
 /*
  * To determine the file name for nlink repair
  *
-- 
2.16.1

Re: [PATCH 02/15] btrfs-progs: lowmem check: find and guess inode filetype

[Qu Wenruo]

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



On 2018年01月26日 16:35, Su Yue wrote:
> Introduce find_file_type_lowmem() and guess_file_type_lowmem().
> 
> find_file_type_lowmem() gets filetypes from inode_item, dir_item and
> dir_index. If two of three's filetype are same and valid, it thinks
> the value is correct.
> 
> guess_file_type_lowmem() searches file_extent and dir_item/index then
> returns with filetype.
> 
> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>

Looks good.

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu
> ---
>  cmds-check.c | 193 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 193 insertions(+)
> 
> diff --git a/cmds-check.c b/cmds-check.c
> index e3505a7f9d6b..b200fdccf0e5 100644
> --- a/cmds-check.c
> +++ b/cmds-check.c
> @@ -3306,6 +3306,199 @@ static int find_file_type(struct inode_record *rec, u8 *type)
>  	return -ENOENT;
>  }
>  
> +/*
> + * Fetch filetype from exited completed dir_item, dir_index and inode_item.
> + * If two of tree items'filetype are same, we think the type is trusted.
> + *
> + * Return 0 if file type is found and BTRFS_FT_* is stored into type.
> + * Return <0 if file type is not found.
> + */
> +static int find_file_type_lowmem(struct btrfs_root *root, u64 ino, u8 *type)
> +{
> +	struct btrfs_key key;
> +	struct btrfs_path path;
> +	struct btrfs_path path2;
> +	struct btrfs_inode_ref *iref;
> +	struct btrfs_dir_item *dir_item;
> +	struct btrfs_dir_item *dir_index;
> +	struct extent_buffer *eb;
> +	u64 dir;
> +	u64 index;
> +	char namebuf[BTRFS_NAME_LEN] = {0};
> +	u32 namelen;
> +	u8 inode_filetype = BTRFS_FT_UNKNOWN;
> +	u8 dir_item_filetype;
> +	u8 dir_index_filetype;
> +	u8 true_file_type;
> +	int slot;
> +	int ret;
> +
> +	key.objectid = ino;
> +	key.type = BTRFS_INODE_ITEM_KEY;
> +	key.offset = 0;
> +
> +	btrfs_init_path(&path);
> +	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
> +	if (ret < 0)
> +		goto out;
> +	if (!ret) {
> +		struct btrfs_inode_item *ii;
> +
> +		ii = btrfs_item_ptr(path.nodes[0], path.slots[0],
> +				    struct btrfs_inode_item);
> +		inode_filetype = imode_to_type(btrfs_inode_mode(path.nodes[0],
> +								ii));
> +	}
> +
> +	key.objectid = ino;
> +	key.type = BTRFS_INODE_REF_KEY;
> +	key.offset = (u64)-1;
> +
> +	btrfs_release_path(&path);
> +	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
> +	if (ret < 0)
> +		goto out;
> +	if (!ret) {
> +		ret = -EIO;
> +		goto out;
> +	}
> +
> +	btrfs_init_path(&path2);
> +next:
> +	btrfs_release_path(&path2);
> +	ret = btrfs_previous_item(root, &path, ino, BTRFS_INODE_REF_KEY);
> +	if (ret) {
> +		ret = -ENOENT;
> +		goto out;
> +	}
> +
> +	eb = path.nodes[0];
> +	slot = path.slots[0];
> +	btrfs_item_key_to_cpu(eb, &key, slot);
> +	dir = key.offset;
> +	iref = btrfs_item_ptr(eb, slot, struct btrfs_inode_ref);
> +	index = btrfs_inode_ref_index(eb, iref);
> +	namelen = btrfs_inode_ref_name_len(eb, iref);
> +	read_extent_buffer(eb, namebuf, (unsigned long)(iref + 1), namelen);
> +
> +	dir_index = btrfs_lookup_dir_index(NULL, root, &path2, dir, namebuf,
> +					   namelen, index, 0);
> +	if (!dir_index)
> +		goto next;
> +	dir_index_filetype = btrfs_dir_type(path2.nodes[0], dir_index);
> +	btrfs_release_path(&path2);
> +	if (dir_index_filetype == inode_filetype) {
> +		true_file_type = inode_filetype;
> +		goto found;
> +	}
> +
> +	dir_item = btrfs_lookup_dir_item(NULL, root, &path2, dir, namebuf,
> +					 namelen, 0);
> +	if (!dir_item)
> +		goto next;
> +	dir_item_filetype = btrfs_dir_type(path2.nodes[0], dir_item);
> +	btrfs_release_path(&path2);
> +	if (dir_item_filetype == inode_filetype) {
> +		true_file_type = inode_filetype;
> +		goto found;
> +	}
> +
> +	if (dir_index_filetype == dir_item_filetype) {
> +		true_file_type = dir_index_filetype;
> +		goto found;
> +	}
> +	goto next;
> +found:
> +	/* rare case, two of three items are both corrupted */
> +	if (true_file_type == BTRFS_FT_UNKNOWN ||
> +	    true_file_type >= BTRFS_FT_MAX)
> +		goto next;
> +	*type = true_file_type;
> +	ret = 0;
> +out:
> +	btrfs_release_path(&path);
> +	return ret;
> +}
> +
> +static int find_normal_file_extent(struct btrfs_root *root, u64 ino);
> +/*
> + * Try to determine inode type if type not found.
> + *
> + * For found regular file extent, it must be FILE.
> + * For found dir_item/index, it must be DIR.
> + *
> + * Return 0 if file type is confirmed and BTRFS_FT_* is stored into type.
> + * Return <0 if file type is unknown.
> + */
> +static int guess_file_type_lowmem(struct btrfs_root *root, u64 ino, u8 *type)
> +{
> +	struct btrfs_key key;
> +	struct btrfs_path *path = NULL;
> +	bool is_dir = false;
> +	bool is_file = false;
> +	int ret;
> +
> +	if (find_normal_file_extent(root, ino)) {
> +		is_file = true;
> +		goto out;
> +	}
> +
> +	key.objectid = ino;
> +	key.type = BTRFS_DIR_ITEM_KEY;
> +	key.offset = (u64)-1;
> +
> +	path = btrfs_alloc_path();
> +	if (!path)
> +		goto out;
> +	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
> +	if (ret < 0)
> +		goto out;
> +	/*
> +	 * (u64)-1 may hit the hashed value in offset.
> +	 */
> +	if (!ret) {
> +		is_dir = true;
> +		goto out;
> +	}
> +
> +	ret = btrfs_previous_item(root, path, ino, BTRFS_DIR_ITEM_KEY);
> +	if (!ret) {
> +		is_dir = true;
> +		goto out;
> +	}
> +
> +	key.type = BTRFS_DIR_INDEX_KEY;
> +	key.offset = (u64)-1;
> +
> +	btrfs_release_path(path);
> +	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
> +	if (ret < 0)
> +		goto out;
> +	if (!ret) {
> +		is_dir = true;
> +		goto out;
> +	}
> +	ret = btrfs_previous_item(root, path, ino, BTRFS_DIR_INDEX_KEY);
> +	if (!ret) {
> +		is_dir = true;
> +		goto out;
> +	}
> +out:
> +	if (path)
> +		btrfs_release_path(path);
> +
> +	if (is_dir) {
> +		*type = BTRFS_FT_DIR;
> +		ret = 0;
> +	} else if (is_file) {
> +		*type = BTRFS_FT_REG_FILE;
> +		ret = 0;
> +	} else {
> +		ret = -ENOENT;
> +	}
> +	return ret;
> +}
> +
>  /*
>   * To determine the file name for nlink repair
>   *
> 


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

Re: [PATCH 02/15] btrfs-progs: lowmem check: find and guess inode filetype

[Qu Wenruo]

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



On 2018年01月26日 16:35, Su Yue wrote:
> Introduce find_file_type_lowmem() and guess_file_type_lowmem().
> 
> find_file_type_lowmem() gets filetypes from inode_item, dir_item and
> dir_index. If two of three's filetype are same and valid, it thinks
> the value is correct.
> 
> guess_file_type_lowmem() searches file_extent and dir_item/index then
> returns with filetype.
> 
> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
> ---
>  cmds-check.c | 193 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 193 insertions(+)
> 
> diff --git a/cmds-check.c b/cmds-check.c
> index e3505a7f9d6b..b200fdccf0e5 100644
> --- a/cmds-check.c
> +++ b/cmds-check.c
> @@ -3306,6 +3306,199 @@ static int find_file_type(struct inode_record *rec, u8 *type)
>  	return -ENOENT;
>  }
>  
> +/*
> + * Fetch filetype from exited completed dir_item, dir_index and inode_item.
                          ^^^^^^
                          existing?
> + * If two of tree items'filetype are same, we think the type is trusted.
> + *
> + * Return 0 if file type is found and BTRFS_FT_* is stored into type.
> + * Return <0 if file type is not found.

This also includes extra error like -EIO from btrfs_search_slot().

> + */
> +static int find_file_type_lowmem(struct btrfs_root *root, u64 ino, u8 *type)
> +{
> +	struct btrfs_key key;
> +	struct btrfs_path path;
> +	struct btrfs_path path2;
> +	struct btrfs_inode_ref *iref;
> +	struct btrfs_dir_item *dir_item;
> +	struct btrfs_dir_item *dir_index;
> +	struct extent_buffer *eb;
> +	u64 dir;
> +	u64 index;
> +	char namebuf[BTRFS_NAME_LEN] = {0};
> +	u32 namelen;
> +	u8 inode_filetype = BTRFS_FT_UNKNOWN;
> +	u8 dir_item_filetype;
> +	u8 dir_index_filetype;
> +	u8 true_file_type;
> +	int slot;
> +	int ret;
> +
> +	key.objectid = ino;
> +	key.type = BTRFS_INODE_ITEM_KEY;
> +	key.offset = 0;
> +
> +	btrfs_init_path(&path);
> +	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
> +	if (ret < 0)
> +		goto out;
> +	if (!ret) {
> +		struct btrfs_inode_item *ii;
> +
> +		ii = btrfs_item_ptr(path.nodes[0], path.slots[0],
> +				    struct btrfs_inode_item);
> +		inode_filetype = imode_to_type(btrfs_inode_mode(path.nodes[0],
> +								ii));
> +	}
> +
> +	key.objectid = ino;
> +	key.type = BTRFS_INODE_REF_KEY;
> +	key.offset = (u64)-1;
> +
> +	btrfs_release_path(&path);
> +	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
> +	if (ret < 0)
> +		goto out;
> +	if (!ret) {
> +		ret = -EIO;
> +		goto out;
> +	}
> +
> +	btrfs_init_path(&path2);
> +next:
> +	btrfs_release_path(&path2)> +	ret = btrfs_previous_item(root, &path, ino, BTRFS_INODE_REF_KEY);
> +	if (ret) {
> +		ret = -ENOENT;

For ret < 0 case, return value is overwritten.


> +		goto out;
> +	}
> +
> +	eb = path.nodes[0];
> +	slot = path.slots[0];
> +	btrfs_item_key_to_cpu(eb, &key, slot);
> +	dir = key.offset;
> +	iref = btrfs_item_ptr(eb, slot, struct btrfs_inode_ref);
> +	index = btrfs_inode_ref_index(eb, iref);
> +	namelen = btrfs_inode_ref_name_len(eb, iref);
> +	read_extent_buffer(eb, namebuf, (unsigned long)(iref + 1), namelen);
> +
> +	dir_index = btrfs_lookup_dir_index(NULL, root, &path2, dir, namebuf,
> +					   namelen, index, 0);
> +	if (!dir_index)
> +		goto next;
> +	dir_index_filetype = btrfs_dir_type(path2.nodes[0], dir_index);
> +	btrfs_release_path(&path2);
> +	if (dir_index_filetype == inode_filetype) {
> +		true_file_type = inode_filetype;
> +		goto found;
> +	}
> +
> +	dir_item = btrfs_lookup_dir_item(NULL, root, &path2, dir, namebuf,
> +					 namelen, 0);
> +	if (!dir_item)
> +		goto next;
> +	dir_item_filetype = btrfs_dir_type(path2.nodes[0], dir_item);
> +	btrfs_release_path(&path2);
> +	if (dir_item_filetype == inode_filetype) {
> +		true_file_type = inode_filetype;
> +		goto found;
> +	}
> +
> +	if (dir_index_filetype == dir_item_filetype) {
> +		true_file_type = dir_index_filetype;
> +		goto found;
> +	}
> +	goto next;
> +found:
> +	/* rare case, two of three items are both corrupted */
> +	if (true_file_type == BTRFS_FT_UNKNOWN ||
> +	    true_file_type >= BTRFS_FT_MAX)
> +		goto next;
> +	*type = true_file_type;
> +	ret = 0;
> +out:
> +	btrfs_release_path(&path);
> +	return ret;
> +}
> +
> +static int find_normal_file_extent(struct btrfs_root *root, u64 ino);
> +/*
> + * Try to determine inode type if type not found.
> + *
> + * For found regular file extent, it must be FILE.
> + * For found dir_item/index, it must be DIR.
> + *
> + * Return 0 if file type is confirmed and BTRFS_FT_* is stored into type.
> + * Return <0 if file type is unknown.
> + */
> +static int guess_file_type_lowmem(struct btrfs_root *root, u64 ino, u8 *type)
> +{
> +	struct btrfs_key key;
> +	struct btrfs_path *path = NULL;
> +	bool is_dir = false;
> +	bool is_file = false;
> +	int ret;
> +
> +	if (find_normal_file_extent(root, ino)) {

Nice reusing the existing code.

> +		is_file = true;
> +		goto out;
> +	}
> +
> +	key.objectid = ino;
> +	key.type = BTRFS_DIR_ITEM_KEY;

You could merge the code together, using just one loop instead of using
two search separately for DIR_ITEM and DIR_INDEX.

Search for key (ino, DIR_INDEX, -1), as DIR_INDEX is larger than DIR_ITEM.

And search backward, not using btrfs_previous_item() but manually
checking the type (and do prev_leaf() manually).
It should save you several lines.

Thanks,
Qu

> +	key.offset = (u64)-1;
> +
> +	path = btrfs_alloc_path();
> +	if (!path)
> +		goto out;
> +	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
> +	if (ret < 0)
> +		goto out;
> +	/*
> +	 * (u64)-1 may hit the hashed value in offset.
> +	 */
> +	if (!ret) {
> +		is_dir = true;
> +		goto out;
> +	}
> +
> +	ret = btrfs_previous_item(root, path, ino, BTRFS_DIR_ITEM_KEY);> +	if (!ret) {
> +		is_dir = true;
> +		goto out;
> +	}
> +
> +	key.type = BTRFS_DIR_INDEX_KEY;
> +	key.offset = (u64)-1;
> +
> +	btrfs_release_path(path);
> +	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
> +	if (ret < 0)
> +		goto out;
> +	if (!ret) {
> +		is_dir = true;
> +		goto out;
> +	}
> +	ret = btrfs_previous_item(root, path, ino, BTRFS_DIR_INDEX_KEY);
> +	if (!ret) {
> +		is_dir = true;
> +		goto out;
> +	}
> +out:
> +	if (path)
> +		btrfs_release_path(path);
> +
> +	if (is_dir) {
> +		*type = BTRFS_FT_DIR;
> +		ret = 0;
> +	} else if (is_file) {
> +		*type = BTRFS_FT_REG_FILE;
> +		ret = 0;
> +	} else {
> +		ret = -ENOENT;
> +	}
> +	return ret;
> +}
> +
>  /*
>   * To determine the file name for nlink repair
>   *
> 


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

Re: [PATCH 02/15] btrfs-progs: lowmem check: find and guess inode filetype

[Qu Wenruo]

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



On 2018年01月26日 17:14, Qu Wenruo wrote:
> 
> 
> On 2018年01月26日 16:35, Su Yue wrote:
>> Introduce find_file_type_lowmem() and guess_file_type_lowmem().
>>
>> find_file_type_lowmem() gets filetypes from inode_item, dir_item and
>> dir_index. If two of three's filetype are same and valid, it thinks
>> the value is correct.
>>
>> guess_file_type_lowmem() searches file_extent and dir_item/index then
>> returns with filetype.
>>
>> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
>> ---
>>  cmds-check.c | 193 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 193 insertions(+)
>>
>> diff --git a/cmds-check.c b/cmds-check.c
>> index e3505a7f9d6b..b200fdccf0e5 100644
>> --- a/cmds-check.c
>> +++ b/cmds-check.c
>> @@ -3306,6 +3306,199 @@ static int find_file_type(struct inode_record *rec, u8 *type)
>>  	return -ENOENT;
>>  }
>>  
>> +/*
>> + * Fetch filetype from exited completed dir_item, dir_index and inode_item.
>                           ^^^^^^
>                           existing?
>> + * If two of tree items'filetype are same, we think the type is trusted.
>> + *
>> + * Return 0 if file type is found and BTRFS_FT_* is stored into type.
>> + * Return <0 if file type is not found.
> 
> This also includes extra error like -EIO from btrfs_search_slot().
> 
>> + */
>> +static int find_file_type_lowmem(struct btrfs_root *root, u64 ino, u8 *type)
>> +{
>> +	struct btrfs_key key;
>> +	struct btrfs_path path;
>> +	struct btrfs_path path2;
>> +	struct btrfs_inode_ref *iref;
>> +	struct btrfs_dir_item *dir_item;
>> +	struct btrfs_dir_item *dir_index;
>> +	struct extent_buffer *eb;
>> +	u64 dir;
>> +	u64 index;
>> +	char namebuf[BTRFS_NAME_LEN] = {0};
>> +	u32 namelen;
>> +	u8 inode_filetype = BTRFS_FT_UNKNOWN;
>> +	u8 dir_item_filetype;
>> +	u8 dir_index_filetype;
>> +	u8 true_file_type;
>> +	int slot;
>> +	int ret;
>> +
>> +	key.objectid = ino;
>> +	key.type = BTRFS_INODE_ITEM_KEY;
>> +	key.offset = 0;
>> +
>> +	btrfs_init_path(&path);
>> +	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
>> +	if (ret < 0)
>> +		goto out;
>> +	if (!ret) {
>> +		struct btrfs_inode_item *ii;
>> +
>> +		ii = btrfs_item_ptr(path.nodes[0], path.slots[0],
>> +				    struct btrfs_inode_item);
>> +		inode_filetype = imode_to_type(btrfs_inode_mode(path.nodes[0],
>> +								ii));
>> +	}
>> +
>> +	key.objectid = ino;
>> +	key.type = BTRFS_INODE_REF_KEY;
>> +	key.offset = (u64)-1;
>> +
>> +	btrfs_release_path(&path);
>> +	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
>> +	if (ret < 0)
>> +		goto out;
>> +	if (!ret) {
>> +		ret = -EIO;
>> +		goto out;
>> +	}
>> +
>> +	btrfs_init_path(&path2);
>> +next:
>> +	btrfs_release_path(&path2)> +	ret = btrfs_previous_item(root, &path, ino, BTRFS_INODE_REF_KEY);
>> +	if (ret) {
>> +		ret = -ENOENT;
> 
> For ret < 0 case, return value is overwritten.
> 
> 
>> +		goto out;
>> +	}
>> +
>> +	eb = path.nodes[0];
>> +	slot = path.slots[0];
>> +	btrfs_item_key_to_cpu(eb, &key, slot);
>> +	dir = key.offset;
>> +	iref = btrfs_item_ptr(eb, slot, struct btrfs_inode_ref);
>> +	index = btrfs_inode_ref_index(eb, iref);
>> +	namelen = btrfs_inode_ref_name_len(eb, iref);
>> +	read_extent_buffer(eb, namebuf, (unsigned long)(iref + 1), namelen);
>> +
>> +	dir_index = btrfs_lookup_dir_index(NULL, root, &path2, dir, namebuf,
>> +					   namelen, index, 0);
>> +	if (!dir_index)
>> +		goto next;
>> +	dir_index_filetype = btrfs_dir_type(path2.nodes[0], dir_index);
>> +	btrfs_release_path(&path2);
>> +	if (dir_index_filetype == inode_filetype) {
>> +		true_file_type = inode_filetype;
>> +		goto found;
>> +	}
>> +
>> +	dir_item = btrfs_lookup_dir_item(NULL, root, &path2, dir, namebuf,
>> +					 namelen, 0);
>> +	if (!dir_item)
>> +		goto next;
>> +	dir_item_filetype = btrfs_dir_type(path2.nodes[0], dir_item);
>> +	btrfs_release_path(&path2);
>> +	if (dir_item_filetype == inode_filetype) {
>> +		true_file_type = inode_filetype;
>> +		goto found;
>> +	}
>> +
>> +	if (dir_index_filetype == dir_item_filetype) {
>> +		true_file_type = dir_index_filetype;
>> +		goto found;
>> +	}
>> +	goto next;
>> +found:
>> +	/* rare case, two of three items are both corrupted */
>> +	if (true_file_type == BTRFS_FT_UNKNOWN ||
>> +	    true_file_type >= BTRFS_FT_MAX)
>> +		goto next;
>> +	*type = true_file_type;
>> +	ret = 0;
>> +out:
>> +	btrfs_release_path(&path);
>> +	return ret;
>> +}
>> +
>> +static int find_normal_file_extent(struct btrfs_root *root, u64 ino);
>> +/*
>> + * Try to determine inode type if type not found.
>> + *
>> + * For found regular file extent, it must be FILE.
>> + * For found dir_item/index, it must be DIR.
>> + *
>> + * Return 0 if file type is confirmed and BTRFS_FT_* is stored into type.
>> + * Return <0 if file type is unknown.
>> + */
>> +static int guess_file_type_lowmem(struct btrfs_root *root, u64 ino, u8 *type)
>> +{
>> +	struct btrfs_key key;
>> +	struct btrfs_path *path = NULL;
>> +	bool is_dir = false;
>> +	bool is_file = false;
>> +	int ret;
>> +
>> +	if (find_normal_file_extent(root, ino)) {
> 
> Nice reusing the existing code.
> 
>> +		is_file = true;
>> +		goto out;
>> +	}
>> +
>> +	key.objectid = ino;
>> +	key.type = BTRFS_DIR_ITEM_KEY;
> 
> You could merge the code together, using just one loop instead of using
> two search separately for DIR_ITEM and DIR_INDEX.
> 
> Search for key (ino, DIR_INDEX, -1), as DIR_INDEX is larger than DIR_ITEM.
> 
> And search backward, not using btrfs_previous_item() but manually
> checking the type (and do prev_leaf() manually).
> It should save you several lines.

In fact you could use btrfs_previous_item() twice, to locate any
DIR_INDEX/DIR_ITEM.

btrfs_previous_item(DIR_INDEX) to locate any DIR_INDEX, and then
btrfs_previous_item(DIR_ITEM) again if DIR_INDEX not found.

Thanks,
Qu

> 
> Thanks,
> Qu
> 
>> +	key.offset = (u64)-1;
>> +
>> +	path = btrfs_alloc_path();
>> +	if (!path)
>> +		goto out;
>> +	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
>> +	if (ret < 0)
>> +		goto out;
>> +	/*
>> +	 * (u64)-1 may hit the hashed value in offset.
>> +	 */
>> +	if (!ret) {
>> +		is_dir = true;
>> +		goto out;
>> +	}
>> +
>> +	ret = btrfs_previous_item(root, path, ino, BTRFS_DIR_ITEM_KEY);> +	if (!ret) {
>> +		is_dir = true;
>> +		goto out;
>> +	}
>> +
>> +	key.type = BTRFS_DIR_INDEX_KEY;
>> +	key.offset = (u64)-1;
>> +
>> +	btrfs_release_path(path);
>> +	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
>> +	if (ret < 0)
>> +		goto out;
>> +	if (!ret) {
>> +		is_dir = true;
>> +		goto out;
>> +	}
>> +	ret = btrfs_previous_item(root, path, ino, BTRFS_DIR_INDEX_KEY);
>> +	if (!ret) {
>> +		is_dir = true;
>> +		goto out;
>> +	}
>> +out:
>> +	if (path)
>> +		btrfs_release_path(path);
>> +
>> +	if (is_dir) {
>> +		*type = BTRFS_FT_DIR;
>> +		ret = 0;
>> +	} else if (is_file) {
>> +		*type = BTRFS_FT_REG_FILE;
>> +		ret = 0;
>> +	} else {
>> +		ret = -ENOENT;
>> +	}
>> +	return ret;
>> +}
>> +
>>  /*
>>   * To determine the file name for nlink repair
>>   *
>>
> 


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

Re: [PATCH 02/15] btrfs-progs: lowmem check: find and guess inode filetype

[Su Yue]

On 01/26/2018 05:14 PM, Qu Wenruo wrote:
> 
> 
> On 2018年01月26日 16:35, Su Yue wrote:
>> Introduce find_file_type_lowmem() and guess_file_type_lowmem().
>>
>> find_file_type_lowmem() gets filetypes from inode_item, dir_item and
>> dir_index. If two of three's filetype are same and valid, it thinks
>> the value is correct.
>>
>> guess_file_type_lowmem() searches file_extent and dir_item/index then
>> returns with filetype.
>>
>> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
>> ---
>>   cmds-check.c | 193 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 193 insertions(+)
>>
>> diff --git a/cmds-check.c b/cmds-check.c
>> index e3505a7f9d6b..b200fdccf0e5 100644
>> --- a/cmds-check.c
>> +++ b/cmds-check.c
>> @@ -3306,6 +3306,199 @@ static int find_file_type(struct inode_record *rec, u8 *type)
>>   	return -ENOENT;
>>   }
>>   
>> +/*
>> + * Fetch filetype from exited completed dir_item, dir_index and inode_item.
>                            ^^^^^^
>                            existing?

Oh.. a Typo.
>> + * If two of tree items'filetype are same, we think the type is trusted.
>> + *
>> + * Return 0 if file type is found and BTRFS_FT_* is stored into type.
>> + * Return <0 if file type is not found.
> 
> This also includes extra error like -EIO from btrfs_search_slot().
> 
>> + */
>> +static int find_file_type_lowmem(struct btrfs_root *root, u64 ino, u8 *type)
>> +{
>> +	struct btrfs_key key;
>> +	struct btrfs_path path;
>> +	struct btrfs_path path2;
>> +	struct btrfs_inode_ref *iref;
>> +	struct btrfs_dir_item *dir_item;
>> +	struct btrfs_dir_item *dir_index;
>> +	struct extent_buffer *eb;
>> +	u64 dir;
>> +	u64 index;
>> +	char namebuf[BTRFS_NAME_LEN] = {0};
>> +	u32 namelen;
>> +	u8 inode_filetype = BTRFS_FT_UNKNOWN;
>> +	u8 dir_item_filetype;
>> +	u8 dir_index_filetype;
>> +	u8 true_file_type;
>> +	int slot;
>> +	int ret;
>> +
>> +	key.objectid = ino;
>> +	key.type = BTRFS_INODE_ITEM_KEY;
>> +	key.offset = 0;
>> +
>> +	btrfs_init_path(&path);
>> +	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
>> +	if (ret < 0)
>> +		goto out;
>> +	if (!ret) {
>> +		struct btrfs_inode_item *ii;
>> +
>> +		ii = btrfs_item_ptr(path.nodes[0], path.slots[0],
>> +				    struct btrfs_inode_item);
>> +		inode_filetype = imode_to_type(btrfs_inode_mode(path.nodes[0],
>> +								ii));
>> +	}
>> +
>> +	key.objectid = ino;
>> +	key.type = BTRFS_INODE_REF_KEY;
>> +	key.offset = (u64)-1;
>> +
>> +	btrfs_release_path(&path);
>> +	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
>> +	if (ret < 0)
>> +		goto out;
>> +	if (!ret) {
>> +		ret = -EIO;
>> +		goto out;
>> +	}
>> +
>> +	btrfs_init_path(&path2);
>> +next:
>> +	btrfs_release_path(&path2)> +	ret = btrfs_previous_item(root, &path, ino, BTRFS_INODE_REF_KEY);
>> +	if (ret) {
>> +		ret = -ENOENT;
> 
> For ret < 0 case, return value is overwritten.
> 
Will fix it.
> 
>> +		goto out;
>> +	}
>> +
>> +	eb = path.nodes[0];
>> +	slot = path.slots[0];
>> +	btrfs_item_key_to_cpu(eb, &key, slot);
>> +	dir = key.offset;
>> +	iref = btrfs_item_ptr(eb, slot, struct btrfs_inode_ref);
>> +	index = btrfs_inode_ref_index(eb, iref);
>> +	namelen = btrfs_inode_ref_name_len(eb, iref);
>> +	read_extent_buffer(eb, namebuf, (unsigned long)(iref + 1), namelen);
>> +
>> +	dir_index = btrfs_lookup_dir_index(NULL, root, &path2, dir, namebuf,
>> +					   namelen, index, 0);
>> +	if (!dir_index)
>> +		goto next;
>> +	dir_index_filetype = btrfs_dir_type(path2.nodes[0], dir_index);
>> +	btrfs_release_path(&path2);
>> +	if (dir_index_filetype == inode_filetype) {
>> +		true_file_type = inode_filetype;
>> +		goto found;
>> +	}
>> +
>> +	dir_item = btrfs_lookup_dir_item(NULL, root, &path2, dir, namebuf,
>> +					 namelen, 0);
>> +	if (!dir_item)
>> +		goto next;
>> +	dir_item_filetype = btrfs_dir_type(path2.nodes[0], dir_item);
>> +	btrfs_release_path(&path2);
>> +	if (dir_item_filetype == inode_filetype) {
>> +		true_file_type = inode_filetype;
>> +		goto found;
>> +	}
>> +
>> +	if (dir_index_filetype == dir_item_filetype) {
>> +		true_file_type = dir_index_filetype;
>> +		goto found;
>> +	}
>> +	goto next;
>> +found:
>> +	/* rare case, two of three items are both corrupted */
>> +	if (true_file_type == BTRFS_FT_UNKNOWN ||
>> +	    true_file_type >= BTRFS_FT_MAX)
>> +		goto next;
>> +	*type = true_file_type;
>> +	ret = 0;
>> +out:
>> +	btrfs_release_path(&path);
>> +	return ret;
>> +}
>> +
>> +static int find_normal_file_extent(struct btrfs_root *root, u64 ino);
>> +/*
>> + * Try to determine inode type if type not found.
>> + *
>> + * For found regular file extent, it must be FILE.
>> + * For found dir_item/index, it must be DIR.
>> + *
>> + * Return 0 if file type is confirmed and BTRFS_FT_* is stored into type.
>> + * Return <0 if file type is unknown.
>> + */
>> +static int guess_file_type_lowmem(struct btrfs_root *root, u64 ino, u8 *type)
>> +{
>> +	struct btrfs_key key;
>> +	struct btrfs_path *path = NULL;
>> +	bool is_dir = false;
>> +	bool is_file = false;
>> +	int ret;
>> +
>> +	if (find_normal_file_extent(root, ino)) {
> 
> Nice reusing the existing code.
> 
>> +		is_file = true;
>> +		goto out;
>> +	}
>> +
>> +	key.objectid = ino;
>> +	key.type = BTRFS_DIR_ITEM_KEY;
> 
> You could merge the code together, using just one loop instead of using
> two search separately for DIR_ITEM and DIR_INDEX.
> 
> Search for key (ino, DIR_INDEX, -1), as DIR_INDEX is larger than DIR_ITEM.
> 
> And search backward, not using btrfs_previous_item() but manually
> checking the type (and do prev_leaf() manually).
> It should save you several lines.
> 
I have thought up this way. I prefer to call btrfs_previous_item()
instead because it make code more readable and manually move path
pointer is pretty tricky.

Since your suggestion is more efficient, I will do it in next version.

Thanks,
Su

> Thanks,
> Qu
> 
>> +	key.offset = (u64)-1;
>> +
>> +	path = btrfs_alloc_path();
>> +	if (!path)
>> +		goto out;
>> +	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
>> +	if (ret < 0)
>> +		goto out;
>> +	/*
>> +	 * (u64)-1 may hit the hashed value in offset.
>> +	 */
>> +	if (!ret) {
>> +		is_dir = true;
>> +		goto out;
>> +	}
>> +
>> +	ret = btrfs_previous_item(root, path, ino, BTRFS_DIR_ITEM_KEY);> +	if (!ret) {
>> +		is_dir = true;
>> +		goto out;
>> +	}
>> +
>> +	key.type = BTRFS_DIR_INDEX_KEY;
>> +	key.offset = (u64)-1;
>> +
>> +	btrfs_release_path(path);
>> +	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
>> +	if (ret < 0)
>> +		goto out;
>> +	if (!ret) {
>> +		is_dir = true;
>> +		goto out;
>> +	}
>> +	ret = btrfs_previous_item(root, path, ino, BTRFS_DIR_INDEX_KEY);
>> +	if (!ret) {
>> +		is_dir = true;
>> +		goto out;
>> +	}
>> +out:
>> +	if (path)
>> +		btrfs_release_path(path);
>> +
>> +	if (is_dir) {
>> +		*type = BTRFS_FT_DIR;
>> +		ret = 0;
>> +	} else if (is_file) {
>> +		*type = BTRFS_FT_REG_FILE;
>> +		ret = 0;
>> +	} else {
>> +		ret = -ENOENT;
>> +	}
>> +	return ret;
>> +}
>> +
>>   /*
>>    * To determine the file name for nlink repair
>>    *
>>
> 

Re: [PATCH 02/15] btrfs-progs: lowmem check: find and guess inode filetype

[Qu Wenruo]

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



On 2018年01月26日 17:31, Su Yue wrote:
> 
> 
> On 01/26/2018 05:14 PM, Qu Wenruo wrote:
>>
>>
>> On 2018年01月26日 16:35, Su Yue wrote:
>>> Introduce find_file_type_lowmem() and guess_file_type_lowmem().
>>>
>>> find_file_type_lowmem() gets filetypes from inode_item, dir_item and
>>> dir_index. If two of three's filetype are same and valid, it thinks
>>> the value is correct.
>>>
>>> guess_file_type_lowmem() searches file_extent and dir_item/index then
>>> returns with filetype.
>>>
>>> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
>>> ---
>>>   cmds-check.c | 193
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>   1 file changed, 193 insertions(+)
>>>
>>> diff --git a/cmds-check.c b/cmds-check.c
>>> index e3505a7f9d6b..b200fdccf0e5 100644
>>> --- a/cmds-check.c
>>> +++ b/cmds-check.c
>>> @@ -3306,6 +3306,199 @@ static int find_file_type(struct inode_record
>>> *rec, u8 *type)
>>>       return -ENOENT;
>>>   }
>>>   +/*
>>> + * Fetch filetype from exited completed dir_item, dir_index and
>>> inode_item.
>>                            ^^^^^^
>>                            existing?
> 
> Oh.. a Typo.
>>> + * If two of tree items'filetype are same, we think the type is
>>> trusted.
>>> + *
>>> + * Return 0 if file type is found and BTRFS_FT_* is stored into type.
>>> + * Return <0 if file type is not found.
>>
>> This also includes extra error like -EIO from btrfs_search_slot().
>>
>>> + */
>>> +static int find_file_type_lowmem(struct btrfs_root *root, u64 ino,
>>> u8 *type)
>>> +{
>>> +    struct btrfs_key key;
>>> +    struct btrfs_path path;
>>> +    struct btrfs_path path2;
>>> +    struct btrfs_inode_ref *iref;
>>> +    struct btrfs_dir_item *dir_item;
>>> +    struct btrfs_dir_item *dir_index;
>>> +    struct extent_buffer *eb;
>>> +    u64 dir;
>>> +    u64 index;
>>> +    char namebuf[BTRFS_NAME_LEN] = {0};
>>> +    u32 namelen;
>>> +    u8 inode_filetype = BTRFS_FT_UNKNOWN;
>>> +    u8 dir_item_filetype;
>>> +    u8 dir_index_filetype;
>>> +    u8 true_file_type;
>>> +    int slot;
>>> +    int ret;
>>> +
>>> +    key.objectid = ino;
>>> +    key.type = BTRFS_INODE_ITEM_KEY;
>>> +    key.offset = 0;
>>> +
>>> +    btrfs_init_path(&path);
>>> +    ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
>>> +    if (ret < 0)
>>> +        goto out;
>>> +    if (!ret) {
>>> +        struct btrfs_inode_item *ii;
>>> +
>>> +        ii = btrfs_item_ptr(path.nodes[0], path.slots[0],
>>> +                    struct btrfs_inode_item);
>>> +        inode_filetype = imode_to_type(btrfs_inode_mode(path.nodes[0],
>>> +                                ii));
>>> +    }
>>> +
>>> +    key.objectid = ino;
>>> +    key.type = BTRFS_INODE_REF_KEY;
>>> +    key.offset = (u64)-1;
>>> +
>>> +    btrfs_release_path(&path);
>>> +    ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
>>> +    if (ret < 0)
>>> +        goto out;
>>> +    if (!ret) {
>>> +        ret = -EIO;
>>> +        goto out;
>>> +    }
>>> +
>>> +    btrfs_init_path(&path2);
>>> +next:
>>> +    btrfs_release_path(&path2)> +    ret = btrfs_previous_item(root,
>>> &path, ino, BTRFS_INODE_REF_KEY);
>>> +    if (ret) {
>>> +        ret = -ENOENT;
>>
>> For ret < 0 case, return value is overwritten.
>>
> Will fix it.
>>
>>> +        goto out;
>>> +    }
>>> +
>>> +    eb = path.nodes[0];
>>> +    slot = path.slots[0];
>>> +    btrfs_item_key_to_cpu(eb, &key, slot);
>>> +    dir = key.offset;
>>> +    iref = btrfs_item_ptr(eb, slot, struct btrfs_inode_ref);
>>> +    index = btrfs_inode_ref_index(eb, iref);
>>> +    namelen = btrfs_inode_ref_name_len(eb, iref);
>>> +    read_extent_buffer(eb, namebuf, (unsigned long)(iref + 1),
>>> namelen);
>>> +
>>> +    dir_index = btrfs_lookup_dir_index(NULL, root, &path2, dir,
>>> namebuf,
>>> +                       namelen, index, 0);
>>> +    if (!dir_index)
>>> +        goto next;
>>> +    dir_index_filetype = btrfs_dir_type(path2.nodes[0], dir_index);
>>> +    btrfs_release_path(&path2);
>>> +    if (dir_index_filetype == inode_filetype) {
>>> +        true_file_type = inode_filetype;
>>> +        goto found;
>>> +    }
>>> +
>>> +    dir_item = btrfs_lookup_dir_item(NULL, root, &path2, dir, namebuf,
>>> +                     namelen, 0);
>>> +    if (!dir_item)
>>> +        goto next;
>>> +    dir_item_filetype = btrfs_dir_type(path2.nodes[0], dir_item);
>>> +    btrfs_release_path(&path2);
>>> +    if (dir_item_filetype == inode_filetype) {
>>> +        true_file_type = inode_filetype;
>>> +        goto found;
>>> +    }
>>> +
>>> +    if (dir_index_filetype == dir_item_filetype) {
>>> +        true_file_type = dir_index_filetype;
>>> +        goto found;
>>> +    }
>>> +    goto next;
>>> +found:
>>> +    /* rare case, two of three items are both corrupted */
>>> +    if (true_file_type == BTRFS_FT_UNKNOWN ||
>>> +        true_file_type >= BTRFS_FT_MAX)
>>> +        goto next;
>>> +    *type = true_file_type;
>>> +    ret = 0;
>>> +out:
>>> +    btrfs_release_path(&path);
>>> +    return ret;
>>> +}
>>> +
>>> +static int find_normal_file_extent(struct btrfs_root *root, u64 ino);
>>> +/*
>>> + * Try to determine inode type if type not found.
>>> + *
>>> + * For found regular file extent, it must be FILE.
>>> + * For found dir_item/index, it must be DIR.
>>> + *
>>> + * Return 0 if file type is confirmed and BTRFS_FT_* is stored into
>>> type.
>>> + * Return <0 if file type is unknown.
>>> + */
>>> +static int guess_file_type_lowmem(struct btrfs_root *root, u64 ino,
>>> u8 *type)
>>> +{
>>> +    struct btrfs_key key;
>>> +    struct btrfs_path *path = NULL;
>>> +    bool is_dir = false;
>>> +    bool is_file = false;
>>> +    int ret;
>>> +
>>> +    if (find_normal_file_extent(root, ino)) {
>>
>> Nice reusing the existing code.
>>
>>> +        is_file = true;
>>> +        goto out;
>>> +    }
>>> +
>>> +    key.objectid = ino;
>>> +    key.type = BTRFS_DIR_ITEM_KEY;
>>
>> You could merge the code together, using just one loop instead of using
>> two search separately for DIR_ITEM and DIR_INDEX.
>>
>> Search for key (ino, DIR_INDEX, -1), as DIR_INDEX is larger than
>> DIR_ITEM.
>>
>> And search backward, not using btrfs_previous_item() but manually
>> checking the type (and do prev_leaf() manually).
>> It should save you several lines.
>>
> I have thought up this way. I prefer to call btrfs_previous_item()
> instead because it make code more readable and manually move path
> pointer is pretty tricky.

Double btrfs_previous_item() call would save you.

Although you need to pay extra attention after first
btrfs_previous_item() found no DIR_INDEX items.

In that case, you need to manually check the key type before call
btrfs_preivous_item() for DIR_ITEM.

Thanks,
Qu

> 
> Since your suggestion is more efficient, I will do it in next version.
> 
> Thanks,
> Su
> 
>> Thanks,
>> Qu
>>
>>> +    key.offset = (u64)-1;
>>> +
>>> +    path = btrfs_alloc_path();
>>> +    if (!path)
>>> +        goto out;
>>> +    ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
>>> +    if (ret < 0)
>>> +        goto out;
>>> +    /*
>>> +     * (u64)-1 may hit the hashed value in offset.
>>> +     */
>>> +    if (!ret) {
>>> +        is_dir = true;
>>> +        goto out;
>>> +    }
>>> +
>>> +    ret = btrfs_previous_item(root, path, ino, BTRFS_DIR_ITEM_KEY);>
>>> +    if (!ret) {
>>> +        is_dir = true;
>>> +        goto out;
>>> +    }
>>> +
>>> +    key.type = BTRFS_DIR_INDEX_KEY;
>>> +    key.offset = (u64)-1;
>>> +
>>> +    btrfs_release_path(path);
>>> +    ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
>>> +    if (ret < 0)
>>> +        goto out;
>>> +    if (!ret) {
>>> +        is_dir = true;
>>> +        goto out;
>>> +    }
>>> +    ret = btrfs_previous_item(root, path, ino, BTRFS_DIR_INDEX_KEY);
>>> +    if (!ret) {
>>> +        is_dir = true;
>>> +        goto out;
>>> +    }
>>> +out:
>>> +    if (path)
>>> +        btrfs_release_path(path);
>>> +
>>> +    if (is_dir) {
>>> +        *type = BTRFS_FT_DIR;
>>> +        ret = 0;
>>> +    } else if (is_file) {
>>> +        *type = BTRFS_FT_REG_FILE;
>>> +        ret = 0;
>>> +    } else {
>>> +        ret = -ENOENT;
>>> +    }
>>> +    return ret;
>>> +}
>>> +
>>>   /*
>>>    * To determine the file name for nlink repair
>>>    *
>>>
>>
> 
> 


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

[PATCH 03/15] btrfs-progs: lowmem check: find filetype in repair_inode_missing()

[Su Yue]

If parameter @filetype is 0, repair_inode_missing will find filetype
automatically.

And let it return -EEXIST instead of 0 if inode item is existed.

Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
 cmds-check.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/cmds-check.c b/cmds-check.c
index b200fdccf0e5..08a2662e603c 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -5161,6 +5161,9 @@ out:
 /*
  * Insert the missing inode item.
  *
+ * @filetype: if 0, find file type automatically.
+ *                  if find nothing, set inode as regular file.
+ *
  * Returns 0 means success.
  * Returns <0 means error.
  */
@@ -5176,6 +5179,19 @@ static int repair_inode_item_missing(struct btrfs_root *root, u64 ino,
 	key.type = BTRFS_INODE_ITEM_KEY;
 	key.offset = 0;
 
+	if (!filetype) {
+		ret = find_file_type_lowmem(root, ino, &filetype);
+		if (ret) {
+			ret = guess_file_type_lowmem(root, ino, &filetype);
+			if (ret) {
+				filetype = BTRFS_FT_REG_FILE;
+				error(
+		"can't get file type for inode %llu, using FILE as fallback",
+				      ino);
+			}
+		}
+	}
+
 	btrfs_init_path(&path);
 	trans = btrfs_start_transaction(root, 1);
 	if (IS_ERR(trans)) {
@@ -5184,7 +5200,9 @@ static int repair_inode_item_missing(struct btrfs_root *root, u64 ino,
 	}
 
 	ret = btrfs_search_slot(trans, root, &key, &path, 1, 1);
-	if (ret < 0 || !ret)
+	if (!ret)
+		ret = -EEXIST;
+	if (ret < 0)
 		goto fail;
 
 	/* insert inode item */
-- 
2.16.1

Re: [PATCH 03/15] btrfs-progs: lowmem check: find filetype in repair_inode_missing()

[Qu Wenruo]

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



On 2018年01月26日 16:35, Su Yue wrote:
> If parameter @filetype is 0, repair_inode_missing will find filetype
> automatically.
> 
> And let it return -EEXIST instead of 0 if inode item is existed.
> 
> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>  cmds-check.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/cmds-check.c b/cmds-check.c
> index b200fdccf0e5..08a2662e603c 100644
> --- a/cmds-check.c
> +++ b/cmds-check.c
> @@ -5161,6 +5161,9 @@ out:
>  /*
>   * Insert the missing inode item.
>   *
> + * @filetype: if 0, find file type automatically.
> + *                  if find nothing, set inode as regular file.
> + *
>   * Returns 0 means success.
>   * Returns <0 means error.
>   */
> @@ -5176,6 +5179,19 @@ static int repair_inode_item_missing(struct btrfs_root *root, u64 ino,
>  	key.type = BTRFS_INODE_ITEM_KEY;
>  	key.offset = 0;
>  
> +	if (!filetype) {
> +		ret = find_file_type_lowmem(root, ino, &filetype);
> +		if (ret) {
> +			ret = guess_file_type_lowmem(root, ino, &filetype);
> +			if (ret) {
> +				filetype = BTRFS_FT_REG_FILE;
> +				error(
> +		"can't get file type for inode %llu, using FILE as fallback",
> +				      ino);
> +			}
> +		}
> +	}
> +
>  	btrfs_init_path(&path);
>  	trans = btrfs_start_transaction(root, 1);
>  	if (IS_ERR(trans)) {
> @@ -5184,7 +5200,9 @@ static int repair_inode_item_missing(struct btrfs_root *root, u64 ino,
>  	}
>  
>  	ret = btrfs_search_slot(trans, root, &key, &path, 1, 1);
> -	if (ret < 0 || !ret)
> +	if (!ret)
> +		ret = -EEXIST;
> +	if (ret < 0)
>  		goto fail;
>  
>  	/* insert inode item */
> 


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

[PATCH 04/15] btrfs-progs: lowmem check: repair complex cases in repair_dir_item()

[Su Yue]

If inode item is missing or filetype is corrupted maybe, and filetypes
of dir_item and dir_index are corrupted too, lowmem repair may insert
wrong inode item and dir_item/index.

First, find and guess filetype of inode item, if failed, use
BTRFS_REG_FILE as fallback for insertion.
If filetype is not available, just delete current dir_item/index.

And repair_dir_item also calls repair_inode_item_mismatch() now.

Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
 cmds-check.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 78 insertions(+), 17 deletions(-)

diff --git a/cmds-check.c b/cmds-check.c
index 08a2662e603c..e33dd7db0048 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -2811,7 +2811,7 @@ static int walk_down_tree_v2(struct btrfs_trans_handle *trans,
 
 		ret = check_child_node(cur, path->slots[*level], next);
 		err |= ret;
-		if (ret < 0) 
+		if (ret < 0)
 			break;
 
 		if (btrfs_is_leaf(next))
@@ -5697,29 +5697,91 @@ static void print_dir_item_err(struct btrfs_root *root, struct btrfs_key *key,
 /*
  * Call repair_inode_item_missing and repair_ternary_lowmem to repair
  *
+ * @filetype:	filetype of the dir_item/index
+ *
  * Returns error after repair
  */
-static int repair_dir_item(struct btrfs_root *root, u64 dirid, u64 ino,
-			   u64 index, u8 filetype, char *namebuf, u32 name_len,
-			   int err)
+static int repair_dir_item(struct btrfs_root *root, struct btrfs_key *key,
+			   u64 ino, u64 index, u8 filetype, char *namebuf,
+			   u32 name_len, int err)
 {
 	int ret;
+	u64 dirid = key->objectid;
+	u8 true_filetype;
 
-	if (err & INODE_ITEM_MISSING) {
-		ret = repair_inode_item_missing(root, ino, filetype);
-		if (!ret)
-			err &= ~(INODE_ITEM_MISMATCH | INODE_ITEM_MISSING);
+	if (err & (INODE_ITEM_MISMATCH | INODE_ITEM_MISSING)) {
+		ret = find_file_type_lowmem(root, ino, &true_filetype);
+		if (ret) {
+			ret = guess_file_type_lowmem(root, ino,
+						     &true_filetype);
+			if (ret) {
+				true_filetype = BTRFS_FT_REG_FILE;
+				error(
+		"can't get file type for inode %llu, using FILE as fallback",
+				      ino);
+			}
+		}
 	}
 
-	if (err & ~(INODE_ITEM_MISMATCH | INODE_ITEM_MISSING)) {
-		ret = repair_ternary_lowmem(root, dirid, ino, index, namebuf,
-					    name_len, filetype, err);
+	/*
+	 * Case: the dir_item has corresponding inode_ref but
+	 * mismatch/missed inode_item and mismatch/missed another
+	 * dir_item/index.
+	 * repair_ternary_lowmem prefer to change another dir_item/index with
+	 * wrong filetype. So delete the item here.
+	 */
+	if (filetype != true_filetype &&
+	    (err & (DIR_ITEM_MISMATCH | DIR_ITEM_MISSING) ||
+	     err & (DIR_INDEX_MISMATCH | DIR_INDEX_MISSING))) {
+		struct btrfs_trans_handle *trans;
+		struct btrfs_path *path;
+
+		path = btrfs_alloc_path();
+		if (!path)
+			goto out;
+		trans = btrfs_start_transaction(root, 0);
+		ret = btrfs_search_slot(trans, root, key, path, -1, 1);
+		if (ret) {
+			btrfs_commit_transaction(trans, root);
+			btrfs_release_path(path);
+			goto out;
+		}
+		ret = btrfs_del_item(trans, root, path);
 		if (!ret) {
-			err &= ~(DIR_INDEX_MISMATCH | DIR_INDEX_MISSING);
-			err &= ~(DIR_ITEM_MISMATCH | DIR_ITEM_MISSING);
-			err &= ~(INODE_REF_MISSING);
+			err = 0;
+			printf(
+		"Deleted dir_item[%llu %u %llu]root %llu name %s filetype %u\n",
+			       key->objectid, key->type, key->offset,
+			       root->objectid, namebuf, filetype);
 		}
+		btrfs_commit_transaction(trans, root);
+		btrfs_release_path(path);
+		/*
+		 * Leave remains to check_inode_item() and check_inode_ref().
+		 */
+		goto out;
+	}
+
+	ret = repair_ternary_lowmem(root, dirid, ino, index, namebuf,
+				    name_len, true_filetype, err);
+	if (!ret) {
+		err &= ~(DIR_INDEX_MISMATCH | DIR_INDEX_MISSING);
+		err &= ~(DIR_ITEM_MISMATCH | DIR_ITEM_MISSING);
+		err &= ~(INODE_REF_MISSING);
+	}
+
+	if (err & INODE_ITEM_MISSING) {
+		ret = repair_inode_item_missing(root, ino, true_filetype);
+		if (!ret || ret == -EEXIST)
+			err &= ~INODE_ITEM_MISSING;
 	}
+
+	if (err & INODE_ITEM_MISMATCH) {
+		ret = repair_inode_item_mismatch(root, ino, true_filetype);
+		if (!ret)
+			err &= ~INODE_ITEM_MISMATCH;
+	}
+out:
 	return err;
 }
 
@@ -5958,9 +6020,8 @@ begin:
 next:
 
 		if (tmp_err && repair) {
-			ret = repair_dir_item(root, di_key->objectid,
-					      location.objectid, index,
-					      imode_to_type(mode), namebuf,
+			ret = repair_dir_item(root, di_key, location.objectid,
+					      index, filetype, namebuf,
 					      name_len, tmp_err);
 			if (ret != tmp_err) {
 				need_research = 1;
-- 
2.16.1

Re: [PATCH 04/15] btrfs-progs: lowmem check: repair complex cases in repair_dir_item()

[Qu Wenruo]

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



On 2018年01月26日 16:35, Su Yue wrote:
> If inode item is missing or filetype is corrupted maybe, and filetypes
> of dir_item and dir_index are corrupted too, lowmem repair may insert
> wrong inode item and dir_item/index.
> 
> First, find and guess filetype of inode item, if failed, use
> BTRFS_REG_FILE as fallback for insertion.
> If filetype is not available, just delete current dir_item/index.
> 
> And repair_dir_item also calls repair_inode_item_mismatch() now.
> 
> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>

Looks good.

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu
> ---
>  cmds-check.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 78 insertions(+), 17 deletions(-)
> 
> diff --git a/cmds-check.c b/cmds-check.c
> index 08a2662e603c..e33dd7db0048 100644
> --- a/cmds-check.c
> +++ b/cmds-check.c
> @@ -2811,7 +2811,7 @@ static int walk_down_tree_v2(struct btrfs_trans_handle *trans,
>  
>  		ret = check_child_node(cur, path->slots[*level], next);
>  		err |= ret;
> -		if (ret < 0) 
> +		if (ret < 0)
>  			break;
>  
>  		if (btrfs_is_leaf(next))
> @@ -5697,29 +5697,91 @@ static void print_dir_item_err(struct btrfs_root *root, struct btrfs_key *key,
>  /*
>   * Call repair_inode_item_missing and repair_ternary_lowmem to repair
>   *
> + * @filetype:	filetype of the dir_item/index
> + *
>   * Returns error after repair
>   */
> -static int repair_dir_item(struct btrfs_root *root, u64 dirid, u64 ino,
> -			   u64 index, u8 filetype, char *namebuf, u32 name_len,
> -			   int err)
> +static int repair_dir_item(struct btrfs_root *root, struct btrfs_key *key,
> +			   u64 ino, u64 index, u8 filetype, char *namebuf,
> +			   u32 name_len, int err)
>  {
>  	int ret;
> +	u64 dirid = key->objectid;
> +	u8 true_filetype;
>  
> -	if (err & INODE_ITEM_MISSING) {
> -		ret = repair_inode_item_missing(root, ino, filetype);
> -		if (!ret)
> -			err &= ~(INODE_ITEM_MISMATCH | INODE_ITEM_MISSING);
> +	if (err & (INODE_ITEM_MISMATCH | INODE_ITEM_MISSING)) {
> +		ret = find_file_type_lowmem(root, ino, &true_filetype);
> +		if (ret) {
> +			ret = guess_file_type_lowmem(root, ino,
> +						     &true_filetype);
> +			if (ret) {
> +				true_filetype = BTRFS_FT_REG_FILE;
> +				error(
> +		"can't get file type for inode %llu, using FILE as fallback",
> +				      ino);
> +			}
> +		}
>  	}
>  
> -	if (err & ~(INODE_ITEM_MISMATCH | INODE_ITEM_MISSING)) {
> -		ret = repair_ternary_lowmem(root, dirid, ino, index, namebuf,
> -					    name_len, filetype, err);
> +	/*
> +	 * Case: the dir_item has corresponding inode_ref but
> +	 * mismatch/missed inode_item and mismatch/missed another
> +	 * dir_item/index.
> +	 * repair_ternary_lowmem prefer to change another dir_item/index with
> +	 * wrong filetype. So delete the item here.
> +	 */
> +	if (filetype != true_filetype &&
> +	    (err & (DIR_ITEM_MISMATCH | DIR_ITEM_MISSING) ||
> +	     err & (DIR_INDEX_MISMATCH | DIR_INDEX_MISSING))) {
> +		struct btrfs_trans_handle *trans;
> +		struct btrfs_path *path;
> +
> +		path = btrfs_alloc_path();
> +		if (!path)
> +			goto out;
> +		trans = btrfs_start_transaction(root, 0);
> +		ret = btrfs_search_slot(trans, root, key, path, -1, 1);
> +		if (ret) {
> +			btrfs_commit_transaction(trans, root);
> +			btrfs_release_path(path);
> +			goto out;
> +		}
> +		ret = btrfs_del_item(trans, root, path);
>  		if (!ret) {
> -			err &= ~(DIR_INDEX_MISMATCH | DIR_INDEX_MISSING);
> -			err &= ~(DIR_ITEM_MISMATCH | DIR_ITEM_MISSING);
> -			err &= ~(INODE_REF_MISSING);
> +			err = 0;
> +			printf(
> +		"Deleted dir_item[%llu %u %llu]root %llu name %s filetype %u\n",
> +			       key->objectid, key->type, key->offset,
> +			       root->objectid, namebuf, filetype);
>  		}
> +		btrfs_commit_transaction(trans, root);
> +		btrfs_release_path(path);
> +		/*
> +		 * Leave remains to check_inode_item() and check_inode_ref().
> +		 */
> +		goto out;
> +	}
> +
> +	ret = repair_ternary_lowmem(root, dirid, ino, index, namebuf,
> +				    name_len, true_filetype, err);
> +	if (!ret) {
> +		err &= ~(DIR_INDEX_MISMATCH | DIR_INDEX_MISSING);
> +		err &= ~(DIR_ITEM_MISMATCH | DIR_ITEM_MISSING);
> +		err &= ~(INODE_REF_MISSING);
> +	}
> +
> +	if (err & INODE_ITEM_MISSING) {
> +		ret = repair_inode_item_missing(root, ino, true_filetype);
> +		if (!ret || ret == -EEXIST)
> +			err &= ~INODE_ITEM_MISSING;
>  	}
> +
> +	if (err & INODE_ITEM_MISMATCH) {
> +		ret = repair_inode_item_mismatch(root, ino, true_filetype);
> +		if (!ret)
> +			err &= ~INODE_ITEM_MISMATCH;
> +	}
> +out:
>  	return err;
>  }
>  
> @@ -5958,9 +6020,8 @@ begin:
>  next:
>  
>  		if (tmp_err && repair) {
> -			ret = repair_dir_item(root, di_key->objectid,
> -					      location.objectid, index,
> -					      imode_to_type(mode), namebuf,
> +			ret = repair_dir_item(root, di_key, location.objectid,
> +					      index, filetype, namebuf,
>  					      name_len, tmp_err);
>  			if (ret != tmp_err) {
>  				need_research = 1;
> 


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

[PATCH 05/15] btrfs-progs: lowmem check: let check_dir_item() continue if find wrong inode_item

[Su Yue]

check_dir_item can handle missed/mismatched inode item well.
Let it continue to check corresponding dir_item/index and inode_ref.

Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
 cmds-check.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/cmds-check.c b/cmds-check.c
index e33dd7db0048..eb65a18fe64b 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -5984,15 +5984,12 @@ begin:
 		ret = btrfs_search_slot(NULL, root, &location, path, 0, 0);
 		if (ret) {
 			tmp_err |= INODE_ITEM_MISSING;
-			goto next;
-		}
-
-		ii = btrfs_item_ptr(path->nodes[0], path->slots[0],
-				    struct btrfs_inode_item);
-		mode = btrfs_inode_mode(path->nodes[0], ii);
-		if (imode_to_type(mode) != filetype) {
-			tmp_err |= INODE_ITEM_MISMATCH;
-			goto next;
+		} else {
+			ii = btrfs_item_ptr(path->nodes[0], path->slots[0],
+					    struct btrfs_inode_item);
+			mode = btrfs_inode_mode(path->nodes[0], ii);
+			if (imode_to_type(mode) != filetype)
+				tmp_err |= INODE_ITEM_MISMATCH;
 		}
 
 		/* Check relative INODE_REF/INODE_EXTREF */
-- 
2.16.1

Re: [PATCH 05/15] btrfs-progs: lowmem check: let check_dir_item() continue if find wrong inode_item

[Qu Wenruo]

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



On 2018年01月26日 16:35, Su Yue wrote:
> check_dir_item can handle missed/mismatched inode item well.
> Let it continue to check corresponding dir_item/index and inode_ref.
> 
> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>  cmds-check.c | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/cmds-check.c b/cmds-check.c
> index e33dd7db0048..eb65a18fe64b 100644
> --- a/cmds-check.c
> +++ b/cmds-check.c
> @@ -5984,15 +5984,12 @@ begin:
>  		ret = btrfs_search_slot(NULL, root, &location, path, 0, 0);
>  		if (ret) {
>  			tmp_err |= INODE_ITEM_MISSING;
> -			goto next;
> -		}
> -
> -		ii = btrfs_item_ptr(path->nodes[0], path->slots[0],
> -				    struct btrfs_inode_item);
> -		mode = btrfs_inode_mode(path->nodes[0], ii);
> -		if (imode_to_type(mode) != filetype) {
> -			tmp_err |= INODE_ITEM_MISMATCH;
> -			goto next;
> +		} else {
> +			ii = btrfs_item_ptr(path->nodes[0], path->slots[0],
> +					    struct btrfs_inode_item);
> +			mode = btrfs_inode_mode(path->nodes[0], ii);
> +			if (imode_to_type(mode) != filetype)
> +				tmp_err |= INODE_ITEM_MISMATCH;
>  		}
>  
>  		/* Check relative INODE_REF/INODE_EXTREF */
> 


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

[PATCH 06/15] btrfs-progs: lowmem check: let check_dir_item() return if repaired

[Su Yue]

If repair_dir_item deleted the item, goto last checked then returns
instead of searching di_key again then returns -ENOENT.

Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
 cmds-check.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/cmds-check.c b/cmds-check.c
index eb65a18fe64b..4ce6139b3ab1 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -5931,7 +5931,7 @@ begin:
 				path->slots[0]--;
 		}
 		if (ret)
-			goto out;
+			return err;
 	}
 
 	node = path->nodes[0];
@@ -6040,7 +6040,7 @@ next:
 			break;
 		}
 	}
-out:
+
 	/* research path */
 	btrfs_release_path(path);
 	ret = btrfs_search_slot(NULL, root, di_key, path, 0, 0);
-- 
2.16.1

Re: [PATCH 06/15] btrfs-progs: lowmem check: let check_dir_item() return if repaired

[Qu Wenruo]

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



On 2018年01月26日 16:35, Su Yue wrote:
> If repair_dir_item deleted the item, goto last checked then returns
> instead of searching di_key again then returns -ENOENT.
> 
> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
> ---
>  cmds-check.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/cmds-check.c b/cmds-check.c
> index eb65a18fe64b..4ce6139b3ab1 100644
> --- a/cmds-check.c
> +++ b/cmds-check.c
> @@ -5931,7 +5931,7 @@ begin:
>  				path->slots[0]--;

Well, not a problem of this patch, but I found previous lines have problem:
------
		ret = btrfs_search_slot(NULL, root, di_key, path, 0, 0);
		/* the item was deleted, let path point the last checked item */
		if (ret > 0) {
			if (path->slots[0] == 0)
				btrfs_prev_leaf(root, path);
			else
				path->slots[0]--;
		}
------

Here return value of btrfs_prev_leaf() is ignored, which can return
error or >1.

Thanks,
Qu

>  		}
>  		if (ret)
> -			goto out;
> +			return err;>  	}
>  
>  	node = path->nodes[0];
> @@ -6040,7 +6040,7 @@ next:
>  			break;
>  		}
>  	}
> -out:
> +
>  	/* research path */
>  	btrfs_release_path(path);
>  	ret = btrfs_search_slot(NULL, root, di_key, path, 0, 0);
> 


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

[PATCH 07/15] btrfs-progs: lowmem check: find_dir_item by di_key in check_dir_item()

[Su Yue]

In check_dir_item, we are going to search corresponding
dir_item/index.
@key shouldn't be used here. It should be @di_key.

Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
 cmds-check.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmds-check.c b/cmds-check.c
index 4ce6139b3ab1..caac71a67472 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -6001,7 +6001,7 @@ begin:
 
 		/* check relative INDEX/ITEM */
 		key.objectid = di_key->objectid;
-		if (key.type == BTRFS_DIR_ITEM_KEY) {
+		if (di_key->type == BTRFS_DIR_ITEM_KEY) {
 			key.type = BTRFS_DIR_INDEX_KEY;
 			key.offset = index;
 		} else {
-- 
2.16.1

Re: [PATCH 07/15] btrfs-progs: lowmem check: find_dir_item by di_key in check_dir_item()

[Qu Wenruo]

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



On 2018年01月26日 16:35, Su Yue wrote:
> In check_dir_item, we are going to search corresponding
> dir_item/index.
> @key shouldn't be used here. It should be @di_key.
> 
> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>  cmds-check.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/cmds-check.c b/cmds-check.c
> index 4ce6139b3ab1..caac71a67472 100644
> --- a/cmds-check.c
> +++ b/cmds-check.c
> @@ -6001,7 +6001,7 @@ begin:
>  
>  		/* check relative INDEX/ITEM */
>  		key.objectid = di_key->objectid;
> -		if (key.type == BTRFS_DIR_ITEM_KEY) {
> +		if (di_key->type == BTRFS_DIR_ITEM_KEY) {
>  			key.type = BTRFS_DIR_INDEX_KEY;
>  			key.offset = index;
>  		} else {
> 


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

[PATCH 08/15] btrfs-progs: lowmem check: call get_dir_isize() after repair

[Su Yue]

Since lowmem repair may change size of inode item, introduce
get_dir_isize() to fetch isize after traversing items of inode.

Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
 cmds-check.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/cmds-check.c b/cmds-check.c
index caac71a67472..e57eea4e61c9 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -5868,6 +5868,35 @@ out:
 	return ret;
 }
 
+static int get_dir_isize(struct btrfs_root *root, u64 ino, u64 *size_ret)
+{
+	struct btrfs_inode_item *ii;
+	struct btrfs_key key;
+	struct btrfs_path path;
+	int ret;
+
+	key.objectid = ino;
+	key.type = BTRFS_INODE_ITEM_KEY;
+	key.offset = 0;
+
+	btrfs_init_path(&path);
+	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
+	if (ret > 0)
+		ret = -ENOENT;
+	if (ret)
+		goto out;
+
+	ii = btrfs_item_ptr(path.nodes[0], path.slots[0],
+			    struct btrfs_inode_item);
+	*size_ret = btrfs_inode_size(path.nodes[0], ii);
+	ret = 0;
+out:
+	if (ret)
+		error("failed to get isize of inode %llu root %llu",
+		      ino, root->root_key.objectid);
+	return ret;
+}
+
 /*
  * Traverse the given DIR_ITEM/DIR_INDEX and check related INODE_ITEM and
  * call find_inode_ref() to check related INODE_REF/INODE_EXTREF.
@@ -6591,6 +6620,7 @@ out:
 		if (repair && (err & DIR_COUNT_AGAIN)) {
 			err &= ~DIR_COUNT_AGAIN;
 			count_dir_isize(root, inode_id, &size);
+			get_dir_isize(root, inode_id, &isize);
 		}
 
 		if ((nlink != 1 || refs != 1) && repair) {
-- 
2.16.1

[PATCH 09/15] btrfs-progs: lowmem check: change logic of leaf process if repair

[Su Yue]

In lowmem check without repair, process_one_leaf_v2() will process one
entire leaf and inner check_inode_item() leads path point next
leaf.
In the beginning, process_one_leaf_v2() will let path point fist inode
item or first position where inode id changed.

However, in lowmem repair, process_one_leaf_v2() will be interrupted
to process one leaf because repair will CoW the leaf. Then some items
unprocessed is skipped.
Since repair may also delete some items, we can't use tricks like
record last checked key.

So, only for lowmem repair:
1. check_inode_item is responsible for handle case missing inode item.
2. process_one_leaf_v2() do not modify path manually, and check_inode()
   promise that @path points last checked item.
   Only when something are fixed, process_one_leaf_v2() will continue
   to check in next round.

Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
 cmds-check.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 59 insertions(+), 9 deletions(-)

diff --git a/cmds-check.c b/cmds-check.c
index e57eea4e61c9..ae0a9e146399 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -2007,6 +2007,8 @@ static int process_one_leaf_v2(struct btrfs_root *root, struct btrfs_path *path,
 
 	cur_bytenr = cur->start;
 
+	if (repair)
+		goto again;
 	/* skip to first inode item or the first inode number change */
 	nritems = btrfs_header_nritems(cur);
 	for (i = 0; i < nritems; i++) {
@@ -2033,9 +2035,12 @@ again:
 		goto out;
 
 	/* still have inode items in thie leaf */
-	if (cur->start == cur_bytenr)
+	if (cur->start == cur_bytenr) {
+		ret = btrfs_next_item(root, path);
+		if (ret > 0)
+			goto out;
 		goto again;
-
+	}
 	/*
 	 * we have switched to another leaf, above nodes may
 	 * have changed, here walk down the path, if a node
@@ -5721,6 +5726,8 @@ static int repair_dir_item(struct btrfs_root *root, struct btrfs_key *key,
 				      ino);
 			}
 		}
+	} else {
+		true_filetype = filetype;
 	}
 
 	/*
@@ -6489,6 +6496,45 @@ out:
 	return ret;
 }
 
+/*
+ * Try insert new inode item frist.
+ * If failed, jump to next inode item.
+ */
+static int handle_inode_item_missing(struct btrfs_root *root,
+				     struct btrfs_path *path)
+{
+	struct btrfs_key key;
+	int ret;
+
+	btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
+
+	ret = repair_inode_item_missing(root, key.objectid, 0);
+	if (!ret) {
+		btrfs_release_path(path);
+		ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
+		if (ret)
+			goto next_inode;
+		else
+			goto out;
+	}
+
+next_inode:
+	error("inode item[%llu] is missing, skip to check next inode",
+	      key.objectid);
+	while (1) {
+		ret = btrfs_next_item(root, path);
+		if (ret > 0)
+			goto out;
+		btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
+		if (key.type == BTRFS_INODE_ITEM_KEY) {
+			ret = 0;
+			break;
+		}
+	}
+out:
+	return ret;
+}
+
 /*
  * Check INODE_ITEM and related ITEMs (the same inode number)
  * 1. check link count
@@ -6536,6 +6582,13 @@ static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path,
 			err |= LAST_ITEM;
 		return err;
 	}
+	if (key.type != BTRFS_INODE_ITEM_KEY && repair) {
+		ret = handle_inode_item_missing(root, path);
+		if (ret > 0)
+			err |= LAST_ITEM;
+		if (ret)
+			return err;
+	}
 
 	ii = btrfs_item_ptr(node, slot, struct btrfs_inode_item);
 	isize = btrfs_inode_size(node, ii);
@@ -6561,7 +6614,6 @@ static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path,
 		btrfs_item_key_to_cpu(node, &key, slot);
 		if (key.objectid != inode_id)
 			goto out;
-
 		switch (key.type) {
 		case BTRFS_INODE_REF_KEY:
 			ret = check_inode_ref(root, &key, path, namebuf,
@@ -6608,12 +6660,10 @@ static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path,
 	}
 
 out:
-	if (err & LAST_ITEM) {
-		btrfs_release_path(path);
-		ret = btrfs_search_slot(NULL, root, &last_key, path, 0, 0);
-		if (ret)
-			return err;
-	}
+	btrfs_release_path(path);
+	ret = btrfs_search_slot(NULL, root, &last_key, path, 0, 0);
+	if (ret)
+		return err;
 
 	/* verify INODE_ITEM nlink/isize/nbytes */
 	if (dir) {
-- 
2.16.1

Re: [PATCH 09/15] btrfs-progs: lowmem check: change logic of leaf process if repair

[Qu Wenruo]

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



On 2018年01月26日 16:35, Su Yue wrote:
> In lowmem check without repair, process_one_leaf_v2() will process one
> entire leaf and inner check_inode_item() leads path point next
> leaf.
> In the beginning, process_one_leaf_v2() will let path point fist inode
> item or first position where inode id changed.
> 
> However, in lowmem repair, process_one_leaf_v2() will be interrupted
> to process one leaf because repair will CoW the leaf. Then some items
> unprocessed is skipped.
> Since repair may also delete some items, we can't use tricks like
> record last checked key.
> 
> So, only for lowmem repair:
> 1. check_inode_item is responsible for handle case missing inode item.

I think the idea to use inode item as the indicator is a good idea.
And since only check_inode_item() can delete inode item, let it to
handle the path is reasonable.

> 2. process_one_leaf_v2() do not modify path manually, and check_inode()
>    promise that @path points last checked item.
>    Only when something are fixed, process_one_leaf_v2() will continue
>    to check in next round.
> 
> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
> ---
>  cmds-check.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 59 insertions(+), 9 deletions(-)
> 
> diff --git a/cmds-check.c b/cmds-check.c
> index e57eea4e61c9..ae0a9e146399 100644
> --- a/cmds-check.c
> +++ b/cmds-check.c
> @@ -2007,6 +2007,8 @@ static int process_one_leaf_v2(struct btrfs_root *root, struct btrfs_path *path,
>  
>  	cur_bytenr = cur->start;
>  
> +	if (repair)
> +		goto again;
>  	/* skip to first inode item or the first inode number change */
>  	nritems = btrfs_header_nritems(cur);
>  	for (i = 0; i < nritems; i++) {
> @@ -2033,9 +2035,12 @@ again:
>  		goto out;
>  
>  	/* still have inode items in thie leaf */
> -	if (cur->start == cur_bytenr)
> +	if (cur->start == cur_bytenr) {
> +		ret = btrfs_next_item(root, path);
> +		if (ret > 0)
> +			goto out;
>  		goto again;
> -
> +	}
>  	/*
>  	 * we have switched to another leaf, above nodes may
>  	 * have changed, here walk down the path, if a node
> @@ -5721,6 +5726,8 @@ static int repair_dir_item(struct btrfs_root *root, struct btrfs_key *key,
>  				      ino);
>  			}
>  		}
> +	} else {
> +		true_filetype = filetype;

This modification doesn't seems related to this patch.

Maybe it's better to move it 4th patch?

>  	}
>  
>  	/*
> @@ -6489,6 +6496,45 @@ out:
>  	return ret;
>  }
>  
> +/*
> + * Try insert new inode item frist.
> + * If failed, jump to next inode item.
> + */
> +static int handle_inode_item_missing(struct btrfs_root *root,
> +				     struct btrfs_path *path)
> +{
> +	struct btrfs_key key;
> +	int ret;
> +
> +	btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
> +
> +	ret = repair_inode_item_missing(root, key.objectid, 0);
> +	if (!ret) {
> +		btrfs_release_path(path);
> +		ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
> +		if (ret)
> +			goto next_inode;
> +		else
> +			goto out;
> +	}
> +
> +next_inode:
> +	error("inode item[%llu] is missing, skip to check next inode",
> +	      key.objectid);
> +	while (1) {
> +		ret = btrfs_next_item(root, path);
> +		if (ret > 0)
> +			goto out;

ret < 0 case is not handled.

> +		btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
> +		if (key.type == BTRFS_INODE_ITEM_KEY) {
> +			ret = 0;
> +			break;
> +		}
> +	}
> +out:
> +	return ret;
> +}
> +
>  /*
>   * Check INODE_ITEM and related ITEMs (the same inode number)
>   * 1. check link count
> @@ -6536,6 +6582,13 @@ static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path,
>  			err |= LAST_ITEM;
>  		return err;
>  	}
> +	if (key.type != BTRFS_INODE_ITEM_KEY && repair) {
> +		ret = handle_inode_item_missing(root, path);
> +		if (ret > 0)
> +			err |= LAST_ITEM;
> +		if (ret)
> +			return err;
> +	}
>  
>  	ii = btrfs_item_ptr(node, slot, struct btrfs_inode_item);
>  	isize = btrfs_inode_size(node, ii);
> @@ -6561,7 +6614,6 @@ static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path,
>  		btrfs_item_key_to_cpu(node, &key, slot);
>  		if (key.objectid != inode_id)
>  			goto out;
> -
>  		switch (key.type) {
>  		case BTRFS_INODE_REF_KEY:
>  			ret = check_inode_ref(root, &key, path, namebuf,
> @@ -6608,12 +6660,10 @@ static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path,
>  	}
>  
>  out:
> -	if (err & LAST_ITEM) {
> -		btrfs_release_path(path);
> -		ret = btrfs_search_slot(NULL, root, &last_key, path, 0, 0);
> -		if (ret)
> -			return err;
> -	}
> +	btrfs_release_path(path);
> +	ret = btrfs_search_slot(NULL, root, &last_key, path, 0, 0);
> +	if (ret)
> +		return err;

Why the LAST_ITEM bit is ignored now?

Thanks,
Qu

>  
>  	/* verify INODE_ITEM nlink/isize/nbytes */
>  	if (dir) {
> 


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

Re: [PATCH 09/15] btrfs-progs: lowmem check: change logic of leaf process if repair

[Su Yue]

On 01/26/2018 06:01 PM, Qu Wenruo wrote:
> 
> 
> On 2018年01月26日 16:35, Su Yue wrote:
>> In lowmem check without repair, process_one_leaf_v2() will process one
>> entire leaf and inner check_inode_item() leads path point next
>> leaf.
>> In the beginning, process_one_leaf_v2() will let path point fist inode
>> item or first position where inode id changed.
>>
>> However, in lowmem repair, process_one_leaf_v2() will be interrupted
>> to process one leaf because repair will CoW the leaf. Then some items
>> unprocessed is skipped.
>> Since repair may also delete some items, we can't use tricks like
>> record last checked key.
>>
>> So, only for lowmem repair:
>> 1. check_inode_item is responsible for handle case missing inode item.
> 
> I think the idea to use inode item as the indicator is a good idea.
> And since only check_inode_item() can delete inode item, let it to
> handle the path is reasonable.
> 
>> 2. process_one_leaf_v2() do not modify path manually, and check_inode()
>>     promise that @path points last checked item.
>>     Only when something are fixed, process_one_leaf_v2() will continue
>>     to check in next round.
>>
>> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
>> ---
>>   cmds-check.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++--------
>>   1 file changed, 59 insertions(+), 9 deletions(-)
>>
>> diff --git a/cmds-check.c b/cmds-check.c
>> index e57eea4e61c9..ae0a9e146399 100644
>> --- a/cmds-check.c
>> +++ b/cmds-check.c
>> @@ -2007,6 +2007,8 @@ static int process_one_leaf_v2(struct btrfs_root *root, struct btrfs_path *path,
>>   
>>   	cur_bytenr = cur->start;
>>   
>> +	if (repair)
>> +		goto again;
>>   	/* skip to first inode item or the first inode number change */
>>   	nritems = btrfs_header_nritems(cur);
>>   	for (i = 0; i < nritems; i++) {
>> @@ -2033,9 +2035,12 @@ again:
>>   		goto out;
>>   
>>   	/* still have inode items in thie leaf */
>> -	if (cur->start == cur_bytenr)
>> +	if (cur->start == cur_bytenr) {
>> +		ret = btrfs_next_item(root, path);
>> +		if (ret > 0)
>> +			goto out;
>>   		goto again;
>> -
>> +	}
>>   	/*
>>   	 * we have switched to another leaf, above nodes may
>>   	 * have changed, here walk down the path, if a node
>> @@ -5721,6 +5726,8 @@ static int repair_dir_item(struct btrfs_root *root, struct btrfs_key *key,
>>   				      ino);
>>   			}
>>   		}
>> +	} else {
>> +		true_filetype = filetype;
> 
> This modification doesn't seems related to this patch.
> 
> Maybe it's better to move it 4th patch?
> 
Yep.
>>   	}
>>   
>>   	/*
>> @@ -6489,6 +6496,45 @@ out:
>>   	return ret;
>>   }
>>   
>> +/*
>> + * Try insert new inode item frist.
>> + * If failed, jump to next inode item.
>> + */
>> +static int handle_inode_item_missing(struct btrfs_root *root,
>> +				     struct btrfs_path *path)
>> +{
>> +	struct btrfs_key key;
>> +	int ret;
>> +
>> +	btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
>> +
>> +	ret = repair_inode_item_missing(root, key.objectid, 0);
>> +	if (!ret) {
>> +		btrfs_release_path(path);
>> +		ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
>> +		if (ret)
>> +			goto next_inode;
>> +		else
>> +			goto out;
>> +	}
>> +
>> +next_inode:
>> +	error("inode item[%llu] is missing, skip to check next inode",
>> +	      key.objectid);
>> +	while (1) {
>> +		ret = btrfs_next_item(root, path);
>> +		if (ret > 0)
>> +			goto out;
> 
> ret < 0 case is not handled.
> 
>> +		btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
>> +		if (key.type == BTRFS_INODE_ITEM_KEY) {
>> +			ret = 0;
>> +			break;
>> +		}
>> +	}
>> +out:
>> +	return ret;
>> +}
>> +
>>   /*
>>    * Check INODE_ITEM and related ITEMs (the same inode number)
>>    * 1. check link count
>> @@ -6536,6 +6582,13 @@ static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path,
>>   			err |= LAST_ITEM;
>>   		return err;
>>   	}
>> +	if (key.type != BTRFS_INODE_ITEM_KEY && repair) {
>> +		ret = handle_inode_item_missing(root, path);
>> +		if (ret > 0)
>> +			err |= LAST_ITEM;
>> +		if (ret)
>> +			return err;
>> +	}
>>   
>>   	ii = btrfs_item_ptr(node, slot, struct btrfs_inode_item);
>>   	isize = btrfs_inode_size(node, ii);
>> @@ -6561,7 +6614,6 @@ static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path,
>>   		btrfs_item_key_to_cpu(node, &key, slot);
>>   		if (key.objectid != inode_id)
>>   			goto out;
>> -
>>   		switch (key.type) {
>>   		case BTRFS_INODE_REF_KEY:
>>   			ret = check_inode_ref(root, &key, path, namebuf,
>> @@ -6608,12 +6660,10 @@ static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path,
>>   	}
>>   
>>   out:
>> -	if (err & LAST_ITEM) {
>> -		btrfs_release_path(path);
>> -		ret = btrfs_search_slot(NULL, root, &last_key, path, 0, 0);
>> -		if (ret)
>> -			return err;
>> -	}
>> +	btrfs_release_path(path);
>> +	ret = btrfs_search_slot(NULL, root, &last_key, path, 0, 0);
>> +	if (ret)
>> +		return err;
> 
> Why the LAST_ITEM bit is ignored now?
> 
My bad. Wrong commit message. This patch influnces lowmem check too.
Here check_inode_item() points to last checked item.
Above process_one_leaf_v2() calls btrfs_next_item() but needs more
strict condition. Will fix it.

Thank,
Su

> Thanks,
> Qu
> 
>>   
>>   	/* verify INODE_ITEM nlink/isize/nbytes */
>>   	if (dir) {
>>
> 

[PATCH 10/15] btrfs-progs: check: clear I_ERR_FILE_EXTENT_DISCOUNT after repair

[Su Yue]

Original check forgets to clear bit I_ERR_FILE_EXTENT_DISCOUNT
in rec->errors after repair.

So just do it.

Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
 cmds-check.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/cmds-check.c b/cmds-check.c
index ae0a9e146399..d8d9a3227c06 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -3978,6 +3978,7 @@ static int repair_inode_discount_extent(struct btrfs_trans_handle *trans,
 		if (ret < 0)
 			goto out;
 	}
+	rec->errors &= ~I_ERR_FILE_EXTENT_DISCOUNT;
 	printf("Fixed discount file extents for inode: %llu in root: %llu\n",
 	       rec->ino, root->objectid);
 out:
-- 
2.16.1

Re: [PATCH 10/15] btrfs-progs: check: clear I_ERR_FILE_EXTENT_DISCOUNT after repair

[Qu Wenruo]

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



On 2018年01月26日 16:35, Su Yue wrote:
> Original check forgets to clear bit I_ERR_FILE_EXTENT_DISCOUNT
> in rec->errors after repair.
> 
> So just do it.
> 
> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

And a test case for this would help us to avoid further regression.

Thanks,
Qu

> ---
>  cmds-check.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/cmds-check.c b/cmds-check.c
> index ae0a9e146399..d8d9a3227c06 100644
> --- a/cmds-check.c
> +++ b/cmds-check.c
> @@ -3978,6 +3978,7 @@ static int repair_inode_discount_extent(struct btrfs_trans_handle *trans,
>  		if (ret < 0)
>  			goto out;
>  	}
> +	rec->errors &= ~I_ERR_FILE_EXTENT_DISCOUNT;
>  	printf("Fixed discount file extents for inode: %llu in root: %llu\n",
>  	       rec->ino, root->objectid);
>  out:
> 


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

[PATCH 11/15] btrfs-progs: check: modify indoe_rec and backref after repair

[Su Yue]

repair_inode_backrefs() forgets to modify backrefs and inode records
after repair them.

Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
 cmds-check.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/cmds-check.c b/cmds-check.c
index d8d9a3227c06..b23a4493b12b 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -3192,6 +3192,7 @@ static int repair_inode_backrefs(struct btrfs_root *root,
 				ret = create_inode_item(root, rec, 1);
 				if (ret)
 					break;
+				rec->found_inode_item = 1;
 				repaired++;
 			}
 		}
@@ -3268,6 +3269,8 @@ static int repair_inode_backrefs(struct btrfs_root *root,
 						    backref->index);
 			BUG_ON(ret);
 			btrfs_commit_transaction(trans, root);
+			backref->found_dir_index = 1;
+			backref->found_dir_item = 1;
 			repaired++;
 		}
 
@@ -3279,6 +3282,7 @@ static int repair_inode_backrefs(struct btrfs_root *root,
 			ret = create_inode_item(root, rec, 0);
 			if (ret)
 				break;
+			rec->found_inode_item = 1;
 			repaired++;
 		}
 
-- 
2.16.1

[PATCH 12/15] btrfs-progs: check: increase counter error in check_inode_recs()

[Su Yue]

Counter @error decides return values of check_inode_recs().

Previously, @error won't be incremented even repair_inode_recs() failed.
It causes 'btrfs check --repair' prints some error information but
returns 0.

So, if root dir is missing and repair is disabled, @error should be
incremented.
And after repair_inode_recs(), increase @error if any errors in inodes
and backrefs.

Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
 cmds-check.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/cmds-check.c b/cmds-check.c
index b23a4493b12b..a83f0a92f48b 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -4137,6 +4137,7 @@ static int check_inode_recs(struct btrfs_root *root,
 			return -EAGAIN;
 		}
 
+		error++;
 		fprintf(stderr, "root %llu root dir %llu not found\n",
 			(unsigned long long)root->root_key.objectid,
 			(unsigned long long)root_dirid);
@@ -4176,10 +4177,9 @@ static int check_inode_recs(struct btrfs_root *root,
 				free_inode_rec(rec);
 				continue;
 			}
-			ret = 0;
 		}
 
-		if (!(repair && ret == 0))
+		if (rec->errors)
 			error++;
 		print_inode_error(root, rec);
 		list_for_each_entry(backref, &rec->backrefs, list) {
@@ -4189,6 +4189,8 @@ static int check_inode_recs(struct btrfs_root *root,
 				backref->errors |= REF_ERR_NO_DIR_INDEX;
 			if (!backref->found_inode_ref)
 				backref->errors |= REF_ERR_NO_INODE_REF;
+			if (backref->errors)
+				error++;
 			fprintf(stderr, "\tunresolved ref dir %llu index %llu"
 				" namelen %u name %s filetype %d errors %x",
 				(unsigned long long)backref->dir,
-- 
2.16.1

Re: [PATCH 12/15] btrfs-progs: check: increase counter error in check_inode_recs()

[Qu Wenruo]

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



On 2018年01月26日 16:35, Su Yue wrote:
> Counter @error decides return values of check_inode_recs().
> 
> Previously, @error won't be incremented even repair_inode_recs() failed.
> It causes 'btrfs check --repair' prints some error information but
> returns 0.
> 
> So, if root dir is missing and repair is disabled, @error should be
> incremented.
> And after repair_inode_recs(), increase @error if any errors in inodes
> and backrefs.
> 
> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>  cmds-check.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/cmds-check.c b/cmds-check.c
> index b23a4493b12b..a83f0a92f48b 100644
> --- a/cmds-check.c
> +++ b/cmds-check.c
> @@ -4137,6 +4137,7 @@ static int check_inode_recs(struct btrfs_root *root,
>  			return -EAGAIN;
>  		}
>  
> +		error++;
>  		fprintf(stderr, "root %llu root dir %llu not found\n",
>  			(unsigned long long)root->root_key.objectid,
>  			(unsigned long long)root_dirid);
> @@ -4176,10 +4177,9 @@ static int check_inode_recs(struct btrfs_root *root,
>  				free_inode_rec(rec);
>  				continue;
>  			}
> -			ret = 0;
>  		}
>  
> -		if (!(repair && ret == 0))
> +		if (rec->errors)
>  			error++;
>  		print_inode_error(root, rec);
>  		list_for_each_entry(backref, &rec->backrefs, list) {
> @@ -4189,6 +4189,8 @@ static int check_inode_recs(struct btrfs_root *root,
>  				backref->errors |= REF_ERR_NO_DIR_INDEX;
>  			if (!backref->found_inode_ref)
>  				backref->errors |= REF_ERR_NO_INODE_REF;
> +			if (backref->errors)
> +				error++;
>  			fprintf(stderr, "\tunresolved ref dir %llu index %llu"
>  				" namelen %u name %s filetype %d errors %x",
>  				(unsigned long long)backref->dir,
> 


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

[PATCH 13/15] btrfs-progs: check: find inode filetype in create_inode_item()

[Su Yue]

Now, find_file_type() doesn't return 0 if mismatched filetype is in a
backref.

Let create_inode_item() first call find_file_type() to get filetype.
If it failed, then guess filetype.

Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
 cmds-check.c | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/cmds-check.c b/cmds-check.c
index a83f0a92f48b..6091c7ef3442 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -3140,6 +3140,8 @@ static int create_inode_item_lowmem(struct btrfs_trans_handle *trans,
 	return __create_inode_item(trans, root, ino, 0, 0, 0, mode);
 }
 
+static u32 btrfs_type_to_imode(u8 type);
+static int find_file_type(struct inode_record *rec, u8 *type);
 static int create_inode_item(struct btrfs_root *root,
 			     struct inode_record *rec, int root_dir)
 {
@@ -3148,14 +3150,19 @@ static int create_inode_item(struct btrfs_root *root,
 	u32 mode = 0;
 	u64 size = 0;
 	int ret;
+	u8 type = 0;
 
-	trans = btrfs_start_transaction(root, 1);
-	if (IS_ERR(trans)) {
-		ret = PTR_ERR(trans);
-		return ret;
+	nlink = root_dir ? 1 : rec->found_link;
+	ret = find_file_type(rec, &type);
+	if (!ret) {
+		mode = btrfs_type_to_imode(type) | 0755;
+		if (type == BTRFS_FT_REG_FILE)
+			size = rec->found_size;
+		else
+			size = rec->extent_end;
+		goto create_inode;
 	}
 
-	nlink = root_dir ? 1 : rec->found_link;
 	if (rec->found_dir_item) {
 		if (rec->found_file_extent)
 			fprintf(stderr, "root %llu inode %llu has both a dir "
@@ -3167,7 +3174,14 @@ static int create_inode_item(struct btrfs_root *root,
 		size = rec->found_size;
 	} else if (!rec->found_dir_item) {
 		size = rec->extent_end;
-		mode =  S_IFREG | 0755;
+		mode = S_IFREG | 0755;
+	}
+
+create_inode:
+	trans = btrfs_start_transaction(root, 1);
+	if (IS_ERR(trans)) {
+		ret = PTR_ERR(trans);
+		return ret;
 	}
 
 	ret = __create_inode_item(trans, root, rec->ino, size, rec->nbytes,
@@ -3307,7 +3321,8 @@ static int find_file_type(struct inode_record *rec, u8 *type)
 	}
 
 	list_for_each_entry(backref, &rec->backrefs, list) {
-		if (backref->found_dir_index || backref->found_dir_item) {
+		if ((backref->found_dir_index || backref->found_dir_item) &&
+		    (!(backref->errors & REF_ERR_FILETYPE_UNMATCH))) {
 			*type = backref->filetype;
 			return 0;
 		}
-- 
2.16.1

Re: [PATCH 13/15] btrfs-progs: check: find inode filetype in create_inode_item()

[Qu Wenruo]

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



On 2018年01月26日 16:35, Su Yue wrote:
> Now, find_file_type() doesn't return 0 if mismatched filetype is in a
> backref.
> 
> Let create_inode_item() first call find_file_type() to get filetype.
> If it failed, then guess filetype.
> 
> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
> ---
>  cmds-check.c | 29 ++++++++++++++++++++++-------
>  1 file changed, 22 insertions(+), 7 deletions(-)
> 
> diff --git a/cmds-check.c b/cmds-check.c
> index a83f0a92f48b..6091c7ef3442 100644
> --- a/cmds-check.c
> +++ b/cmds-check.c
> @@ -3140,6 +3140,8 @@ static int create_inode_item_lowmem(struct btrfs_trans_handle *trans,
>  	return __create_inode_item(trans, root, ino, 0, 0, 0, mode);
>  }
>  
> +static u32 btrfs_type_to_imode(u8 type);
> +static int find_file_type(struct inode_record *rec, u8 *type);

Nice to see code shared between original and lowmem mode.

>  static int create_inode_item(struct btrfs_root *root,
>  			     struct inode_record *rec, int root_dir)
>  {
> @@ -3148,14 +3150,19 @@ static int create_inode_item(struct btrfs_root *root,
>  	u32 mode = 0;
>  	u64 size = 0;
>  	int ret;
> +	u8 type = 0;
>  
> -	trans = btrfs_start_transaction(root, 1);
> -	if (IS_ERR(trans)) {
> -		ret = PTR_ERR(trans);
> -		return ret;
> +	nlink = root_dir ? 1 : rec->found_link;
> +	ret = find_file_type(rec, &type);
> +	if (!ret) {
> +		mode = btrfs_type_to_imode(type) | 0755;
> +		if (type == BTRFS_FT_REG_FILE)
> +			size = rec->found_size;
> +		else
> +			size = rec->extent_end;
> +		goto create_inode;
>  	}

Just a nicpick, here we could use simple if else branch to avoid extra tag:

if (!ret) {
    ...
} else if (rec->found_dir_item) {
    ...
} else if (!rec->found_dir_item) {
    ...
}

Despite that, feel free to add my tag:

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

>  
> -	nlink = root_dir ? 1 : rec->found_link;
>  	if (rec->found_dir_item) {
>  		if (rec->found_file_extent)
>  			fprintf(stderr, "root %llu inode %llu has both a dir "
> @@ -3167,7 +3174,14 @@ static int create_inode_item(struct btrfs_root *root,
>  		size = rec->found_size;
>  	} else if (!rec->found_dir_item) {
>  		size = rec->extent_end;
> -		mode =  S_IFREG | 0755;
> +		mode = S_IFREG | 0755;
> +	}
> +
> +create_inode:
> +	trans = btrfs_start_transaction(root, 1);
> +	if (IS_ERR(trans)) {
> +		ret = PTR_ERR(trans);
> +		return ret;
>  	}
>  
>  	ret = __create_inode_item(trans, root, rec->ino, size, rec->nbytes,
> @@ -3307,7 +3321,8 @@ static int find_file_type(struct inode_record *rec, u8 *type)
>  	}
>  
>  	list_for_each_entry(backref, &rec->backrefs, list) {
> -		if (backref->found_dir_index || backref->found_dir_item) {
> +		if ((backref->found_dir_index || backref->found_dir_item) &&
> +		    (!(backref->errors & REF_ERR_FILETYPE_UNMATCH))) {
>  			*type = backref->filetype;
>  			return 0;
>  		}
> 


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

[PATCH 14/15] btrfs-progs: check: handle mismatched filetype in repair_inode_backref

[Su Yue]

Original can't handle mismatched well.
Since it only records inode mode and one filetype of dir_item or
dir_index. If backref has mismatched filetype, the one filetype
is untrusted. The only trusted thing is inode mode.

So, if we found mismatched filetype in a backref, just delete
dir_index and dir_item. Then next round of repair will add new
dir_index and dir_item whose filetype bases on the inode mode.

Transform delete_dir_index() to __delete_dir_item() for code reuse.

Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
 cmds-check.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 57 insertions(+), 9 deletions(-)

diff --git a/cmds-check.c b/cmds-check.c
index 6091c7ef3442..156b4063bf40 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -3059,27 +3059,41 @@ static int add_missing_dir_index(struct btrfs_root *root,
 	return 0;
 }
 
-static int delete_dir_index(struct btrfs_root *root,
-			    struct inode_backref *backref)
+static int __delete_dir_item(struct btrfs_root *root,
+			     struct inode_backref *backref, bool is_index)
 {
 	struct btrfs_trans_handle *trans;
 	struct btrfs_dir_item *di;
 	struct btrfs_path path;
 	int ret = 0;
+	u64 offset;
+	u8 type;
+
+	if (is_index) {
+		type = BTRFS_DIR_INDEX_KEY;
+		offset = backref->index;
+	} else {
+		type = BTRFS_DIR_ITEM_KEY;
+		offset = btrfs_name_hash(backref->name, backref->namelen);
+	}
 
 	trans = btrfs_start_transaction(root, 1);
 	if (IS_ERR(trans))
 		return PTR_ERR(trans);
 
-	fprintf(stderr, "Deleting bad dir index [%llu,%u,%llu] root %llu\n",
-		(unsigned long long)backref->dir,
-		BTRFS_DIR_INDEX_KEY, (unsigned long long)backref->index,
-		(unsigned long long)root->objectid);
+	fprintf(stderr, "Deleting bad dir %s [%llu,%u,%llu] root %llu\n",
+		is_index ? "index" : "item", (unsigned long long)backref->dir,
+		type, offset, (unsigned long long)root->objectid);
 
 	btrfs_init_path(&path);
-	di = btrfs_lookup_dir_index(trans, root, &path, backref->dir,
-				    backref->name, backref->namelen,
-				    backref->index, -1);
+	if (is_index)
+		di = btrfs_lookup_dir_index(trans, root, &path, backref->dir,
+					    backref->name, backref->namelen,
+					    backref->index, -1);
+	else
+		di = btrfs_lookup_dir_item(trans, root, &path, backref->dir,
+					   backref->name, backref->namelen,
+					   -1);
 	if (IS_ERR(di)) {
 		ret = PTR_ERR(di);
 		btrfs_release_path(&path);
@@ -3099,6 +3113,18 @@ static int delete_dir_index(struct btrfs_root *root,
 	return ret;
 }
 
+static int delete_dir_index(struct btrfs_root *root,
+			    struct inode_backref *backref)
+{
+	return __delete_dir_item(root, backref, true);
+}
+
+static int delete_dir_item(struct btrfs_root *root,
+			   struct inode_backref *backref)
+{
+	return __delete_dir_item(root, backref, false);
+}
+
 static int __create_inode_item(struct btrfs_trans_handle *trans,
 			       struct btrfs_root *root, u64 ino, u64 size,
 			       u64 nbytes, u64 nlink, u32 mode)
@@ -3228,6 +3254,22 @@ static int repair_inode_backrefs(struct btrfs_root *root,
 			continue;
 		}
 
+		if (delete &&
+		    (backref->errors & REF_ERR_FILETYPE_UNMATCH)) {
+			if (backref->found_dir_index)
+				ret = delete_dir_index(root, backref);
+			if (ret)
+				break;
+			if (backref->found_dir_item)
+				ret = delete_dir_item(root, backref);
+			if (ret)
+				break;
+			backref->found_dir_index = 0;
+			backref->found_dir_item = 0;
+			repaired++;
+			continue;
+		}
+
 		if (!delete && !backref->found_dir_index &&
 		    backref->found_dir_item && backref->found_inode_ref) {
 			ret = add_missing_dir_index(root, inode_cache, rec,
@@ -3285,6 +3327,12 @@ static int repair_inode_backrefs(struct btrfs_root *root,
 			btrfs_commit_transaction(trans, root);
 			backref->found_dir_index = 1;
 			backref->found_dir_item = 1;
+			/*
+			 * We can't judge backre->filetype is right or not.
+			 * Just rely on the inode mode.
+			 */
+			if (rec->found_inode_item)
+				backref->errors &= ~REF_ERR_FILETYPE_UNMATCH;
 			repaired++;
 		}
 
-- 
2.16.1

[PATCH 15/15] btrfs-progs: fsck-tests: add image for original and lowmem check

[Su Yue]

This image have two cases mixed:
1) Both filetypes of dir_item and dir_index about inode 258
   are corrupted.
2) inode item 258 is missing.

Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
 .../029-mismatched-filetype-no-inode/default_case.img    | Bin 0 -> 3072 bytes
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 tests/fsck-tests/029-mismatched-filetype-no-inode/default_case.img

diff --git a/tests/fsck-tests/029-mismatched-filetype-no-inode/default_case.img b/tests/fsck-tests/029-mismatched-filetype-no-inode/default_case.img
new file mode 100644
index 0000000000000000000000000000000000000000..57fc55b13c18bb9e5098a2fc4cb78f44fcf4abd9
GIT binary patch
literal 3072
zcmeHIdr;F?7EXCTRG?Bxc@>Nzl**%3o;ExN5K61)k|sa|c?E0?0tS<45}pcFF|3HG
zP(T(C5ekF^)P$A90ufn+LPC@zJVi+;1Oh=If!~6&Gaa4TKlhKF@ywljzjJ23d(ZjK
zojF%g#i^6~k^cn!$27LiPu{I9Fuxu2_TKVAAg%4)oBa+Gw*w%~J8ZGd_-G#$_^81D
zqXPJmONab%?&_8cU%hU@e3P9%Ckpzm)Ff$v)Wc@$>`BOZ#`Pp<)M#>*a!#6gR-n6^
z)1=c4bzkSqAeVbqNE#^6(OCUuf%ng!<@)45b#aP846yZF4X-R_(Y-V^`b{r7%mwmG
zSWwtole>bWF5})+N6j;AXb0Gxy74YnHE}t5`;DOxH;HrQz7u)mWsTfIuMAkZ<(_L<
zC!F*GIJRKTD$r+#ynp(^1;1avE`FkKNIg+L5PnOuanrCw<r7L^+;xULn#a2B7=Ctr
zs4O>R`tt$)P*2%T!%ObT%-uhb?}*&D?n%}JRS;pn;z#C$w4a`Mn{yX|Ga9U}x~oOl
zPdy&K^u@GIhKy*>SRG86A0MnN+-sb03Tf_vZ5-E+liKXMd_&vzazm2Gne+MktgGVy
zvel#Bjd}T{tkT9P=zgwDEa(bJDo>c|p2U#nuEy7Oq}9Yu^C{YMx%Hc`+EvW8NX3^E
zk5b)CX?>IaMfBjmD-m+Yq}J{ZNNVX0?AQ<#|9zRx-J>V|%LlTp#^N=X)8>mh-)=f9
zEYlDVvLL+$VG(Ui8HDAoY(D;gCVKkJ-@HePR*lull}K3i@%ZJsPNw}D=_Eh9%JD@h
z?Erk8B2lV5KN*FmZmG~q(|r;G^8W)cQe$Qm!0sCQG}HAsPz|4CwFrA_^+e2sR_0gZ
zT6?nGG7Cq$kNKf{KyPloT`ZqIZM2Cu7Dyak38$c#V`;-zUb>Sns-7=GO_RFo*VdgR
zuqI*rZ54Ur!&<Nktt_=qwYT0D`{aeDnQJcqW;k4U@!_JtC&3Y6hCc_~3~StNabqtN
zucrxtJ82q>Sl>f9N%#3``J4tLz`huK;mr8Mfx0Knd3o)jenuy1{i@HR#*`hTB~${6
zUv_Afxa)cPYb5}TDS<Zybm~}XtG1UA%v>e(!p&B4Ya=~W>ofT{n+nz8oS~p4#lC`~
zb8u11IWe6w=f)s&>P;TI(=k?N@SPvz&PJ9UlXj%_3Z(%0d_CZmZU@ut8ci9#8f$HC
z*WTjhvI#~*-wngRh#7=X-Fmp=jPY=Ih0Wy^83noKY+xx`U!G=AWh5CjX{nVD9LF{R
zU-WeEL{Wb4;-3oGNM9i?p>g7ijF+qbFuJxU3sZdd{#01uO$yD{GvFk&d(6xiY^PD`
z`WsWygqmUnBww<AN=f-z|I3v7FQyO>#H6qGUm0wp*HH~P%N?E<<}ITl?5LserSj}(
z297#+hWh84_Y2p@xTsd%x#--#Nni#Q|4Ipxp<3r7+8+PXxx%Y-r5X!!=_|}{J!L^2
zj)_*GCH6DoSd?di6o{=gi$ToxU!<VWg!#+Ul@A+ZSO@@+3tK%|eS#l>sEUY<bfUnd
z#z3(u>%_(a-G;Yng@p_aR!m56_S-4Kkx`wb-~;)P!B|l#<=%tqh!TpM*i_|yOSAQ4
zvW?d6n(=HfFPM`F&Z!$djU!(>x!?y?d%v=A;NURFAf7^dPE5VSt3>8OZ!b$jB>d=R
zUi2N&gsLW#yNe%L557Lz@L)84{Q4l9BWL3)GF6Y&!utQ%Wd=U5E>~6_7O${=+w`i(
z%3#13Z=w)Kdf}4|UKpK;xUcCxgl6f)p}g4$b6lc71QCwFLt}^}kC=Pd@dZitR_Fgr
zf%3Tpr&Fw75HCo1nW(9PfPwbUeOrmw&Kp#7!4BH?vg6sS-ETPM%E9-Nx3v?98T^30
z!E<e6$T@1JTsO|k&(!1Bgxa*?+N!22&m4JD_)=S!<bs%(BImU<xmqX_rwtN-wO0o#
z+MuM7R4Ar0B=B~zLu5(*{mK>+k^4=*tXx5D5H3qd4x8U8kRwaU7RMO#zpG5`D%hQ{
zQ@Id0-jr`iP(?4BQ3O;KRS<hPP9Rcj0zpavn@1$>T&=3_7=bz}(DFsr==Y@6keZ0m
ztya!P0$!o(C1)SCJ9hSoSk5^t{Zb`iYVMb$BO^M)46RnlO3RAX#_TMYwqi77@8KNm
zMEX%lf3IE;C<ATPVc`?mUoqC6zzlCka><GfbQpT?a&7vz*jX<DY0X42bXNZZ)<^e$
HufV?n{Xac9

literal 0
HcmV?d00001

-- 
2.16.1

Re: [PATCH 15/15] btrfs-progs: fsck-tests: add image for original and lowmem check

[Qu Wenruo]

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



On 2018年01月26日 16:35, Su Yue wrote:
> This image have two cases mixed:
> 1) Both filetypes of dir_item and dir_index about inode 258
>    are corrupted.
> 2) inode item 258 is missing.

It would be better to provide some debug tree output in commit message.

Something like:
------
        item 6 key (257 DIR_ITEM 4128386376) itemoff 15834 itemsize 35
                location key (258 INODE_ITEM 0) type DIR_ITEM.100
                transid 7 data_len 0 name_len 5
                name: file1
        item 7 key (257 DIR_INDEX 2) itemoff 15799 itemsize 35
                location key (258 INODE_ITEM 0) type DIR_ITEM.34
                transid 7 data_len 0 name_len 5
                name: file1
        item 8 key (258 INODE_REF 257) itemoff 15784 itemsize 15
                index 2 namelen 5 name: file1
------

would explain the problem easier.

And I think the coverage is not that good.

The image will fall into the fallback case as no reliable filetype.

But it would be better to have another image with regular/prealloc file
extent to allow us to exam the filetype guess/find code.

Thanks,
Qu
> 
> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
> ---
>  .../029-mismatched-filetype-no-inode/default_case.img    | Bin 0 -> 3072 bytes
>  1 file changed, 0 insertions(+), 0 deletions(-)
>  create mode 100644 tests/fsck-tests/029-mismatched-filetype-no-inode/default_case.img
> 
> diff --git a/tests/fsck-tests/029-mismatched-filetype-no-inode/default_case.img b/tests/fsck-tests/029-mismatched-filetype-no-inode/default_case.img
> new file mode 100644
> index 0000000000000000000000000000000000000000..57fc55b13c18bb9e5098a2fc4cb78f44fcf4abd9
> GIT binary patch
> literal 3072
> zcmeHIdr;F?7EXCTRG?Bxc@>Nzl**%3o;ExN5K61)k|sa|c?E0?0tS<45}pcFF|3HG
> zP(T(C5ekF^)P$A90ufn+LPC@zJVi+;1Oh=If!~6&Gaa4TKlhKF@ywljzjJ23d(ZjK
> zojF%g#i^6~k^cn!$27LiPu{I9Fuxu2_TKVAAg%4)oBa+Gw*w%~J8ZGd_-G#$_^81D
> zqXPJmONab%?&_8cU%hU@e3P9%Ckpzm)Ff$v)Wc@$>`BOZ#`Pp<)M#>*a!#6gR-n6^
> z)1=c4bzkSqAeVbqNE#^6(OCUuf%ng!<@)45b#aP846yZF4X-R_(Y-V^`b{r7%mwmG
> zSWwtole>bWF5})+N6j;AXb0Gxy74YnHE}t5`;DOxH;HrQz7u)mWsTfIuMAkZ<(_L<
> zC!F*GIJRKTD$r+#ynp(^1;1avE`FkKNIg+L5PnOuanrCw<r7L^+;xULn#a2B7=Ctr
> zs4O>R`tt$)P*2%T!%ObT%-uhb?}*&D?n%}JRS;pn;z#C$w4a`Mn{yX|Ga9U}x~oOl
> zPdy&K^u@GIhKy*>SRG86A0MnN+-sb03Tf_vZ5-E+liKXMd_&vzazm2Gne+MktgGVy
> zvel#Bjd}T{tkT9P=zgwDEa(bJDo>c|p2U#nuEy7Oq}9Yu^C{YMx%Hc`+EvW8NX3^E
> zk5b)CX?>IaMfBjmD-m+Yq}J{ZNNVX0?AQ<#|9zRx-J>V|%LlTp#^N=X)8>mh-)=f9
> zEYlDVvLL+$VG(Ui8HDAoY(D;gCVKkJ-@HePR*lull}K3i@%ZJsPNw}D=_Eh9%JD@h
> z?Erk8B2lV5KN*FmZmG~q(|r;G^8W)cQe$Qm!0sCQG}HAsPz|4CwFrA_^+e2sR_0gZ
> zT6?nGG7Cq$kNKf{KyPloT`ZqIZM2Cu7Dyak38$c#V`;-zUb>Sns-7=GO_RFo*VdgR
> zuqI*rZ54Ur!&<Nktt_=qwYT0D`{aeDnQJcqW;k4U@!_JtC&3Y6hCc_~3~StNabqtN
> zucrxtJ82q>Sl>f9N%#3``J4tLz`huK;mr8Mfx0Knd3o)jenuy1{i@HR#*`hTB~${6
> zUv_Afxa)cPYb5}TDS<Zybm~}XtG1UA%v>e(!p&B4Ya=~W>ofT{n+nz8oS~p4#lC`~
> zb8u11IWe6w=f)s&>P;TI(=k?N@SPvz&PJ9UlXj%_3Z(%0d_CZmZU@ut8ci9#8f$HC
> z*WTjhvI#~*-wngRh#7=X-Fmp=jPY=Ih0Wy^83noKY+xx`U!G=AWh5CjX{nVD9LF{R
> zU-WeEL{Wb4;-3oGNM9i?p>g7ijF+qbFuJxU3sZdd{#01uO$yD{GvFk&d(6xiY^PD`
> z`WsWygqmUnBww<AN=f-z|I3v7FQyO>#H6qGUm0wp*HH~P%N?E<<}ITl?5LserSj}(
> z297#+hWh84_Y2p@xTsd%x#--#Nni#Q|4Ipxp<3r7+8+PXxx%Y-r5X!!=_|}{J!L^2
> zj)_*GCH6DoSd?di6o{=gi$ToxU!<VWg!#+Ul@A+ZSO@@+3tK%|eS#l>sEUY<bfUnd
> z#z3(u>%_(a-G;Yng@p_aR!m56_S-4Kkx`wb-~;)P!B|l#<=%tqh!TpM*i_|yOSAQ4
> zvW?d6n(=HfFPM`F&Z!$djU!(>x!?y?d%v=A;NURFAf7^dPE5VSt3>8OZ!b$jB>d=R
> zUi2N&gsLW#yNe%L557Lz@L)84{Q4l9BWL3)GF6Y&!utQ%Wd=U5E>~6_7O${=+w`i(
> z%3#13Z=w)Kdf}4|UKpK;xUcCxgl6f)p}g4$b6lc71QCwFLt}^}kC=Pd@dZitR_Fgr
> zf%3Tpr&Fw75HCo1nW(9PfPwbUeOrmw&Kp#7!4BH?vg6sS-ETPM%E9-Nx3v?98T^30
> z!E<e6$T@1JTsO|k&(!1Bgxa*?+N!22&m4JD_)=S!<bs%(BImU<xmqX_rwtN-wO0o#
> z+MuM7R4Ar0B=B~zLu5(*{mK>+k^4=*tXx5D5H3qd4x8U8kRwaU7RMO#zpG5`D%hQ{
> zQ@Id0-jr`iP(?4BQ3O;KRS<hPP9Rcj0zpavn@1$>T&=3_7=bz}(DFsr==Y@6keZ0m
> ztya!P0$!o(C1)SCJ9hSoSk5^t{Zb`iYVMb$BO^M)46RnlO3RAX#_TMYwqi77@8KNm
> zMEX%lf3IE;C<ATPVc`?mUoqC6zzlCka><GfbQpT?a&7vz*jX<DY0X42bXNZZ)<^e$
> HufV?n{Xac9
> 
> literal 0
> HcmV?d00001
> 


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