From: Filipe Manana <fdman...@suse.com>

During the mount procedure we are calling btrfs_orphan_cleanup() against
the root tree, which will find all orphans items in this tree. When an
orphan item corresponds to a deleted subvolume/snapshot (instead of an
inode space cache), it must not delete the orphan item, because that will
cause btrfs_find_orphan_roots() to not find the orphan item and therefore
not add the corresponding subvolume root to the list of dead roots, which
results in the subvolume's tree never being deleted by the cleanup thread.

The same applies to the remount from RO to RW path.

Fix this by making btrfs_find_orphan_roots() run before calling
btrfs_orphan_cleanup() against the root tree.

A test case for fstests will follow soon.

Reported-by: Robbie Ko <robbi...@synology.com>
Link: 
https://lore.kernel.org/linux-btrfs/b19f4310-35e0-606e-1eea-2dd84d28c...@synology.com/
Fixes: 638331fa56caea ("btrfs: fix transaction leak and crash after cleaning up 
orphans on RO mount")
Signed-off-by: Filipe Manana <fdman...@suse.com>
---
 fs/btrfs/disk-io.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 41b718cfea40..7f50b3f65f3a 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3009,6 +3009,21 @@ int btrfs_start_pre_rw_mount(struct btrfs_fs_info 
*fs_info)
                }
        }
 
+       /*
+        * btrfs_find_orphan_roots() is responsible for finding all the dead
+        * roots (with 0 refs), flag them with BTRFS_ROOT_DEAD_TREE and load
+        * them into the fs_info->fs_roots_radix tree. This must be done before
+        * calling btrfs_orphan_cleanup() on the tree root. If we don't do it
+        * first, then btrfs_orphan_cleanup() will delete a dead root's orphan
+        * item before the root's tree is deleted - this means that if we 
unmount
+        * or crash before the deletion completes, on the next mount we will not
+        * delete what remains of the tree because the orphan item does not
+        * exists anymore, which is what tells us we have a pending deletion.
+        */
+       ret = btrfs_find_orphan_roots(fs_info);
+       if (ret)
+               goto out;
+
        ret = btrfs_cleanup_fs_roots(fs_info);
        if (ret)
                goto out;
@@ -3068,7 +3083,6 @@ int btrfs_start_pre_rw_mount(struct btrfs_fs_info 
*fs_info)
                }
        }
 
-       ret = btrfs_find_orphan_roots(fs_info);
 out:
        return ret;
 }
-- 
2.28.0

Reply via email to