[PATCH v2 0/7] btrfs-progs: lowmem: bug fixes and inode_extref repair

[PATCH v2 0/7] btrfs-progs: lowmem: bug fixes and inode_extref repair

[damenly.su]

From: Su Yue <suy.fnst@cn.fujitsu.com>

This patchset can be fetched from my github(based on v4.17.1):
https://github.com/Damenly/btrfs-progs/tree/lowmem_extref

The patchset aims to support check and repair errors about
inode_extref in lowmem mode. 

patch[1-2] let btrfs_unlink() detect inode_extref.
patch[3] fixes a minor bug in check_dir_item due to my careless
	 long ago.
patch[4] fixes a bug about inconsistent path in check_fs_roots()
	 under repair.
patch[5] fixes a corner case about traversal of inode items.
patch[6] enable inode_extref repair support and remove unnecessary
	 checks.
patch[7] add a test image, it can verify above patches except
	 patch[3].

Changelog:
v2:
   Resend with patches in right order.	

Su Yue (7):
  btrfs-progs: adjust arguments of btrfs_lookup_inode_extref()
  btrfs-progs: make btrfs_unlink() lookup inode_extref
  btrfs-progs: lowmem check: find dir_item by di_key in check_dir_item()
  btrfs-progs: lowmem: search key of root again after check_fs_root()
    under repair
  btrfs-progs: lowmem: continue to check item in last slot while
    checking inodes
  btrfs-progs: lowmem: improve check_inode_extref()
  btrfs-progs: fsck-tests: add test case inode_extref without dir_item
    and dir_index

 check/mode-lowmem.c                           | 148 ++++++++++++++----
 ctree.h                                       |   6 +-
 inode-item.c                                  |   6 +-
 inode.c                                       |  14 +-
 .../inode_extref_without_dir_item_index.img   | Bin 0 -> 10240 bytes
 ... inode_ref_without_dir_item_and_index.img} | Bin
 6 files changed, 139 insertions(+), 35 deletions(-)
 create mode 100644 tests/fsck-tests/009-no-dir-item-or-index/inode_extref_without_dir_item_index.img
 rename tests/fsck-tests/009-no-dir-item-or-index/{default_case.img => inode_ref_without_dir_item_and_index.img} (100%)

-- 
2.18.0

[PATCH v2 1/7] btrfs-progs: adjust arguments of btrfs_lookup_inode_extref()

[damenly.su]

From: Su Yue <suy.fnst@cn.fujitsu.com>

The argument index is not used in btrfs_lookup_inode_extref(),
so remove it.
And adjust positions its arguments to make it consistent with
kernel part.

No functional change.

Fixes: 260675657767 ("btrfs-progs: Import btrfs_insert/del/lookup_extref() functions.")
Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
 ctree.h      | 6 +++---
 inode-item.c | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/ctree.h b/ctree.h
index 4719962df67d..e7f6c5df95f1 100644
--- a/ctree.h
+++ b/ctree.h
@@ -2708,9 +2708,9 @@ int btrfs_lookup_inode(struct btrfs_trans_handle *trans, struct btrfs_root
 		       *root, struct btrfs_path *path,
 		       struct btrfs_key *location, int mod);
 struct btrfs_inode_extref *btrfs_lookup_inode_extref(struct btrfs_trans_handle
-		*trans, struct btrfs_path *path, struct btrfs_root *root,
-		u64 ino, u64 parent_ino, u64 index, const char *name,
-		int namelen, int ins_len);
+		*trans, struct btrfs_root *root, struct btrfs_path *path,
+		const char *name, int namelen, u64 ino, u64 parent_ino,
+		int ins_len);
 int btrfs_del_inode_extref(struct btrfs_trans_handle *trans,
 			   struct btrfs_root *root,
 			   const char *name, int name_len,
diff --git a/inode-item.c b/inode-item.c
index 1cc106670cd4..461557cb83d6 100644
--- a/inode-item.c
+++ b/inode-item.c
@@ -228,9 +228,9 @@ static int btrfs_find_name_in_ext_backref(struct btrfs_path *path,
 }
 
 struct btrfs_inode_extref *btrfs_lookup_inode_extref(struct btrfs_trans_handle
-		*trans, struct btrfs_path *path, struct btrfs_root *root,
-		u64 ino, u64 parent_ino, u64 index, const char *name,
-		int namelen, int ins_len)
+		*trans, struct btrfs_root *root, struct btrfs_path *path,
+		const char *name, int namelen, u64 ino, u64 parent_ino,
+		int ins_len)
 {
 	struct btrfs_key key;
 	struct btrfs_inode_extref *extref;
-- 
2.18.0

[PATCH v2 2/7] btrfs-progs: make btrfs_unlink() lookup inode_extref

[damenly.su]

From: Su Yue <suy.fnst@cn.fujitsu.com>

btrfs_unlink() uses btrfs_lookup_inode_ref() to look up inode_ref
but forget inode_extref case.

Let btrfs_unlink() call btrfs_lookup_inode_extref() if inode_ref is
found and EXTENDED_IREF feature is enabled.

Fixes: 0cc75eddd093 ("btrfs-progs: Add btrfs_unlink() and btrfs_add_link() functions.")
Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
 inode.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/inode.c b/inode.c
index 2398bca4a109..598ad0ab6b4c 100644
--- a/inode.c
+++ b/inode.c
@@ -277,6 +277,7 @@ int btrfs_unlink(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 	struct btrfs_key key;
 	struct btrfs_inode_item *inode_item;
 	struct btrfs_inode_ref *inode_ref;
+	struct btrfs_inode_extref *inode_extref = NULL;
 	struct btrfs_dir_item *dir_item;
 	u64 inode_size;
 	u32 nlinks;
@@ -296,7 +297,18 @@ int btrfs_unlink(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 		ret = PTR_ERR(inode_ref);
 		goto out;
 	}
-	if (inode_ref)
+
+	if (!inode_ref && btrfs_fs_incompat(root->fs_info, EXTENDED_IREF)) {
+		btrfs_release_path(path);
+		inode_extref = btrfs_lookup_inode_extref(trans, root, path,
+					 name, namelen, ino, parent_ino, 0);
+		if (IS_ERR(inode_extref)) {
+			ret = PTR_ERR(inode_extref);
+			goto out;
+		}
+	}
+
+	if (inode_ref || inode_extref)
 		del_inode_ref = 1;
 	btrfs_release_path(path);
 
-- 
2.18.0

[PATCH v2 3/7] btrfs-progs: lowmem check: find dir_item by di_key in check_dir_item()

[damenly.su]

From: Su Yue <suy.fnst@cn.fujitsu.com>

In check_dir_item, we are going to search corresponding
dir_item/index.

Commit 564901eac7a4 ("btrfs-progs: check: introduce
print_dir_item_err()") Changed argument name from key to di_key but
forgot to change the key name for dir_item search.
So @key shouldn't be used here. It should be @di_key.

Fixes: 564901eac7a4 ("btrfs-progs: check: introduce print_dir_item_err()")
Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
 check/mode-lowmem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 1bce44f5658a..89a304bbdd69 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -1658,7 +1658,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.18.0

[PATCH v2 4/7] btrfs-progs: lowmem: search key of root again after check_fs_root() under repair

[damenly.su]

From: Su Yue <suy.fnst@cn.fujitsu.com>

In check_fs_roots_lowmem(), we do search and follow the resulted path
to call check_fs_root(), then call btrfs_next_item() to check next
root.
However, if repair is enabled, the root tree can be cowed, the
existed path can cause strange errors.

Solution:
  If repair, save the key before calling check_fs_root,
  search the saved key again before checking next root.

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

diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 89a304bbdd69..8fc9edab1d66 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -4967,9 +4967,13 @@ int check_fs_roots_lowmem(struct btrfs_fs_info *fs_info)
 	}
 
 	while (1) {
+		struct btrfs_key saved_key;
+
 		node = path.nodes[0];
 		slot = path.slots[0];
 		btrfs_item_key_to_cpu(node, &key, slot);
+		if (repair)
+			saved_key = key;
 		if (key.objectid > BTRFS_LAST_FREE_OBJECTID)
 			goto out;
 		if (key.type == BTRFS_ROOT_ITEM_KEY &&
@@ -5000,6 +5004,17 @@ int check_fs_roots_lowmem(struct btrfs_fs_info *fs_info)
 			err |= ret;
 		}
 next:
+		/*
+		 * Since root tree can be cowed during repair,
+		 * here search the saved key again.
+		 */
+		if (repair) {
+			btrfs_release_path(&path);
+			ret = btrfs_search_slot(NULL, fs_info->tree_root,
+						&saved_key, &path, 0, 0);
+			/* Repair never deletes trees, search must succeed. */
+			BUG_ON(ret);
+		}
 		ret = btrfs_next_item(tree_root, &path);
 		if (ret > 0)
 			goto out;
-- 
2.18.0

[PATCH v2 5/7] btrfs-progs: lowmem: continue to check item in last slot while checking inodes

[damenly.su]

From: Su Yue <suy.fnst@cn.fujitsu.com>

After call of check_inode_item(), path may point to the last unchecked
slot of the leaf. The outer walk_up_tree() always treats the position
as checked item then skips to next item.

If the last item was an inode item, yes, it was unchecked.
Then walk_up_tree() will think the leaf is checked and walk up to
upper node, process_one_leaf() in walk_down_tree() would skip to
check next inode item. Which means, the inode item won't be checked.

Solution:
After check_inode_item returns, if found path point to the last item
of a leaf, decrease path slot manually, so walk_up_tree() will stay
on the leaf.

Fixes: 5e2dc770471b ("btrfs-progs: check: skip shared node or leaf check for low_memory mode")
Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
 check/mode-lowmem.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 8fc9edab1d66..b6b33786d02b 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -2612,6 +2612,18 @@ again:
 	if (cur->start == cur_bytenr)
 		goto again;
 
+	/*
+	 * path may point at the last item(a inode item maybe) in a leaf.
+	 * Without below lines, walk_up_tree() will skip the item which
+	 * means all items related to the inode will never be checked.
+	 * Decrease the slot manually, walk_up_tree won't skip to next node
+	 * if it occurs.
+	 */
+	if (path->slots[0] + 1 >= btrfs_header_nritems(path->nodes[0])) {
+		if (path->slots[0])
+			path->slots[0]--;
+	}
+
 	/*
 	 * we have switched to another leaf, above nodes may
 	 * have changed, here walk down the path, if a node
-- 
2.18.0

[PATCH v2 6/7] btrfs-progs: lowmem: improve check_inode_extref()

[damenly.su]

From: Su Yue <suy.fnst@cn.fujitsu.com>

inode_extref is much similar with inode_ref, most codes are reused in
check_inode_extref().
Exception:
There is no need to check root directory, so remove it.
Make check_inode_extref() verify hash value with key offset now.

And lowmem check can detect errors about inode_extref and try to
repair errors.

Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
 check/mode-lowmem.c | 119 ++++++++++++++++++++++++++++++++++----------
 1 file changed, 92 insertions(+), 27 deletions(-)

diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index b6b33786d02b..bec2ee185cc8 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -1182,37 +1182,79 @@ out:
  *
  * Return 0 if no error occurred.
  */
-static int check_inode_extref(struct btrfs_root *root,
-			      struct btrfs_key *ref_key,
-			      struct extent_buffer *node, int slot, u64 *refs,
-			      int mode)
+static int check_inode_extref(struct btrfs_root *root, struct btrfs_key *ref_key,
+			      struct btrfs_path *path, char *name_ret,
+			      u32 *namelen_ret, u64 *refs_ret, int mode)
 {
 	struct btrfs_key key;
 	struct btrfs_key location;
 	struct btrfs_inode_extref *extref;
+	struct extent_buffer *node;
 	char namebuf[BTRFS_NAME_LEN] = {0};
 	u32 total;
-	u32 cur = 0;
+	u32 cur;
 	u32 len;
 	u32 name_len;
 	u64 index;
 	u64 parent;
+	int err;
+	int tmp_err;
 	int ret;
-	int err = 0;
+	int slot;
+	u64 refs;
+	bool search_again = false;
 
 	location.objectid = ref_key->objectid;
 	location.type = BTRFS_INODE_ITEM_KEY;
 	location.offset = 0;
+begin:
+	cur = 0;
+	err = 0;
+	refs = *refs_ret;
+	*namelen_ret = 0;
+
+	/* since after repair, path and the dir item may be changed */
+	if (search_again) {
+		search_again = false;
+		btrfs_release_path(path);
+		ret = btrfs_search_slot(NULL, root, ref_key, path, 0, 0);
+		/*
+		 * The item was deleted, let the path point to the last checked
+		 * item.
+		 */
+		if (ret > 0) {
+			if (path->slots[0] == 0) {
+				ret = btrfs_prev_leaf(root, path);
+				/*
+				 * we are checking the inode item, there must
+				 * be some items before, the case shall never
+				 * happen.
+				 */
+				BUG_ON(ret);
+			} else {
+				path->slots[0]--;
+			}
+			goto out;
+		} else if (ret < 0) {
+			err |= ret;
+			goto out;
+		}
+	}
+
+	node = path->nodes[0];
+	slot = path->slots[0];
 
 	extref = btrfs_item_ptr(node, slot, struct btrfs_inode_extref);
 	total = btrfs_item_size_nr(node, slot);
 
-next:
-	/* update inode ref count */
-	(*refs)++;
-	name_len = btrfs_inode_extref_name_len(node, extref);
-	index = btrfs_inode_extref_index(node, extref);
+loop:
+	/* Update inode ref count */
+	refs++;
+	tmp_err = 0;
 	parent = btrfs_inode_extref_parent(node, extref);
+	index = btrfs_inode_extref_index(node, extref);
+	name_len = btrfs_inode_extref_name_len(node, extref);
+
 	if (name_len <= BTRFS_NAME_LEN) {
 		len = name_len;
 	} else {
@@ -1220,37 +1262,60 @@ next:
 		warning("root %llu INODE_EXTREF[%llu %llu] name too long",
 			root->objectid, ref_key->objectid, ref_key->offset);
 	}
+
 	read_extent_buffer(node, namebuf, (unsigned long)(extref + 1), len);
 
-	/* Check root dir ref name */
-	if (index == 0 && strncmp(namebuf, "..", name_len)) {
-		error("root %llu INODE_EXTREF[%llu %llu] ROOT_DIR name shouldn't be %s",
+	/* verify hash value */
+	if (ref_key->offset != btrfs_extref_hash(parent, namebuf, len)) {
+		err |= -EIO;
+		error("root %llu INODE_EXTREF[%llu %llu] name %s namelen %u mode %d mismatch with its hash, wanted %llu have %llu",
 		      root->objectid, ref_key->objectid, ref_key->offset,
-		      namebuf);
-		err |= ROOT_DIR_ERROR;
+		      namebuf, len, mode, ref_key->offset,
+		      btrfs_extref_hash(parent, namebuf, len));
+		goto out;
 	}
 
-	/* find related dir_index */
+	/* copy the first name found to name_ret */
+	if (refs == 1 && name_ret) {
+		memcpy(name_ret, namebuf, len);
+		*namelen_ret = len;
+	}
+
+	/* Find related DIR_INDEX */
 	key.objectid = parent;
 	key.type = BTRFS_DIR_INDEX_KEY;
 	key.offset = index;
-	ret = find_dir_item(root, &key, &location, namebuf, len, mode);
-	err |= ret;
+	tmp_err |= find_dir_item(root, &key, &location, namebuf, len,
+			    imode_to_type(mode));
 
-	/* find related dir_item */
+	/* Find related dir_item */
 	key.objectid = parent;
 	key.type = BTRFS_DIR_ITEM_KEY;
 	key.offset = btrfs_name_hash(namebuf, len);
-	ret = find_dir_item(root, &key, &location, namebuf, len, mode);
-	err |= ret;
+	tmp_err |= find_dir_item(root, &key, &location, namebuf, len,
+			    imode_to_type(mode));
+
+	if (tmp_err && repair) {
+		ret = repair_ternary_lowmem(root, parent, ref_key->objectid,
+			    index, namebuf, name_len, imode_to_type(mode),
+			    tmp_err);
+		if (!ret) {
+			search_again = true;
+			goto begin;
+		}
+	}
 
+	print_inode_ref_err(root, ref_key, index, namebuf, name_len,
+			    imode_to_type(mode), tmp_err);
+
+	err |= tmp_err;
 	len = sizeof(*extref) + name_len;
 	extref = (struct btrfs_inode_extref *)((char *)extref + len);
 	cur += len;
-
 	if (cur < total)
-		goto next;
-
+		goto loop;
+out:
+	*refs_ret = refs;
 	return err;
 }
 
@@ -2426,8 +2491,8 @@ static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path)
 				warning("root %llu EXTREF[%llu %llu] isn't supported",
 					root->objectid, key.objectid,
 					key.offset);
-			ret = check_inode_extref(root, &key, node, slot, &refs,
-						 mode);
+			ret = check_inode_extref(root, &key, path, namebuf,
+						 &name_len, &refs, mode);
 			err |= ret;
 			break;
 		}
-- 
2.18.0

[PATCH v2 7/7] btrfs-progs: fsck-tests: add test case inode_extref without dir_item and dir_index

[damenly.su]

From: Su Yue <suy.fnst@cn.fujitsu.com>

This case contains an inode_extref:
==========================================
...
   item 1 key (257 INODE_EXTREF 3460996356) itemoff 3947 itemsize 24
                index 257 parent 256 namelen 6 name: foo255
...
==========================================
The related dir_item and dir_index are missing.

Add the case to ensure both original and lowmem mode check can handle
the case of inode_extref.

Lowmem part is supported since patch named
       'btrfs-progs: lowmem: improve check_inode_extref()'.

And rename default_case.img to inode_ref_without_dir_item_index.img.

Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
 .../inode_extref_without_dir_item_index.img     | Bin 0 -> 10240 bytes
 ...=> inode_ref_without_dir_item_and_index.img} | Bin
 2 files changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 tests/fsck-tests/009-no-dir-item-or-index/inode_extref_without_dir_item_index.img
 rename tests/fsck-tests/009-no-dir-item-or-index/{default_case.img => inode_ref_without_dir_item_and_index.img} (100%)

diff --git a/tests/fsck-tests/009-no-dir-item-or-index/inode_extref_without_dir_item_index.img b/tests/fsck-tests/009-no-dir-item-or-index/inode_extref_without_dir_item_index.img
new file mode 100644
index 0000000000000000000000000000000000000000..85f7a8211fe66dfa9b2b1f3a98e72f54cd402616
GIT binary patch
literal 10240
zcmeHscQo8j8?UlhEOymZqDGCfdWp7rf~Y~1#Of{4q9>NMSe=L(Wkn}~5JZVy61@b`
zOY|NsB<z*<ci-F3{qz2D&pT(%^L*whb7r3J`OZ8u6XTHLwZd)Lg7;qw*$v14SG@si
z{}O`#MQ8s~cKv^0N5UJX{146m++Z;H#s?3t<S&8!gY*7k>wmBv=mszSgNy&l!H|F1
z$NlwH{s)Ks?ejn7KN0v(1pZ$}z<*}eU&ND$kky0Zga5{R>1!Q|jBj!Kk_IdwP^`75
zM#+ojWKDf@k7f}D9aT8qYmSi4F2VupuXr{D%*M11tY*yA57Z39C|MYjWMVhzVK5j1
zieWerkiPZjcdzsBS22#CPt1DVI!`Y9h*_LYFoYww&j3lOEQw=`DnvA?wF8(}YD85$
zG7$zaGL3dVC{eyOEl3W_-qsf~#zJSUyqf<-HM2v|!&%~jSR%lz|08~jx;7^e3~;sY
zT^phzaQf-A9pzJu&sDmw_AnE#I*l;xF8RnBNniHgj!g0JPRgY|`Ok<a{Mq^vLSl&#
zT==vTr(~q+;FBMuOG`o+hG&)q;T&{&zBb;Y>hQSmPV3fQT{G?g$ZX6e{@Ov^aKMAG
zr|O5JwU)}O00QwdZZ<+yyfQ#~Ka9r6hA@3Oik}0|;rd7T)~&xIk460t)3|9Uli=;g
zh~{}Qn(6<FU0Pt{f_C5)P)(K3JwH5PE8f!<&bb(~y;Tzz$`OkHCLq>qP?bLNIenC5
zLO3=6UuM9&&4o2fB&4lmozz;AiUWx^ndhJ)O`@bM4QWxL!mC>|7xSNK`Sw)H$N)-m
zSXz2)pA0>PYZPT59_=l(%tNBo?D|v)$%|6$`GKhK<Yx|_Z4h?!&ekU8CN7em<`<~c
z3`TXnj3nB0HqI`I=#f5$3l_;9nqB$anY63#2cOROvo^B4lgXan6XVTQ><>+y93Fiy
zTiWZzmC*aeUfXv1K)7a?^YiuO;WXH%#MegmjkD8b)YP2MzV{*|{#*5|R9Ov&-Q^d%
z_IMBdg|z+-kf1lZG%Y^>3;_8eTVDY0gLuVuK^iH*1%Z5W_7lF*&eh~uM$6u+hgOC_
z4rWyxVvQeo1hp}4g+fK$4!+0}!ov|G-z-7PF$8c+7m_drD7D<mkV->1!U-DzS0t`X
z>Y@fV(K_9a{<$w^me4NJyvG7Sf}U~z9#&01Q3o5cr>bn8t5a2O%6)B<A`FcOJ5V(M
z^Po>Wf6SW`JBx#V08j00K~k%COtKaNNL64F+f;tgKG=lY(fg3Y1(No%L^e>S=)3E(
z&!A-jV%H0qu$NE*8j|<QFkAg_;Yu`kJB;TDF|ABDtMDtjNB&e2*tr0d;93REh?baW
zJXGa7*BIuzqutPc#vY;Wdne2cv}U-aTSp=~9Zl%M&Ue@uo?r{oXa+;oE6suP&`d!i
zP#H`@FWLwE;X3r-?mMs`bkeHA5!(ySKs*bn0(aIF=`f>3Wl=2@yv!W(Y57SB519_C
z6)+t9-_I4%1fb5Z&csckFPKh{cpNOM?t%BnXTyFy@$oS?=g{%z#FR7*F40BkJ{o-r
zeg|!2s2At!us$=cRn=-Ad2{xrofs7EHO~dqhBX`nd|fIgA*e6HJ^;GIet=Fzs|Guf
zxQ2n&#dYN2A8l5#wAUxk;ou8NrqCx`Oc#>ow_1A&mZ!ALgX2PB!dwW;z>bFJXiw+c
zypsFg$DKl>+n>1KhFaK2cxvlV`)R$kUYXhip;KvQ_#vs`?2wszPATytUWAA(sQSJH
z^RMiYpTGHknufRKDjB%5pBN2SjvkV5L(I9UU1DFG7??kh5jsJqY6o_B73<X0_zI=Z
z=b<CyX3d_xi$Kx{wY~`qj#8FskoeQ0EW`i&wQBZHFLps@!nHz8vmWZT*SB}&le{=x
zNy#G@$&wB~tSqW8u|1QcLB#7K@o*ys6#g@NSr$&Tj<mgwzS2$m4m7sv+hG&HkA_`N
zev{K9innXkt5g_V_#Zk=1%_gz$V-8bJzA9WD!4~hT#W*`05h-YqfgtdYlTv5Mifs9
zH^b7@@<pqAEF_;f3}p<k`n{@oSg-pF$2upnc(D6DL%b&Kc+mQoYTOZOJ94FQ9GZLi
znADEBe$Tw|<r;0iqW$ojcqtkrDA=#T_xpNXUS^n2@jh$R=<(Oh>bdweC+VkG5<x7>
ztzX_b9lWd=Vzn<PBWXTcd6#`wGV?vY1muq^ziV~IXC<R5*GI#c8$z7pbVNGcNmHSS
zuj&7`?C@F9@S)nwck=g{_wYUIjlW$m%3wKvc4cksjXGCT{Cj^=#$OMvYW`GmtyH*!
zBRu&fBiV7*`ZUXD$G9_n6_Ooc;7+wL6qRwt*UkBqDzzH>c~SRNqWEoil+=D!ag|n-
zV^-zuB>ELcdPa`Vc1d(LA~EzFlT|RepXaM<_ZQ&B?gu~2%>~O?mWu*ojfALeu&8rY
z%(K7N_DOUrT=YUJZZ@L=U0(FX;pxMuNkvPDt$~o5dsYG#-1_04K`yn1Sj1qQyyU)u
zvE-QOkSY++v*PYgE>*?e4q8%GvE@?)$zlkmkNxg6rFV14ha&=lZU@XI5tK3agYnuh
zNl&@>6QbOf!&O)HQ{2|dd))AuM$=SF{g1$7-j)@?DAPh2O`AO0jeD=PM^~MXkJUt?
zg!zRNe;G%|+^)Mfs=4CBFPv0W(4b>eMtgNz*aYYq-X&rPO8jLR4X3_a*l^z_g%+7?
z#z~Y*GhPVg9A#rzk&Ci5oMS)wNGpABH)(}Fs=lB>cSV-}difg<|9)bFxp0>ib#Eb*
zXH=MoY9X0ZIZX1lPC&i3>Avo0w7cHTw5>ZJkc7`dE1L@Mdl;H}&TNlnmp(t_N>G?|
z^yzi_Ggk2#>$h<C^MPjXW<&&q_e&!q8_x<(o>ELTtthGbfOvGe`fBfOy5Bi3OdvIC
zko*#aS2d@G!A8N`p?m7t$<cDWbUt;o|F`;pZ#zqWLN>3&`>n6@`X>2J>=><Zx%Ct6
z<&;6PI~kT17x}V{FP5W;JT(u0!m}8J9oD)eh8t18V|_$77!gMuJ)<S#KH>IdgriB!
zyPDn6G?7$$%#1Hy(ReG^dp%jL#2hmj@aK@-MKNSpj?#Jo@UUHd=T`c{ujnyN6Z(yN
z<X<STO_8A7y^~py49dug+d?5t_|H`*Tjkv#EOld_XVV22o-VK%o{Za12RRT=zPD%%
zJnr!e;IW>V09=VFyj>lbukif?9_n-pzQrmm`U(GZMSfRsU0y<VK9_m@YN4TNS!3sX
z@3$Z5#dO6r?Qtjr^Mb4_?MdqSD6(-(_8f_mCtdWrRvMX7hCU(W=HTHqq&krVzquW`
zXFYtaWpIV`-Mv<l_!2z-@4pEzIkZc<z31&Ej@}A4Y}!Q_Fq(Dh%RLVf!nHPC-`|*#
z|F<O!kq-3JohQt>YL)e$aXtDIlz>mw0Djc$J;jVHiWu0|IM3vxB*w?rNR;7H%N*VO
zS{xTS(0t!CczpJ00B#W?Ria4GHyG_Utydx(VG~SDOo8SHIYOa;HjpD0jbEmQm(CH(
zUyVQDKM=`*@44(y()CYf_P^#gA<VE&XjL$Gt!b1X>d6%`Up5}k=<hG-+k{%iB6x+j
zs>*mLbMZ>r<nQ~>><rPE+O`o2tI;3Ko%ZCGa?~`Aw(vxUwJV1}ZVv&I93&clI<Inw
zSB|ADp7>fU;QY|H$`WYqi;J_DM`T>SNeD&YzXDRl9_3JVA*>sc3WKF;z8v))5WO+g
zE5DldUY6BIg(3d86zre0(!VXb$I)hF+_tCC3@@c0(-pj<^_ls>%k4Oxa0<LFF(I9s
z6@D(@1>H9~l0yI|nmaMo$^eKahAa=BIpo|d9PE9Z*6iRf$|2xFG&hv)I|QsY7E#6b
zTAfIg-LO=R07{WWr5?`IP9zXaMug_0Gp-udKRDIV4lbHY9)5E@^=I|y<-tJE+^1uQ
zAqBt3^DSWun1-Ikyn-+8yMlYBPaQ5N9x3ENnQkrjXhctOA=kC8I>~1jD9<uKY&B65
z<*lv-xsLfxjUo@?8_G_x;mpB;)Nqq44CYsP-etUEHzyQxxU@BU9;5u5)ZB{VIDc0u
zhMl+`^OZ04P+cL!<-(IhO+WT<Of?mKss!IDUrz0NN$tQsAi$pl1No&?1#~rWdU(Vz
zoULg&p{o?7b`&6%7<|l|%K~&5P-!8_OA8PndbpkzsB@_dIEIML4!=%}hLyS*M*;WM
ze#r%8pqYcK%i}i^FiVD3AwN>`mFU|eeU?{)r-I(1O+91yg*LSOnU;TruThVAnkI`B
zzT`yhH*_nRtlh_`PEUBu6>~Nil0*z2Rf@3Dv{%a=vPSVzE7G(x10xw&Qma6Dx7F_o
zqrL#I202NuAR0pbO8lrZpQY7cS?Rskz_*Z|+~E|D4^nMrjw{X(TFxwH8O;s>bkHmF
zEJ(ZU$7Q4WDMO_@C^d9g3pqCw_}jEQmj*7VQmL(Z`^6)2?DO^#L&$C=QLeyhml&xC
zCv7&<aUsxQ_5Il3J?e;6dPgIN`wbM!@4L0RCEBeD&+f^JpkhX=y2}VX7yG=ydEGPA
zqyfOe54-MbvY11+V;j;^AjP&7@?K6DFk4(qBnBHG@nTNF;Q027Cp7G>c0bNJWl$sh
zN){ciz!{6CeznB7k+qc|C5-h8T^Uc41J{SbsK=7at21#ZYHAPD+2Vr~aJa|#x+=9|
zC4Q5O+_z><lofEe`!g4UD8wR8So)C?Jj-t{`b5}pfm{;z@=-Y}M3#qZm5IWok2sLM
z<XQfsZK(*)IH{>GQXEF#yQ;q(6p*|h<+)#HfTZUtaI+~rw;_J5i-OzJSj@L%Zk5bJ
zGaKXHm(K!pe3;`ru{u6%k<cblf>_Z!MU(h1GK^3{D<ifMY@y=%c8l3|yL}n%dmzTM
z?P(9bBZw?awBdOV`$v}3)~r!vY(93s6?@6&#IeRe?P$pudsKQ2i|>)KfG%D{G@h%(
zcJ9F<a)%hrvpBv&&GCC2Da#^r2Y;<NS}IPwbQ7gadA|^sU!K5Yy9CX8L)0_n)25#z
zy6rcn5?gJ5cTH$WqQvl8DbM8scgqw>P%qbV3AH<ZM%!);a95&Np+FG>7kGa)vMNio
z!_npd3$P1~ph}5qV!VFZ606qnJQOtvJZ;X=DAB9L(fve9r&{xNR031E{$#S#wBJ2B
z=aRU+88U*wQmWXZ#1m_qbb-;3OzL-<P8u-6<v^4jni;4993*Apn$z-tczK`mxG!<l
z9%_)PXrsb`k2zmOj6`FZRa{XrXn&MfU+ugUKob(InwXHNoES5bYE9iS2)F{zKx#D`
z!m;!!K`5g{R8u`QEyr_qvzU=2YbKQtYd(~QHP<G90s=Z{0vLKs-S%f|=j>AjQVS|q
zk!o<sEYJ$5V6a3TPXS8GMSH%SzW~d=s40jX%@6iDI1GRJ0ZI|*2dO<?!Xa^|dM_6X
zK0hRN37)I7h~>vS&BoeC`cd@`Jro>IU?kU`Z%SR)b}8GpN={n;9&Cq-{p~h>!u4J1
zTZ^kp@PL45Q6<RdfN6U{a-yZbkCwvcKy01rhW?chEDvVr)A$^0Z!diN*3z%F|NO7E
z_lEwJbY=<X@!aqmzx2}aiI(!xAXB6B_eJ|wy7Nt$f!h`e3~PKA(+K;GZN|!6i*$tU
zDsE22v~GmJQR<-b<_p)jTp-iPPI1j55&dtbG<Q__OZIjLsaciN=arOxe8b&&jfzNM
z5<GC(S?vh<75{Nf!G+&sM%uSY5TVr3n6>_M-G@-6y#L9)^e~UMXTk?AMpf6jMQLA=
zM|%zG_2i^h>LaxE)Ac{A(C$@ZiYvzE(vUwyJ2<38)}3WKbh0Y1Z`wE=@X5yq`y8lh
zV7T9of<0%n`@-qE3aJpJF|Bv;juGfxZEJFnT1PMNAT{-rzTFoc?A?2k{IU0mn`dw-
z+7no*;c&_T*F#UI{iX!NnJq=DTz7Ipf8so2Qi#^V)D9UZ*KMV{xudao9Q8>GW|OQC
zCOwf<7*QU(xHBfps@>(fE`1=6vw;rICa$L<1n{2iLva4OYjW)OB2#Et2#tDY{Q+pg
zB{O)=6Lg9r1CL@eES#@e&s5j@?(g9A)IW=V7`sHy&qo^GslX{I7R)+9SW1W-pZ0EL
zP)pg227BiRP_MJsmS()>8HS^OIo+`WWP_!O_O;Kqadg;k)e5Tu6}XKSC7Tz8r?@(i
z_1MbV>qUg2iN;M+XfS7>r1l#5>0p4Avw^(-&)OYLUE20I;JC10@}Zt!iKCD}x*#e6
zO#F_&|Kz5>g+H4szckgV0EoU)E^1Z{8DptSPsoWfG`+=A#TBjR$Suz?wPY=+w6%*M
z)r1(zxsIz<5iXnAB%xWTt{I~0OpPdE_dSVi<FgG~DaF>08M~1}T_r1-+e``1Jipq-
zv+=*%*PiSssdtsiPYBEUNE~bXOtgox_++5utLr;;0!mv;Sp28LNFUt514d{mE!X@<
zXt|2nL-Pj)IpSV2T#2GyZaqM|f~43B3XT1NzJ65fD1&|ivqrV0v@9=Z)k7{TTsf9T
zrtg!ohp<F!UT1o5%`GKE&z=`4L%$3^$S~YFh1ySi^680<s<;q6A4=!@xLf4_Z7vtF
z{m5=Rh_Id?J>4jKjdP5wzFyo@skFI1SkMi*Xq`<_*4)(4XUn^bzmD5i0HouZ6o4?C
zUl6tRjRo^j?)FQfP4yARJn8mR;5o#y0~N@IqJmsfyQyw!9)eVms7S~@?`9M%1WVDO
zjB){n0po28ukdd+j3~ez2r-R33Wm03SD}XEA^6~+4j8J&%vq0TI`WXcogCOn>cCL|
z25EYb%5lW9J4M4@Em;Vb6jQ$lfkj*2>2N|7Lwdlm;ZgX~gBI^YV5wMcl}1xcgwE_s
z%#tlSlC+L1ma9+Wmr%P5`e*^G(Z`O_Oka9A%b`;+l>i^xfAG`9lwF6%EIM)P+aRO}
zJQsNggj=8m4{SmY`Pw&bR=<RR4fc*zz#VWAwH>ECe1r{=Sq}vu<lqt=AXIWgCXxrv
z(IJR>2&`!#*S;Bt26c}X1bQyUWII?=WN|HcHbH@;|KL5zv=a(5qNt2p?SC7fYkMNT
z8UUIMd>WVRuyrliY%fS|zs8K;n|rz&yYX0}XBF>VBsW2X5_vLV7ST}X;ZIVR74-Qg
zv7`ZmwQ4F`0!4W2A$_^HiE_}En*Tj{NzaZTaCBx#&{TCtkYGf%gXm=1-R%XRi5Az$
zKj{QjPnB3lsu{;2MQp>^=|;9M3L6>W`u;{%6J2Mkr7}IL#_;4LT8)(s-`VolxJrur
z>w}>@)1wQd-}|)h9P;-@AP83YE%XJTIB#rUn4M3>KiZn2Lj_d|i~$mJ$?ICP3!W9-
z0q1bYkfiwfMWtal?P%l^4)3BEfvvpIh(uLuE<tXmkVI_{R<iAVF%%)NpHzzT-Jydi
z*o3-(U4Cb65I78xfSaj%KyD0NP+>qMd!MSADvS*w1Q+O#L*0c;(#(Y%HcK<2Nl(cE
z+(41XDeI;o6o4486m<bdv)Tw9f(A~GP9yyab06aR1w@Fqy8<nMsqD=wgH{}F+?%0?
z0&rLKQ-CSV{hF)Y06hitW%pmqkOd++`;`5&vC<usC;<8eY8$vfRmakc4&;U7tKheL
zp{)SP5MP=T)oE1^@{TtPlEit5O8}ldcSv*8p;Ws!P<`Pv>><waH420jKxe%&<Xaj(
z#>TG>o8w2;thfCv*4Tqyzn+sxF^e>U+bb}CaatX3BphOb%$^jBxrtwXpZvpbL9B{U
z)Z6`7ef3DS^U)$i;);KVSa#UZ_4*yKN(FK47BbayR!lqih@zbj_hHiXQEYIX1afjW
zodX#h(^Y%*NXW*^Bs=~3owLn5=5eSi#%yOBq@lzduhbgT=I`=E9D3#E5nSAUxZ$WG
z(`s*9HHp0<|72&|;6Jn4;Il>Wf4Xd^0zly_v0!J7{5!c)tYXIkRu{VN^=ns1&FQ>L
zvpflMGdg2Qk*2J-6qwP0;WqYA!FI8!9TrSdobVBc-nK9X5-WUEw0%kr?qm`W{cl(4
z|I>jDWRJIZa2@IO9Xj^&I8G-Hv7!^px8iUZFI(2YuakxUa1MY+t;K73a}=6#t~D`&
zwjBjA(^Ax<6Z86X_5yOP`dAoMw8HO4>u``t3*-$Lt#hD_i$De(nHvE)Bxug)0io{C
z6~{_!&o6iPKQ5fUU%XH{Xgq(Zm=&bw`8&^t?0R*;X*6K^&_}obWYcV}#<M<^j=b!l
z$XsB{;i6^9x*JsNtK^oLhfxazaj`B+x3clI_k4z^bW6&1<=cC8A|ju0gd1q^1Huw?
zT7!52O8J0L0lAhrbaX*A96Jl3m-d}MpwTsm`=G%ZgbPTHxu{$JHO08CnFTkELl;bu
z*`a4diP@ohM1k3%cSL}By=$ok@eCyS0r42LbH}mc50uBT69px<>+FCA+IPN%nmcx`
zK$#pmUqM|QI>(?=CSTF^qB_G7Yoef`2s+V|p@<aG+Wv@#q7q*tzKKo<kQNr7yv`Xj
z3cRnkb)l5q@d{a>Eo?lU>MOvvJ5ri#DAgPLMKs@gpoB$StT%T0Iu<1B@QOuTu{X9~
zG}(4otT)F#UZI~QC{^}M(0ljMkLjAeYR4qoVb0zh^>~FCmLU2Yh1b6n)7Lrn!2j-T
z>phU36n1$spSOXVxqJ5KN&4q64}~wx!h|a8)McL~*slbAM+vn)`;PIl;OH+b{90V-
zcfa3QtKUSYe=jR_k-MV6&^+&FoqOGBaf|@=9a$NtzA2%u&&WPmBNKL!LdTb+lhs+k
zfaY}&%0)!~fFjczJ5TcbH!4^O_MHv%aeU$}B_9>uw@~bR^2{*hAlN1){#Bl}7ZggV
zH;&;_4!B(sPvE04GC@*-mknqkprIg6E~2A<O=-+JLFz={#2oUTV>5v`*|yEYE%Si{
z!I~#lj&*`JWL<3}2hf-dN0OyW5O<b32YNE`y$RWVuAhsaDGm>!-^@f^yTN2hKpRCF
z7zeC8mGMT{CBcXU$zC|t75_5TdW;OjP{`QTjco=PSHS0qC7!~A_%~-luf<@_R1uW3
z*LYXn40LJZKgrB}3QkjMKSkG>j_)(gJkjRO-t@R7*nL{t9BPv7{F3Ww<H#A$j8Ea!
z;BznXThGE%_=@bM&F3|CW}U~g>lB=(lF$4`?up3Fk-B;^2ptqXbT>cG8&s%mN%K7)
zy$+TO2%)6v(2!T_k90l-R<IQg+ScxAH}yK>0NL{QB8#o><P@!m*9QK5WLXcG`K;U&
zT#HP4@c3<Ke-+J@^95PO<HA$dT1MsBDQD@#v#?D&*sX<-7XRc__tEXA=B=u4IBH*}
z`5ug3zm&TOp;TQ~d!f~z;3OG4+375fs$(19r<p<C=jGZuWMt{usAW+vAFX9oeN$a~
zOS9>yHdmRKW(&r@Cp2uI>?|8@V&tq4CpGSTH_TVuSt;g$!#ILcu-npnDF53Fa}n&I
z%8c;ad7BxLyyU{?X2f@@vr_LkeS9@p?o^Xm_he*Wdj_e_>$-U;`L0{K_D`ybt8+`d
z)Z*w_&`d?~71i^fT(_j!G~5c-^lC3NCf_(6lRrfk&b!y*vL9SyJ9De(kfUDW71zWr
zB%6}{GjXX;ZXUQ;dg~_!ufFlAI@XPmRhu^<>w+R_9q#eJGC0EuIZ$0ygK&0C6Jdp%
z28xVl8ytjr9}IWYYeb{K)D?-;Y<7t!b~4F2-C~Y8xfXG`KV;AC7w3o1{q{d?ycm|1
zetNkf*-bX3yb%80^mEq3hv8|J`>n6oO0_I1Zl9-Un|+~r@Y&yXxN%y{*7L67@1Tj$
zMogP~Cg;R0H=m&reR<%ge5z<tu8U);9~x6>Dn?f^*4}(ha*2hUD-hzH<i`XhJ#7+e
z=!H+PhS>>Xb~-}Ems+Z7yPR5NL`^RpgdpR@0C}HPJVl!&#ez}DXnbgv9LW+7@%g!Z
zhgOy-_Hk5PFTUMU^w5PbUKI59?sLM!9Rs7I8eu|6Cv?u)iD)Dg9{5F-3sO%Hx8eI$
zG+D|07GRns@{_QyXDMrzR#i=mIG->duR~Pk*0J*~2$vkdEA!YU>Tq-9P3z7}vL#mH
zm+M(FoC2bKg6DXB-R!mvb83J{csq$`D3Eafq2?(lL{x~WoYZQ9m~+9!K;58SKEwg^
zI9DP)s$}eKWbR%p;ZhgflNMV%JJ1;u176osQBmNE6!no+n1H}+FJ5ym_QY_4htnLE
zZYfTMr6+9FL?_8q?=lX%Ea8MfIddkwG>}So*rphx5hguO?Lc(il>i2f$}DqYB|2mw
zbl-BvJ8dX^<#i>+N#@y2Rl}xCOsq-%p}v!;OP6A+5nr$b0zF(mVDE@&Pa5jQYlI!9
zN}6Gux|S|eHR3U1lG_jWC>Zp*;2>kJ3OqxYzE%bSL;zHl{S5Dkaf8a70Aw@{uA6NX
zA&Ci{nIgs#5Zb$>hG9D4Rl9~OH0oCjrL<p-y1Sd%M-^B!&&>kLBE9>bQ}K={nB1A~
zoZVt=*ErXTr;~?hvu3zJxXZeE427umK29?hMXxdzCFd*NFB&Eo7BA4owl>%n>Z3*C
zU@ryiBYu6q1sb)%#I=Ey;yHRuLKS6vBSalAM0$E@Znj$SeTm9Za=D;e^jkae#TL7u
zop{9Q*i=kkmFf(1#nC>BUR#aP*L>lAHec!>dMy@#iBr}q>C~VS?F?Mr8h6h3(<rF7
zDc00}OJZz2{T-01n!I*skD>1c8;nAltM4PK(Hw8o8Nj2PVLWOH2NyUJs=e(u@092#
kuJE&u`WfiUR27hTDGCa0|Gszr5C2aD{u6=!zasEo0Lh(cb^rhX

literal 0
HcmV?d00001

diff --git a/tests/fsck-tests/009-no-dir-item-or-index/default_case.img b/tests/fsck-tests/009-no-dir-item-or-index/inode_ref_without_dir_item_and_index.img
similarity index 100%
rename from tests/fsck-tests/009-no-dir-item-or-index/default_case.img
rename to tests/fsck-tests/009-no-dir-item-or-index/inode_ref_without_dir_item_and_index.img
-- 
2.18.0

Re: [PATCH v2 1/7] btrfs-progs: adjust arguments of btrfs_lookup_inode_extref()

[Qu Wenruo]

On 2018/9/13 上午4:49, damenly.su@gmail.com wrote:
> From: Su Yue <suy.fnst@cn.fujitsu.com>
> 
> The argument index is not used in btrfs_lookup_inode_extref(),
> so remove it.
> And adjust positions its arguments to make it consistent with
> kernel part.
> 
> No functional change.
> 
> Fixes: 260675657767 ("btrfs-progs: Import btrfs_insert/del/lookup_extref() functions.")
> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>

Looks good, just small nitpick on coding style.

After fixing that, feel free to add my tag:

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


> ---
>  ctree.h      | 6 +++---
>  inode-item.c | 6 +++---
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/ctree.h b/ctree.h
> index 4719962df67d..e7f6c5df95f1 100644
> --- a/ctree.h
> +++ b/ctree.h
> @@ -2708,9 +2708,9 @@ int btrfs_lookup_inode(struct btrfs_trans_handle *trans, struct btrfs_root
>  		       *root, struct btrfs_path *path,
>  		       struct btrfs_key *location, int mod);
>  struct btrfs_inode_extref *btrfs_lookup_inode_extref(struct btrfs_trans_handle
> -		*trans, struct btrfs_path *path, struct btrfs_root *root,
> -		u64 ino, u64 parent_ino, u64 index, const char *name,
> -		int namelen, int ins_len);
> +		*trans, struct btrfs_root *root, struct btrfs_path *path,

*trans and struct btrfs_trans_handle are in different lines, looks
pretty strange.

It would be much better to keep them in the same line, just what kernel
does.

> +		const char *name, int namelen, u64 ino, u64 parent_ino,
> +		int ins_len);
>  int btrfs_del_inode_extref(struct btrfs_trans_handle *trans,
>  			   struct btrfs_root *root,
>  			   const char *name, int name_len,
> diff --git a/inode-item.c b/inode-item.c
> index 1cc106670cd4..461557cb83d6 100644
> --- a/inode-item.c
> +++ b/inode-item.c
> @@ -228,9 +228,9 @@ static int btrfs_find_name_in_ext_backref(struct btrfs_path *path,
>  }
>  
>  struct btrfs_inode_extref *btrfs_lookup_inode_extref(struct btrfs_trans_handle
> -		*trans, struct btrfs_path *path, struct btrfs_root *root,
> -		u64 ino, u64 parent_ino, u64 index, const char *name,
> -		int namelen, int ins_len)
> +		*trans, struct btrfs_root *root, struct btrfs_path *path,

Same here.

Thanks,
Qu

> +		const char *name, int namelen, u64 ino, u64 parent_ino,
> +		int ins_len)
>  {
>  	struct btrfs_key key;
>  	struct btrfs_inode_extref *extref;
> 

Re: [PATCH v2 2/7] btrfs-progs: make btrfs_unlink() lookup inode_extref

[Qu Wenruo]

On 2018/9/13 上午4:49, damenly.su@gmail.com wrote:
> From: Su Yue <suy.fnst@cn.fujitsu.com>
> 
> btrfs_unlink() uses btrfs_lookup_inode_ref() to look up inode_ref
> but forget inode_extref case.
> 
> Let btrfs_unlink() call btrfs_lookup_inode_extref() if inode_ref is
> found and EXTENDED_IREF feature is enabled.
> 
> Fixes: 0cc75eddd093 ("btrfs-progs: Add btrfs_unlink() and btrfs_add_link() functions.")

My fault.

Indeed we need to check extref especially EXTENTD_IREF is already
default mkfs features.

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

Thanks,
Qu

> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
> ---
>  inode.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/inode.c b/inode.c
> index 2398bca4a109..598ad0ab6b4c 100644
> --- a/inode.c
> +++ b/inode.c
> @@ -277,6 +277,7 @@ int btrfs_unlink(struct btrfs_trans_handle *trans, struct btrfs_root *root,
>  	struct btrfs_key key;
>  	struct btrfs_inode_item *inode_item;
>  	struct btrfs_inode_ref *inode_ref;
> +	struct btrfs_inode_extref *inode_extref = NULL;
>  	struct btrfs_dir_item *dir_item;
>  	u64 inode_size;
>  	u32 nlinks;
> @@ -296,7 +297,18 @@ int btrfs_unlink(struct btrfs_trans_handle *trans, struct btrfs_root *root,
>  		ret = PTR_ERR(inode_ref);
>  		goto out;
>  	}
> -	if (inode_ref)
> +
> +	if (!inode_ref && btrfs_fs_incompat(root->fs_info, EXTENDED_IREF)) {
> +		btrfs_release_path(path);
> +		inode_extref = btrfs_lookup_inode_extref(trans, root, path,
> +					 name, namelen, ino, parent_ino, 0);
> +		if (IS_ERR(inode_extref)) {
> +			ret = PTR_ERR(inode_extref);
> +			goto out;
> +		}
> +	}
> +
> +	if (inode_ref || inode_extref)
>  		del_inode_ref = 1;
>  	btrfs_release_path(path);
>  
> 

Re: [PATCH v2 3/7] btrfs-progs: lowmem check: find dir_item by di_key in check_dir_item()

[Qu Wenruo]

On 2018/9/13 上午4:49, damenly.su@gmail.com wrote:
> From: Su Yue <suy.fnst@cn.fujitsu.com>
> 
> In check_dir_item, we are going to search corresponding
> dir_item/index.
> 
> Commit 564901eac7a4 ("btrfs-progs: check: introduce
> print_dir_item_err()") Changed argument name from key to di_key but
> forgot to change the key name for dir_item search.
> So @key shouldn't be used here. It should be @di_key.
> 
> Fixes: 564901eac7a4 ("btrfs-progs: check: introduce print_dir_item_err()")

Which also forgot to modify comment about parameters.

> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
> ---
>  check/mode-lowmem.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index 1bce44f5658a..89a304bbdd69 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -1658,7 +1658,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) {

To avoid such problem, I recommend to rename @key to @search_key, and
move the declaration inside the while loop.

Thanks,
Qu

>  			key.type = BTRFS_DIR_INDEX_KEY;
>  			key.offset = index;
>  		} else {
> 

Re: [PATCH v2 4/7] btrfs-progs: lowmem: search key of root again after check_fs_root() under repair

[Qu Wenruo]

On 2018/9/13 上午4:49, damenly.su@gmail.com wrote:
> From: Su Yue <suy.fnst@cn.fujitsu.com>
> 
> In check_fs_roots_lowmem(), we do search and follow the resulted path
> to call check_fs_root(), then call btrfs_next_item() to check next
> root.
> However, if repair is enabled, the root tree can be cowed, the
> existed path can cause strange errors.
> 
> Solution:
>   If repair, save the key before calling check_fs_root,
>   search the saved key again before checking next root.

Both reason and solution looks good.

> 
> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
> ---
>  check/mode-lowmem.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index 89a304bbdd69..8fc9edab1d66 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -4967,9 +4967,13 @@ int check_fs_roots_lowmem(struct btrfs_fs_info *fs_info)
>  	}
>  
>  	while (1) {
> +		struct btrfs_key saved_key;
> +
>  		node = path.nodes[0];
>  		slot = path.slots[0];
>  		btrfs_item_key_to_cpu(node, &key, slot);
> +		if (repair)
> +			saved_key = key;
>  		if (key.objectid > BTRFS_LAST_FREE_OBJECTID)
>  			goto out;
>  		if (key.type == BTRFS_ROOT_ITEM_KEY &&
> @@ -5000,6 +5004,17 @@ int check_fs_roots_lowmem(struct btrfs_fs_info *fs_info)
>  			err |= ret;
>  		}
>  next:
> +		/*
> +		 * Since root tree can be cowed during repair,
> +		 * here search the saved key again.
> +		 */
> +		if (repair) {
> +			btrfs_release_path(&path);
> +			ret = btrfs_search_slot(NULL, fs_info->tree_root,
> +						&saved_key, &path, 0, 0);
> +			/* Repair never deletes trees, search must succeed. */
> +			BUG_ON(ret);

But this doesn't look good to me.

Your assumption here is valid (at least for now), but it's possible that
some tree blocks get corrupted in a large root tree, and in that case,
we could still read part of the root tree, but btrfs_search_slot() could
still return -EIO for certain search key.

So I still prefer to do some error handling other than BUG_ON(ret).

Thanks,
Qu

> +		}
>  		ret = btrfs_next_item(tree_root, &path);
>  		if (ret > 0)
>  			goto out;
> 

Re: [PATCH v2 5/7] btrfs-progs: lowmem: continue to check item in last slot while checking inodes

[Qu Wenruo]

On 2018/9/13 上午4:49, damenly.su@gmail.com wrote:
> From: Su Yue <suy.fnst@cn.fujitsu.com>
> 
> After call of check_inode_item(), path may point to the last unchecked
> slot of the leaf. The outer walk_up_tree() always treats the position
> as checked item then skips to next item.

So check_inode_item() always set its path to *unchecked* slot, while
walk_up_tree() always think its path is set to *checked* slot.

Then this is indeed a problem.

> 
> If the last item was an inode item, yes, it was unchecked.
> Then walk_up_tree() will think the leaf is checked and walk up to
> upper node, process_one_leaf() in walk_down_tree() would skip to
> check next inode item. Which means, the inode item won't be checked.
> 
> Solution:
> After check_inode_item returns, if found path point to the last item
> of a leaf,

Would you please explain more about why last item makes a difference here?

>From previous statement, it looks like it's the difference in how
walk_up_tree() and check_inode_item() handles the path.
Not really related to last item.

Or did I miss something?

Thanks,
Qu

> decrease path slot manually, so walk_up_tree() will stay
> on the leaf.
> 
> Fixes: 5e2dc770471b ("btrfs-progs: check: skip shared node or leaf check for low_memory mode")
> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
> ---
>  check/mode-lowmem.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index 8fc9edab1d66..b6b33786d02b 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -2612,6 +2612,18 @@ again:
>  	if (cur->start == cur_bytenr)
>  		goto again;
>  
> +	/*
> +	 * path may point at the last item(a inode item maybe) in a leaf.
> +	 * Without below lines, walk_up_tree() will skip the item which
> +	 * means all items related to the inode will never be checked.
> +	 * Decrease the slot manually, walk_up_tree won't skip to next node
> +	 * if it occurs.
> +	 */
> +	if (path->slots[0] + 1 >= btrfs_header_nritems(path->nodes[0])) {
> +		if (path->slots[0])
> +			path->slots[0]--;
> +	}
> +
>  	/*
>  	 * we have switched to another leaf, above nodes may
>  	 * have changed, here walk down the path, if a node
> 

Re: [PATCH v2 6/7] btrfs-progs: lowmem: improve check_inode_extref()

[Qu Wenruo]

On 2018/9/13 上午4:49, damenly.su@gmail.com wrote:
> From: Su Yue <suy.fnst@cn.fujitsu.com>
> 
> inode_extref is much similar with inode_ref, most codes are reused in
> check_inode_extref().
> Exception:
> There is no need to check root directory, so remove it.
> Make check_inode_extref() verify hash value with key offset now.
> 
> And lowmem check can detect errors about inode_extref and try to
> repair errors.
> 
> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
> ---
>  check/mode-lowmem.c | 119 ++++++++++++++++++++++++++++++++++----------
>  1 file changed, 92 insertions(+), 27 deletions(-)
> 
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index b6b33786d02b..bec2ee185cc8 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -1182,37 +1182,79 @@ out:
>   *
>   * Return 0 if no error occurred.
>   */
> -static int check_inode_extref(struct btrfs_root *root,
> -			      struct btrfs_key *ref_key,
> -			      struct extent_buffer *node, int slot, u64 *refs,
> -			      int mode)
> +static int check_inode_extref(struct btrfs_root *root, struct btrfs_key *ref_key,
> +			      struct btrfs_path *path, char *name_ret,
> +			      u32 *namelen_ret, u64 *refs_ret, int mode)
>  {
>  	struct btrfs_key key;
>  	struct btrfs_key location;
>  	struct btrfs_inode_extref *extref;
> +	struct extent_buffer *node;
>  	char namebuf[BTRFS_NAME_LEN] = {0};
>  	u32 total;
> -	u32 cur = 0;
> +	u32 cur;
>  	u32 len;
>  	u32 name_len;
>  	u64 index;
>  	u64 parent;
> +	int err;
> +	int tmp_err;
>  	int ret;
> -	int err = 0;
> +	int slot;
> +	u64 refs;
> +	bool search_again = false;
>  
>  	location.objectid = ref_key->objectid;
>  	location.type = BTRFS_INODE_ITEM_KEY;
>  	location.offset = 0;
> +begin:
> +	cur = 0;
> +	err = 0;
> +	refs = *refs_ret;
> +	*namelen_ret = 0;
> +
> +	/* since after repair, path and the dir item may be changed */
> +	if (search_again) {
> +		search_again = false;
> +		btrfs_release_path(path);
> +		ret = btrfs_search_slot(NULL, root, ref_key, path, 0, 0);
> +		/*
> +		 * The item was deleted, let the path point to the last checked
> +		 * item.
> +		 */
> +		if (ret > 0) {
> +			if (path->slots[0] == 0) {
> +				ret = btrfs_prev_leaf(root, path);
> +				/*
> +				 * we are checking the inode item, there must
> +				 * be some items before, the case shall never
> +				 * happen.
> +				 */

This assumption looks valid by a quick glance, but doesn't stand in the
following case:
1) It's really the first INODE_ITEM.
2) Previous leaf/node is corrupted.

So we still need some error handling for it.

> +				BUG_ON(ret);
> +			} else {
> +				path->slots[0]--;
> +			}
> +			goto out;
> +		} else if (ret < 0) {
> +			err |= ret;
> +			goto out;
> +		}
> +	}
> +
> +	node = path->nodes[0];
> +	slot = path->slots[0];
>  
>  	extref = btrfs_item_ptr(node, slot, struct btrfs_inode_extref);
>  	total = btrfs_item_size_nr(node, slot);
>  
> -next:
> -	/* update inode ref count */
> -	(*refs)++;
> -	name_len = btrfs_inode_extref_name_len(node, extref);
> -	index = btrfs_inode_extref_index(node, extref);
> +loop:
> +	/* Update inode ref count */
> +	refs++;
> +	tmp_err = 0;
>  	parent = btrfs_inode_extref_parent(node, extref);
> +	index = btrfs_inode_extref_index(node, extref);
> +	name_len = btrfs_inode_extref_name_len(node, extref);
> +
>  	if (name_len <= BTRFS_NAME_LEN) {
>  		len = name_len;
>  	} else {
> @@ -1220,37 +1262,60 @@ next:
>  		warning("root %llu INODE_EXTREF[%llu %llu] name too long",
>  			root->objectid, ref_key->objectid, ref_key->offset);
>  	}
> +
>  	read_extent_buffer(node, namebuf, (unsigned long)(extref + 1), len);
>  
> -	/* Check root dir ref name */
> -	if (index == 0 && strncmp(namebuf, "..", name_len)) {
> -		error("root %llu INODE_EXTREF[%llu %llu] ROOT_DIR name shouldn't be %s",
> +	/* verify hash value */
> +	if (ref_key->offset != btrfs_extref_hash(parent, namebuf, len)) {
> +		err |= -EIO;

It's never a good idea to mix minus error number with bitmap errors.

Thanks,
Qu

> +		error("root %llu INODE_EXTREF[%llu %llu] name %s namelen %u mode %d mismatch with its hash, wanted %llu have %llu",
>  		      root->objectid, ref_key->objectid, ref_key->offset,
> -		      namebuf);
> -		err |= ROOT_DIR_ERROR;
> +		      namebuf, len, mode, ref_key->offset,
> +		      btrfs_extref_hash(parent, namebuf, len));
> +		goto out;
>  	}
>  
> -	/* find related dir_index */
> +	/* copy the first name found to name_ret */
> +	if (refs == 1 && name_ret) {
> +		memcpy(name_ret, namebuf, len);
> +		*namelen_ret = len;
> +	}
> +
> +	/* Find related DIR_INDEX */
>  	key.objectid = parent;
>  	key.type = BTRFS_DIR_INDEX_KEY;
>  	key.offset = index;
> -	ret = find_dir_item(root, &key, &location, namebuf, len, mode);
> -	err |= ret;
> +	tmp_err |= find_dir_item(root, &key, &location, namebuf, len,
> +			    imode_to_type(mode));
>  
> -	/* find related dir_item */
> +	/* Find related dir_item */
>  	key.objectid = parent;
>  	key.type = BTRFS_DIR_ITEM_KEY;
>  	key.offset = btrfs_name_hash(namebuf, len);
> -	ret = find_dir_item(root, &key, &location, namebuf, len, mode);
> -	err |= ret;
> +	tmp_err |= find_dir_item(root, &key, &location, namebuf, len,
> +			    imode_to_type(mode));
> +
> +	if (tmp_err && repair) {
> +		ret = repair_ternary_lowmem(root, parent, ref_key->objectid,
> +			    index, namebuf, name_len, imode_to_type(mode),
> +			    tmp_err);
> +		if (!ret) {
> +			search_again = true;
> +			goto begin;
> +		}
> +	}
>  
> +	print_inode_ref_err(root, ref_key, index, namebuf, name_len,
> +			    imode_to_type(mode), tmp_err);
> +
> +	err |= tmp_err;
>  	len = sizeof(*extref) + name_len;
>  	extref = (struct btrfs_inode_extref *)((char *)extref + len);
>  	cur += len;
> -
>  	if (cur < total)
> -		goto next;
> -
> +		goto loop;
> +out:
> +	*refs_ret = refs;
>  	return err;
>  }
>  
> @@ -2426,8 +2491,8 @@ static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path)
>  				warning("root %llu EXTREF[%llu %llu] isn't supported",
>  					root->objectid, key.objectid,
>  					key.offset);
> -			ret = check_inode_extref(root, &key, node, slot, &refs,
> -						 mode);
> +			ret = check_inode_extref(root, &key, path, namebuf,
> +						 &name_len, &refs, mode);
>  			err |= ret;
>  			break;
>  		}
> 

Re: [PATCH v2 7/7] btrfs-progs: fsck-tests: add test case inode_extref without dir_item and dir_index

[Qu Wenruo]

On 2018/9/13 上午4:49, damenly.su@gmail.com wrote:
> From: Su Yue <suy.fnst@cn.fujitsu.com>
> 
> This case contains an inode_extref:
> ==========================================
> ...
>    item 1 key (257 INODE_EXTREF 3460996356) itemoff 3947 itemsize 24
>                 index 257 parent 256 namelen 6 name: foo255
> ...
> ==========================================
> The related dir_item and dir_index are missing.
> 
> Add the case to ensure both original and lowmem mode check can handle
> the case of inode_extref.
> 
> Lowmem part is supported since patch named
>        'btrfs-progs: lowmem: improve check_inode_extref()'.
> 
> And rename default_case.img to inode_ref_without_dir_item_index.img.
> 
> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
> ---
>  .../inode_extref_without_dir_item_index.img     | Bin 0 -> 10240 bytes
>  ...=> inode_ref_without_dir_item_and_index.img} | Bin
>  2 files changed, 0 insertions(+), 0 deletions(-)
>  create mode 100644 tests/fsck-tests/009-no-dir-item-or-index/inode_extref_without_dir_item_index.img

Checked the image, it's normal INODE_REF with INODE_EXTREF. Looks good
by itself.

It could be even better, by providing image with INODE_EXTREF only
without any INODE_REF.

Thanks,
Qu

>  rename tests/fsck-tests/009-no-dir-item-or-index/{default_case.img => inode_ref_without_dir_item_and_index.img} (100%)
> 
> diff --git a/tests/fsck-tests/009-no-dir-item-or-index/inode_extref_without_dir_item_index.img b/tests/fsck-tests/009-no-dir-item-or-index/inode_extref_without_dir_item_index.img
> new file mode 100644
> index 0000000000000000000000000000000000000000..85f7a8211fe66dfa9b2b1f3a98e72f54cd402616
> GIT binary patch
> literal 10240
> zcmeHscQo8j8?UlhEOymZqDGCfdWp7rf~Y~1#Of{4q9>NMSe=L(Wkn}~5JZVy61@b`
> zOY|NsB<z*<ci-F3{qz2D&pT(%^L*whb7r3J`OZ8u6XTHLwZd)Lg7;qw*$v14SG@si
> z{}O`#MQ8s~cKv^0N5UJX{146m++Z;H#s?3t<S&8!gY*7k>wmBv=mszSgNy&l!H|F1
> z$NlwH{s)Ks?ejn7KN0v(1pZ$}z<*}eU&ND$kky0Zga5{R>1!Q|jBj!Kk_IdwP^`75
> zM#+ojWKDf@k7f}D9aT8qYmSi4F2VupuXr{D%*M11tY*yA57Z39C|MYjWMVhzVK5j1
> zieWerkiPZjcdzsBS22#CPt1DVI!`Y9h*_LYFoYww&j3lOEQw=`DnvA?wF8(}YD85$
> zG7$zaGL3dVC{eyOEl3W_-qsf~#zJSUyqf<-HM2v|!&%~jSR%lz|08~jx;7^e3~;sY
> zT^phzaQf-A9pzJu&sDmw_AnE#I*l;xF8RnBNniHgj!g0JPRgY|`Ok<a{Mq^vLSl&#
> zT==vTr(~q+;FBMuOG`o+hG&)q;T&{&zBb;Y>hQSmPV3fQT{G?g$ZX6e{@Ov^aKMAG
> zr|O5JwU)}O00QwdZZ<+yyfQ#~Ka9r6hA@3Oik}0|;rd7T)~&xIk460t)3|9Uli=;g
> zh~{}Qn(6<FU0Pt{f_C5)P)(K3JwH5PE8f!<&bb(~y;Tzz$`OkHCLq>qP?bLNIenC5
> zLO3=6UuM9&&4o2fB&4lmozz;AiUWx^ndhJ)O`@bM4QWxL!mC>|7xSNK`Sw)H$N)-m
> zSXz2)pA0>PYZPT59_=l(%tNBo?D|v)$%|6$`GKhK<Yx|_Z4h?!&ekU8CN7em<`<~c
> z3`TXnj3nB0HqI`I=#f5$3l_;9nqB$anY63#2cOROvo^B4lgXan6XVTQ><>+y93Fiy
> zTiWZzmC*aeUfXv1K)7a?^YiuO;WXH%#MegmjkD8b)YP2MzV{*|{#*5|R9Ov&-Q^d%
> z_IMBdg|z+-kf1lZG%Y^>3;_8eTVDY0gLuVuK^iH*1%Z5W_7lF*&eh~uM$6u+hgOC_
> z4rWyxVvQeo1hp}4g+fK$4!+0}!ov|G-z-7PF$8c+7m_drD7D<mkV->1!U-DzS0t`X
> z>Y@fV(K_9a{<$w^me4NJyvG7Sf}U~z9#&01Q3o5cr>bn8t5a2O%6)B<A`FcOJ5V(M
> z^Po>Wf6SW`JBx#V08j00K~k%COtKaNNL64F+f;tgKG=lY(fg3Y1(No%L^e>S=)3E(
> z&!A-jV%H0qu$NE*8j|<QFkAg_;Yu`kJB;TDF|ABDtMDtjNB&e2*tr0d;93REh?baW
> zJXGa7*BIuzqutPc#vY;Wdne2cv}U-aTSp=~9Zl%M&Ue@uo?r{oXa+;oE6suP&`d!i
> zP#H`@FWLwE;X3r-?mMs`bkeHA5!(ySKs*bn0(aIF=`f>3Wl=2@yv!W(Y57SB519_C
> z6)+t9-_I4%1fb5Z&csckFPKh{cpNOM?t%BnXTyFy@$oS?=g{%z#FR7*F40BkJ{o-r
> zeg|!2s2At!us$=cRn=-Ad2{xrofs7EHO~dqhBX`nd|fIgA*e6HJ^;GIet=Fzs|Guf
> zxQ2n&#dYN2A8l5#wAUxk;ou8NrqCx`Oc#>ow_1A&mZ!ALgX2PB!dwW;z>bFJXiw+c
> zypsFg$DKl>+n>1KhFaK2cxvlV`)R$kUYXhip;KvQ_#vs`?2wszPATytUWAA(sQSJH
> z^RMiYpTGHknufRKDjB%5pBN2SjvkV5L(I9UU1DFG7??kh5jsJqY6o_B73<X0_zI=Z
> z=b<CyX3d_xi$Kx{wY~`qj#8FskoeQ0EW`i&wQBZHFLps@!nHz8vmWZT*SB}&le{=x
> zNy#G@$&wB~tSqW8u|1QcLB#7K@o*ys6#g@NSr$&Tj<mgwzS2$m4m7sv+hG&HkA_`N
> zev{K9innXkt5g_V_#Zk=1%_gz$V-8bJzA9WD!4~hT#W*`05h-YqfgtdYlTv5Mifs9
> zH^b7@@<pqAEF_;f3}p<k`n{@oSg-pF$2upnc(D6DL%b&Kc+mQoYTOZOJ94FQ9GZLi
> znADEBe$Tw|<r;0iqW$ojcqtkrDA=#T_xpNXUS^n2@jh$R=<(Oh>bdweC+VkG5<x7>
> ztzX_b9lWd=Vzn<PBWXTcd6#`wGV?vY1muq^ziV~IXC<R5*GI#c8$z7pbVNGcNmHSS
> zuj&7`?C@F9@S)nwck=g{_wYUIjlW$m%3wKvc4cksjXGCT{Cj^=#$OMvYW`GmtyH*!
> zBRu&fBiV7*`ZUXD$G9_n6_Ooc;7+wL6qRwt*UkBqDzzH>c~SRNqWEoil+=D!ag|n-
> zV^-zuB>ELcdPa`Vc1d(LA~EzFlT|RepXaM<_ZQ&B?gu~2%>~O?mWu*ojfALeu&8rY
> z%(K7N_DOUrT=YUJZZ@L=U0(FX;pxMuNkvPDt$~o5dsYG#-1_04K`yn1Sj1qQyyU)u
> zvE-QOkSY++v*PYgE>*?e4q8%GvE@?)$zlkmkNxg6rFV14ha&=lZU@XI5tK3agYnuh
> zNl&@>6QbOf!&O)HQ{2|dd))AuM$=SF{g1$7-j)@?DAPh2O`AO0jeD=PM^~MXkJUt?
> zg!zRNe;G%|+^)Mfs=4CBFPv0W(4b>eMtgNz*aYYq-X&rPO8jLR4X3_a*l^z_g%+7?
> z#z~Y*GhPVg9A#rzk&Ci5oMS)wNGpABH)(}Fs=lB>cSV-}difg<|9)bFxp0>ib#Eb*
> zXH=MoY9X0ZIZX1lPC&i3>Avo0w7cHTw5>ZJkc7`dE1L@Mdl;H}&TNlnmp(t_N>G?|
> z^yzi_Ggk2#>$h<C^MPjXW<&&q_e&!q8_x<(o>ELTtthGbfOvGe`fBfOy5Bi3OdvIC
> zko*#aS2d@G!A8N`p?m7t$<cDWbUt;o|F`;pZ#zqWLN>3&`>n6@`X>2J>=><Zx%Ct6
> z<&;6PI~kT17x}V{FP5W;JT(u0!m}8J9oD)eh8t18V|_$77!gMuJ)<S#KH>IdgriB!
> zyPDn6G?7$$%#1Hy(ReG^dp%jL#2hmj@aK@-MKNSpj?#Jo@UUHd=T`c{ujnyN6Z(yN
> z<X<STO_8A7y^~py49dug+d?5t_|H`*Tjkv#EOld_XVV22o-VK%o{Za12RRT=zPD%%
> zJnr!e;IW>V09=VFyj>lbukif?9_n-pzQrmm`U(GZMSfRsU0y<VK9_m@YN4TNS!3sX
> z@3$Z5#dO6r?Qtjr^Mb4_?MdqSD6(-(_8f_mCtdWrRvMX7hCU(W=HTHqq&krVzquW`
> zXFYtaWpIV`-Mv<l_!2z-@4pEzIkZc<z31&Ej@}A4Y}!Q_Fq(Dh%RLVf!nHPC-`|*#
> z|F<O!kq-3JohQt>YL)e$aXtDIlz>mw0Djc$J;jVHiWu0|IM3vxB*w?rNR;7H%N*VO
> zS{xTS(0t!CczpJ00B#W?Ria4GHyG_Utydx(VG~SDOo8SHIYOa;HjpD0jbEmQm(CH(
> zUyVQDKM=`*@44(y()CYf_P^#gA<VE&XjL$Gt!b1X>d6%`Up5}k=<hG-+k{%iB6x+j
> zs>*mLbMZ>r<nQ~>><rPE+O`o2tI;3Ko%ZCGa?~`Aw(vxUwJV1}ZVv&I93&clI<Inw
> zSB|ADp7>fU;QY|H$`WYqi;J_DM`T>SNeD&YzXDRl9_3JVA*>sc3WKF;z8v))5WO+g
> zE5DldUY6BIg(3d86zre0(!VXb$I)hF+_tCC3@@c0(-pj<^_ls>%k4Oxa0<LFF(I9s
> z6@D(@1>H9~l0yI|nmaMo$^eKahAa=BIpo|d9PE9Z*6iRf$|2xFG&hv)I|QsY7E#6b
> zTAfIg-LO=R07{WWr5?`IP9zXaMug_0Gp-udKRDIV4lbHY9)5E@^=I|y<-tJE+^1uQ
> zAqBt3^DSWun1-Ikyn-+8yMlYBPaQ5N9x3ENnQkrjXhctOA=kC8I>~1jD9<uKY&B65
> z<*lv-xsLfxjUo@?8_G_x;mpB;)Nqq44CYsP-etUEHzyQxxU@BU9;5u5)ZB{VIDc0u
> zhMl+`^OZ04P+cL!<-(IhO+WT<Of?mKss!IDUrz0NN$tQsAi$pl1No&?1#~rWdU(Vz
> zoULg&p{o?7b`&6%7<|l|%K~&5P-!8_OA8PndbpkzsB@_dIEIML4!=%}hLyS*M*;WM
> ze#r%8pqYcK%i}i^FiVD3AwN>`mFU|eeU?{)r-I(1O+91yg*LSOnU;TruThVAnkI`B
> zzT`yhH*_nRtlh_`PEUBu6>~Nil0*z2Rf@3Dv{%a=vPSVzE7G(x10xw&Qma6Dx7F_o
> zqrL#I202NuAR0pbO8lrZpQY7cS?Rskz_*Z|+~E|D4^nMrjw{X(TFxwH8O;s>bkHmF
> zEJ(ZU$7Q4WDMO_@C^d9g3pqCw_}jEQmj*7VQmL(Z`^6)2?DO^#L&$C=QLeyhml&xC
> zCv7&<aUsxQ_5Il3J?e;6dPgIN`wbM!@4L0RCEBeD&+f^JpkhX=y2}VX7yG=ydEGPA
> zqyfOe54-MbvY11+V;j;^AjP&7@?K6DFk4(qBnBHG@nTNF;Q027Cp7G>c0bNJWl$sh
> zN){ciz!{6CeznB7k+qc|C5-h8T^Uc41J{SbsK=7at21#ZYHAPD+2Vr~aJa|#x+=9|
> zC4Q5O+_z><lofEe`!g4UD8wR8So)C?Jj-t{`b5}pfm{;z@=-Y}M3#qZm5IWok2sLM
> z<XQfsZK(*)IH{>GQXEF#yQ;q(6p*|h<+)#HfTZUtaI+~rw;_J5i-OzJSj@L%Zk5bJ
> zGaKXHm(K!pe3;`ru{u6%k<cblf>_Z!MU(h1GK^3{D<ifMY@y=%c8l3|yL}n%dmzTM
> z?P(9bBZw?awBdOV`$v}3)~r!vY(93s6?@6&#IeRe?P$pudsKQ2i|>)KfG%D{G@h%(
> zcJ9F<a)%hrvpBv&&GCC2Da#^r2Y;<NS}IPwbQ7gadA|^sU!K5Yy9CX8L)0_n)25#z
> zy6rcn5?gJ5cTH$WqQvl8DbM8scgqw>P%qbV3AH<ZM%!);a95&Np+FG>7kGa)vMNio
> z!_npd3$P1~ph}5qV!VFZ606qnJQOtvJZ;X=DAB9L(fve9r&{xNR031E{$#S#wBJ2B
> z=aRU+88U*wQmWXZ#1m_qbb-;3OzL-<P8u-6<v^4jni;4993*Apn$z-tczK`mxG!<l
> z9%_)PXrsb`k2zmOj6`FZRa{XrXn&MfU+ugUKob(InwXHNoES5bYE9iS2)F{zKx#D`
> z!m;!!K`5g{R8u`QEyr_qvzU=2YbKQtYd(~QHP<G90s=Z{0vLKs-S%f|=j>AjQVS|q
> zk!o<sEYJ$5V6a3TPXS8GMSH%SzW~d=s40jX%@6iDI1GRJ0ZI|*2dO<?!Xa^|dM_6X
> zK0hRN37)I7h~>vS&BoeC`cd@`Jro>IU?kU`Z%SR)b}8GpN={n;9&Cq-{p~h>!u4J1
> zTZ^kp@PL45Q6<RdfN6U{a-yZbkCwvcKy01rhW?chEDvVr)A$^0Z!diN*3z%F|NO7E
> z_lEwJbY=<X@!aqmzx2}aiI(!xAXB6B_eJ|wy7Nt$f!h`e3~PKA(+K;GZN|!6i*$tU
> zDsE22v~GmJQR<-b<_p)jTp-iPPI1j55&dtbG<Q__OZIjLsaciN=arOxe8b&&jfzNM
> z5<GC(S?vh<75{Nf!G+&sM%uSY5TVr3n6>_M-G@-6y#L9)^e~UMXTk?AMpf6jMQLA=
> zM|%zG_2i^h>LaxE)Ac{A(C$@ZiYvzE(vUwyJ2<38)}3WKbh0Y1Z`wE=@X5yq`y8lh
> zV7T9of<0%n`@-qE3aJpJF|Bv;juGfxZEJFnT1PMNAT{-rzTFoc?A?2k{IU0mn`dw-
> z+7no*;c&_T*F#UI{iX!NnJq=DTz7Ipf8so2Qi#^V)D9UZ*KMV{xudao9Q8>GW|OQC
> zCOwf<7*QU(xHBfps@>(fE`1=6vw;rICa$L<1n{2iLva4OYjW)OB2#Et2#tDY{Q+pg
> zB{O)=6Lg9r1CL@eES#@e&s5j@?(g9A)IW=V7`sHy&qo^GslX{I7R)+9SW1W-pZ0EL
> zP)pg227BiRP_MJsmS()>8HS^OIo+`WWP_!O_O;Kqadg;k)e5Tu6}XKSC7Tz8r?@(i
> z_1MbV>qUg2iN;M+XfS7>r1l#5>0p4Avw^(-&)OYLUE20I;JC10@}Zt!iKCD}x*#e6
> zO#F_&|Kz5>g+H4szckgV0EoU)E^1Z{8DptSPsoWfG`+=A#TBjR$Suz?wPY=+w6%*M
> z)r1(zxsIz<5iXnAB%xWTt{I~0OpPdE_dSVi<FgG~DaF>08M~1}T_r1-+e``1Jipq-
> zv+=*%*PiSssdtsiPYBEUNE~bXOtgox_++5utLr;;0!mv;Sp28LNFUt514d{mE!X@<
> zXt|2nL-Pj)IpSV2T#2GyZaqM|f~43B3XT1NzJ65fD1&|ivqrV0v@9=Z)k7{TTsf9T
> zrtg!ohp<F!UT1o5%`GKE&z=`4L%$3^$S~YFh1ySi^680<s<;q6A4=!@xLf4_Z7vtF
> z{m5=Rh_Id?J>4jKjdP5wzFyo@skFI1SkMi*Xq`<_*4)(4XUn^bzmD5i0HouZ6o4?C
> zUl6tRjRo^j?)FQfP4yARJn8mR;5o#y0~N@IqJmsfyQyw!9)eVms7S~@?`9M%1WVDO
> zjB){n0po28ukdd+j3~ez2r-R33Wm03SD}XEA^6~+4j8J&%vq0TI`WXcogCOn>cCL|
> z25EYb%5lW9J4M4@Em;Vb6jQ$lfkj*2>2N|7Lwdlm;ZgX~gBI^YV5wMcl}1xcgwE_s
> z%#tlSlC+L1ma9+Wmr%P5`e*^G(Z`O_Oka9A%b`;+l>i^xfAG`9lwF6%EIM)P+aRO}
> zJQsNggj=8m4{SmY`Pw&bR=<RR4fc*zz#VWAwH>ECe1r{=Sq}vu<lqt=AXIWgCXxrv
> z(IJR>2&`!#*S;Bt26c}X1bQyUWII?=WN|HcHbH@;|KL5zv=a(5qNt2p?SC7fYkMNT
> z8UUIMd>WVRuyrliY%fS|zs8K;n|rz&yYX0}XBF>VBsW2X5_vLV7ST}X;ZIVR74-Qg
> zv7`ZmwQ4F`0!4W2A$_^HiE_}En*Tj{NzaZTaCBx#&{TCtkYGf%gXm=1-R%XRi5Az$
> zKj{QjPnB3lsu{;2MQp>^=|;9M3L6>W`u;{%6J2Mkr7}IL#_;4LT8)(s-`VolxJrur
> z>w}>@)1wQd-}|)h9P;-@AP83YE%XJTIB#rUn4M3>KiZn2Lj_d|i~$mJ$?ICP3!W9-
> z0q1bYkfiwfMWtal?P%l^4)3BEfvvpIh(uLuE<tXmkVI_{R<iAVF%%)NpHzzT-Jydi
> z*o3-(U4Cb65I78xfSaj%KyD0NP+>qMd!MSADvS*w1Q+O#L*0c;(#(Y%HcK<2Nl(cE
> z+(41XDeI;o6o4486m<bdv)Tw9f(A~GP9yyab06aR1w@Fqy8<nMsqD=wgH{}F+?%0?
> z0&rLKQ-CSV{hF)Y06hitW%pmqkOd++`;`5&vC<usC;<8eY8$vfRmakc4&;U7tKheL
> zp{)SP5MP=T)oE1^@{TtPlEit5O8}ldcSv*8p;Ws!P<`Pv>><waH420jKxe%&<Xaj(
> z#>TG>o8w2;thfCv*4Tqyzn+sxF^e>U+bb}CaatX3BphOb%$^jBxrtwXpZvpbL9B{U
> z)Z6`7ef3DS^U)$i;);KVSa#UZ_4*yKN(FK47BbayR!lqih@zbj_hHiXQEYIX1afjW
> zodX#h(^Y%*NXW*^Bs=~3owLn5=5eSi#%yOBq@lzduhbgT=I`=E9D3#E5nSAUxZ$WG
> z(`s*9HHp0<|72&|;6Jn4;Il>Wf4Xd^0zly_v0!J7{5!c)tYXIkRu{VN^=ns1&FQ>L
> zvpflMGdg2Qk*2J-6qwP0;WqYA!FI8!9TrSdobVBc-nK9X5-WUEw0%kr?qm`W{cl(4
> z|I>jDWRJIZa2@IO9Xj^&I8G-Hv7!^px8iUZFI(2YuakxUa1MY+t;K73a}=6#t~D`&
> zwjBjA(^Ax<6Z86X_5yOP`dAoMw8HO4>u``t3*-$Lt#hD_i$De(nHvE)Bxug)0io{C
> z6~{_!&o6iPKQ5fUU%XH{Xgq(Zm=&bw`8&^t?0R*;X*6K^&_}obWYcV}#<M<^j=b!l
> z$XsB{;i6^9x*JsNtK^oLhfxazaj`B+x3clI_k4z^bW6&1<=cC8A|ju0gd1q^1Huw?
> zT7!52O8J0L0lAhrbaX*A96Jl3m-d}MpwTsm`=G%ZgbPTHxu{$JHO08CnFTkELl;bu
> z*`a4diP@ohM1k3%cSL}By=$ok@eCyS0r42LbH}mc50uBT69px<>+FCA+IPN%nmcx`
> zK$#pmUqM|QI>(?=CSTF^qB_G7Yoef`2s+V|p@<aG+Wv@#q7q*tzKKo<kQNr7yv`Xj
> z3cRnkb)l5q@d{a>Eo?lU>MOvvJ5ri#DAgPLMKs@gpoB$StT%T0Iu<1B@QOuTu{X9~
> zG}(4otT)F#UZI~QC{^}M(0ljMkLjAeYR4qoVb0zh^>~FCmLU2Yh1b6n)7Lrn!2j-T
> z>phU36n1$spSOXVxqJ5KN&4q64}~wx!h|a8)McL~*slbAM+vn)`;PIl;OH+b{90V-
> zcfa3QtKUSYe=jR_k-MV6&^+&FoqOGBaf|@=9a$NtzA2%u&&WPmBNKL!LdTb+lhs+k
> zfaY}&%0)!~fFjczJ5TcbH!4^O_MHv%aeU$}B_9>uw@~bR^2{*hAlN1){#Bl}7ZggV
> zH;&;_4!B(sPvE04GC@*-mknqkprIg6E~2A<O=-+JLFz={#2oUTV>5v`*|yEYE%Si{
> z!I~#lj&*`JWL<3}2hf-dN0OyW5O<b32YNE`y$RWVuAhsaDGm>!-^@f^yTN2hKpRCF
> z7zeC8mGMT{CBcXU$zC|t75_5TdW;OjP{`QTjco=PSHS0qC7!~A_%~-luf<@_R1uW3
> z*LYXn40LJZKgrB}3QkjMKSkG>j_)(gJkjRO-t@R7*nL{t9BPv7{F3Ww<H#A$j8Ea!
> z;BznXThGE%_=@bM&F3|CW}U~g>lB=(lF$4`?up3Fk-B;^2ptqXbT>cG8&s%mN%K7)
> zy$+TO2%)6v(2!T_k90l-R<IQg+ScxAH}yK>0NL{QB8#o><P@!m*9QK5WLXcG`K;U&
> zT#HP4@c3<Ke-+J@^95PO<HA$dT1MsBDQD@#v#?D&*sX<-7XRc__tEXA=B=u4IBH*}
> z`5ug3zm&TOp;TQ~d!f~z;3OG4+375fs$(19r<p<C=jGZuWMt{usAW+vAFX9oeN$a~
> zOS9>yHdmRKW(&r@Cp2uI>?|8@V&tq4CpGSTH_TVuSt;g$!#ILcu-npnDF53Fa}n&I
> z%8c;ad7BxLyyU{?X2f@@vr_LkeS9@p?o^Xm_he*Wdj_e_>$-U;`L0{K_D`ybt8+`d
> z)Z*w_&`d?~71i^fT(_j!G~5c-^lC3NCf_(6lRrfk&b!y*vL9SyJ9De(kfUDW71zWr
> zB%6}{GjXX;ZXUQ;dg~_!ufFlAI@XPmRhu^<>w+R_9q#eJGC0EuIZ$0ygK&0C6Jdp%
> z28xVl8ytjr9}IWYYeb{K)D?-;Y<7t!b~4F2-C~Y8xfXG`KV;AC7w3o1{q{d?ycm|1
> zetNkf*-bX3yb%80^mEq3hv8|J`>n6oO0_I1Zl9-Un|+~r@Y&yXxN%y{*7L67@1Tj$
> zMogP~Cg;R0H=m&reR<%ge5z<tu8U);9~x6>Dn?f^*4}(ha*2hUD-hzH<i`XhJ#7+e
> z=!H+PhS>>Xb~-}Ems+Z7yPR5NL`^RpgdpR@0C}HPJVl!&#ez}DXnbgv9LW+7@%g!Z
> zhgOy-_Hk5PFTUMU^w5PbUKI59?sLM!9Rs7I8eu|6Cv?u)iD)Dg9{5F-3sO%Hx8eI$
> zG+D|07GRns@{_QyXDMrzR#i=mIG->duR~Pk*0J*~2$vkdEA!YU>Tq-9P3z7}vL#mH
> zm+M(FoC2bKg6DXB-R!mvb83J{csq$`D3Eafq2?(lL{x~WoYZQ9m~+9!K;58SKEwg^
> zI9DP)s$}eKWbR%p;ZhgflNMV%JJ1;u176osQBmNE6!no+n1H}+FJ5ym_QY_4htnLE
> zZYfTMr6+9FL?_8q?=lX%Ea8MfIddkwG>}So*rphx5hguO?Lc(il>i2f$}DqYB|2mw
> zbl-BvJ8dX^<#i>+N#@y2Rl}xCOsq-%p}v!;OP6A+5nr$b0zF(mVDE@&Pa5jQYlI!9
> zN}6Gux|S|eHR3U1lG_jWC>Zp*;2>kJ3OqxYzE%bSL;zHl{S5Dkaf8a70Aw@{uA6NX
> zA&Ci{nIgs#5Zb$>hG9D4Rl9~OH0oCjrL<p-y1Sd%M-^B!&&>kLBE9>bQ}K={nB1A~
> zoZVt=*ErXTr;~?hvu3zJxXZeE427umK29?hMXxdzCFd*NFB&Eo7BA4owl>%n>Z3*C
> zU@ryiBYu6q1sb)%#I=Ey;yHRuLKS6vBSalAM0$E@Znj$SeTm9Za=D;e^jkae#TL7u
> zop{9Q*i=kkmFf(1#nC>BUR#aP*L>lAHec!>dMy@#iBr}q>C~VS?F?Mr8h6h3(<rF7
> zDc00}OJZz2{T-01n!I*skD>1c8;nAltM4PK(Hw8o8Nj2PVLWOH2NyUJs=e(u@092#
> kuJE&u`WfiUR27hTDGCa0|Gszr5C2aD{u6=!zasEo0Lh(cb^rhX
> 
> literal 0
> HcmV?d00001
> 
> diff --git a/tests/fsck-tests/009-no-dir-item-or-index/default_case.img b/tests/fsck-tests/009-no-dir-item-or-index/inode_ref_without_dir_item_and_index.img
> similarity index 100%
> rename from tests/fsck-tests/009-no-dir-item-or-index/default_case.img
> rename to tests/fsck-tests/009-no-dir-item-or-index/inode_ref_without_dir_item_and_index.img
> 

Re: [PATCH v2 3/7] btrfs-progs: lowmem check: find dir_item by di_key in check_dir_item()

[Su Yue]

On 09/14/2018 07:33 AM, Qu Wenruo wrote:
> 
> 
> On 2018/9/13 上午4:49, damenly.su@gmail.com wrote:
>> From: Su Yue <suy.fnst@cn.fujitsu.com>
>>
>> In check_dir_item, we are going to search corresponding
>> dir_item/index.
>>
>> Commit 564901eac7a4 ("btrfs-progs: check: introduce
>> print_dir_item_err()") Changed argument name from key to di_key but
>> forgot to change the key name for dir_item search.
>> So @key shouldn't be used here. It should be @di_key.
>>
>> Fixes: 564901eac7a4 ("btrfs-progs: check: introduce print_dir_item_err()")
> 
> Which also forgot to modify comment about parameters.
Oh....Noticed
> 
>> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
>> ---
>>   check/mode-lowmem.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
>> index 1bce44f5658a..89a304bbdd69 100644
>> --- a/check/mode-lowmem.c
>> +++ b/check/mode-lowmem.c
>> @@ -1658,7 +1658,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) {
> 
> To avoid such problem, I recommend to rename @key to @search_key, and
> move the declaration inside the while loop.
> 
Nice suggestion. Will do it in next version.

Thanks,
Su
> Thanks,
> Qu
> 
>>   			key.type = BTRFS_DIR_INDEX_KEY;
>>   			key.offset = index;
>>   		} else {
>>
> 
> 

Re: [PATCH v2 4/7] btrfs-progs: lowmem: search key of root again after check_fs_root() under repair

[Su Yue]

On 09/14/2018 07:37 AM, Qu Wenruo wrote:
> 
> 
> On 2018/9/13 上午4:49, damenly.su@gmail.com wrote:
>> From: Su Yue <suy.fnst@cn.fujitsu.com>
>>
>> In check_fs_roots_lowmem(), we do search and follow the resulted path
>> to call check_fs_root(), then call btrfs_next_item() to check next
>> root.
>> However, if repair is enabled, the root tree can be cowed, the
>> existed path can cause strange errors.
>>
>> Solution:
>>    If repair, save the key before calling check_fs_root,
>>    search the saved key again before checking next root.
> 
> Both reason and solution looks good.
> 
>>
>> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
>> ---
>>   check/mode-lowmem.c | 15 +++++++++++++++
>>   1 file changed, 15 insertions(+)
>>
>> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
>> index 89a304bbdd69..8fc9edab1d66 100644
>> --- a/check/mode-lowmem.c
>> +++ b/check/mode-lowmem.c
>> @@ -4967,9 +4967,13 @@ int check_fs_roots_lowmem(struct btrfs_fs_info *fs_info)
>>   	}
>>   
>>   	while (1) {
>> +		struct btrfs_key saved_key;
>> +
>>   		node = path.nodes[0];
>>   		slot = path.slots[0];
>>   		btrfs_item_key_to_cpu(node, &key, slot);
>> +		if (repair)
>> +			saved_key = key;
>>   		if (key.objectid > BTRFS_LAST_FREE_OBJECTID)
>>   			goto out;
>>   		if (key.type == BTRFS_ROOT_ITEM_KEY &&
>> @@ -5000,6 +5004,17 @@ int check_fs_roots_lowmem(struct btrfs_fs_info *fs_info)
>>   			err |= ret;
>>   		}
>>   next:
>> +		/*
>> +		 * Since root tree can be cowed during repair,
>> +		 * here search the saved key again.
>> +		 */
>> +		if (repair) {
>> +			btrfs_release_path(&path);
>> +			ret = btrfs_search_slot(NULL, fs_info->tree_root,
>> +						&saved_key, &path, 0, 0);
>> +			/* Repair never deletes trees, search must succeed. */
>> +			BUG_ON(ret);
> 
> But this doesn't look good to me.
> 
> Your assumption here is valid (at least for now), but it's possible that
> some tree blocks get corrupted in a large root tree, and in that case,
> we could still read part of the root tree, but btrfs_search_slot() could
> still return -EIO for certain search key.
> 
> So I still prefer to do some error handling other than BUG_ON(ret).
> 
Okay, will try it.

Thanks,
Su
> Thanks,
> Qu
> 
>> +		}
>>   		ret = btrfs_next_item(tree_root, &path);
>>   		if (ret > 0)
>>   			goto out;
>>
> 
> 

Re: [PATCH v2 5/7] btrfs-progs: lowmem: continue to check item in last slot while checking inodes

[Su Yue]

On 09/14/2018 07:43 AM, Qu Wenruo wrote:
> 
> 
> On 2018/9/13 上午4:49, damenly.su@gmail.com wrote:
>> From: Su Yue <suy.fnst@cn.fujitsu.com>
>>
>> After call of check_inode_item(), path may point to the last unchecked
>> slot of the leaf. The outer walk_up_tree() always treats the position
>> as checked item then skips to next item.
> 
> So check_inode_item() always set its path to *unchecked* slot, while
> walk_up_tree() always think its path is set to *checked* slot.
> 
> Then this is indeed a problem.
> 
Yep, that's the case.

>>
>> If the last item was an inode item, yes, it was unchecked.
>> Then walk_up_tree() will think the leaf is checked and walk up to
>> upper node, process_one_leaf() in walk_down_tree() would skip to
>> check next inode item. Which means, the inode item won't be checked.
>>
>> Solution:
>> After check_inode_item returns, if found path point to the last item
>> of a leaf,
> 
> Would you please explain more about why last item makes a difference here?
> 
>  From previous statement, it looks like it's the difference in how
> walk_up_tree() and check_inode_item() handles the path.
> Not really related to last item.
>
Yes, the change is tricky. The core problem is walk_up_tree() will
skip to other nodes. Decreament of slot will let path still
point to the leaf after  walk_up_tree() returns.
Or we must change logical of walk_up_tree() or check_inode_item().

BTW,now process_one_leaf() checks inode_item from first inode_item
or second item with different ino. Change it start from last slot
should be the right way.

Thanks,
Su


Thanks,
Su
> Or did I miss something?
> 
> Thanks,
> Qu
> 
>> decrease path slot manually, so walk_up_tree() will stay
>> on the leaf.
>>
>> Fixes: 5e2dc770471b ("btrfs-progs: check: skip shared node or leaf check for low_memory mode")
>> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
>> ---
>>   check/mode-lowmem.c | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
>> index 8fc9edab1d66..b6b33786d02b 100644
>> --- a/check/mode-lowmem.c
>> +++ b/check/mode-lowmem.c
>> @@ -2612,6 +2612,18 @@ again:
>>   	if (cur->start == cur_bytenr)
>>   		goto again;
>>   
>> +	/*
>> +	 * path may point at the last item(a inode item maybe) in a leaf.
>> +	 * Without below lines, walk_up_tree() will skip the item which
>> +	 * means all items related to the inode will never be checked.
>> +	 * Decrease the slot manually, walk_up_tree won't skip to next node
>> +	 * if it occurs.
>> +	 */
>> +	if (path->slots[0] + 1 >= btrfs_header_nritems(path->nodes[0])) {
>> +		if (path->slots[0])
>> +			path->slots[0]--;
>> +	}
>> +
>>   	/*
>>   	 * we have switched to another leaf, above nodes may
>>   	 * have changed, here walk down the path, if a node
>>
> 
> 

Re: [PATCH v2 4/7] btrfs-progs: lowmem: search key of root again after check_fs_root() under repair

[Nikolay Borisov]

On 14.09.2018 03:58, Su Yue wrote:
> 
> 
> On 09/14/2018 07:37 AM, Qu Wenruo wrote:
>>
>>
>> On 2018/9/13 上午4:49, damenly.su@gmail.com wrote:
>>> From: Su Yue <suy.fnst@cn.fujitsu.com>
>>>
>>> In check_fs_roots_lowmem(), we do search and follow the resulted path
>>> to call check_fs_root(), then call btrfs_next_item() to check next
>>> root.
>>> However, if repair is enabled, the root tree can be cowed, the
>>> existed path can cause strange errors.
>>>
>>> Solution:
>>>    If repair, save the key before calling check_fs_root,
>>>    search the saved key again before checking next root.
>>
>> Both reason and solution looks good.
>>
>>>
>>> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
>>> ---
>>>   check/mode-lowmem.c | 15 +++++++++++++++
>>>   1 file changed, 15 insertions(+)
>>>
>>> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
>>> index 89a304bbdd69..8fc9edab1d66 100644
>>> --- a/check/mode-lowmem.c
>>> +++ b/check/mode-lowmem.c
>>> @@ -4967,9 +4967,13 @@ int check_fs_roots_lowmem(struct btrfs_fs_info
>>> *fs_info)
>>>       }
>>>         while (1) {
>>> +        struct btrfs_key saved_key;
>>> +
>>>           node = path.nodes[0];
>>>           slot = path.slots[0];
>>>           btrfs_item_key_to_cpu(node, &key, slot);
>>> +        if (repair)
>>> +            saved_key = key;
>>>           if (key.objectid > BTRFS_LAST_FREE_OBJECTID)
>>>               goto out;
>>>           if (key.type == BTRFS_ROOT_ITEM_KEY &&
>>> @@ -5000,6 +5004,17 @@ int check_fs_roots_lowmem(struct btrfs_fs_info
>>> *fs_info)
>>>               err |= ret;
>>>           }
>>>   next:
>>> +        /*
>>> +         * Since root tree can be cowed during repair,
>>> +         * here search the saved key again.
>>> +         */
>>> +        if (repair) {
>>> +            btrfs_release_path(&path);
>>> +            ret = btrfs_search_slot(NULL, fs_info->tree_root,
>>> +                        &saved_key, &path, 0, 0);
>>> +            /* Repair never deletes trees, search must succeed. */
>>> +            BUG_ON(ret);
>>
>> But this doesn't look good to me.
>>
>> Your assumption here is valid (at least for now), but it's possible that
>> some tree blocks get corrupted in a large root tree, and in that case,
>> we could still read part of the root tree, but btrfs_search_slot() could
>> still return -EIO for certain search key.
>>
>> So I still prefer to do some error handling other than BUG_ON(ret).
>>
> Okay, will try it.

Just to emphasize Qu's point - we should strive to remove existing
BUG_ON and should never introduce new ones. btrfs-progs is already quite
messy and we should be improving that.

> 
> Thanks,
> Su
>> Thanks,
>> Qu
>>
>>> +        }
>>>           ret = btrfs_next_item(tree_root, &path);
>>>           if (ret > 0)
>>>               goto out;
>>>
>>
>>
> 
> 
> 

Re: [PATCH v2 4/7] btrfs-progs: lowmem: search key of root again after check_fs_root() under repair

[Su Yue]

On 09/14/2018 02:27 PM, Nikolay Borisov wrote:
> 
> 
> On 14.09.2018 03:58, Su Yue wrote:
>>
>>
>> On 09/14/2018 07:37 AM, Qu Wenruo wrote:
>>>
>>>
>>> On 2018/9/13 上午4:49, damenly.su@gmail.com wrote:
>>>> From: Su Yue <suy.fnst@cn.fujitsu.com>
>>>>
>>>> In check_fs_roots_lowmem(), we do search and follow the resulted path
>>>> to call check_fs_root(), then call btrfs_next_item() to check next
>>>> root.
>>>> However, if repair is enabled, the root tree can be cowed, the
>>>> existed path can cause strange errors.
>>>>
>>>> Solution:
>>>>     If repair, save the key before calling check_fs_root,
>>>>     search the saved key again before checking next root.
>>>
>>> Both reason and solution looks good.
>>>
>>>>
>>>> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
>>>> ---
>>>>    check/mode-lowmem.c | 15 +++++++++++++++
>>>>    1 file changed, 15 insertions(+)
>>>>
>>>> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
>>>> index 89a304bbdd69..8fc9edab1d66 100644
>>>> --- a/check/mode-lowmem.c
>>>> +++ b/check/mode-lowmem.c
>>>> @@ -4967,9 +4967,13 @@ int check_fs_roots_lowmem(struct btrfs_fs_info
>>>> *fs_info)
>>>>        }
>>>>          while (1) {
>>>> +        struct btrfs_key saved_key;
>>>> +
>>>>            node = path.nodes[0];
>>>>            slot = path.slots[0];
>>>>            btrfs_item_key_to_cpu(node, &key, slot);
>>>> +        if (repair)
>>>> +            saved_key = key;
>>>>            if (key.objectid > BTRFS_LAST_FREE_OBJECTID)
>>>>                goto out;
>>>>            if (key.type == BTRFS_ROOT_ITEM_KEY &&
>>>> @@ -5000,6 +5004,17 @@ int check_fs_roots_lowmem(struct btrfs_fs_info
>>>> *fs_info)
>>>>                err |= ret;
>>>>            }
>>>>    next:
>>>> +        /*
>>>> +         * Since root tree can be cowed during repair,
>>>> +         * here search the saved key again.
>>>> +         */
>>>> +        if (repair) {
>>>> +            btrfs_release_path(&path);
>>>> +            ret = btrfs_search_slot(NULL, fs_info->tree_root,
>>>> +                        &saved_key, &path, 0, 0);
>>>> +            /* Repair never deletes trees, search must succeed. */
>>>> +            BUG_ON(ret);
>>>
>>> But this doesn't look good to me.
>>>
>>> Your assumption here is valid (at least for now), but it's possible that
>>> some tree blocks get corrupted in a large root tree, and in that case,
>>> we could still read part of the root tree, but btrfs_search_slot() could
>>> still return -EIO for certain search key.
>>>
>>> So I still prefer to do some error handling other than BUG_ON(ret).
>>>
>> Okay, will try it.
> 
> Just to emphasize Qu's point - we should strive to remove existing
> BUG_ON and should never introduce new ones. btrfs-progs is already quite
> messy and we should be improving that.
> 
Understood, thanks for your emphasis．

Su
>>
>> Thanks,
>> Su
>>> Thanks,
>>> Qu
>>>
>>>> +        }
>>>>            ret = btrfs_next_item(tree_root, &path);
>>>>            if (ret > 0)
>>>>                goto out;
>>>>
>>>
>>>
>>
>>
>>
> 
>