Re: [PATCH 11/17] btrfs: move flush related definitions to space-info.h

[Qu Wenruo <wqu@suse.com>]

On 2022/9/14 23:06, Josef Bacik wrote:
> This code is used in space-info.c, move the definitions to space-info.h.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Looks good overall, but one new header makes me wonder:

> ---
>   fs/btrfs/ctree.h         | 59 ----------------------------------------
>   fs/btrfs/delayed-inode.c |  1 +
>   fs/btrfs/inode-item.c    |  1 +
>   fs/btrfs/props.c         |  1 +
>   fs/btrfs/relocation.c    |  1 +
>   fs/btrfs/space-info.h    | 59 ++++++++++++++++++++++++++++++++++++++++
>   fs/btrfs/super.c         |  1 +
>   7 files changed, 64 insertions(+), 59 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 60f8817f5b7c..d99720cf4835 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -2654,65 +2654,6 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
>   
>   void btrfs_clear_space_info_full(struct btrfs_fs_info *info);
>   
> -/*
> - * Different levels for to flush space when doing space reservations.
> - *
> - * The higher the level, the more methods we try to reclaim space.
> - */
> -enum btrfs_reserve_flush_enum {
> -	/* If we are in the transaction, we can't flush anything.*/
> -	BTRFS_RESERVE_NO_FLUSH,
> -
> -	/*
> -	 * Flush space by:
> -	 * - Running delayed inode items
> -	 * - Allocating a new chunk
> -	 */
> -	BTRFS_RESERVE_FLUSH_LIMIT,
> -
> -	/*
> -	 * Flush space by:
> -	 * - Running delayed inode items
> -	 * - Running delayed refs
> -	 * - Running delalloc and waiting for ordered extents
> -	 * - Allocating a new chunk
> -	 */
> -	BTRFS_RESERVE_FLUSH_EVICT,
> -
> -	/*
> -	 * Flush space by above mentioned methods and by:
> -	 * - Running delayed iputs
> -	 * - Committing transaction
> -	 *
> -	 * Can be interrupted by a fatal signal.
> -	 */
> -	BTRFS_RESERVE_FLUSH_DATA,
> -	BTRFS_RESERVE_FLUSH_FREE_SPACE_INODE,
> -	BTRFS_RESERVE_FLUSH_ALL,
> -
> -	/*
> -	 * Pretty much the same as FLUSH_ALL, but can also steal space from
> -	 * global rsv.
> -	 *
> -	 * Can be interrupted by a fatal signal.
> -	 */
> -	BTRFS_RESERVE_FLUSH_ALL_STEAL,
> -};
> -
> -enum btrfs_flush_state {
> -	FLUSH_DELAYED_ITEMS_NR	=	1,
> -	FLUSH_DELAYED_ITEMS	=	2,
> -	FLUSH_DELAYED_REFS_NR	=	3,
> -	FLUSH_DELAYED_REFS	=	4,
> -	FLUSH_DELALLOC		=	5,
> -	FLUSH_DELALLOC_WAIT	=	6,
> -	FLUSH_DELALLOC_FULL	=	7,
> -	ALLOC_CHUNK		=	8,
> -	ALLOC_CHUNK_FORCE	=	9,
> -	RUN_DELAYED_IPUTS	=	10,
> -	COMMIT_TRANS		=	11,
> -};
> -
>   int btrfs_subvolume_reserve_metadata(struct btrfs_root *root,
>   				     struct btrfs_block_rsv *rsv,
>   				     int nitems, bool use_global_rsv);
> diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
> index cac5169eaf8d..a411f04a7b97 100644
> --- a/fs/btrfs/delayed-inode.c
> +++ b/fs/btrfs/delayed-inode.c
> @@ -14,6 +14,7 @@
>   #include "qgroup.h"
>   #include "locking.h"
>   #include "inode-item.h"
> +#include "space-info.h"
>   
>   #define BTRFS_DELAYED_WRITEBACK		512
>   #define BTRFS_DELAYED_BACKGROUND	128
> diff --git a/fs/btrfs/inode-item.c b/fs/btrfs/inode-item.c
> index 0eeb5ea87894..366f3a788c6a 100644
> --- a/fs/btrfs/inode-item.c
> +++ b/fs/btrfs/inode-item.c
> @@ -8,6 +8,7 @@
>   #include "disk-io.h"
>   #include "transaction.h"
>   #include "print-tree.h"
> +#include "space-info.h"
>   
>   struct btrfs_inode_ref *btrfs_find_name_in_backref(struct extent_buffer *leaf,
>   						   int slot, const char *name,
> diff --git a/fs/btrfs/props.c b/fs/btrfs/props.c
> index 055a631276ce..07f62e3ba6a5 100644
> --- a/fs/btrfs/props.c
> +++ b/fs/btrfs/props.c
> @@ -10,6 +10,7 @@
>   #include "ctree.h"
>   #include "xattr.h"
>   #include "compression.h"
> +#include "space-info.h"
>   
>   #define BTRFS_PROP_HANDLERS_HT_BITS 8
>   static DEFINE_HASHTABLE(prop_handlers_ht, BTRFS_PROP_HANDLERS_HT_BITS);

>   struct btrfs_space_info {
>   	spinlock_t lock;
>   
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index be7df8d1d5b8..2add5b23c476 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -49,6 +49,7 @@
>   #include "discard.h"
>   #include "qgroup.h"
>   #include "raid56.h"
> +#include "space-info.h"

Why super.c needs this header?

The moved code is definition for btrfs_reserve_flush_enum and 
btrfs_flush_state, but I didn't see any direct usage inside super.c.

Is it for trace event?

Or you just did the same search using "FLUSH" as keyword and got hit on 
FLUSHONCOMMIT?

Thanks,
Qu

>   #define CREATE_TRACE_POINTS
>   #include <trace/events/btrfs.h>
>