[PATCH 0/3] btrfs: allow BTRFS_IOC_SNAP_DESTROY_V2 to remove ghost subvolume

[PATCH 0/3] btrfs: allow BTRFS_IOC_SNAP_DESTROY_V2 to remove ghost subvolume

[Qu Wenruo]

Since we're busting ghost subvolumes, the branch is now called
ghost_busters:
https://github.com/adam900710/linux/tree/ghost_busters

The first two patches are just cleanup found during the development.

The first is a missing check for subvolid range, the missing check
itself won't cause any harm, just returning -ENOENT from dentry lookup,
other than the expected -EINVAL.

The 2nd is a super old dead comment from the early age of btrfs.

The final patch is the real work to allow patched "btrfs subvolume delete -i"
to delete ghost subvolume.
Tested with the image dump of previous submitted btrfs-progs patchset.

Qu Wenruo (3):
  btrfs: return -EINVAL if some user wants to remove uuid/data_reloc
    tree
  btrfs: remove dead comment on btrfs_add_dead_root()
  btrfs: allow BTRFS_IOC_SNAP_DESTROY_V2 to remove ghost subvolume

 fs/btrfs/ioctl.c       | 81 +++++++++++++++++++++++++++++++++++++++++-
 fs/btrfs/transaction.c |  7 ++--
 2 files changed, 84 insertions(+), 4 deletions(-)

-- 
2.32.0

[PATCH 1/3] btrfs: return -EINVAL if some user wants to remove uuid/data_reloc tree

[Qu Wenruo]

Ioctl BTRFS_IOC_SNAP_DESTROY_V2 supports a flag to delete a subvolume
using root id directly.

We check the target root id against BTRFS_FIRST_FREE_OBJECTID, but not
again BTRFS_LAST_FREE_OBJECTID.

This means if user passes rootid like DATA_RELOC (-9) or TREE_RELOC
(-8), we can pass the check, then get caught by later dentry check and
got error number -ENOENT, other than -EINVAL.

It's not a big deal as we have extra safe nets to prevent those
trees get removed, it's still better to do the extra check and return
proper -EINVAL error.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/ioctl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 0ba98e08a029..889e27c24e3a 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -2932,7 +2932,8 @@ static noinline int btrfs_ioctl_snap_destroy(struct file *file,
 			if (err)
 				goto out;
 		} else {
-			if (vol_args2->subvolid < BTRFS_FIRST_FREE_OBJECTID) {
+			if (vol_args2->subvolid < BTRFS_FIRST_FREE_OBJECTID ||
+			    vol_args2->subvolid > BTRFS_LAST_FREE_OBJECTID) {
 				err = -EINVAL;
 				goto out;
 			}
-- 
2.32.0

[PATCH 2/3] btrfs: remove dead comment on btrfs_add_dead_root()

[Qu Wenruo]

The old comment is from the initial merge of btrfs, but since commit
5d4f98a28c7d ("Btrfs: Mixed back reference  (FORWARD ROLLING FORMAT
CHANGE)") changed the behavior to not to allocate any extra memory,
the comment on the memory allocation part is out-of-date.

Fix it by removing the dead part and change it to modern behavior.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/transaction.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index 73df8b81496e..29316c062237 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -1319,9 +1319,10 @@ static noinline int commit_cowonly_roots(struct btrfs_trans_handle *trans)
 }
 
 /*
- * dead roots are old snapshots that need to be deleted.  This allocates
- * a dirty root struct and adds it into the list of dead roots that need to
- * be deleted
+ * Dead roots are old snapshots that need to be deleted.
+ *
+ * This helper will queue them to the dead_roots list to be deleted by
+ * cleaner thread.
  */
 void btrfs_add_dead_root(struct btrfs_root *root)
 {
-- 
2.32.0

[PATCH 3/3] btrfs: allow BTRFS_IOC_SNAP_DESTROY_V2 to remove ghost subvolume

[Qu Wenruo]

There is a report from the mail list that some subvolumes don't have any
ROOT_REF/BACKREF and has 0 ref.
But without an ORPHAN item.

Such ghost subvolumes can't be deleted by any ioctl but only rely on
btrfs-progs to add ORPHAN item.

Normally kernel only needs to gracefully abort/reject such corrupted
structure, but in this case we have all the needed infrastructures and
interface to allow BTRFS_IOC_SNAP_DESTROY_V2 to delete it.

So add the ability to delete such ghost subvolumes to
BTRFS_IOC_SNAP_DESTROY_V2.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/ioctl.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 78 insertions(+)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 889e27c24e3a..06d3c293cffd 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -2892,6 +2892,81 @@ static int btrfs_ioctl_get_subvol_rootref(struct file *file, void __user *argp)
 	return ret;
 }
 
+/*
+ * Special case that some subvolume has missing ORPHAN_ITEM, but its refs is
+ * already 0 (without any ROOT_REF/BACKREF).
+ * In that case such subvolume is only taking space while unable to be deleted.
+ *
+ * No reproducer to reproduce such corruption, but it won't hurt to cleanup them
+ * as we can reuse existing code since we only need to insert an orphan item and
+ * queue them to be deleted.
+ */
+static int __cold remove_ghost_subvol(struct btrfs_fs_info *fs_info,
+				      u64 rootid)
+{
+	struct btrfs_trans_handle *trans;
+	struct btrfs_root *root;
+	struct btrfs_path *path;
+	struct btrfs_key key;
+	int ret;
+
+	root = btrfs_get_fs_root(fs_info, rootid, false);
+	if (IS_ERR(root)) {
+		ret = PTR_ERR(root);
+		return ret;
+	}
+
+	/* A ghost subvolume is already a problem, better to output a warning */
+	btrfs_warn(fs_info, "root %llu has no refs nor orphan item", rootid); 
+	if (btrfs_root_refs(&root->root_item) != 0) {
+		/* We get some strange root */
+		btrfs_warn(fs_info,
+			"root %llu has %u refs, but no proper root backref",
+			rootid, btrfs_root_refs(&root->root_item));
+		ret = -EUCLEAN;
+		goto out;
+	}
+
+	/* Already has orphan inserted */
+	if (test_bit(BTRFS_ROOT_ORPHAN_ITEM_INSERTED, &root->state))
+		goto out;
+
+	path = btrfs_alloc_path();
+	if (!path) {
+		ret = -ENOMEM;
+		goto out;
+	}
+	key.objectid = BTRFS_ORPHAN_OBJECTID;
+	key.type = BTRFS_ORPHAN_ITEM_KEY;
+	key.offset = rootid;
+
+	ret = btrfs_search_slot(NULL, fs_info->tree_root, &key, path, 0, 0);
+	btrfs_free_path(path);
+	/* Either error or there is already an orphan item */
+	if (ret <= 0)
+		goto out;
+
+	trans = btrfs_start_transaction(fs_info->tree_root, 1);
+	if (IS_ERR(trans)) {
+		ret = PTR_ERR(trans);
+		goto out;
+	}
+
+	ret = btrfs_insert_orphan_item(trans, fs_info->tree_root, rootid);
+	if (ret < 0 && ret != -EEXIST) {
+		btrfs_abort_transaction(trans, ret);
+		goto end_trans;
+	}
+	ret = 0;
+	btrfs_add_dead_root(root);
+
+end_trans:
+	btrfs_end_transaction(trans);
+out:
+	btrfs_put_root(root);
+	return ret;
+}
+
 static noinline int btrfs_ioctl_snap_destroy(struct file *file,
 					     void __user *arg,
 					     bool destroy_v2)
@@ -2947,6 +3022,9 @@ static noinline int btrfs_ioctl_snap_destroy(struct file *file,
 					vol_args2->subvolid, 0, 0);
 			if (IS_ERR(dentry)) {
 				err = PTR_ERR(dentry);
+				if (err == -ENOENT)
+					err = remove_ghost_subvol(fs_info,
+							vol_args2->subvolid);
 				goto out_drop_write;
 			}
 
-- 
2.32.0

Re: [PATCH 1/3] btrfs: return -EINVAL if some user wants to remove uuid/data_reloc tree

[Anand Jain]

On 28/6/21 6:16 pm, Qu Wenruo wrote:
> Ioctl BTRFS_IOC_SNAP_DESTROY_V2 supports a flag to delete a subvolume
> using root id directly.
> 
> We check the target root id against BTRFS_FIRST_FREE_OBJECTID, but not
> again BTRFS_LAST_FREE_OBJECTID.
> 
> This means if user passes rootid like DATA_RELOC (-9) or TREE_RELOC
> (-8), we can pass the check, then get caught by later dentry check and
> got error number -ENOENT, other than -EINVAL.
> 
> It's not a big deal as we have extra safe nets to prevent those
> trees get removed, it's still better to do the extra check and return
> proper -EINVAL error.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Reviewed-by: Anand Jain <anand.jain@oracle.com>

Thanks,
-Anand


> ---
>   fs/btrfs/ioctl.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 0ba98e08a029..889e27c24e3a 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -2932,7 +2932,8 @@ static noinline int btrfs_ioctl_snap_destroy(struct file *file,
>   			if (err)
>   				goto out;
>   		} else {
> -			if (vol_args2->subvolid < BTRFS_FIRST_FREE_OBJECTID) {
> +			if (vol_args2->subvolid < BTRFS_FIRST_FREE_OBJECTID ||
> +			    vol_args2->subvolid > BTRFS_LAST_FREE_OBJECTID) {
>   				err = -EINVAL;
>   				goto out;
>   			}
> 

Re: [PATCH 2/3] btrfs: remove dead comment on btrfs_add_dead_root()

[Anand Jain]

On 28/6/21 6:16 pm, Qu Wenruo wrote:
> The old comment is from the initial merge of btrfs, but since commit
> 5d4f98a28c7d ("Btrfs: Mixed back reference  (FORWARD ROLLING FORMAT
> CHANGE)") changed the behavior to not to allocate any extra memory,
> the comment on the memory allocation part is out-of-date.
> 
> Fix it by removing the dead part and change it to modern behavior.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Reviewed-by: Anand Jain <anand.jain@oracle.com>

Thanks, Anand

> ---
>   fs/btrfs/transaction.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index 73df8b81496e..29316c062237 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -1319,9 +1319,10 @@ static noinline int commit_cowonly_roots(struct btrfs_trans_handle *trans)
>   }
>   
>   /*
> - * dead roots are old snapshots that need to be deleted.  This allocates
> - * a dirty root struct and adds it into the list of dead roots that need to
> - * be deleted
> + * Dead roots are old snapshots that need to be deleted.
> + *
> + * This helper will queue them to the dead_roots list to be deleted by
> + * cleaner thread.
>    */
>   void btrfs_add_dead_root(struct btrfs_root *root)
>   {
> 

Re: [PATCH 3/3] btrfs: allow BTRFS_IOC_SNAP_DESTROY_V2 to remove ghost subvolume

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 5109 bytes --]

Hi Qu,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on kdave/for-next]
[also build test WARNING on v5.13 next-20210628]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Qu-Wenruo/btrfs-allow-BTRFS_IOC_SNAP_DESTROY_V2-to-remove-ghost-subvolume/20210628-181747
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
config: x86_64-randconfig-a002-20210628 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 4c92e31dd0f1bd152eda883af20ff7fbcaa14945)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/391ab0041fef5776e7517ab363701d6f86d9406b
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Qu-Wenruo/btrfs-allow-BTRFS_IOC_SNAP_DESTROY_V2-to-remove-ghost-subvolume/20210628-181747
        git checkout 391ab0041fef5776e7517ab363701d6f86d9406b
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> fs/btrfs/ioctl.c:2931:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
           if (test_bit(BTRFS_ROOT_ORPHAN_ITEM_INSERTED, &root->state))
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/ioctl.c:2967:9: note: uninitialized use occurs here
           return ret;
                  ^~~
   fs/btrfs/ioctl.c:2931:2: note: remove the 'if' if its condition is always false
           if (test_bit(BTRFS_ROOT_ORPHAN_ITEM_INSERTED, &root->state))
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/ioctl.c:2911:9: note: initialize the variable 'ret' to silence this warning
           int ret;
                  ^
                   = 0
   1 warning generated.


vim +2931 fs/btrfs/ioctl.c

  2894	
  2895	/*
  2896	 * Special case that some subvolume has missing ORPHAN_ITEM, but its refs is
  2897	 * already 0 (without any ROOT_REF/BACKREF).
  2898	 * In that case such subvolume is only taking space while unable to be deleted.
  2899	 *
  2900	 * No reproducer to reproduce such corruption, but it won't hurt to cleanup them
  2901	 * as we can reuse existing code since we only need to insert an orphan item and
  2902	 * queue them to be deleted.
  2903	 */
  2904	static int __cold remove_ghost_subvol(struct btrfs_fs_info *fs_info,
  2905					      u64 rootid)
  2906	{
  2907		struct btrfs_trans_handle *trans;
  2908		struct btrfs_root *root;
  2909		struct btrfs_path *path;
  2910		struct btrfs_key key;
  2911		int ret;
  2912	
  2913		root = btrfs_get_fs_root(fs_info, rootid, false);
  2914		if (IS_ERR(root)) {
  2915			ret = PTR_ERR(root);
  2916			return ret;
  2917		}
  2918	
  2919		/* A ghost subvolume is already a problem, better to output a warning */
  2920		btrfs_warn(fs_info, "root %llu has no refs nor orphan item", rootid); 
  2921		if (btrfs_root_refs(&root->root_item) != 0) {
  2922			/* We get some strange root */
  2923			btrfs_warn(fs_info,
  2924				"root %llu has %u refs, but no proper root backref",
  2925				rootid, btrfs_root_refs(&root->root_item));
  2926			ret = -EUCLEAN;
  2927			goto out;
  2928		}
  2929	
  2930		/* Already has orphan inserted */
> 2931		if (test_bit(BTRFS_ROOT_ORPHAN_ITEM_INSERTED, &root->state))
  2932			goto out;
  2933	
  2934		path = btrfs_alloc_path();
  2935		if (!path) {
  2936			ret = -ENOMEM;
  2937			goto out;
  2938		}
  2939		key.objectid = BTRFS_ORPHAN_OBJECTID;
  2940		key.type = BTRFS_ORPHAN_ITEM_KEY;
  2941		key.offset = rootid;
  2942	
  2943		ret = btrfs_search_slot(NULL, fs_info->tree_root, &key, path, 0, 0);
  2944		btrfs_free_path(path);
  2945		/* Either error or there is already an orphan item */
  2946		if (ret <= 0)
  2947			goto out;
  2948	
  2949		trans = btrfs_start_transaction(fs_info->tree_root, 1);
  2950		if (IS_ERR(trans)) {
  2951			ret = PTR_ERR(trans);
  2952			goto out;
  2953		}
  2954	
  2955		ret = btrfs_insert_orphan_item(trans, fs_info->tree_root, rootid);
  2956		if (ret < 0 && ret != -EEXIST) {
  2957			btrfs_abort_transaction(trans, ret);
  2958			goto end_trans;
  2959		}
  2960		ret = 0;
  2961		btrfs_add_dead_root(root);
  2962	
  2963	end_trans:
  2964		btrfs_end_transaction(trans);
  2965	out:
  2966		btrfs_put_root(root);
  2967		return ret;
  2968	}
  2969	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 45377 bytes --]

Re: [PATCH 3/3] btrfs: allow BTRFS_IOC_SNAP_DESTROY_V2 to remove ghost subvolume

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 5238 bytes --]

Hi Qu,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on kdave/for-next]
[also build test WARNING on v5.13 next-20210628]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Qu-Wenruo/btrfs-allow-BTRFS_IOC_SNAP_DESTROY_V2-to-remove-ghost-subvolume/20210628-181747
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
config: x86_64-randconfig-a002-20210628 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 4c92e31dd0f1bd152eda883af20ff7fbcaa14945)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/391ab0041fef5776e7517ab363701d6f86d9406b
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Qu-Wenruo/btrfs-allow-BTRFS_IOC_SNAP_DESTROY_V2-to-remove-ghost-subvolume/20210628-181747
        git checkout 391ab0041fef5776e7517ab363701d6f86d9406b
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> fs/btrfs/ioctl.c:2931:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
           if (test_bit(BTRFS_ROOT_ORPHAN_ITEM_INSERTED, &root->state))
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/ioctl.c:2967:9: note: uninitialized use occurs here
           return ret;
                  ^~~
   fs/btrfs/ioctl.c:2931:2: note: remove the 'if' if its condition is always false
           if (test_bit(BTRFS_ROOT_ORPHAN_ITEM_INSERTED, &root->state))
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/ioctl.c:2911:9: note: initialize the variable 'ret' to silence this warning
           int ret;
                  ^
                   = 0
   1 warning generated.


vim +2931 fs/btrfs/ioctl.c

  2894	
  2895	/*
  2896	 * Special case that some subvolume has missing ORPHAN_ITEM, but its refs is
  2897	 * already 0 (without any ROOT_REF/BACKREF).
  2898	 * In that case such subvolume is only taking space while unable to be deleted.
  2899	 *
  2900	 * No reproducer to reproduce such corruption, but it won't hurt to cleanup them
  2901	 * as we can reuse existing code since we only need to insert an orphan item and
  2902	 * queue them to be deleted.
  2903	 */
  2904	static int __cold remove_ghost_subvol(struct btrfs_fs_info *fs_info,
  2905					      u64 rootid)
  2906	{
  2907		struct btrfs_trans_handle *trans;
  2908		struct btrfs_root *root;
  2909		struct btrfs_path *path;
  2910		struct btrfs_key key;
  2911		int ret;
  2912	
  2913		root = btrfs_get_fs_root(fs_info, rootid, false);
  2914		if (IS_ERR(root)) {
  2915			ret = PTR_ERR(root);
  2916			return ret;
  2917		}
  2918	
  2919		/* A ghost subvolume is already a problem, better to output a warning */
  2920		btrfs_warn(fs_info, "root %llu has no refs nor orphan item", rootid); 
  2921		if (btrfs_root_refs(&root->root_item) != 0) {
  2922			/* We get some strange root */
  2923			btrfs_warn(fs_info,
  2924				"root %llu has %u refs, but no proper root backref",
  2925				rootid, btrfs_root_refs(&root->root_item));
  2926			ret = -EUCLEAN;
  2927			goto out;
  2928		}
  2929	
  2930		/* Already has orphan inserted */
> 2931		if (test_bit(BTRFS_ROOT_ORPHAN_ITEM_INSERTED, &root->state))
  2932			goto out;
  2933	
  2934		path = btrfs_alloc_path();
  2935		if (!path) {
  2936			ret = -ENOMEM;
  2937			goto out;
  2938		}
  2939		key.objectid = BTRFS_ORPHAN_OBJECTID;
  2940		key.type = BTRFS_ORPHAN_ITEM_KEY;
  2941		key.offset = rootid;
  2942	
  2943		ret = btrfs_search_slot(NULL, fs_info->tree_root, &key, path, 0, 0);
  2944		btrfs_free_path(path);
  2945		/* Either error or there is already an orphan item */
  2946		if (ret <= 0)
  2947			goto out;
  2948	
  2949		trans = btrfs_start_transaction(fs_info->tree_root, 1);
  2950		if (IS_ERR(trans)) {
  2951			ret = PTR_ERR(trans);
  2952			goto out;
  2953		}
  2954	
  2955		ret = btrfs_insert_orphan_item(trans, fs_info->tree_root, rootid);
  2956		if (ret < 0 && ret != -EEXIST) {
  2957			btrfs_abort_transaction(trans, ret);
  2958			goto end_trans;
  2959		}
  2960		ret = 0;
  2961		btrfs_add_dead_root(root);
  2962	
  2963	end_trans:
  2964		btrfs_end_transaction(trans);
  2965	out:
  2966		btrfs_put_root(root);
  2967		return ret;
  2968	}
  2969	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 45377 bytes --]

Re: [PATCH 3/3] btrfs: allow BTRFS_IOC_SNAP_DESTROY_V2 to remove ghost subvolume

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 5101 bytes --]

Hi Qu,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on kdave/for-next]
[also build test WARNING on v5.13 next-20210628]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Qu-Wenruo/btrfs-allow-BTRFS_IOC_SNAP_DESTROY_V2-to-remove-ghost-subvolume/20210628-181747
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
config: arm-randconfig-r025-20210628 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 4c92e31dd0f1bd152eda883af20ff7fbcaa14945)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/0day-ci/linux/commit/391ab0041fef5776e7517ab363701d6f86d9406b
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Qu-Wenruo/btrfs-allow-BTRFS_IOC_SNAP_DESTROY_V2-to-remove-ghost-subvolume/20210628-181747
        git checkout 391ab0041fef5776e7517ab363701d6f86d9406b
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> fs/btrfs/ioctl.c:2931:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
           if (test_bit(BTRFS_ROOT_ORPHAN_ITEM_INSERTED, &root->state))
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/ioctl.c:2967:9: note: uninitialized use occurs here
           return ret;
                  ^~~
   fs/btrfs/ioctl.c:2931:2: note: remove the 'if' if its condition is always false
           if (test_bit(BTRFS_ROOT_ORPHAN_ITEM_INSERTED, &root->state))
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/ioctl.c:2911:9: note: initialize the variable 'ret' to silence this warning
           int ret;
                  ^
                   = 0
   1 warning generated.


vim +2931 fs/btrfs/ioctl.c

  2894	
  2895	/*
  2896	 * Special case that some subvolume has missing ORPHAN_ITEM, but its refs is
  2897	 * already 0 (without any ROOT_REF/BACKREF).
  2898	 * In that case such subvolume is only taking space while unable to be deleted.
  2899	 *
  2900	 * No reproducer to reproduce such corruption, but it won't hurt to cleanup them
  2901	 * as we can reuse existing code since we only need to insert an orphan item and
  2902	 * queue them to be deleted.
  2903	 */
  2904	static int __cold remove_ghost_subvol(struct btrfs_fs_info *fs_info,
  2905					      u64 rootid)
  2906	{
  2907		struct btrfs_trans_handle *trans;
  2908		struct btrfs_root *root;
  2909		struct btrfs_path *path;
  2910		struct btrfs_key key;
  2911		int ret;
  2912	
  2913		root = btrfs_get_fs_root(fs_info, rootid, false);
  2914		if (IS_ERR(root)) {
  2915			ret = PTR_ERR(root);
  2916			return ret;
  2917		}
  2918	
  2919		/* A ghost subvolume is already a problem, better to output a warning */
  2920		btrfs_warn(fs_info, "root %llu has no refs nor orphan item", rootid); 
  2921		if (btrfs_root_refs(&root->root_item) != 0) {
  2922			/* We get some strange root */
  2923			btrfs_warn(fs_info,
  2924				"root %llu has %u refs, but no proper root backref",
  2925				rootid, btrfs_root_refs(&root->root_item));
  2926			ret = -EUCLEAN;
  2927			goto out;
  2928		}
  2929	
  2930		/* Already has orphan inserted */
> 2931		if (test_bit(BTRFS_ROOT_ORPHAN_ITEM_INSERTED, &root->state))
  2932			goto out;
  2933	
  2934		path = btrfs_alloc_path();
  2935		if (!path) {
  2936			ret = -ENOMEM;
  2937			goto out;
  2938		}
  2939		key.objectid = BTRFS_ORPHAN_OBJECTID;
  2940		key.type = BTRFS_ORPHAN_ITEM_KEY;
  2941		key.offset = rootid;
  2942	
  2943		ret = btrfs_search_slot(NULL, fs_info->tree_root, &key, path, 0, 0);
  2944		btrfs_free_path(path);
  2945		/* Either error or there is already an orphan item */
  2946		if (ret <= 0)
  2947			goto out;
  2948	
  2949		trans = btrfs_start_transaction(fs_info->tree_root, 1);
  2950		if (IS_ERR(trans)) {
  2951			ret = PTR_ERR(trans);
  2952			goto out;
  2953		}
  2954	
  2955		ret = btrfs_insert_orphan_item(trans, fs_info->tree_root, rootid);
  2956		if (ret < 0 && ret != -EEXIST) {
  2957			btrfs_abort_transaction(trans, ret);
  2958			goto end_trans;
  2959		}
  2960		ret = 0;
  2961		btrfs_add_dead_root(root);
  2962	
  2963	end_trans:
  2964		btrfs_end_transaction(trans);
  2965	out:
  2966		btrfs_put_root(root);
  2967		return ret;
  2968	}
  2969	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35286 bytes --]

Re: [PATCH 3/3] btrfs: allow BTRFS_IOC_SNAP_DESTROY_V2 to remove ghost subvolume

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 5230 bytes --]

Hi Qu,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on kdave/for-next]
[also build test WARNING on v5.13 next-20210628]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Qu-Wenruo/btrfs-allow-BTRFS_IOC_SNAP_DESTROY_V2-to-remove-ghost-subvolume/20210628-181747
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
config: arm-randconfig-r025-20210628 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 4c92e31dd0f1bd152eda883af20ff7fbcaa14945)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/0day-ci/linux/commit/391ab0041fef5776e7517ab363701d6f86d9406b
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Qu-Wenruo/btrfs-allow-BTRFS_IOC_SNAP_DESTROY_V2-to-remove-ghost-subvolume/20210628-181747
        git checkout 391ab0041fef5776e7517ab363701d6f86d9406b
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> fs/btrfs/ioctl.c:2931:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
           if (test_bit(BTRFS_ROOT_ORPHAN_ITEM_INSERTED, &root->state))
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/ioctl.c:2967:9: note: uninitialized use occurs here
           return ret;
                  ^~~
   fs/btrfs/ioctl.c:2931:2: note: remove the 'if' if its condition is always false
           if (test_bit(BTRFS_ROOT_ORPHAN_ITEM_INSERTED, &root->state))
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/ioctl.c:2911:9: note: initialize the variable 'ret' to silence this warning
           int ret;
                  ^
                   = 0
   1 warning generated.


vim +2931 fs/btrfs/ioctl.c

  2894	
  2895	/*
  2896	 * Special case that some subvolume has missing ORPHAN_ITEM, but its refs is
  2897	 * already 0 (without any ROOT_REF/BACKREF).
  2898	 * In that case such subvolume is only taking space while unable to be deleted.
  2899	 *
  2900	 * No reproducer to reproduce such corruption, but it won't hurt to cleanup them
  2901	 * as we can reuse existing code since we only need to insert an orphan item and
  2902	 * queue them to be deleted.
  2903	 */
  2904	static int __cold remove_ghost_subvol(struct btrfs_fs_info *fs_info,
  2905					      u64 rootid)
  2906	{
  2907		struct btrfs_trans_handle *trans;
  2908		struct btrfs_root *root;
  2909		struct btrfs_path *path;
  2910		struct btrfs_key key;
  2911		int ret;
  2912	
  2913		root = btrfs_get_fs_root(fs_info, rootid, false);
  2914		if (IS_ERR(root)) {
  2915			ret = PTR_ERR(root);
  2916			return ret;
  2917		}
  2918	
  2919		/* A ghost subvolume is already a problem, better to output a warning */
  2920		btrfs_warn(fs_info, "root %llu has no refs nor orphan item", rootid); 
  2921		if (btrfs_root_refs(&root->root_item) != 0) {
  2922			/* We get some strange root */
  2923			btrfs_warn(fs_info,
  2924				"root %llu has %u refs, but no proper root backref",
  2925				rootid, btrfs_root_refs(&root->root_item));
  2926			ret = -EUCLEAN;
  2927			goto out;
  2928		}
  2929	
  2930		/* Already has orphan inserted */
> 2931		if (test_bit(BTRFS_ROOT_ORPHAN_ITEM_INSERTED, &root->state))
  2932			goto out;
  2933	
  2934		path = btrfs_alloc_path();
  2935		if (!path) {
  2936			ret = -ENOMEM;
  2937			goto out;
  2938		}
  2939		key.objectid = BTRFS_ORPHAN_OBJECTID;
  2940		key.type = BTRFS_ORPHAN_ITEM_KEY;
  2941		key.offset = rootid;
  2942	
  2943		ret = btrfs_search_slot(NULL, fs_info->tree_root, &key, path, 0, 0);
  2944		btrfs_free_path(path);
  2945		/* Either error or there is already an orphan item */
  2946		if (ret <= 0)
  2947			goto out;
  2948	
  2949		trans = btrfs_start_transaction(fs_info->tree_root, 1);
  2950		if (IS_ERR(trans)) {
  2951			ret = PTR_ERR(trans);
  2952			goto out;
  2953		}
  2954	
  2955		ret = btrfs_insert_orphan_item(trans, fs_info->tree_root, rootid);
  2956		if (ret < 0 && ret != -EEXIST) {
  2957			btrfs_abort_transaction(trans, ret);
  2958			goto end_trans;
  2959		}
  2960		ret = 0;
  2961		btrfs_add_dead_root(root);
  2962	
  2963	end_trans:
  2964		btrfs_end_transaction(trans);
  2965	out:
  2966		btrfs_put_root(root);
  2967		return ret;
  2968	}
  2969	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 35286 bytes --]

Re: [PATCH 3/3] btrfs: allow BTRFS_IOC_SNAP_DESTROY_V2 to remove ghost subvolume

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 7359 bytes --]

CC: kbuild-all(a)lists.01.org
In-Reply-To: <20210628101637.349718-4-wqu@suse.com>
References: <20210628101637.349718-4-wqu@suse.com>
TO: Qu Wenruo <wqu@suse.com>
TO: linux-btrfs(a)vger.kernel.org

Hi Qu,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on kdave/for-next]
[also build test WARNING on v5.13 next-20210628]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Qu-Wenruo/btrfs-allow-BTRFS_IOC_SNAP_DESTROY_V2-to-remove-ghost-subvolume/20210628-181747
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
:::::: branch date: 7 hours ago
:::::: commit date: 7 hours ago
config: i386-randconfig-m021-20210628 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

New smatch warnings:
fs/btrfs/ioctl.c:2967 remove_ghost_subvol() error: uninitialized symbol 'ret'.

Old smatch warnings:
fs/btrfs/ioctl.c:808 create_snapshot() warn: '&pending_snapshot->list' not removed from list
fs/btrfs/ioctl.c:1568 btrfs_defrag_file() warn: should 'ret << 12' be a 64 bit type?

vim +/ret +2967 fs/btrfs/ioctl.c

42e4b520c812da Tomohiro Misono 2018-05-21  2894  
391ab0041fef57 Qu Wenruo       2021-06-28  2895  /*
391ab0041fef57 Qu Wenruo       2021-06-28  2896   * Special case that some subvolume has missing ORPHAN_ITEM, but its refs is
391ab0041fef57 Qu Wenruo       2021-06-28  2897   * already 0 (without any ROOT_REF/BACKREF).
391ab0041fef57 Qu Wenruo       2021-06-28  2898   * In that case such subvolume is only taking space while unable to be deleted.
391ab0041fef57 Qu Wenruo       2021-06-28  2899   *
391ab0041fef57 Qu Wenruo       2021-06-28  2900   * No reproducer to reproduce such corruption, but it won't hurt to cleanup them
391ab0041fef57 Qu Wenruo       2021-06-28  2901   * as we can reuse existing code since we only need to insert an orphan item and
391ab0041fef57 Qu Wenruo       2021-06-28  2902   * queue them to be deleted.
391ab0041fef57 Qu Wenruo       2021-06-28  2903   */
391ab0041fef57 Qu Wenruo       2021-06-28  2904  static int __cold remove_ghost_subvol(struct btrfs_fs_info *fs_info,
391ab0041fef57 Qu Wenruo       2021-06-28  2905  				      u64 rootid)
391ab0041fef57 Qu Wenruo       2021-06-28  2906  {
391ab0041fef57 Qu Wenruo       2021-06-28  2907  	struct btrfs_trans_handle *trans;
391ab0041fef57 Qu Wenruo       2021-06-28  2908  	struct btrfs_root *root;
391ab0041fef57 Qu Wenruo       2021-06-28  2909  	struct btrfs_path *path;
391ab0041fef57 Qu Wenruo       2021-06-28  2910  	struct btrfs_key key;
391ab0041fef57 Qu Wenruo       2021-06-28  2911  	int ret;
391ab0041fef57 Qu Wenruo       2021-06-28  2912  
391ab0041fef57 Qu Wenruo       2021-06-28  2913  	root = btrfs_get_fs_root(fs_info, rootid, false);
391ab0041fef57 Qu Wenruo       2021-06-28  2914  	if (IS_ERR(root)) {
391ab0041fef57 Qu Wenruo       2021-06-28  2915  		ret = PTR_ERR(root);
391ab0041fef57 Qu Wenruo       2021-06-28  2916  		return ret;
391ab0041fef57 Qu Wenruo       2021-06-28  2917  	}
391ab0041fef57 Qu Wenruo       2021-06-28  2918  
391ab0041fef57 Qu Wenruo       2021-06-28  2919  	/* A ghost subvolume is already a problem, better to output a warning */
391ab0041fef57 Qu Wenruo       2021-06-28  2920  	btrfs_warn(fs_info, "root %llu has no refs nor orphan item", rootid); 
391ab0041fef57 Qu Wenruo       2021-06-28  2921  	if (btrfs_root_refs(&root->root_item) != 0) {
391ab0041fef57 Qu Wenruo       2021-06-28  2922  		/* We get some strange root */
391ab0041fef57 Qu Wenruo       2021-06-28  2923  		btrfs_warn(fs_info,
391ab0041fef57 Qu Wenruo       2021-06-28  2924  			"root %llu has %u refs, but no proper root backref",
391ab0041fef57 Qu Wenruo       2021-06-28  2925  			rootid, btrfs_root_refs(&root->root_item));
391ab0041fef57 Qu Wenruo       2021-06-28  2926  		ret = -EUCLEAN;
391ab0041fef57 Qu Wenruo       2021-06-28  2927  		goto out;
391ab0041fef57 Qu Wenruo       2021-06-28  2928  	}
391ab0041fef57 Qu Wenruo       2021-06-28  2929  
391ab0041fef57 Qu Wenruo       2021-06-28  2930  	/* Already has orphan inserted */
391ab0041fef57 Qu Wenruo       2021-06-28  2931  	if (test_bit(BTRFS_ROOT_ORPHAN_ITEM_INSERTED, &root->state))
391ab0041fef57 Qu Wenruo       2021-06-28  2932  		goto out;
391ab0041fef57 Qu Wenruo       2021-06-28  2933  
391ab0041fef57 Qu Wenruo       2021-06-28  2934  	path = btrfs_alloc_path();
391ab0041fef57 Qu Wenruo       2021-06-28  2935  	if (!path) {
391ab0041fef57 Qu Wenruo       2021-06-28  2936  		ret = -ENOMEM;
391ab0041fef57 Qu Wenruo       2021-06-28  2937  		goto out;
391ab0041fef57 Qu Wenruo       2021-06-28  2938  	}
391ab0041fef57 Qu Wenruo       2021-06-28  2939  	key.objectid = BTRFS_ORPHAN_OBJECTID;
391ab0041fef57 Qu Wenruo       2021-06-28  2940  	key.type = BTRFS_ORPHAN_ITEM_KEY;
391ab0041fef57 Qu Wenruo       2021-06-28  2941  	key.offset = rootid;
391ab0041fef57 Qu Wenruo       2021-06-28  2942  
391ab0041fef57 Qu Wenruo       2021-06-28  2943  	ret = btrfs_search_slot(NULL, fs_info->tree_root, &key, path, 0, 0);
391ab0041fef57 Qu Wenruo       2021-06-28  2944  	btrfs_free_path(path);
391ab0041fef57 Qu Wenruo       2021-06-28  2945  	/* Either error or there is already an orphan item */
391ab0041fef57 Qu Wenruo       2021-06-28  2946  	if (ret <= 0)
391ab0041fef57 Qu Wenruo       2021-06-28  2947  		goto out;
391ab0041fef57 Qu Wenruo       2021-06-28  2948  
391ab0041fef57 Qu Wenruo       2021-06-28  2949  	trans = btrfs_start_transaction(fs_info->tree_root, 1);
391ab0041fef57 Qu Wenruo       2021-06-28  2950  	if (IS_ERR(trans)) {
391ab0041fef57 Qu Wenruo       2021-06-28  2951  		ret = PTR_ERR(trans);
391ab0041fef57 Qu Wenruo       2021-06-28  2952  		goto out;
391ab0041fef57 Qu Wenruo       2021-06-28  2953  	}
391ab0041fef57 Qu Wenruo       2021-06-28  2954  
391ab0041fef57 Qu Wenruo       2021-06-28  2955  	ret = btrfs_insert_orphan_item(trans, fs_info->tree_root, rootid);
391ab0041fef57 Qu Wenruo       2021-06-28  2956  	if (ret < 0 && ret != -EEXIST) {
391ab0041fef57 Qu Wenruo       2021-06-28  2957  		btrfs_abort_transaction(trans, ret);
391ab0041fef57 Qu Wenruo       2021-06-28  2958  		goto end_trans;
391ab0041fef57 Qu Wenruo       2021-06-28  2959  	}
391ab0041fef57 Qu Wenruo       2021-06-28  2960  	ret = 0;
391ab0041fef57 Qu Wenruo       2021-06-28  2961  	btrfs_add_dead_root(root);
391ab0041fef57 Qu Wenruo       2021-06-28  2962  
391ab0041fef57 Qu Wenruo       2021-06-28  2963  end_trans:
391ab0041fef57 Qu Wenruo       2021-06-28  2964  	btrfs_end_transaction(trans);
391ab0041fef57 Qu Wenruo       2021-06-28  2965  out:
391ab0041fef57 Qu Wenruo       2021-06-28  2966  	btrfs_put_root(root);
391ab0041fef57 Qu Wenruo       2021-06-28 @2967  	return ret;
391ab0041fef57 Qu Wenruo       2021-06-28  2968  }
391ab0041fef57 Qu Wenruo       2021-06-28  2969  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 39986 bytes --]

[kbuild] Re: [PATCH 3/3] btrfs: allow BTRFS_IOC_SNAP_DESTROY_V2 to remove ghost subvolume

[Dan Carpenter]

Hi Qu,

url:    https://github.com/0day-ci/linux/commits/Qu-Wenruo/btrfs-allow-BTRFS_IOC_SNAP_DESTROY_V2-to-remove-ghost-subvolume/20210628-181747 
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git  for-next
config: i386-randconfig-m021-20210628 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

New smatch warnings:
fs/btrfs/ioctl.c:2967 remove_ghost_subvol() error: uninitialized symbol 'ret'.

Old smatch warnings:
fs/btrfs/ioctl.c:808 create_snapshot() warn: '&pending_snapshot->list' not removed from list
fs/btrfs/ioctl.c:1568 btrfs_defrag_file() warn: should 'ret << 12' be a 64 bit type?

vim +/ret +2967 fs/btrfs/ioctl.c

391ab0041fef57 Qu Wenruo       2021-06-28  2904  static int __cold remove_ghost_subvol(struct btrfs_fs_info *fs_info,
391ab0041fef57 Qu Wenruo       2021-06-28  2905  				      u64 rootid)
391ab0041fef57 Qu Wenruo       2021-06-28  2906  {
391ab0041fef57 Qu Wenruo       2021-06-28  2907  	struct btrfs_trans_handle *trans;
391ab0041fef57 Qu Wenruo       2021-06-28  2908  	struct btrfs_root *root;
391ab0041fef57 Qu Wenruo       2021-06-28  2909  	struct btrfs_path *path;
391ab0041fef57 Qu Wenruo       2021-06-28  2910  	struct btrfs_key key;
391ab0041fef57 Qu Wenruo       2021-06-28  2911  	int ret;
391ab0041fef57 Qu Wenruo       2021-06-28  2912  
391ab0041fef57 Qu Wenruo       2021-06-28  2913  	root = btrfs_get_fs_root(fs_info, rootid, false);
391ab0041fef57 Qu Wenruo       2021-06-28  2914  	if (IS_ERR(root)) {
391ab0041fef57 Qu Wenruo       2021-06-28  2915  		ret = PTR_ERR(root);
391ab0041fef57 Qu Wenruo       2021-06-28  2916  		return ret;

return PTR_ERR(root);

391ab0041fef57 Qu Wenruo       2021-06-28  2917  	}
391ab0041fef57 Qu Wenruo       2021-06-28  2918  
391ab0041fef57 Qu Wenruo       2021-06-28  2919  	/* A ghost subvolume is already a problem, better to output a warning */
391ab0041fef57 Qu Wenruo       2021-06-28  2920  	btrfs_warn(fs_info, "root %llu has no refs nor orphan item", rootid); 
391ab0041fef57 Qu Wenruo       2021-06-28  2921  	if (btrfs_root_refs(&root->root_item) != 0) {
391ab0041fef57 Qu Wenruo       2021-06-28  2922  		/* We get some strange root */
391ab0041fef57 Qu Wenruo       2021-06-28  2923  		btrfs_warn(fs_info,
391ab0041fef57 Qu Wenruo       2021-06-28  2924  			"root %llu has %u refs, but no proper root backref",
391ab0041fef57 Qu Wenruo       2021-06-28  2925  			rootid, btrfs_root_refs(&root->root_item));
391ab0041fef57 Qu Wenruo       2021-06-28  2926  		ret = -EUCLEAN;
391ab0041fef57 Qu Wenruo       2021-06-28  2927  		goto out;
391ab0041fef57 Qu Wenruo       2021-06-28  2928  	}
391ab0041fef57 Qu Wenruo       2021-06-28  2929  
391ab0041fef57 Qu Wenruo       2021-06-28  2930  	/* Already has orphan inserted */
391ab0041fef57 Qu Wenruo       2021-06-28  2931  	if (test_bit(BTRFS_ROOT_ORPHAN_ITEM_INSERTED, &root->state))
391ab0041fef57 Qu Wenruo       2021-06-28  2932  		goto out;

ret not intialized on this path.

391ab0041fef57 Qu Wenruo       2021-06-28  2933  
391ab0041fef57 Qu Wenruo       2021-06-28  2934  	path = btrfs_alloc_path();
391ab0041fef57 Qu Wenruo       2021-06-28  2935  	if (!path) {
391ab0041fef57 Qu Wenruo       2021-06-28  2936  		ret = -ENOMEM;
391ab0041fef57 Qu Wenruo       2021-06-28  2937  		goto out;
391ab0041fef57 Qu Wenruo       2021-06-28  2938  	}
391ab0041fef57 Qu Wenruo       2021-06-28  2939  	key.objectid = BTRFS_ORPHAN_OBJECTID;
391ab0041fef57 Qu Wenruo       2021-06-28  2940  	key.type = BTRFS_ORPHAN_ITEM_KEY;
391ab0041fef57 Qu Wenruo       2021-06-28  2941  	key.offset = rootid;
391ab0041fef57 Qu Wenruo       2021-06-28  2942  
391ab0041fef57 Qu Wenruo       2021-06-28  2943  	ret = btrfs_search_slot(NULL, fs_info->tree_root, &key, path, 0, 0);
391ab0041fef57 Qu Wenruo       2021-06-28  2944  	btrfs_free_path(path);
391ab0041fef57 Qu Wenruo       2021-06-28  2945  	/* Either error or there is already an orphan item */
391ab0041fef57 Qu Wenruo       2021-06-28  2946  	if (ret <= 0)
391ab0041fef57 Qu Wenruo       2021-06-28  2947  		goto out;
391ab0041fef57 Qu Wenruo       2021-06-28  2948  
391ab0041fef57 Qu Wenruo       2021-06-28  2949  	trans = btrfs_start_transaction(fs_info->tree_root, 1);
391ab0041fef57 Qu Wenruo       2021-06-28  2950  	if (IS_ERR(trans)) {
391ab0041fef57 Qu Wenruo       2021-06-28  2951  		ret = PTR_ERR(trans);
391ab0041fef57 Qu Wenruo       2021-06-28  2952  		goto out;
391ab0041fef57 Qu Wenruo       2021-06-28  2953  	}
391ab0041fef57 Qu Wenruo       2021-06-28  2954  
391ab0041fef57 Qu Wenruo       2021-06-28  2955  	ret = btrfs_insert_orphan_item(trans, fs_info->tree_root, rootid);
391ab0041fef57 Qu Wenruo       2021-06-28  2956  	if (ret < 0 && ret != -EEXIST) {
391ab0041fef57 Qu Wenruo       2021-06-28  2957  		btrfs_abort_transaction(trans, ret);
391ab0041fef57 Qu Wenruo       2021-06-28  2958  		goto end_trans;
391ab0041fef57 Qu Wenruo       2021-06-28  2959  	}
391ab0041fef57 Qu Wenruo       2021-06-28  2960  	ret = 0;
391ab0041fef57 Qu Wenruo       2021-06-28  2961  	btrfs_add_dead_root(root);
391ab0041fef57 Qu Wenruo       2021-06-28  2962  
391ab0041fef57 Qu Wenruo       2021-06-28  2963  end_trans:
391ab0041fef57 Qu Wenruo       2021-06-28  2964  	btrfs_end_transaction(trans);
391ab0041fef57 Qu Wenruo       2021-06-28  2965  out:
391ab0041fef57 Qu Wenruo       2021-06-28  2966  	btrfs_put_root(root);
391ab0041fef57 Qu Wenruo       2021-06-28 @2967  	return ret;
391ab0041fef57 Qu Wenruo       2021-06-28  2968  }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org 

_______________________________________________
kbuild mailing list -- kbuild@lists.01.org
To unsubscribe send an email to kbuild-leave@lists.01.org

[kbuild] Re: [PATCH 3/3] btrfs: allow BTRFS_IOC_SNAP_DESTROY_V2 to remove ghost subvolume

[Dan Carpenter]

[-- Attachment #1: Type: text/plain, Size: 6017 bytes --]

Hi Qu,

url:    https://github.com/0day-ci/linux/commits/Qu-Wenruo/btrfs-allow-BTRFS_IOC_SNAP_DESTROY_V2-to-remove-ghost-subvolume/20210628-181747 
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git  for-next
config: i386-randconfig-m021-20210628 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

New smatch warnings:
fs/btrfs/ioctl.c:2967 remove_ghost_subvol() error: uninitialized symbol 'ret'.

Old smatch warnings:
fs/btrfs/ioctl.c:808 create_snapshot() warn: '&pending_snapshot->list' not removed from list
fs/btrfs/ioctl.c:1568 btrfs_defrag_file() warn: should 'ret << 12' be a 64 bit type?

vim +/ret +2967 fs/btrfs/ioctl.c

391ab0041fef57 Qu Wenruo       2021-06-28  2904  static int __cold remove_ghost_subvol(struct btrfs_fs_info *fs_info,
391ab0041fef57 Qu Wenruo       2021-06-28  2905  				      u64 rootid)
391ab0041fef57 Qu Wenruo       2021-06-28  2906  {
391ab0041fef57 Qu Wenruo       2021-06-28  2907  	struct btrfs_trans_handle *trans;
391ab0041fef57 Qu Wenruo       2021-06-28  2908  	struct btrfs_root *root;
391ab0041fef57 Qu Wenruo       2021-06-28  2909  	struct btrfs_path *path;
391ab0041fef57 Qu Wenruo       2021-06-28  2910  	struct btrfs_key key;
391ab0041fef57 Qu Wenruo       2021-06-28  2911  	int ret;
391ab0041fef57 Qu Wenruo       2021-06-28  2912  
391ab0041fef57 Qu Wenruo       2021-06-28  2913  	root = btrfs_get_fs_root(fs_info, rootid, false);
391ab0041fef57 Qu Wenruo       2021-06-28  2914  	if (IS_ERR(root)) {
391ab0041fef57 Qu Wenruo       2021-06-28  2915  		ret = PTR_ERR(root);
391ab0041fef57 Qu Wenruo       2021-06-28  2916  		return ret;

return PTR_ERR(root);

391ab0041fef57 Qu Wenruo       2021-06-28  2917  	}
391ab0041fef57 Qu Wenruo       2021-06-28  2918  
391ab0041fef57 Qu Wenruo       2021-06-28  2919  	/* A ghost subvolume is already a problem, better to output a warning */
391ab0041fef57 Qu Wenruo       2021-06-28  2920  	btrfs_warn(fs_info, "root %llu has no refs nor orphan item", rootid); 
391ab0041fef57 Qu Wenruo       2021-06-28  2921  	if (btrfs_root_refs(&root->root_item) != 0) {
391ab0041fef57 Qu Wenruo       2021-06-28  2922  		/* We get some strange root */
391ab0041fef57 Qu Wenruo       2021-06-28  2923  		btrfs_warn(fs_info,
391ab0041fef57 Qu Wenruo       2021-06-28  2924  			"root %llu has %u refs, but no proper root backref",
391ab0041fef57 Qu Wenruo       2021-06-28  2925  			rootid, btrfs_root_refs(&root->root_item));
391ab0041fef57 Qu Wenruo       2021-06-28  2926  		ret = -EUCLEAN;
391ab0041fef57 Qu Wenruo       2021-06-28  2927  		goto out;
391ab0041fef57 Qu Wenruo       2021-06-28  2928  	}
391ab0041fef57 Qu Wenruo       2021-06-28  2929  
391ab0041fef57 Qu Wenruo       2021-06-28  2930  	/* Already has orphan inserted */
391ab0041fef57 Qu Wenruo       2021-06-28  2931  	if (test_bit(BTRFS_ROOT_ORPHAN_ITEM_INSERTED, &root->state))
391ab0041fef57 Qu Wenruo       2021-06-28  2932  		goto out;

ret not intialized on this path.

391ab0041fef57 Qu Wenruo       2021-06-28  2933  
391ab0041fef57 Qu Wenruo       2021-06-28  2934  	path = btrfs_alloc_path();
391ab0041fef57 Qu Wenruo       2021-06-28  2935  	if (!path) {
391ab0041fef57 Qu Wenruo       2021-06-28  2936  		ret = -ENOMEM;
391ab0041fef57 Qu Wenruo       2021-06-28  2937  		goto out;
391ab0041fef57 Qu Wenruo       2021-06-28  2938  	}
391ab0041fef57 Qu Wenruo       2021-06-28  2939  	key.objectid = BTRFS_ORPHAN_OBJECTID;
391ab0041fef57 Qu Wenruo       2021-06-28  2940  	key.type = BTRFS_ORPHAN_ITEM_KEY;
391ab0041fef57 Qu Wenruo       2021-06-28  2941  	key.offset = rootid;
391ab0041fef57 Qu Wenruo       2021-06-28  2942  
391ab0041fef57 Qu Wenruo       2021-06-28  2943  	ret = btrfs_search_slot(NULL, fs_info->tree_root, &key, path, 0, 0);
391ab0041fef57 Qu Wenruo       2021-06-28  2944  	btrfs_free_path(path);
391ab0041fef57 Qu Wenruo       2021-06-28  2945  	/* Either error or there is already an orphan item */
391ab0041fef57 Qu Wenruo       2021-06-28  2946  	if (ret <= 0)
391ab0041fef57 Qu Wenruo       2021-06-28  2947  		goto out;
391ab0041fef57 Qu Wenruo       2021-06-28  2948  
391ab0041fef57 Qu Wenruo       2021-06-28  2949  	trans = btrfs_start_transaction(fs_info->tree_root, 1);
391ab0041fef57 Qu Wenruo       2021-06-28  2950  	if (IS_ERR(trans)) {
391ab0041fef57 Qu Wenruo       2021-06-28  2951  		ret = PTR_ERR(trans);
391ab0041fef57 Qu Wenruo       2021-06-28  2952  		goto out;
391ab0041fef57 Qu Wenruo       2021-06-28  2953  	}
391ab0041fef57 Qu Wenruo       2021-06-28  2954  
391ab0041fef57 Qu Wenruo       2021-06-28  2955  	ret = btrfs_insert_orphan_item(trans, fs_info->tree_root, rootid);
391ab0041fef57 Qu Wenruo       2021-06-28  2956  	if (ret < 0 && ret != -EEXIST) {
391ab0041fef57 Qu Wenruo       2021-06-28  2957  		btrfs_abort_transaction(trans, ret);
391ab0041fef57 Qu Wenruo       2021-06-28  2958  		goto end_trans;
391ab0041fef57 Qu Wenruo       2021-06-28  2959  	}
391ab0041fef57 Qu Wenruo       2021-06-28  2960  	ret = 0;
391ab0041fef57 Qu Wenruo       2021-06-28  2961  	btrfs_add_dead_root(root);
391ab0041fef57 Qu Wenruo       2021-06-28  2962  
391ab0041fef57 Qu Wenruo       2021-06-28  2963  end_trans:
391ab0041fef57 Qu Wenruo       2021-06-28  2964  	btrfs_end_transaction(trans);
391ab0041fef57 Qu Wenruo       2021-06-28  2965  out:
391ab0041fef57 Qu Wenruo       2021-06-28  2966  	btrfs_put_root(root);
391ab0041fef57 Qu Wenruo       2021-06-28 @2967  	return ret;
391ab0041fef57 Qu Wenruo       2021-06-28  2968  }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org 

_______________________________________________
kbuild mailing list -- kbuild(a)lists.01.org
To unsubscribe send an email to kbuild-leave@lists.01.org

Re: [PATCH 3/3] btrfs: allow BTRFS_IOC_SNAP_DESTROY_V2 to remove ghost subvolume

[David Sterba]

On Mon, Jun 28, 2021 at 06:16:37PM +0800, Qu Wenruo wrote:
> There is a report from the mail list that some subvolumes don't have any
> ROOT_REF/BACKREF and has 0 ref.
> But without an ORPHAN item.

Do you have link to the mail report?

> Such ghost subvolumes can't be deleted by any ioctl but only rely on
> btrfs-progs to add ORPHAN item.

Is there a way to list such subvolumes from progs?
> 
> Normally kernel only needs to gracefully abort/reject such corrupted
> structure, but in this case we have all the needed infrastructures and
> interface to allow BTRFS_IOC_SNAP_DESTROY_V2 to delete it.
> 
> So add the ability to delete such ghost subvolumes to
> BTRFS_IOC_SNAP_DESTROY_V2.

So this is only extending the functionality and we don't need to handle
backward compatibility.

Re: [PATCH 3/3] btrfs: allow BTRFS_IOC_SNAP_DESTROY_V2 to remove ghost subvolume

[Qu Wenruo]

On 2021/6/30 下午9:16, David Sterba wrote:
> On Mon, Jun 28, 2021 at 06:16:37PM +0800, Qu Wenruo wrote:
>> There is a report from the mail list that some subvolumes don't have any
>> ROOT_REF/BACKREF and has 0 ref.
>> But without an ORPHAN item.
> 
> Do you have link to the mail report?

Here it is:
https://lore.kernel.org/linux-btrfs/CAJ9tZB-G7KZkxGfrADbmHruuEfwyhV1bihUvRZrJ_ypt_iUVKg@mail.gmail.com/T/#t

> 
>> Such ghost subvolumes can't be deleted by any ioctl but only rely on
>> btrfs-progs to add ORPHAN item.
> 
> Is there a way to list such subvolumes from progs?

No, root with 0 ref will not show up in "btrfs subv list".

In fact unless we pass @check_ref = false, btrfs_get_fs_root() won't 
return such ghost root at all.

>>
>> Normally kernel only needs to gracefully abort/reject such corrupted
>> structure, but in this case we have all the needed infrastructures and
>> interface to allow BTRFS_IOC_SNAP_DESTROY_V2 to delete it.
>>
>> So add the ability to delete such ghost subvolumes to
>> BTRFS_IOC_SNAP_DESTROY_V2.
> 
> So this is only extending the functionality and we don't need to handle
> backward compatibility.
> 
Exactly.

In fact, I hope we never need to bother such case in the future...

Thanks,
Qu

Re: [PATCH 3/3] btrfs: allow BTRFS_IOC_SNAP_DESTROY_V2 to remove ghost subvolume

[David Sterba]

On Wed, Jun 30, 2021 at 09:26:20PM +0800, Qu Wenruo wrote:
> > Is there a way to list such subvolumes from progs?
> 
> No, root with 0 ref will not show up in "btrfs subv list".

We might need one then, but I'll read the full report, maybe it's a
one-off bug.

> In fact unless we pass @check_ref = false, btrfs_get_fs_root() won't 
> return such ghost root at all.

In progs the subvolumes are searched directly by the key, so this
(kernel function) is not relevant.

Re: [PATCH 3/3] btrfs: allow BTRFS_IOC_SNAP_DESTROY_V2 to remove ghost subvolume

[Qu Wenruo]

On 2021/6/30 下午9:30, David Sterba wrote:
> On Wed, Jun 30, 2021 at 09:26:20PM +0800, Qu Wenruo wrote:
>>> Is there a way to list such subvolumes from progs?
>>
>> No, root with 0 ref will not show up in "btrfs subv list".
>
> We might need one then, but I'll read the full report, maybe it's a
> one-off bug.
>
>> In fact unless we pass @check_ref = false, btrfs_get_fs_root() won't
>> return such ghost root at all.
>
> In progs the subvolumes are searched directly by the key, so this
> (kernel function) is not relevant.
>
Anyway, you can use the test image from the btrfs-progs patchset to test
btrfs-progs modification.

THanks,
Qu

Re: [PATCH 0/3] btrfs: allow BTRFS_IOC_SNAP_DESTROY_V2 to remove ghost subvolume

[Zygo Blaxell]

On Mon, Jun 28, 2021 at 06:16:34PM +0800, Qu Wenruo wrote:
> Since we're busting ghost subvolumes, the branch is now called
> ghost_busters:
> https://github.com/adam900710/linux/tree/ghost_busters
> 
> The first two patches are just cleanup found during the development.
> 
> The first is a missing check for subvolid range, the missing check
> itself won't cause any harm, just returning -ENOENT from dentry lookup,
> other than the expected -EINVAL.
> 
> The 2nd is a super old dead comment from the early age of btrfs.
> 
> The final patch is the real work to allow patched "btrfs subvolume delete -i"
> to delete ghost subvolume.
> Tested with the image dump of previous submitted btrfs-progs patchset.
> 
> Qu Wenruo (3):
>   btrfs: return -EINVAL if some user wants to remove uuid/data_reloc
>     tree
>   btrfs: remove dead comment on btrfs_add_dead_root()
>   btrfs: allow BTRFS_IOC_SNAP_DESTROY_V2 to remove ghost subvolume

I hit this bug on several machines while they were running 5.11.  The
ghost subvols seem to occur naturally--I didn't change my usual workloads
to get them, they just showed up in fairly normal snapshot rotation.

They don't seem to occur on 5.10 (up to .46) or on 5.12 and later, but
once they are created, they don't go away without using this patch to
remove them.

This patch does get rid of the ghost subvols after the fact, quite nicely.

Some users on IRC have hit the same problem.  One was running Debian's
backported 5.10, which doesn't fit the pattern of kernel versions I've
observed, but maybe Debian backported something?

>  fs/btrfs/ioctl.c       | 81 +++++++++++++++++++++++++++++++++++++++++-
>  fs/btrfs/transaction.c |  7 ++--
>  2 files changed, 84 insertions(+), 4 deletions(-)
> 
> -- 
> 2.32.0
> 

Re: [PATCH 0/3] btrfs: allow BTRFS_IOC_SNAP_DESTROY_V2 to remove ghost subvolume

[Qu Wenruo]

On 2021/7/20 下午12:05, Zygo Blaxell wrote:
> On Mon, Jun 28, 2021 at 06:16:34PM +0800, Qu Wenruo wrote:
>> Since we're busting ghost subvolumes, the branch is now called
>> ghost_busters:
>> https://github.com/adam900710/linux/tree/ghost_busters
>>
>> The first two patches are just cleanup found during the development.
>>
>> The first is a missing check for subvolid range, the missing check
>> itself won't cause any harm, just returning -ENOENT from dentry lookup,
>> other than the expected -EINVAL.
>>
>> The 2nd is a super old dead comment from the early age of btrfs.
>>
>> The final patch is the real work to allow patched "btrfs subvolume delete -i"
>> to delete ghost subvolume.
>> Tested with the image dump of previous submitted btrfs-progs patchset.
>>
>> Qu Wenruo (3):
>>    btrfs: return -EINVAL if some user wants to remove uuid/data_reloc
>>      tree
>>    btrfs: remove dead comment on btrfs_add_dead_root()
>>    btrfs: allow BTRFS_IOC_SNAP_DESTROY_V2 to remove ghost subvolume
>
> I hit this bug on several machines while they were running 5.11.  The
> ghost subvols seem to occur naturally--I didn't change my usual workloads
> to get them, they just showed up in fairly normal snapshot rotation.

And there is no powerloss involved?

Then it's a much serious problem than I thought.

Thanks,
Qu
>
> They don't seem to occur on 5.10 (up to .46) or on 5.12 and later, but
> once they are created, they don't go away without using this patch to
> remove them.
>
> This patch does get rid of the ghost subvols after the fact, quite nicely.
>
> Some users on IRC have hit the same problem.  One was running Debian's
> backported 5.10, which doesn't fit the pattern of kernel versions I've
> observed, but maybe Debian backported something?
>
>>   fs/btrfs/ioctl.c       | 81 +++++++++++++++++++++++++++++++++++++++++-
>>   fs/btrfs/transaction.c |  7 ++--
>>   2 files changed, 84 insertions(+), 4 deletions(-)
>>
>> --
>> 2.32.0
>>
>

Re: [PATCH 0/3] btrfs: allow BTRFS_IOC_SNAP_DESTROY_V2 to remove ghost subvolume

[Zygo Blaxell]

On Tue, Jul 20, 2021 at 12:33:47PM +0800, Qu Wenruo wrote:
> 
> 
> On 2021/7/20 下午12:05, Zygo Blaxell wrote:
> > On Mon, Jun 28, 2021 at 06:16:34PM +0800, Qu Wenruo wrote:
> > > Since we're busting ghost subvolumes, the branch is now called
> > > ghost_busters:
> > > https://github.com/adam900710/linux/tree/ghost_busters
> > > 
> > > The first two patches are just cleanup found during the development.
> > > 
> > > The first is a missing check for subvolid range, the missing check
> > > itself won't cause any harm, just returning -ENOENT from dentry lookup,
> > > other than the expected -EINVAL.
> > > 
> > > The 2nd is a super old dead comment from the early age of btrfs.
> > > 
> > > The final patch is the real work to allow patched "btrfs subvolume delete -i"
> > > to delete ghost subvolume.
> > > Tested with the image dump of previous submitted btrfs-progs patchset.
> > > 
> > > Qu Wenruo (3):
> > >    btrfs: return -EINVAL if some user wants to remove uuid/data_reloc
> > >      tree
> > >    btrfs: remove dead comment on btrfs_add_dead_root()
> > >    btrfs: allow BTRFS_IOC_SNAP_DESTROY_V2 to remove ghost subvolume
> > 
> > I hit this bug on several machines while they were running 5.11.  The
> > ghost subvols seem to occur naturally--I didn't change my usual workloads
> > to get them, they just showed up in fairly normal snapshot rotation.
> 
> And there is no powerloss involved?

There could be.  The test VMs have frequent simulated powerloss events
because we're testing for regressions in post-powerloss behavior (I
guess this is one?).  It also affected laptops that probably did have
a forced shutdown or two (WiFi and Bluetooth are huge crash generators
on new hardware).

Nothing running 5.11 on stable power seems to have been affected so far.

> Then it's a much serious problem than I thought.
> 
> Thanks,
> Qu
> > 
> > They don't seem to occur on 5.10 (up to .46) or on 5.12 and later, but
> > once they are created, they don't go away without using this patch to
> > remove them.

It's odd that it doesn't seem to happen on kernels other than 5.11.
That would suggest it was broken in 5.11-rc and fixed in 5.12, which
might be a useful range to search for regressions and accidental fixes.

In any case, if a user has been running 5.11, they're going to have
ghost subvols lying around on their filesystem, so we might as well get
ready to deal with them in the kernel and tools.

> > This patch does get rid of the ghost subvols after the fact, quite nicely.
> > 
> > Some users on IRC have hit the same problem.  One was running Debian's
> > backported 5.10, which doesn't fit the pattern of kernel versions I've
> > observed, but maybe Debian backported something?
> > 
> > >   fs/btrfs/ioctl.c       | 81 +++++++++++++++++++++++++++++++++++++++++-
> > >   fs/btrfs/transaction.c |  7 ++--
> > >   2 files changed, 84 insertions(+), 4 deletions(-)
> > > 
> > > --
> > > 2.32.0
> > > 
> > 

Re: [PATCH 0/3] btrfs: allow BTRFS_IOC_SNAP_DESTROY_V2 to remove ghost subvolume

[Qu Wenruo]

On 2021/7/20 下午11:41, Zygo Blaxell wrote:
> On Tue, Jul 20, 2021 at 12:33:47PM +0800, Qu Wenruo wrote:
>>
>>
>> On 2021/7/20 下午12:05, Zygo Blaxell wrote:
>>> On Mon, Jun 28, 2021 at 06:16:34PM +0800, Qu Wenruo wrote:
>>>> Since we're busting ghost subvolumes, the branch is now called
>>>> ghost_busters:
>>>> https://github.com/adam900710/linux/tree/ghost_busters
>>>>
>>>> The first two patches are just cleanup found during the development.
>>>>
>>>> The first is a missing check for subvolid range, the missing check
>>>> itself won't cause any harm, just returning -ENOENT from dentry lookup,
>>>> other than the expected -EINVAL.
>>>>
>>>> The 2nd is a super old dead comment from the early age of btrfs.
>>>>
>>>> The final patch is the real work to allow patched "btrfs subvolume delete -i"
>>>> to delete ghost subvolume.
>>>> Tested with the image dump of previous submitted btrfs-progs patchset.
>>>>
>>>> Qu Wenruo (3):
>>>>     btrfs: return -EINVAL if some user wants to remove uuid/data_reloc
>>>>       tree
>>>>     btrfs: remove dead comment on btrfs_add_dead_root()
>>>>     btrfs: allow BTRFS_IOC_SNAP_DESTROY_V2 to remove ghost subvolume
>>>
>>> I hit this bug on several machines while they were running 5.11.  The
>>> ghost subvols seem to occur naturally--I didn't change my usual workloads
>>> to get them, they just showed up in fairly normal snapshot rotation.
>>
>> And there is no powerloss involved?
>
> There could be.  The test VMs have frequent simulated powerloss events
> because we're testing for regressions in post-powerloss behavior (I
> guess this is one?).  It also affected laptops that probably did have
> a forced shutdown or two (WiFi and Bluetooth are huge crash generators
> on new hardware).
>
> Nothing running 5.11 on stable power seems to have been affected so far.
>
>> Then it's a much serious problem than I thought.
>>
>> Thanks,
>> Qu
>>>
>>> They don't seem to occur on 5.10 (up to .46) or on 5.12 and later, but
>>> once they are created, they don't go away without using this patch to
>>> remove them.
>
> It's odd that it doesn't seem to happen on kernels other than 5.11.
> That would suggest it was broken in 5.11-rc and fixed in 5.12, which
> might be a useful range to search for regressions and accidental fixes.
>
> In any case, if a user has been running 5.11, they're going to have
> ghost subvols lying around on their filesystem, so we might as well get
> ready to deal with them in the kernel and tools.


So my previous guess on subvolume unlink and orphan item insert get into
two different transaction could be true.

The last time I checked the code, I'm checking the latest upstream,
which is already v5.14-rc kernels.

Let me try to pin down the offending patch, but at least it's a good
news it only happens in v5.11.

Thanks,
Qu
>
>>> This patch does get rid of the ghost subvols after the fact, quite nicely.
>>>
>>> Some users on IRC have hit the same problem.  One was running Debian's
>>> backported 5.10, which doesn't fit the pattern of kernel versions I've
>>> observed, but maybe Debian backported something?
>>>
>>>>    fs/btrfs/ioctl.c       | 81 +++++++++++++++++++++++++++++++++++++++++-
>>>>    fs/btrfs/transaction.c |  7 ++--
>>>>    2 files changed, 84 insertions(+), 4 deletions(-)
>>>>
>>>> --
>>>> 2.32.0
>>>>
>>>

Re: [PATCH 0/3] btrfs: allow BTRFS_IOC_SNAP_DESTROY_V2 to remove ghost subvolume

[Qu Wenruo]

Gentle ping?

Or we don't need this feature and just let btrfs-check to repair such 
problem?

Thanks,
Qu

On 2021/6/28 下午6:16, Qu Wenruo wrote:
> Since we're busting ghost subvolumes, the branch is now called
> ghost_busters:
> https://github.com/adam900710/linux/tree/ghost_busters
> 
> The first two patches are just cleanup found during the development.
> 
> The first is a missing check for subvolid range, the missing check
> itself won't cause any harm, just returning -ENOENT from dentry lookup,
> other than the expected -EINVAL.
> 
> The 2nd is a super old dead comment from the early age of btrfs.
> 
> The final patch is the real work to allow patched "btrfs subvolume delete -i"
> to delete ghost subvolume.
> Tested with the image dump of previous submitted btrfs-progs patchset.
> 
> Qu Wenruo (3):
>    btrfs: return -EINVAL if some user wants to remove uuid/data_reloc
>      tree
>    btrfs: remove dead comment on btrfs_add_dead_root()
>    btrfs: allow BTRFS_IOC_SNAP_DESTROY_V2 to remove ghost subvolume
> 
>   fs/btrfs/ioctl.c       | 81 +++++++++++++++++++++++++++++++++++++++++-
>   fs/btrfs/transaction.c |  7 ++--
>   2 files changed, 84 insertions(+), 4 deletions(-)
>