Re: [PATCH 3/4] btrfs: get rid of root->orphan_cleanup_state

[Nikolay Borisov <nborisov@suse.com>]

On 28.10.21 г. 22:50, Josef Bacik wrote:
> Now that we don't care about the stage of the orphan_cleanup_state,
> simply replace it with a bit on ->state to make sure we don't call the
> orphan cleanup every time we wander into this root.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>  fs/btrfs/ctree.h   | 9 ++-------
>  fs/btrfs/disk-io.c | 1 -
>  fs/btrfs/inode.c   | 4 +---
>  3 files changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 7553e9dc5f93..01f6a40ba2dd 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -511,11 +511,6 @@ struct btrfs_discard_ctl {
>  	atomic64_t discard_bytes_saved;
>  };
>  
> -enum btrfs_orphan_cleanup_state {
> -	ORPHAN_CLEANUP_STARTED	= 1,
> -	ORPHAN_CLEANUP_DONE	= 2,
> -};
> -
>  void btrfs_init_async_reclaim_work(struct btrfs_fs_info *fs_info);
>  
>  /* fs_info */
> @@ -1110,6 +1105,8 @@ enum {
>  	BTRFS_ROOT_HAS_LOG_TREE,
>  	/* Qgroup flushing is in progress */
>  	BTRFS_ROOT_QGROUP_FLUSHING,
> +	/* We started the orphan cleanup for this root. */
> +	BTRFS_ROOT_ORPHAN_CLEANUP,
>  };
>  
>  /*
> @@ -1178,8 +1175,6 @@ struct btrfs_root {
>  	spinlock_t log_extents_lock[2];
>  	struct list_head logged_list[2];
>  
> -	int orphan_cleanup_state;
> -
>  	spinlock_t inode_lock;
>  	/* red-black tree that keeps track of in-memory inodes */
>  	struct rb_root inode_tree;
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index c7254331cf38..5be0ae67ceb7 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -1144,7 +1144,6 @@ static void __setup_root(struct btrfs_root *root, struct btrfs_fs_info *fs_info,
>  	root->node = NULL;
>  	root->commit_root = NULL;
>  	root->state = 0;
> -	root->orphan_cleanup_state = 0;
>  
>  	root->last_trans = 0;
>  	root->free_objectid = 0;
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index c783a3e434b9..a9ebafb168b5 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -3475,7 +3475,7 @@ int btrfs_orphan_cleanup(struct btrfs_root *root)
>  	u64 last_objectid = 0;
>  	int ret = 0, nr_unlink = 0;
>  
> -	if (cmpxchg(&root->orphan_cleanup_state, 0, ORPHAN_CLEANUP_STARTED))
> +	if (test_and_set_bit(BTRFS_ROOT_ORPHAN_CLEANUP, &root->state))
>  		return 0;
>  
>  	path = btrfs_alloc_path();
> @@ -3633,8 +3633,6 @@ int btrfs_orphan_cleanup(struct btrfs_root *root)
>  	/* release the path since we're done with it */
>  	btrfs_release_path(path);
>  
> -	root->orphan_cleanup_state = ORPHAN_CLEANUP_DONE;

Don't you need to clear the bit at this stage?

> -
>  	if (test_bit(BTRFS_ROOT_ORPHAN_ITEM_INSERTED, &root->state)) {
>  		trans = btrfs_join_transaction(root);
>  		if (!IS_ERR(trans))
>