[PATCH 0/4] btrfs: fix missing error handling when replaying directory entries

[PATCH 0/4] btrfs: fix missing error handling when replaying directory entries

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

Some places during log replay are ignoring errors when looking up directory
entries in a subvolume/fs tree. This fixes all those places and makes sure
we get consistent error hanlding when looking dir items and dir index items.

Filipe Manana (4):
  btrfs: deal with errors when checking if a dir entry exists during log replay
  btrfs: deal with errors when replaying dir entry during log replay
  btrfs: deal with errors when adding inode reference during log replay
  btrfs: unify lookup return value when dir entry is missing

 fs/btrfs/ctree.h    |  2 +-
 fs/btrfs/dir-item.c | 48 +++++++++++++++++++++++++--------
 fs/btrfs/tree-log.c | 65 ++++++++++++++++++++++++++++-----------------
 3 files changed, 79 insertions(+), 36 deletions(-)

-- 
2.33.0

[PATCH 1/4] btrfs: deal with errors when checking if a dir entry exists during log replay

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

Currently inode_in_dir() ignores errors returned from
btrfs_lookup_dir_index_item() and from btrfs_lookup_dir_item(), treating
any errors as if the directory entry does not exists in the fs/subvolume
tree, which is obviously not correct, as we can get errors such as -EIO
when reading extent buffers while searching the fs/subvolume's tree.

Fix that by making inode_in_dir() return the errors and making its only
caller, add_inode_ref(), deal with returned errors as well.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/tree-log.c | 47 ++++++++++++++++++++++++++++-----------------
 1 file changed, 29 insertions(+), 18 deletions(-)

diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index b765ca7536fe..79d7cca704fb 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -967,9 +967,11 @@ static noinline int drop_one_dir_item(struct btrfs_trans_handle *trans,
 }
 
 /*
- * helper function to see if a given name and sequence number found
- * in an inode back reference are already in a directory and correctly
- * point to this inode
+ * Helper function to see if a given name and sequence number found in an inode
+ * back reference are already in a directory and correctly point to this inode.
+ *
+ * Returns: < 0 on error, 0 if the directory entry does not exists and 1 if it
+ * exists.
  */
 static noinline int inode_in_dir(struct btrfs_root *root,
 				 struct btrfs_path *path,
@@ -978,29 +980,35 @@ static noinline int inode_in_dir(struct btrfs_root *root,
 {
 	struct btrfs_dir_item *di;
 	struct btrfs_key location;
-	int match = 0;
+	int ret = 0;
 
 	di = btrfs_lookup_dir_index_item(NULL, root, path, dirid,
 					 index, name, name_len, 0);
-	if (di && !IS_ERR(di)) {
+	if (IS_ERR(di)) {
+		if (PTR_ERR(di) != -ENOENT)
+			ret = PTR_ERR(di);
+		goto out;
+	} else if (di) {
 		btrfs_dir_item_key_to_cpu(path->nodes[0], di, &location);
 		if (location.objectid != objectid)
 			goto out;
-	} else
+	} else {
 		goto out;
-	btrfs_release_path(path);
+	}
 
+	btrfs_release_path(path);
 	di = btrfs_lookup_dir_item(NULL, root, path, dirid, name, name_len, 0);
-	if (di && !IS_ERR(di)) {
-		btrfs_dir_item_key_to_cpu(path->nodes[0], di, &location);
-		if (location.objectid != objectid)
-			goto out;
-	} else
+	if (IS_ERR(di)) {
+		ret = PTR_ERR(di);
 		goto out;
-	match = 1;
+	} else if (di) {
+		btrfs_dir_item_key_to_cpu(path->nodes[0], di, &location);
+		if (location.objectid == objectid)
+			ret = 1;
+	}
 out:
 	btrfs_release_path(path);
-	return match;
+	return ret;
 }
 
 /*
@@ -1545,10 +1553,12 @@ static noinline int add_inode_ref(struct btrfs_trans_handle *trans,
 		if (ret)
 			goto out;
 
-		/* if we already have a perfect match, we're done */
-		if (!inode_in_dir(root, path, btrfs_ino(BTRFS_I(dir)),
-					btrfs_ino(BTRFS_I(inode)), ref_index,
-					name, namelen)) {
+		ret = inode_in_dir(root, path, btrfs_ino(BTRFS_I(dir)),
+				   btrfs_ino(BTRFS_I(inode)), ref_index,
+				   name, namelen);
+		if (ret < 0) {
+			goto out;
+		} else if (ret == 0) {
 			/*
 			 * look for a conflicting back reference in the
 			 * metadata. if we find one we have to unlink that name
@@ -1608,6 +1618,7 @@ static noinline int add_inode_ref(struct btrfs_trans_handle *trans,
 			if (ret)
 				goto out;
 		}
+		/* Else, ret == 1, we already have a perfect match, we're done. */
 
 		ref_ptr = (unsigned long)(ref_ptr + ref_struct_size) + namelen;
 		kfree(name);
-- 
2.33.0

[PATCH 2/4] btrfs: deal with errors when replaying dir entry during log replay

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

At replay_one_one(), we are treating any error returned from
btrfs_lookup_dir_item() or from btrfs_lookup_dir_index_item() as meaning
that there is no existing directory entry in the fs/subvolume tree.
This is not correct since we can get errors such as, for example, -EIO
when reading extent buffers while searching the fs/subvolume's btree.

So fix that and return the error to the caller when it is not -ENOENT.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/tree-log.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 79d7cca704fb..61bcd09feedc 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -2016,7 +2016,14 @@ static noinline int replay_one_name(struct btrfs_trans_handle *trans,
 		ret = -EINVAL;
 		goto out;
 	}
-	if (IS_ERR_OR_NULL(dst_di)) {
+
+	if (dst_di == ERR_PTR(-ENOENT))
+		dst_di = NULL;
+
+	if (IS_ERR(dst_di)) {
+		ret = PTR_ERR(dst_di);
+		goto out;
+	} else if (!dst_di) {
 		/* we need a sequence number to insert, so we only
 		 * do inserts for the BTRFS_DIR_INDEX_KEY types
 		 */
-- 
2.33.0

[PATCH 3/4] btrfs: deal with errors when adding inode reference during log replay

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

At __inode_add_ref(), we treating any error returned from
btrfs_lookup_dir_item() or from btrfs_lookup_dir_index_item() as meaning
that there is no existing directory entry in the fs/subvolume tree.
This is not correct since we can get errors such as, for example, -EIO
when reading extent buffers while searching the fs/subvolume's btree.

So fix that and return the error to the caller when it is not -ENOENT.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/tree-log.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 61bcd09feedc..0091ec39f57c 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -1218,7 +1218,10 @@ static inline int __add_inode_ref(struct btrfs_trans_handle *trans,
 	/* look for a conflicting sequence number */
 	di = btrfs_lookup_dir_index_item(trans, root, path, btrfs_ino(dir),
 					 ref_index, name, namelen, 0);
-	if (di && !IS_ERR(di)) {
+	if (IS_ERR(di)) {
+		if (PTR_ERR(di) != -ENOENT)
+			return PTR_ERR(di);
+	} else if (di) {
 		ret = drop_one_dir_item(trans, root, path, dir, di);
 		if (ret)
 			return ret;
@@ -1228,7 +1231,9 @@ static inline int __add_inode_ref(struct btrfs_trans_handle *trans,
 	/* look for a conflicting name */
 	di = btrfs_lookup_dir_item(trans, root, path, btrfs_ino(dir),
 				   name, namelen, 0);
-	if (di && !IS_ERR(di)) {
+	if (IS_ERR(di)) {
+		return PTR_ERR(di);
+	} else if (di) {
 		ret = drop_one_dir_item(trans, root, path, dir, di);
 		if (ret)
 			return ret;
-- 
2.33.0

[PATCH 4/4] btrfs: unify lookup return value when dir entry is missing

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

btrfs_lookup_dir_index_item() and btrfs_lookup_dir_item() lookup for dir
entries and both are used during log replay or when updating a log tree
during an unlink.

However when the dir item does not exists, btrfs_lookup_dir_item() returns
NULL while btrfs_lookup_dir_index_item() returns PTR_ERR(-ENOENT), and if
the dir item exists but there is no matching entry for a given name or
index, both return NULL. This makes the call sites during log replay to
be more verbose than necessary and it makes it easy to miss this slight
difference. Since we don't need to distinguish between those two cases,
make btrfs_lookup_dir_index_item() always return NULL when there is no
matching directory entry - either because there isn't any dir entry or
because there is one but it does not match the given name and index.

Also rename the argument 'objectid' of btrfs_lookup_dir_index_item() to
'index' since it is supposed to match an index number, and the name
'objectid' is not very good because it can easily be confused with an
inode number (like the inode number a dir entry points to).

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/ctree.h    |  2 +-
 fs/btrfs/dir-item.c | 48 ++++++++++++++++++++++++++++++++++-----------
 fs/btrfs/tree-log.c | 14 ++++---------
 3 files changed, 42 insertions(+), 22 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 400ce90efbb0..ad674f092b4e 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3075,7 +3075,7 @@ struct btrfs_dir_item *
 btrfs_lookup_dir_index_item(struct btrfs_trans_handle *trans,
 			    struct btrfs_root *root,
 			    struct btrfs_path *path, u64 dir,
-			    u64 objectid, const char *name, int name_len,
+			    u64 index, const char *name, int name_len,
 			    int mod);
 struct btrfs_dir_item *
 btrfs_search_dir_index_item(struct btrfs_root *root,
diff --git a/fs/btrfs/dir-item.c b/fs/btrfs/dir-item.c
index f1274d5c3805..7721ce0c0604 100644
--- a/fs/btrfs/dir-item.c
+++ b/fs/btrfs/dir-item.c
@@ -190,9 +190,20 @@ static struct btrfs_dir_item *btrfs_lookup_match_dir(
 }
 
 /*
- * lookup a directory item based on name.  'dir' is the objectid
- * we're searching in, and 'mod' tells us if you plan on deleting the
- * item (use mod < 0) or changing the options (use mod > 0)
+ * Lookup for a directory item by name.
+ *
+ * @trans:	The transaction handle to use. Can be NULL if @mod is 0.
+ * @root:	The root of the target tree.
+ * @path:	Path to use for the search.
+ * @dir:	The inode number (objectid) of the directory.
+ * @name:	The name associated to the directory entry we are looking for.
+ * @name_len:	The length of the name.
+ * @mod:	Used to indicate if the tree search is meant for a read only
+ *		lookup, for a modification lookup or for a deletion lookup, so
+ *		its value should be 0, 1 or -1, respectively.
+ *
+ * Returns: NULL if the dir item does not exists, an error pointer if an error
+ * happened, or a pointer to a dir item if a dir item exists for the given name.
  */
 struct btrfs_dir_item *btrfs_lookup_dir_item(struct btrfs_trans_handle *trans,
 					     struct btrfs_root *root,
@@ -273,27 +284,42 @@ int btrfs_check_dir_item_collision(struct btrfs_root *root, u64 dir,
 }
 
 /*
- * lookup a directory item based on index.  'dir' is the objectid
- * we're searching in, and 'mod' tells us if you plan on deleting the
- * item (use mod < 0) or changing the options (use mod > 0)
+ * Lookup for a directory index item by name and index number.
  *
- * The name is used to make sure the index really points to the name you were
- * looking for.
+ * @trans:	The transaction handle to use. Can be NULL if @mod is 0.
+ * @root:	The root of the target tree.
+ * @path:	Path to use for the search.
+ * @dir:	The inode number (objectid) of the directory.
+ * @index:	The index number.
+ * @name:	The name associated to the directory entry we are looking for.
+ * @name_len:	The length of the name.
+ * @mod:	Used to indicate if the tree search is meant for a read only
+ *		lookup, for a modification lookup or for a deletion lookup, so
+ *		its value should be 0, 1 or -1, respectively.
+ *
+ * Returns: NULL if the dir index item does not exists, an error pointer if an
+ * error happened, or a pointer to a dir item if the dir index item exists and
+ * matches the criteria (name and index number).
  */
 struct btrfs_dir_item *
 btrfs_lookup_dir_index_item(struct btrfs_trans_handle *trans,
 			    struct btrfs_root *root,
 			    struct btrfs_path *path, u64 dir,
-			    u64 objectid, const char *name, int name_len,
+			    u64 index, const char *name, int name_len,
 			    int mod)
 {
+	struct btrfs_dir_item *di;
 	struct btrfs_key key;
 
 	key.objectid = dir;
 	key.type = BTRFS_DIR_INDEX_KEY;
-	key.offset = objectid;
+	key.offset = index;
 
-	return btrfs_lookup_match_dir(trans, root, path, &key, name, name_len, mod);
+	di = btrfs_lookup_match_dir(trans, root, path, &key, name, name_len, mod);
+	if (di == ERR_PTR(-ENOENT))
+		return NULL;
+
+	return di;
 }
 
 struct btrfs_dir_item *
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 0091ec39f57c..68d92835dd0f 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -985,8 +985,7 @@ static noinline int inode_in_dir(struct btrfs_root *root,
 	di = btrfs_lookup_dir_index_item(NULL, root, path, dirid,
 					 index, name, name_len, 0);
 	if (IS_ERR(di)) {
-		if (PTR_ERR(di) != -ENOENT)
-			ret = PTR_ERR(di);
+		ret = PTR_ERR(di);
 		goto out;
 	} else if (di) {
 		btrfs_dir_item_key_to_cpu(path->nodes[0], di, &location);
@@ -1219,8 +1218,7 @@ static inline int __add_inode_ref(struct btrfs_trans_handle *trans,
 	di = btrfs_lookup_dir_index_item(trans, root, path, btrfs_ino(dir),
 					 ref_index, name, namelen, 0);
 	if (IS_ERR(di)) {
-		if (PTR_ERR(di) != -ENOENT)
-			return PTR_ERR(di);
+		return PTR_ERR(di);
 	} else if (di) {
 		ret = drop_one_dir_item(trans, root, path, dir, di);
 		if (ret)
@@ -2022,9 +2020,6 @@ static noinline int replay_one_name(struct btrfs_trans_handle *trans,
 		goto out;
 	}
 
-	if (dst_di == ERR_PTR(-ENOENT))
-		dst_di = NULL;
-
 	if (IS_ERR(dst_di)) {
 		ret = PTR_ERR(dst_di);
 		goto out;
@@ -2332,7 +2327,7 @@ static noinline int check_item_in_log(struct btrfs_trans_handle *trans,
 						     dir_key->offset,
 						     name, name_len, 0);
 		}
-		if (!log_di || log_di == ERR_PTR(-ENOENT)) {
+		if (!log_di) {
 			btrfs_dir_item_key_to_cpu(eb, di, &location);
 			btrfs_release_path(path);
 			btrfs_release_path(log_path);
@@ -3591,8 +3586,7 @@ int btrfs_del_dir_entries_in_log(struct btrfs_trans_handle *trans,
 	if (err == -ENOSPC) {
 		btrfs_set_log_full_commit(trans);
 		err = 0;
-	} else if (err < 0 && err != -ENOENT) {
-		/* ENOENT can be returned if the entry hasn't been fsynced yet */
+	} else if (err < 0) {
 		btrfs_abort_transaction(trans, err);
 	}
 
-- 
2.33.0

Re: [PATCH 1/4] btrfs: deal with errors when checking if a dir entry exists during log replay

[Qu Wenruo]

On 2021/10/1 20:52, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
>
> Currently inode_in_dir() ignores errors returned from
> btrfs_lookup_dir_index_item() and from btrfs_lookup_dir_item(), treating
> any errors as if the directory entry does not exists in the fs/subvolume
> tree, which is obviously not correct, as we can get errors such as -EIO
> when reading extent buffers while searching the fs/subvolume's tree.
>
> Fix that by making inode_in_dir() return the errors and making its only
> caller, add_inode_ref(), deal with returned errors as well.
>
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
>   fs/btrfs/tree-log.c | 47 ++++++++++++++++++++++++++++-----------------
>   1 file changed, 29 insertions(+), 18 deletions(-)
>
> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> index b765ca7536fe..79d7cca704fb 100644
> --- a/fs/btrfs/tree-log.c
> +++ b/fs/btrfs/tree-log.c
> @@ -967,9 +967,11 @@ static noinline int drop_one_dir_item(struct btrfs_trans_handle *trans,
>   }
>
>   /*
> - * helper function to see if a given name and sequence number found
> - * in an inode back reference are already in a directory and correctly
> - * point to this inode
> + * Helper function to see if a given name and sequence number found in an inode
> + * back reference are already in a directory and correctly point to this inode.
> + *
> + * Returns: < 0 on error, 0 if the directory entry does not exists and 1 if it
> + * exists.
>    */
>   static noinline int inode_in_dir(struct btrfs_root *root,
>   				 struct btrfs_path *path,
> @@ -978,29 +980,35 @@ static noinline int inode_in_dir(struct btrfs_root *root,
>   {
>   	struct btrfs_dir_item *di;
>   	struct btrfs_key location;
> -	int match = 0;
> +	int ret = 0;
>
>   	di = btrfs_lookup_dir_index_item(NULL, root, path, dirid,
>   					 index, name, name_len, 0);

I did a quick search for "btrfs_lookup_dir_", and find most if not all
callers are handling the NULL case just like ENOENT.

Can we make btrfs_lookup_match_dir() to return -ENOENT when the target
is not found?

Return NULL while we still have another PTR_ERR(-ENOENT) to indicate not
found is really asking for problems.

Thanks,
Qu
> -	if (di && !IS_ERR(di)) {
> +	if (IS_ERR(di)) {
> +		if (PTR_ERR(di) != -ENOENT)
> +			ret = PTR_ERR(di);
> +		goto out;
> +	} else if (di) {
>   		btrfs_dir_item_key_to_cpu(path->nodes[0], di, &location);
>   		if (location.objectid != objectid)
>   			goto out;
> -	} else
> +	} else {
>   		goto out;
> -	btrfs_release_path(path);
> +	}
>
> +	btrfs_release_path(path);
>   	di = btrfs_lookup_dir_item(NULL, root, path, dirid, name, name_len, 0);
> -	if (di && !IS_ERR(di)) {
> -		btrfs_dir_item_key_to_cpu(path->nodes[0], di, &location);
> -		if (location.objectid != objectid)
> -			goto out;
> -	} else
> +	if (IS_ERR(di)) {
> +		ret = PTR_ERR(di);
>   		goto out;
> -	match = 1;
> +	} else if (di) {
> +		btrfs_dir_item_key_to_cpu(path->nodes[0], di, &location);
> +		if (location.objectid == objectid)
> +			ret = 1;
> +	}
>   out:
>   	btrfs_release_path(path);
> -	return match;
> +	return ret;
>   }
>
>   /*
> @@ -1545,10 +1553,12 @@ static noinline int add_inode_ref(struct btrfs_trans_handle *trans,
>   		if (ret)
>   			goto out;
>
> -		/* if we already have a perfect match, we're done */
> -		if (!inode_in_dir(root, path, btrfs_ino(BTRFS_I(dir)),
> -					btrfs_ino(BTRFS_I(inode)), ref_index,
> -					name, namelen)) {
> +		ret = inode_in_dir(root, path, btrfs_ino(BTRFS_I(dir)),
> +				   btrfs_ino(BTRFS_I(inode)), ref_index,
> +				   name, namelen);
> +		if (ret < 0) {
> +			goto out;
> +		} else if (ret == 0) {
>   			/*
>   			 * look for a conflicting back reference in the
>   			 * metadata. if we find one we have to unlink that name
> @@ -1608,6 +1618,7 @@ static noinline int add_inode_ref(struct btrfs_trans_handle *trans,
>   			if (ret)
>   				goto out;
>   		}
> +		/* Else, ret == 1, we already have a perfect match, we're done. */
>
>   		ref_ptr = (unsigned long)(ref_ptr + ref_struct_size) + namelen;
>   		kfree(name);
>

Re: [PATCH 1/4] btrfs: deal with errors when checking if a dir entry exists during log replay

[Filipe Manana]

On Fri, Oct 1, 2021 at 2:14 PM Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
>
>
>
> On 2021/10/1 20:52, fdmanana@kernel.org wrote:
> > From: Filipe Manana <fdmanana@suse.com>
> >
> > Currently inode_in_dir() ignores errors returned from
> > btrfs_lookup_dir_index_item() and from btrfs_lookup_dir_item(), treating
> > any errors as if the directory entry does not exists in the fs/subvolume
> > tree, which is obviously not correct, as we can get errors such as -EIO
> > when reading extent buffers while searching the fs/subvolume's tree.
> >
> > Fix that by making inode_in_dir() return the errors and making its only
> > caller, add_inode_ref(), deal with returned errors as well.
> >
> > Signed-off-by: Filipe Manana <fdmanana@suse.com>
> > ---
> >   fs/btrfs/tree-log.c | 47 ++++++++++++++++++++++++++++-----------------
> >   1 file changed, 29 insertions(+), 18 deletions(-)
> >
> > diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> > index b765ca7536fe..79d7cca704fb 100644
> > --- a/fs/btrfs/tree-log.c
> > +++ b/fs/btrfs/tree-log.c
> > @@ -967,9 +967,11 @@ static noinline int drop_one_dir_item(struct btrfs_trans_handle *trans,
> >   }
> >
> >   /*
> > - * helper function to see if a given name and sequence number found
> > - * in an inode back reference are already in a directory and correctly
> > - * point to this inode
> > + * Helper function to see if a given name and sequence number found in an inode
> > + * back reference are already in a directory and correctly point to this inode.
> > + *
> > + * Returns: < 0 on error, 0 if the directory entry does not exists and 1 if it
> > + * exists.
> >    */
> >   static noinline int inode_in_dir(struct btrfs_root *root,
> >                                struct btrfs_path *path,
> > @@ -978,29 +980,35 @@ static noinline int inode_in_dir(struct btrfs_root *root,
> >   {
> >       struct btrfs_dir_item *di;
> >       struct btrfs_key location;
> > -     int match = 0;
> > +     int ret = 0;
> >
> >       di = btrfs_lookup_dir_index_item(NULL, root, path, dirid,
> >                                        index, name, name_len, 0);
>
> I did a quick search for "btrfs_lookup_dir_", and find most if not all
> callers are handling the NULL case just like ENOENT.
>
> Can we make btrfs_lookup_match_dir() to return -ENOENT when the target
> is not found?
>
> Return NULL while we still have another PTR_ERR(-ENOENT) to indicate not
> found is really asking for problems.

Yes it is. And that's what the last patch does, it makes the two
functions consistent regarding the return value.

Thanks.

>
> Thanks,
> Qu
> > -     if (di && !IS_ERR(di)) {
> > +     if (IS_ERR(di)) {
> > +             if (PTR_ERR(di) != -ENOENT)
> > +                     ret = PTR_ERR(di);
> > +             goto out;
> > +     } else if (di) {
> >               btrfs_dir_item_key_to_cpu(path->nodes[0], di, &location);
> >               if (location.objectid != objectid)
> >                       goto out;
> > -     } else
> > +     } else {
> >               goto out;
> > -     btrfs_release_path(path);
> > +     }
> >
> > +     btrfs_release_path(path);
> >       di = btrfs_lookup_dir_item(NULL, root, path, dirid, name, name_len, 0);
> > -     if (di && !IS_ERR(di)) {
> > -             btrfs_dir_item_key_to_cpu(path->nodes[0], di, &location);
> > -             if (location.objectid != objectid)
> > -                     goto out;
> > -     } else
> > +     if (IS_ERR(di)) {
> > +             ret = PTR_ERR(di);
> >               goto out;
> > -     match = 1;
> > +     } else if (di) {
> > +             btrfs_dir_item_key_to_cpu(path->nodes[0], di, &location);
> > +             if (location.objectid == objectid)
> > +                     ret = 1;
> > +     }
> >   out:
> >       btrfs_release_path(path);
> > -     return match;
> > +     return ret;
> >   }
> >
> >   /*
> > @@ -1545,10 +1553,12 @@ static noinline int add_inode_ref(struct btrfs_trans_handle *trans,
> >               if (ret)
> >                       goto out;
> >
> > -             /* if we already have a perfect match, we're done */
> > -             if (!inode_in_dir(root, path, btrfs_ino(BTRFS_I(dir)),
> > -                                     btrfs_ino(BTRFS_I(inode)), ref_index,
> > -                                     name, namelen)) {
> > +             ret = inode_in_dir(root, path, btrfs_ino(BTRFS_I(dir)),
> > +                                btrfs_ino(BTRFS_I(inode)), ref_index,
> > +                                name, namelen);
> > +             if (ret < 0) {
> > +                     goto out;
> > +             } else if (ret == 0) {
> >                       /*
> >                        * look for a conflicting back reference in the
> >                        * metadata. if we find one we have to unlink that name
> > @@ -1608,6 +1618,7 @@ static noinline int add_inode_ref(struct btrfs_trans_handle *trans,
> >                       if (ret)
> >                               goto out;
> >               }
> > +             /* Else, ret == 1, we already have a perfect match, we're done. */
> >
> >               ref_ptr = (unsigned long)(ref_ptr + ref_struct_size) + namelen;
> >               kfree(name);
> >

Re: [PATCH 0/4] btrfs: fix missing error handling when replaying directory entries

[David Sterba]

On Fri, Oct 01, 2021 at 01:52:29PM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> Some places during log replay are ignoring errors when looking up directory
> entries in a subvolume/fs tree. This fixes all those places and makes sure
> we get consistent error hanlding when looking dir items and dir index items.
> 
> Filipe Manana (4):
>   btrfs: deal with errors when checking if a dir entry exists during log replay
>   btrfs: deal with errors when replaying dir entry during log replay
>   btrfs: deal with errors when adding inode reference during log replay
>   btrfs: unify lookup return value when dir entry is missing

Added to misc-next, thanks.