* [PATCH 1/4] btrfs: use single return variable in btrfs_find_orphan_roots()
2025-12-16 16:33 [PATCH 0/4] btrfs: some cleanups in btrfs_find_orphan_roots() fdmanana
@ 2025-12-16 16:33 ` fdmanana
2026-02-08 18:52 ` Chris Mason
2025-12-16 16:33 ` [PATCH 2/4] btrfs: remove redundant path release " fdmanana
` (4 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: fdmanana @ 2025-12-16 16:33 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
We use both 'ret' and 'err' which is a pattern that generates confusion
and resulted in subtle bugs in the past. Remove 'err' and use only 'ret'.
Also move simplify the error flow by directy returning from the function
instead of breaking of the loop, since there are no resources to cleanup
after the loop.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/root-tree.c | 40 ++++++++++++++++++----------------------
1 file changed, 18 insertions(+), 22 deletions(-)
diff --git a/fs/btrfs/root-tree.c b/fs/btrfs/root-tree.c
index 6a7e297ab0a7..a7171112d638 100644
--- a/fs/btrfs/root-tree.c
+++ b/fs/btrfs/root-tree.c
@@ -217,8 +217,6 @@ int btrfs_find_orphan_roots(struct btrfs_fs_info *fs_info)
BTRFS_PATH_AUTO_FREE(path);
struct btrfs_key key;
struct btrfs_root *root;
- int err = 0;
- int ret;
path = btrfs_alloc_path();
if (!path)
@@ -230,20 +228,19 @@ int btrfs_find_orphan_roots(struct btrfs_fs_info *fs_info)
while (1) {
u64 root_objectid;
+ int ret;
ret = btrfs_search_slot(NULL, tree_root, &key, path, 0, 0);
- if (ret < 0) {
- err = ret;
- break;
- }
+ if (ret < 0)
+ return ret;
leaf = path->nodes[0];
if (path->slots[0] >= btrfs_header_nritems(leaf)) {
ret = btrfs_next_leaf(tree_root, path);
if (ret < 0)
- err = ret;
- if (ret != 0)
- break;
+ return ret;
+ else if (ret > 0)
+ return 0;
leaf = path->nodes[0];
}
@@ -252,34 +249,33 @@ int btrfs_find_orphan_roots(struct btrfs_fs_info *fs_info)
if (key.objectid != BTRFS_ORPHAN_OBJECTID ||
key.type != BTRFS_ORPHAN_ITEM_KEY)
- break;
+ return 0;
root_objectid = key.offset;
key.offset++;
root = btrfs_get_fs_root(fs_info, root_objectid, false);
- err = PTR_ERR_OR_ZERO(root);
- if (err && err != -ENOENT) {
+ ret = PTR_ERR_OR_ZERO(root);
+ if (ret && ret != -ENOENT) {
break;
- } else if (err == -ENOENT) {
+ } else if (ret == -ENOENT) {
struct btrfs_trans_handle *trans;
btrfs_release_path(path);
trans = btrfs_join_transaction(tree_root);
if (IS_ERR(trans)) {
- err = PTR_ERR(trans);
- btrfs_handle_fs_error(fs_info, err,
+ ret = PTR_ERR(trans);
+ btrfs_handle_fs_error(fs_info, ret,
"Failed to start trans to delete orphan item");
- break;
+ return ret;
}
- err = btrfs_del_orphan_item(trans, tree_root,
- root_objectid);
+ ret = btrfs_del_orphan_item(trans, tree_root, root_objectid);
btrfs_end_transaction(trans);
- if (err) {
- btrfs_handle_fs_error(fs_info, err,
+ if (ret) {
+ btrfs_handle_fs_error(fs_info, ret,
"Failed to delete root orphan item");
- break;
+ return ret;
}
continue;
}
@@ -307,7 +303,7 @@ int btrfs_find_orphan_roots(struct btrfs_fs_info *fs_info)
btrfs_put_root(root);
}
- return err;
+ return 0;
}
/* drop the root item for 'key' from the tree root */
--
2.47.2
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 1/4] btrfs: use single return variable in btrfs_find_orphan_roots()
2025-12-16 16:33 ` [PATCH 1/4] btrfs: use single return variable " fdmanana
@ 2026-02-08 18:52 ` Chris Mason
2026-02-08 19:54 ` Filipe Manana
0 siblings, 1 reply; 11+ messages in thread
From: Chris Mason @ 2026-02-08 18:52 UTC (permalink / raw)
To: fdmanana; +Cc: linux-btrfs
fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
>
> We use both 'ret' and 'err' which is a pattern that generates confusion
> and resulted in subtle bugs in the past. Remove 'err' and use only 'ret'.
> Also move simplify the error flow by directy returning from the function
> instead of breaking of the loop, since there are no resources to cleanup
> after the loop.
Hi everyone,
I'm running the btrfs for-next branch, along with the encryption patch set
through my AI review prompts:
https://github.com/masoncl/review-prompts
I'm working on some btrfs specific instructions, but hopefully these reviews
are already useful. I've kept the headers to make it easier to double check
that I'm actually replying to the right email, but please let me know if any
parts of this are incorrect:
commit 8670a25ecb2fbc35d4e58f8f522e7d5b735d6778
Author: Filipe Manana <fdmanana@suse.com>
btrfs: use single return variable in btrfs_find_orphan_roots()
This commit refactors btrfs_find_orphan_roots() to use a single return
variable 'ret' instead of both 'ret' and 'err', and simplifies the error
flow by directly returning from the function instead of breaking out of
the loop.
> diff --git a/fs/btrfs/root-tree.c b/fs/btrfs/root-tree.c
> index 6a7e297ab0a7a..a7171112d638f 100644
> --- a/fs/btrfs/root-tree.c
> +++ b/fs/btrfs/root-tree.c
[ ... ]
> @@ -217,8 +217,6 @@ int btrfs_find_orphan_roots(struct btrfs_fs_info *fs_info)
> BTRFS_PATH_AUTO_FREE(path);
> struct btrfs_key key;
> struct btrfs_root *root;
> - int err = 0;
> - int ret;
>
> path = btrfs_alloc_path();
> if (!path)
> @@ -230,20 +228,19 @@ int btrfs_find_orphan_roots(struct btrfs_fs_info *fs_info)
>
> while (1) {
> u64 root_objectid;
> + int ret;
>
> ret = btrfs_search_slot(NULL, tree_root, &key, path, 0, 0);
> - if (ret < 0) {
> - err = ret;
> - break;
> - }
> + if (ret < 0)
> + return ret;
>
> leaf = path->nodes[0];
> if (path->slots[0] >= btrfs_header_nritems(leaf)) {
> ret = btrfs_next_leaf(tree_root, path);
> if (ret < 0)
> - err = ret;
> - if (ret != 0)
> - break;
> + return ret;
> + else if (ret > 0)
> + return 0;
> leaf = path->nodes[0];
> }
>
> @@ -252,34 +249,33 @@ int btrfs_find_orphan_roots(struct btrfs_fs_info *fs_info)
>
> if (key.objectid != BTRFS_ORPHAN_OBJECTID ||
> key.type != BTRFS_ORPHAN_ITEM_KEY)
> - break;
> + return 0;
>
> root_objectid = key.offset;
> key.offset++;
>
> root = btrfs_get_fs_root(fs_info, root_objectid, false);
> - err = PTR_ERR_OR_ZERO(root);
> - if (err && err != -ENOENT) {
> + ret = PTR_ERR_OR_ZERO(root);
> + if (ret && ret != -ENOENT) {
> break;
^^^^
Does this code lose the error code from btrfs_get_fs_root()?
The refactoring moved 'int ret' from function scope (line 217 in the old
code) to loop scope (inside the 'while (1)' loop). When btrfs_get_fs_root()
fails with an error like -ENOMEM or -EIO, the code breaks out of the loop.
However, the 'ret' variable containing the error is scoped to the loop body
and goes out of scope after the break.
The function then returns 0 at line 303, losing the error code. This
violates the commit message's stated intent to "directly return from the
function instead of breaking of the loop."
Looking at btrfs_get_fs_root() in fs/btrfs/disk-io.c, it can return:
- -ENOMEM if path allocation fails (line 1316)
- -EIO or other errors from btrfs_read_tree_root() (line 1301)
- errors from btrfs_init_fs_root() (line 1310)
- -EIO from btrfs_search_slot() (line 1323)
If btrfs_get_fs_root() fails during mount due to memory pressure or I/O
errors, the mount will succeed instead of failing. This can lead to
filesystem corruption because the orphan root handling is incomplete but the
mount proceeds anyway.
Shouldn't this 'break;' be 'return ret;' to match all the other error paths
in the refactored function?
> - } else if (err == -ENOENT) {
> + } else if (ret == -ENOENT) {
> struct btrfs_trans_handle *trans;
>
> btrfs_release_path(path);
>
> trans = btrfs_join_transaction(tree_root);
> if (IS_ERR(trans)) {
> - err = PTR_ERR(trans);
> - btrfs_handle_fs_error(fs_info, err,
> + ret = PTR_ERR(trans);
> + btrfs_handle_fs_error(fs_info, ret,
> "Failed to start trans to delete orphan item");
> - break;
> + return ret;
> }
> - err = btrfs_del_orphan_item(trans, tree_root,
> - root_objectid);
> + ret = btrfs_del_orphan_item(trans, tree_root, root_objectid);
> btrfs_end_transaction(trans);
> - if (err) {
> - btrfs_handle_fs_error(fs_info, err,
> + if (ret) {
> + btrfs_handle_fs_error(fs_info, ret,
> "Failed to delete root orphan item");
> - break;
> + return ret;
> }
> continue;
> }
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 1/4] btrfs: use single return variable in btrfs_find_orphan_roots()
2026-02-08 18:52 ` Chris Mason
@ 2026-02-08 19:54 ` Filipe Manana
0 siblings, 0 replies; 11+ messages in thread
From: Filipe Manana @ 2026-02-08 19:54 UTC (permalink / raw)
To: Chris Mason; +Cc: linux-btrfs
On Sun, Feb 8, 2026 at 6:53 PM Chris Mason <clm@meta.com> wrote:
>
> fdmanana@kernel.org wrote:
> > From: Filipe Manana <fdmanana@suse.com>
> >
> > We use both 'ret' and 'err' which is a pattern that generates confusion
> > and resulted in subtle bugs in the past. Remove 'err' and use only 'ret'.
> > Also move simplify the error flow by directy returning from the function
> > instead of breaking of the loop, since there are no resources to cleanup
> > after the loop.
>
> Hi everyone,
>
> I'm running the btrfs for-next branch, along with the encryption patch set
> through my AI review prompts:
>
> https://github.com/masoncl/review-prompts
>
> I'm working on some btrfs specific instructions, but hopefully these reviews
> are already useful. I've kept the headers to make it easier to double check
> that I'm actually replying to the right email, but please let me know if any
> parts of this are incorrect:
>
> commit 8670a25ecb2fbc35d4e58f8f522e7d5b735d6778
> Author: Filipe Manana <fdmanana@suse.com>
>
> btrfs: use single return variable in btrfs_find_orphan_roots()
>
> This commit refactors btrfs_find_orphan_roots() to use a single return
> variable 'ret' instead of both 'ret' and 'err', and simplifies the error
> flow by directly returning from the function instead of breaking out of
> the loop.
>
> > diff --git a/fs/btrfs/root-tree.c b/fs/btrfs/root-tree.c
> > index 6a7e297ab0a7a..a7171112d638f 100644
> > --- a/fs/btrfs/root-tree.c
> > +++ b/fs/btrfs/root-tree.c
>
> [ ... ]
>
> > @@ -217,8 +217,6 @@ int btrfs_find_orphan_roots(struct btrfs_fs_info *fs_info)
> > BTRFS_PATH_AUTO_FREE(path);
> > struct btrfs_key key;
> > struct btrfs_root *root;
> > - int err = 0;
> > - int ret;
> >
> > path = btrfs_alloc_path();
> > if (!path)
> > @@ -230,20 +228,19 @@ int btrfs_find_orphan_roots(struct btrfs_fs_info *fs_info)
> >
> > while (1) {
> > u64 root_objectid;
> > + int ret;
> >
> > ret = btrfs_search_slot(NULL, tree_root, &key, path, 0, 0);
> > - if (ret < 0) {
> > - err = ret;
> > - break;
> > - }
> > + if (ret < 0)
> > + return ret;
> >
> > leaf = path->nodes[0];
> > if (path->slots[0] >= btrfs_header_nritems(leaf)) {
> > ret = btrfs_next_leaf(tree_root, path);
> > if (ret < 0)
> > - err = ret;
> > - if (ret != 0)
> > - break;
> > + return ret;
> > + else if (ret > 0)
> > + return 0;
> > leaf = path->nodes[0];
> > }
> >
> > @@ -252,34 +249,33 @@ int btrfs_find_orphan_roots(struct btrfs_fs_info *fs_info)
> >
> > if (key.objectid != BTRFS_ORPHAN_OBJECTID ||
> > key.type != BTRFS_ORPHAN_ITEM_KEY)
> > - break;
> > + return 0;
> >
> > root_objectid = key.offset;
> > key.offset++;
> >
> > root = btrfs_get_fs_root(fs_info, root_objectid, false);
> > - err = PTR_ERR_OR_ZERO(root);
> > - if (err && err != -ENOENT) {
> > + ret = PTR_ERR_OR_ZERO(root);
> > + if (ret && ret != -ENOENT) {
> > break;
> ^^^^
>
> Does this code lose the error code from btrfs_get_fs_root()?
It does indeed. Fix sent here:
https://lore.kernel.org/linux-btrfs/6400dc5103b5ddbe543961b4e865e311ab396790.1770580302.git.fdmanana@suse.com/
Thanks.
>
> The refactoring moved 'int ret' from function scope (line 217 in the old
> code) to loop scope (inside the 'while (1)' loop). When btrfs_get_fs_root()
> fails with an error like -ENOMEM or -EIO, the code breaks out of the loop.
> However, the 'ret' variable containing the error is scoped to the loop body
> and goes out of scope after the break.
>
> The function then returns 0 at line 303, losing the error code. This
> violates the commit message's stated intent to "directly return from the
> function instead of breaking of the loop."
>
> Looking at btrfs_get_fs_root() in fs/btrfs/disk-io.c, it can return:
> - -ENOMEM if path allocation fails (line 1316)
> - -EIO or other errors from btrfs_read_tree_root() (line 1301)
> - errors from btrfs_init_fs_root() (line 1310)
> - -EIO from btrfs_search_slot() (line 1323)
>
> If btrfs_get_fs_root() fails during mount due to memory pressure or I/O
> errors, the mount will succeed instead of failing. This can lead to
> filesystem corruption because the orphan root handling is incomplete but the
> mount proceeds anyway.
>
> Shouldn't this 'break;' be 'return ret;' to match all the other error paths
> in the refactored function?
>
> > - } else if (err == -ENOENT) {
> > + } else if (ret == -ENOENT) {
> > struct btrfs_trans_handle *trans;
> >
> > btrfs_release_path(path);
> >
> > trans = btrfs_join_transaction(tree_root);
> > if (IS_ERR(trans)) {
> > - err = PTR_ERR(trans);
> > - btrfs_handle_fs_error(fs_info, err,
> > + ret = PTR_ERR(trans);
> > + btrfs_handle_fs_error(fs_info, ret,
> > "Failed to start trans to delete orphan item");
> > - break;
> > + return ret;
> > }
> > - err = btrfs_del_orphan_item(trans, tree_root,
> > - root_objectid);
> > + ret = btrfs_del_orphan_item(trans, tree_root, root_objectid);
> > btrfs_end_transaction(trans);
> > - if (err) {
> > - btrfs_handle_fs_error(fs_info, err,
> > + if (ret) {
> > + btrfs_handle_fs_error(fs_info, ret,
> > "Failed to delete root orphan item");
> > - break;
> > + return ret;
> > }
> > continue;
> > }
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/4] btrfs: remove redundant path release in btrfs_find_orphan_roots()
2025-12-16 16:33 [PATCH 0/4] btrfs: some cleanups in btrfs_find_orphan_roots() fdmanana
2025-12-16 16:33 ` [PATCH 1/4] btrfs: use single return variable " fdmanana
@ 2025-12-16 16:33 ` fdmanana
2025-12-16 16:33 ` [PATCH 3/4] btrfs: don't call btrfs_handle_fs_error() after failure to join transaction fdmanana
` (3 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: fdmanana @ 2025-12-16 16:33 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
There's no need to release the path in the if branch used when the root
does not exists since we released the path before the call to
btrfs_get_fs_root(). So remove that redundant btrfs_release_path() call.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/root-tree.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/fs/btrfs/root-tree.c b/fs/btrfs/root-tree.c
index a7171112d638..40f9bc9485e8 100644
--- a/fs/btrfs/root-tree.c
+++ b/fs/btrfs/root-tree.c
@@ -261,8 +261,6 @@ int btrfs_find_orphan_roots(struct btrfs_fs_info *fs_info)
} else if (ret == -ENOENT) {
struct btrfs_trans_handle *trans;
- btrfs_release_path(path);
-
trans = btrfs_join_transaction(tree_root);
if (IS_ERR(trans)) {
ret = PTR_ERR(trans);
--
2.47.2
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 3/4] btrfs: don't call btrfs_handle_fs_error() after failure to join transaction
2025-12-16 16:33 [PATCH 0/4] btrfs: some cleanups in btrfs_find_orphan_roots() fdmanana
2025-12-16 16:33 ` [PATCH 1/4] btrfs: use single return variable " fdmanana
2025-12-16 16:33 ` [PATCH 2/4] btrfs: remove redundant path release " fdmanana
@ 2025-12-16 16:33 ` fdmanana
2025-12-16 17:00 ` David Sterba
2025-12-16 16:33 ` [PATCH 4/4] btrfs: don't call btrfs_handle_fs_error() after failure to delete orphan item fdmanana
` (2 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: fdmanana @ 2025-12-16 16:33 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
In btrfs_find_orphan_roots() we don't need to call btrfs_handle_fs_error()
if we fail to join a transaction. This is because we haven't done anything
yet regarding the current root and previous iterations of the loop dealt
with other roots, so there's nothing we need to undo. Instead log an error
message and return the error to the caller, which will result either in
a mount failure or remount failure (the only contextes it's called from).
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/root-tree.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/root-tree.c b/fs/btrfs/root-tree.c
index 40f9bc9485e8..a30c7bc3f0e5 100644
--- a/fs/btrfs/root-tree.c
+++ b/fs/btrfs/root-tree.c
@@ -264,8 +264,9 @@ int btrfs_find_orphan_roots(struct btrfs_fs_info *fs_info)
trans = btrfs_join_transaction(tree_root);
if (IS_ERR(trans)) {
ret = PTR_ERR(trans);
- btrfs_handle_fs_error(fs_info, ret,
- "Failed to start trans to delete orphan item");
+ btrfs_err(fs_info,
+ "failed to join transaction to delete orphan item: %d\n",
+ ret);
return ret;
}
ret = btrfs_del_orphan_item(trans, tree_root, root_objectid);
--
2.47.2
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 3/4] btrfs: don't call btrfs_handle_fs_error() after failure to join transaction
2025-12-16 16:33 ` [PATCH 3/4] btrfs: don't call btrfs_handle_fs_error() after failure to join transaction fdmanana
@ 2025-12-16 17:00 ` David Sterba
2025-12-16 17:03 ` Filipe Manana
0 siblings, 1 reply; 11+ messages in thread
From: David Sterba @ 2025-12-16 17:00 UTC (permalink / raw)
To: fdmanana; +Cc: linux-btrfs
On Tue, Dec 16, 2025 at 04:33:18PM +0000, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
>
> In btrfs_find_orphan_roots() we don't need to call btrfs_handle_fs_error()
> if we fail to join a transaction. This is because we haven't done anything
> yet regarding the current root and previous iterations of the loop dealt
> with other roots, so there's nothing we need to undo. Instead log an error
> message and return the error to the caller, which will result either in
> a mount failure or remount failure (the only contextes it's called from).
>
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
> fs/btrfs/root-tree.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/root-tree.c b/fs/btrfs/root-tree.c
> index 40f9bc9485e8..a30c7bc3f0e5 100644
> --- a/fs/btrfs/root-tree.c
> +++ b/fs/btrfs/root-tree.c
> @@ -264,8 +264,9 @@ int btrfs_find_orphan_roots(struct btrfs_fs_info *fs_info)
> trans = btrfs_join_transaction(tree_root);
> if (IS_ERR(trans)) {
> ret = PTR_ERR(trans);
> - btrfs_handle_fs_error(fs_info, ret,
> - "Failed to start trans to delete orphan item");
> + btrfs_err(fs_info,
> + "failed to join transaction to delete orphan item: %d\n",
btrfs_err and the other helpers don't need "\n"
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 3/4] btrfs: don't call btrfs_handle_fs_error() after failure to join transaction
2025-12-16 17:00 ` David Sterba
@ 2025-12-16 17:03 ` Filipe Manana
0 siblings, 0 replies; 11+ messages in thread
From: Filipe Manana @ 2025-12-16 17:03 UTC (permalink / raw)
To: dsterba; +Cc: linux-btrfs
On Tue, Dec 16, 2025 at 5:00 PM David Sterba <dsterba@suse.cz> wrote:
>
> On Tue, Dec 16, 2025 at 04:33:18PM +0000, fdmanana@kernel.org wrote:
> > From: Filipe Manana <fdmanana@suse.com>
> >
> > In btrfs_find_orphan_roots() we don't need to call btrfs_handle_fs_error()
> > if we fail to join a transaction. This is because we haven't done anything
> > yet regarding the current root and previous iterations of the loop dealt
> > with other roots, so there's nothing we need to undo. Instead log an error
> > message and return the error to the caller, which will result either in
> > a mount failure or remount failure (the only contextes it's called from).
> >
> > Signed-off-by: Filipe Manana <fdmanana@suse.com>
> > ---
> > fs/btrfs/root-tree.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/btrfs/root-tree.c b/fs/btrfs/root-tree.c
> > index 40f9bc9485e8..a30c7bc3f0e5 100644
> > --- a/fs/btrfs/root-tree.c
> > +++ b/fs/btrfs/root-tree.c
> > @@ -264,8 +264,9 @@ int btrfs_find_orphan_roots(struct btrfs_fs_info *fs_info)
> > trans = btrfs_join_transaction(tree_root);
> > if (IS_ERR(trans)) {
> > ret = PTR_ERR(trans);
> > - btrfs_handle_fs_error(fs_info, ret,
> > - "Failed to start trans to delete orphan item");
> > + btrfs_err(fs_info,
> > + "failed to join transaction to delete orphan item: %d\n",
>
> btrfs_err and the other helpers don't need "\n"
Yes, I missed that in this patch, the other similar ones don't have
the new line.
Will remove on commit time if there are no other changes needed.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 4/4] btrfs: don't call btrfs_handle_fs_error() after failure to delete orphan item
2025-12-16 16:33 [PATCH 0/4] btrfs: some cleanups in btrfs_find_orphan_roots() fdmanana
` (2 preceding siblings ...)
2025-12-16 16:33 ` [PATCH 3/4] btrfs: don't call btrfs_handle_fs_error() after failure to join transaction fdmanana
@ 2025-12-16 16:33 ` fdmanana
2025-12-16 22:23 ` [PATCH 0/4] btrfs: some cleanups in btrfs_find_orphan_roots() Qu Wenruo
2025-12-17 6:44 ` Johannes Thumshirn
5 siblings, 0 replies; 11+ messages in thread
From: fdmanana @ 2025-12-16 16:33 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
In btrfs_find_orphan_roots() we don't need to call btrfs_handle_fs_error()
if we fail to delete the orphan item for the current root. This is because
we haven't done anything yet regarding the current root and previous
iterations of the loop dealt with other roots, so there's nothing we need
to undo. Instead log an error message and return the error to the caller,
which will result either in a mount failure or remount failure (the only
contextes it's called from).
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/root-tree.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/root-tree.c b/fs/btrfs/root-tree.c
index a30c7bc3f0e5..065c3842122a 100644
--- a/fs/btrfs/root-tree.c
+++ b/fs/btrfs/root-tree.c
@@ -272,8 +272,8 @@ int btrfs_find_orphan_roots(struct btrfs_fs_info *fs_info)
ret = btrfs_del_orphan_item(trans, tree_root, root_objectid);
btrfs_end_transaction(trans);
if (ret) {
- btrfs_handle_fs_error(fs_info, ret,
- "Failed to delete root orphan item");
+ btrfs_err(fs_info,
+ "failed to delete root orphan item: %d", ret);
return ret;
}
continue;
--
2.47.2
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 0/4] btrfs: some cleanups in btrfs_find_orphan_roots()
2025-12-16 16:33 [PATCH 0/4] btrfs: some cleanups in btrfs_find_orphan_roots() fdmanana
` (3 preceding siblings ...)
2025-12-16 16:33 ` [PATCH 4/4] btrfs: don't call btrfs_handle_fs_error() after failure to delete orphan item fdmanana
@ 2025-12-16 22:23 ` Qu Wenruo
2025-12-17 6:44 ` Johannes Thumshirn
5 siblings, 0 replies; 11+ messages in thread
From: Qu Wenruo @ 2025-12-16 22:23 UTC (permalink / raw)
To: fdmanana, linux-btrfs
在 2025/12/17 03:03, fdmanana@kernel.org 写道:
> From: Filipe Manana <fdmanana@suse.com>
>
> Some cleanups in btrfs_find_orphan_roots(). Details in the changelogs.
Reviewed-by: Qu Wenruo <wqu@suse.com>
Except the tailing "\n" is unnecessary in the 3rd patch.
Thanks,
Qu
>
> Filipe Manana (4):
> btrfs: use single return variable in btrfs_find_orphan_roots()
> btrfs: remove redundant path release in btrfs_find_orphan_roots()
> btrfs: don't call btrfs_handle_fs_error() after failure to join transaction
> btrfs: don't call btrfs_handle_fs_error() after failure to delete orphan item
>
> fs/btrfs/root-tree.c | 47 ++++++++++++++++++++------------------------
> 1 file changed, 21 insertions(+), 26 deletions(-)
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 0/4] btrfs: some cleanups in btrfs_find_orphan_roots()
2025-12-16 16:33 [PATCH 0/4] btrfs: some cleanups in btrfs_find_orphan_roots() fdmanana
` (4 preceding siblings ...)
2025-12-16 22:23 ` [PATCH 0/4] btrfs: some cleanups in btrfs_find_orphan_roots() Qu Wenruo
@ 2025-12-17 6:44 ` Johannes Thumshirn
5 siblings, 0 replies; 11+ messages in thread
From: Johannes Thumshirn @ 2025-12-17 6:44 UTC (permalink / raw)
To: fdmanana@kernel.org, linux-btrfs@vger.kernel.org
On 12/16/25 5:39 PM, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
>
> Some cleanups in btrfs_find_orphan_roots(). Details in the changelogs.
>
> Filipe Manana (4):
> btrfs: use single return variable in btrfs_find_orphan_roots()
> btrfs: remove redundant path release in btrfs_find_orphan_roots()
> btrfs: don't call btrfs_handle_fs_error() after failure to join transaction
> btrfs: don't call btrfs_handle_fs_error() after failure to delete orphan item
>
> fs/btrfs/root-tree.c | 47 ++++++++++++++++++++------------------------
> 1 file changed, 21 insertions(+), 26 deletions(-)
>
with aforementioned "\n" fixed
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
^ permalink raw reply [flat|nested] 11+ messages in thread