[PATCH 0/9] Switch defines to enums

[PATCH 0/9] Switch defines to enums

[David Sterba]

This is motivated by a merging mistake that happened a few releases ago.
Two patches updated BTRFS_FS_* flags independently to the same value,
git did not see any direct merge conflict. The two values got mixed at
runtime and caused crash.

Update all #define sequential values, the above merging problem would
not happen as there would be a conflict and the enum value
auto-increment would prevent duplicated values anyway.

David Sterba (9):
  btrfs: switch BTRFS_FS_STATE_* to enums
  btrfs: switch BTRFS_BLOCK_RSV_* to enums
  btrfs: switch BTRFS_FS_* to enums
  btrfs: switch BTRFS_ROOT_* to enums
  btrfs: swtich EXTENT_BUFFER_* to enums
  btrfs: switch EXTENT_FLAG_* to enums
  btrfs: switch BTRFS_*_LOCK to enums
  btrfs: switch BTRFS_ORDERED_* to enums
  btrfs: drop extra enum initialization where using defaults

 fs/btrfs/btrfs_inode.h  |   2 +-
 fs/btrfs/ctree.h        | 168 ++++++++++++++++++++++------------------
 fs/btrfs/disk-io.h      |  10 +--
 fs/btrfs/extent_io.h    |  28 ++++---
 fs/btrfs/extent_map.h   |  21 +++--
 fs/btrfs/locking.h      |  10 ++-
 fs/btrfs/ordered-data.h |  45 ++++++-----
 fs/btrfs/qgroup.h       |   2 +-
 fs/btrfs/sysfs.h        |   2 +-
 fs/btrfs/transaction.h  |  14 ++--
 10 files changed, 169 insertions(+), 133 deletions(-)

-- 
2.19.1

[PATCH 1/9] btrfs: switch BTRFS_FS_STATE_* to enums

[David Sterba]

We can use simple enum for values that are not part of on-disk format:
global filesystem states.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/ctree.h | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index a98507fa9192..f82ec5e41b0c 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -109,13 +109,26 @@ static inline unsigned long btrfs_chunk_item_size(int num_stripes)
 }
 
 /*
- * File system states
+ * Runtime (in-memory) states of filesystem
  */
-#define BTRFS_FS_STATE_ERROR		0
-#define BTRFS_FS_STATE_REMOUNTING	1
-#define BTRFS_FS_STATE_TRANS_ABORTED	2
-#define BTRFS_FS_STATE_DEV_REPLACING	3
-#define BTRFS_FS_STATE_DUMMY_FS_INFO	4
+enum {
+	/* Global indicator of serious filesysystem errors */
+	BTRFS_FS_STATE_ERROR,
+	/*
+	 * Filesystem is being remounted, allow to skip some operations, like
+	 * defrag
+	 */
+	BTRFS_FS_STATE_REMOUNTING,
+	/* Track if the transaction abort has been reported */
+	BTRFS_FS_STATE_TRANS_ABORTED,
+	/*
+	 * Indicate that replace source or target device state is changed and
+	 * allow to block bio operations
+	 */
+	BTRFS_FS_STATE_DEV_REPLACING,
+	/* The btrfs_fs_info created for self-tests */
+	BTRFS_FS_STATE_DUMMY_FS_INFO,
+};
 
 #define BTRFS_BACKREF_REV_MAX		256
 #define BTRFS_BACKREF_REV_SHIFT		56
-- 
2.19.1

[PATCH 2/9] btrfs: switch BTRFS_BLOCK_RSV_* to enums

[David Sterba]

We can use simple enum for values that are not part of on-disk format:
block reserve types.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/ctree.h | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index f82ec5e41b0c..40c405d74a01 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -461,13 +461,18 @@ struct btrfs_space_info {
 	struct kobject *block_group_kobjs[BTRFS_NR_RAID_TYPES];
 };
 
-#define	BTRFS_BLOCK_RSV_GLOBAL		1
-#define	BTRFS_BLOCK_RSV_DELALLOC	2
-#define	BTRFS_BLOCK_RSV_TRANS		3
-#define	BTRFS_BLOCK_RSV_CHUNK		4
-#define	BTRFS_BLOCK_RSV_DELOPS		5
-#define	BTRFS_BLOCK_RSV_EMPTY		6
-#define	BTRFS_BLOCK_RSV_TEMP		7
+/*
+ * Types of block reserves
+ */
+enum {
+	BTRFS_BLOCK_RSV_GLOBAL,
+	BTRFS_BLOCK_RSV_DELALLOC,
+	BTRFS_BLOCK_RSV_TRANS,
+	BTRFS_BLOCK_RSV_CHUNK,
+	BTRFS_BLOCK_RSV_DELOPS,
+	BTRFS_BLOCK_RSV_EMPTY,
+	BTRFS_BLOCK_RSV_TEMP,
+};
 
 struct btrfs_block_rsv {
 	u64 size;
-- 
2.19.1

[PATCH 3/9] btrfs: switch BTRFS_FS_* to enums

[David Sterba]

We can use simple enum for values that are not part of on-disk format:
internal filesystem states.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/ctree.h | 63 ++++++++++++++++++++++++------------------------
 1 file changed, 31 insertions(+), 32 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 40c405d74a01..7176b95b40e7 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -757,38 +757,37 @@ struct btrfs_swapfile_pin {
 
 bool btrfs_pinned_by_swapfile(struct btrfs_fs_info *fs_info, void *ptr);
 
-#define BTRFS_FS_BARRIER			1
-#define BTRFS_FS_CLOSING_START			2
-#define BTRFS_FS_CLOSING_DONE			3
-#define BTRFS_FS_LOG_RECOVERING			4
-#define BTRFS_FS_OPEN				5
-#define BTRFS_FS_QUOTA_ENABLED			6
-#define BTRFS_FS_UPDATE_UUID_TREE_GEN		9
-#define BTRFS_FS_CREATING_FREE_SPACE_TREE	10
-#define BTRFS_FS_BTREE_ERR			11
-#define BTRFS_FS_LOG1_ERR			12
-#define BTRFS_FS_LOG2_ERR			13
-#define BTRFS_FS_QUOTA_OVERRIDE			14
-/* Used to record internally whether fs has been frozen */
-#define BTRFS_FS_FROZEN				15
-
-/*
- * Indicate that a whole-filesystem exclusive operation is running
- * (device replace, resize, device add/delete, balance)
- */
-#define BTRFS_FS_EXCL_OP			16
-
-/*
- * To info transaction_kthread we need an immediate commit so it doesn't
- * need to wait for commit_interval
- */
-#define BTRFS_FS_NEED_ASYNC_COMMIT		17
-
-/*
- * Indicate that balance has been set up from the ioctl and is in the main
- * phase. The fs_info::balance_ctl is initialized.
- */
-#define BTRFS_FS_BALANCE_RUNNING		18
+enum {
+	BTRFS_FS_BARRIER,
+	BTRFS_FS_CLOSING_START,
+	BTRFS_FS_CLOSING_DONE,
+	BTRFS_FS_LOG_RECOVERING,
+	BTRFS_FS_OPEN,
+	BTRFS_FS_QUOTA_ENABLED,
+	BTRFS_FS_UPDATE_UUID_TREE_GEN,
+	BTRFS_FS_CREATING_FREE_SPACE_TREE,
+	BTRFS_FS_BTREE_ERR,
+	BTRFS_FS_LOG1_ERR,
+	BTRFS_FS_LOG2_ERR,
+	BTRFS_FS_QUOTA_OVERRIDE,
+	/* Used to record internally whether fs has been frozen */
+	BTRFS_FS_FROZEN,
+	/*
+	 * Indicate that a whole-filesystem exclusive operation is running
+	 * (device replace, resize, device add/delete, balance)
+	 */
+	BTRFS_FS_EXCL_OP,
+	/*
+	 * To info transaction_kthread we need an immediate commit so it
+	 * doesn't need to wait for commit_interval
+	 */
+	BTRFS_FS_NEED_ASYNC_COMMIT,
+	/*
+	 * Indicate that balance has been set up from the ioctl and is in the
+	 * main phase. The fs_info::balance_ctl is initialized.
+	 */
+	BTRFS_FS_BALANCE_RUNNING,
+};
 
 struct btrfs_fs_info {
 	u8 chunk_tree_uuid[BTRFS_UUID_SIZE];
-- 
2.19.1

[PATCH 4/9] btrfs: switch BTRFS_ROOT_* to enums

[David Sterba]

We can use simple enum for values that are not part of on-disk format:
root tree flags.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/ctree.h | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 7176b95b40e7..4bb0ac3050ff 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -1180,22 +1180,23 @@ struct btrfs_subvolume_writers {
 /*
  * The state of btrfs root
  */
-/*
- * btrfs_record_root_in_trans is a multi-step process,
- * and it can race with the balancing code.   But the
- * race is very small, and only the first time the root
- * is added to each transaction.  So IN_TRANS_SETUP
- * is used to tell us when more checks are required
- */
-#define BTRFS_ROOT_IN_TRANS_SETUP	0
-#define BTRFS_ROOT_REF_COWS		1
-#define BTRFS_ROOT_TRACK_DIRTY		2
-#define BTRFS_ROOT_IN_RADIX		3
-#define BTRFS_ROOT_ORPHAN_ITEM_INSERTED	4
-#define BTRFS_ROOT_DEFRAG_RUNNING	5
-#define BTRFS_ROOT_FORCE_COW		6
-#define BTRFS_ROOT_MULTI_LOG_TASKS	7
-#define BTRFS_ROOT_DIRTY		8
+enum {
+	/*
+	 * btrfs_record_root_in_trans is a multi-step process, and it can race
+	 * with the balancing code.   But the race is very small, and only the
+	 * first time the root is added to each transaction.  So IN_TRANS_SETUP
+	 * is used to tell us when more checks are required
+	 */
+	BTRFS_ROOT_IN_TRANS_SETUP,
+	BTRFS_ROOT_REF_COWS,
+	BTRFS_ROOT_TRACK_DIRTY,
+	BTRFS_ROOT_IN_RADIX,
+	BTRFS_ROOT_ORPHAN_ITEM_INSERTED,
+	BTRFS_ROOT_DEFRAG_RUNNING,
+	BTRFS_ROOT_FORCE_COW,
+	BTRFS_ROOT_MULTI_LOG_TASKS,
+	BTRFS_ROOT_DIRTY,
+};
 
 /*
  * in ram representation of the tree.  extent_root is used for all allocations
-- 
2.19.1

[PATCH 5/9] btrfs: swtich EXTENT_BUFFER_* to enums

[David Sterba]

We can use simple enum for values that are not part of on-disk format:
extent buffer flags;

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/extent_io.h | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index a1d3ea5a0d32..fd42492e62e5 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -37,18 +37,22 @@
 #define EXTENT_BIO_COMPRESSED 1
 #define EXTENT_BIO_FLAG_SHIFT 16
 
-/* these are bit numbers for test/set bit */
-#define EXTENT_BUFFER_UPTODATE 0
-#define EXTENT_BUFFER_DIRTY 2
-#define EXTENT_BUFFER_CORRUPT 3
-#define EXTENT_BUFFER_READAHEAD 4	/* this got triggered by readahead */
-#define EXTENT_BUFFER_TREE_REF 5
-#define EXTENT_BUFFER_STALE 6
-#define EXTENT_BUFFER_WRITEBACK 7
-#define EXTENT_BUFFER_READ_ERR 8        /* read IO error */
-#define EXTENT_BUFFER_UNMAPPED 9
-#define EXTENT_BUFFER_IN_TREE 10
-#define EXTENT_BUFFER_WRITE_ERR 11    /* write IO error */
+enum {
+	EXTENT_BUFFER_UPTODATE,
+	EXTENT_BUFFER_DIRTY,
+	EXTENT_BUFFER_CORRUPT,
+	/* this got triggered by readahead */
+	EXTENT_BUFFER_READAHEAD,
+	EXTENT_BUFFER_TREE_REF,
+	EXTENT_BUFFER_STALE,
+	EXTENT_BUFFER_WRITEBACK,
+	/* read IO error */
+	EXTENT_BUFFER_READ_ERR,
+	EXTENT_BUFFER_UNMAPPED,
+	EXTENT_BUFFER_IN_TREE,
+	/* write IO error */
+	EXTENT_BUFFER_WRITE_ERR,
+};
 
 /* these are flags for __process_pages_contig */
 #define PAGE_UNLOCK		(1 << 0)
-- 
2.19.1

[PATCH 6/9] btrfs: switch EXTENT_FLAG_* to enums

[David Sterba]

We can use simple enum for values that are not part of on-disk format:
extent map flags.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/extent_map.h | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/fs/btrfs/extent_map.h b/fs/btrfs/extent_map.h
index 31977ffd6190..ef05a0121652 100644
--- a/fs/btrfs/extent_map.h
+++ b/fs/btrfs/extent_map.h
@@ -11,13 +11,20 @@
 #define EXTENT_MAP_INLINE ((u64)-2)
 #define EXTENT_MAP_DELALLOC ((u64)-1)
 
-/* bits for the flags field */
-#define EXTENT_FLAG_PINNED 0 /* this entry not yet on disk, don't free it */
-#define EXTENT_FLAG_COMPRESSED 1
-#define EXTENT_FLAG_PREALLOC 3 /* pre-allocated extent */
-#define EXTENT_FLAG_LOGGING 4 /* Logging this extent */
-#define EXTENT_FLAG_FILLING 5 /* Filling in a preallocated extent */
-#define EXTENT_FLAG_FS_MAPPING 6 /* filesystem extent mapping type */
+/* bits for the extent_map::flags field */
+enum {
+	/* this entry not yet on disk, don't free it */
+	EXTENT_FLAG_PINNED,
+	EXTENT_FLAG_COMPRESSED,
+	/* pre-allocated extent */
+	EXTENT_FLAG_PREALLOC,
+	/* Logging this extent */
+	EXTENT_FLAG_LOGGING,
+	/* Filling in a preallocated extent */
+	EXTENT_FLAG_FILLING,
+	/* filesystem extent mapping type */
+	EXTENT_FLAG_FS_MAPPING,
+};
 
 struct extent_map {
 	struct rb_node rb_node;
-- 
2.19.1

[PATCH 7/9] btrfs: switch BTRFS_*_LOCK to enums

[David Sterba]

We can use simple enum for values that are not part of on-disk format:
tree lock types.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/locking.h | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/locking.h b/fs/btrfs/locking.h
index 29135def468e..684d0ef4faa4 100644
--- a/fs/btrfs/locking.h
+++ b/fs/btrfs/locking.h
@@ -6,10 +6,12 @@
 #ifndef BTRFS_LOCKING_H
 #define BTRFS_LOCKING_H
 
-#define BTRFS_WRITE_LOCK 1
-#define BTRFS_READ_LOCK 2
-#define BTRFS_WRITE_LOCK_BLOCKING 3
-#define BTRFS_READ_LOCK_BLOCKING 4
+enum {
+	BTRFS_WRITE_LOCK,
+	BTRFS_READ_LOCK,
+	BTRFS_WRITE_LOCK_BLOCKING,
+	BTRFS_READ_LOCK_BLOCKING,
+};
 
 void btrfs_tree_lock(struct extent_buffer *eb);
 void btrfs_tree_unlock(struct extent_buffer *eb);
-- 
2.19.1

[PATCH 8/9] btrfs: switch BTRFS_ORDERED_* to enums

[David Sterba]

We can use simple enum for values that are not part of on-disk format:
ordered extent flags.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/ordered-data.h | 45 +++++++++++++++++++++++------------------
 1 file changed, 25 insertions(+), 20 deletions(-)

diff --git a/fs/btrfs/ordered-data.h b/fs/btrfs/ordered-data.h
index b10e6765d88f..fb9a161f0215 100644
--- a/fs/btrfs/ordered-data.h
+++ b/fs/btrfs/ordered-data.h
@@ -37,26 +37,31 @@ struct btrfs_ordered_sum {
  * rbtree, just before waking any waiters.  It is used to indicate the
  * IO is done and any metadata is inserted into the tree.
  */
-#define BTRFS_ORDERED_IO_DONE 0 /* set when all the pages are written */
-
-#define BTRFS_ORDERED_COMPLETE 1 /* set when removed from the tree */
-
-#define BTRFS_ORDERED_NOCOW 2 /* set when we want to write in place */
-
-#define BTRFS_ORDERED_COMPRESSED 3 /* writing a zlib compressed extent */
-
-#define BTRFS_ORDERED_PREALLOC 4 /* set when writing to preallocated extent */
-
-#define BTRFS_ORDERED_DIRECT 5 /* set when we're doing DIO with this extent */
-
-#define BTRFS_ORDERED_IOERR 6 /* We had an io error when writing this out */
-
-#define BTRFS_ORDERED_UPDATED_ISIZE 7 /* indicates whether this ordered extent
-				       * has done its due diligence in updating
-				       * the isize. */
-#define BTRFS_ORDERED_TRUNCATED 8 /* Set when we have to truncate an extent */
-
-#define BTRFS_ORDERED_REGULAR 10 /* Regular IO for COW */
+enum {
+	/* set when all the pages are written */
+	BTRFS_ORDERED_IO_DONE,
+	/* set when removed from the tree */
+	BTRFS_ORDERED_COMPLETE,
+	/* set when we want to write in place */
+	BTRFS_ORDERED_NOCOW,
+	/* writing a zlib compressed extent */
+	BTRFS_ORDERED_COMPRESSED,
+	/* set when writing to preallocated extent */
+	BTRFS_ORDERED_PREALLOC,
+	/* set when we're doing DIO with this extent */
+	BTRFS_ORDERED_DIRECT,
+	/* We had an io error when writing this out */
+	BTRFS_ORDERED_IOERR,
+	/*
+	 * indicates whether this ordered extent has done its due diligence in
+	 * updating the isize
+	 */
+	BTRFS_ORDERED_UPDATED_ISIZE,
+	/* Set when we have to truncate an extent */
+	BTRFS_ORDERED_TRUNCATED,
+	/* Regular IO for COW */
+	BTRFS_ORDERED_REGULAR,
+};
 
 struct btrfs_ordered_extent {
 	/* logical offset in the file */
-- 
2.19.1

[PATCH 9/9] btrfs: drop extra enum initialization where using defaults

[David Sterba]

The first auto-assigned value to enum is 0, we can use that and not
initialize all members where the auto-increment does the same. This is
used for values that are not part of on-disk format.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/btrfs_inode.h |  2 +-
 fs/btrfs/ctree.h       | 28 ++++++++++++++--------------
 fs/btrfs/disk-io.h     | 10 +++++-----
 fs/btrfs/qgroup.h      |  2 +-
 fs/btrfs/sysfs.h       |  2 +-
 fs/btrfs/transaction.h | 14 +++++++-------
 6 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index 4de321aee7a5..fc25607304f2 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -20,7 +20,7 @@
  * new data the application may have written before commit.
  */
 enum {
-	BTRFS_INODE_ORDERED_DATA_CLOSE = 0,
+	BTRFS_INODE_ORDERED_DATA_CLOSE,
 	BTRFS_INODE_DUMMY,
 	BTRFS_INODE_IN_DEFRAG,
 	BTRFS_INODE_HAS_ASYNC_EXTENT,
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 4bb0ac3050ff..f1d1c6ba3aa1 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -334,7 +334,7 @@ struct btrfs_node {
  * The slots array records the index of the item or block pointer
  * used while walking the tree.
  */
-enum { READA_NONE = 0, READA_BACK, READA_FORWARD };
+enum { READA_NONE, READA_BACK, READA_FORWARD };
 struct btrfs_path {
 	struct extent_buffer *nodes[BTRFS_MAX_LEVEL];
 	int slots[BTRFS_MAX_LEVEL];
@@ -532,18 +532,18 @@ struct btrfs_free_cluster {
 };
 
 enum btrfs_caching_type {
-	BTRFS_CACHE_NO		= 0,
-	BTRFS_CACHE_STARTED	= 1,
-	BTRFS_CACHE_FAST	= 2,
-	BTRFS_CACHE_FINISHED	= 3,
-	BTRFS_CACHE_ERROR	= 4,
+	BTRFS_CACHE_NO,
+	BTRFS_CACHE_STARTED,
+	BTRFS_CACHE_FAST,
+	BTRFS_CACHE_FINISHED,
+	BTRFS_CACHE_ERROR,
 };
 
 enum btrfs_disk_cache_state {
-	BTRFS_DC_WRITTEN	= 0,
-	BTRFS_DC_ERROR		= 1,
-	BTRFS_DC_CLEAR		= 2,
-	BTRFS_DC_SETUP		= 3,
+	BTRFS_DC_WRITTEN,
+	BTRFS_DC_ERROR,
+	BTRFS_DC_CLEAR,
+	BTRFS_DC_SETUP,
 };
 
 struct btrfs_caching_control {
@@ -2621,10 +2621,10 @@ static inline gfp_t btrfs_alloc_write_mask(struct address_space *mapping)
 /* extent-tree.c */
 
 enum btrfs_inline_ref_type {
-	BTRFS_REF_TYPE_INVALID =	 0,
-	BTRFS_REF_TYPE_BLOCK =		 1,
-	BTRFS_REF_TYPE_DATA =		 2,
-	BTRFS_REF_TYPE_ANY =		 3,
+	BTRFS_REF_TYPE_INVALID,
+	BTRFS_REF_TYPE_BLOCK,
+	BTRFS_REF_TYPE_DATA,
+	BTRFS_REF_TYPE_ANY,
 };
 
 int btrfs_get_extent_inline_ref_type(const struct extent_buffer *eb,
diff --git a/fs/btrfs/disk-io.h b/fs/btrfs/disk-io.h
index 4cccba22640f..987a64bc0c66 100644
--- a/fs/btrfs/disk-io.h
+++ b/fs/btrfs/disk-io.h
@@ -21,11 +21,11 @@
 #define BTRFS_BDEV_BLOCKSIZE	(4096)
 
 enum btrfs_wq_endio_type {
-	BTRFS_WQ_ENDIO_DATA = 0,
-	BTRFS_WQ_ENDIO_METADATA = 1,
-	BTRFS_WQ_ENDIO_FREE_SPACE = 2,
-	BTRFS_WQ_ENDIO_RAID56 = 3,
-	BTRFS_WQ_ENDIO_DIO_REPAIR = 4,
+	BTRFS_WQ_ENDIO_DATA,
+	BTRFS_WQ_ENDIO_METADATA,
+	BTRFS_WQ_ENDIO_FREE_SPACE,
+	BTRFS_WQ_ENDIO_RAID56,
+	BTRFS_WQ_ENDIO_DIO_REPAIR,
 };
 
 static inline u64 btrfs_sb_offset(int mirror)
diff --git a/fs/btrfs/qgroup.h b/fs/btrfs/qgroup.h
index d8f78f5ab854..e4e6ee44073a 100644
--- a/fs/btrfs/qgroup.h
+++ b/fs/btrfs/qgroup.h
@@ -70,7 +70,7 @@ struct btrfs_qgroup_extent_record {
  *	be converted into META_PERTRANS.
  */
 enum btrfs_qgroup_rsv_type {
-	BTRFS_QGROUP_RSV_DATA = 0,
+	BTRFS_QGROUP_RSV_DATA,
 	BTRFS_QGROUP_RSV_META_PERTRANS,
 	BTRFS_QGROUP_RSV_META_PREALLOC,
 	BTRFS_QGROUP_RSV_LAST,
diff --git a/fs/btrfs/sysfs.h b/fs/btrfs/sysfs.h
index c6ee600aff89..40716b357c1d 100644
--- a/fs/btrfs/sysfs.h
+++ b/fs/btrfs/sysfs.h
@@ -9,7 +9,7 @@
 extern u64 btrfs_debugfs_test;
 
 enum btrfs_feature_set {
-	FEAT_COMPAT = 0,
+	FEAT_COMPAT,
 	FEAT_COMPAT_RO,
 	FEAT_INCOMPAT,
 	FEAT_MAX
diff --git a/fs/btrfs/transaction.h b/fs/btrfs/transaction.h
index 703d5116a2fc..f1ba78949d1b 100644
--- a/fs/btrfs/transaction.h
+++ b/fs/btrfs/transaction.h
@@ -12,13 +12,13 @@
 #include "ctree.h"
 
 enum btrfs_trans_state {
-	TRANS_STATE_RUNNING		= 0,
-	TRANS_STATE_BLOCKED		= 1,
-	TRANS_STATE_COMMIT_START	= 2,
-	TRANS_STATE_COMMIT_DOING	= 3,
-	TRANS_STATE_UNBLOCKED		= 4,
-	TRANS_STATE_COMPLETED		= 5,
-	TRANS_STATE_MAX			= 6,
+	TRANS_STATE_RUNNING,
+	TRANS_STATE_BLOCKED,
+	TRANS_STATE_COMMIT_START,
+	TRANS_STATE_COMMIT_DOING,
+	TRANS_STATE_UNBLOCKED,
+	TRANS_STATE_COMPLETED,
+	TRANS_STATE_MAX,
 };
 
 #define BTRFS_TRANS_HAVE_FREE_BGS	0
-- 
2.19.1

Re: [PATCH 1/9] btrfs: switch BTRFS_FS_STATE_* to enums

[Omar Sandoval]

On Tue, Nov 27, 2018 at 08:53:41PM +0100, David Sterba wrote:
> We can use simple enum for values that are not part of on-disk format:
> global filesystem states.

Reviewed-by: Omar Sandoval <osandov@fb.com>

Some typos/wording suggestions below.

> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>  fs/btrfs/ctree.h | 25 +++++++++++++++++++------
>  1 file changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index a98507fa9192..f82ec5e41b0c 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -109,13 +109,26 @@ static inline unsigned long btrfs_chunk_item_size(int num_stripes)
>  }
>  
>  /*
> - * File system states
> + * Runtime (in-memory) states of filesystem
>   */
> -#define BTRFS_FS_STATE_ERROR		0
> -#define BTRFS_FS_STATE_REMOUNTING	1
> -#define BTRFS_FS_STATE_TRANS_ABORTED	2
> -#define BTRFS_FS_STATE_DEV_REPLACING	3
> -#define BTRFS_FS_STATE_DUMMY_FS_INFO	4
> +enum {
> +	/* Global indicator of serious filesysystem errors */

filesysystem -> filesystem

> +	BTRFS_FS_STATE_ERROR,
> +	/*
> +	 * Filesystem is being remounted, allow to skip some operations, like
> +	 * defrag
> +	 */
> +	BTRFS_FS_STATE_REMOUNTING,
> +	/* Track if the transaction abort has been reported */

Which one is "the" transaction abort? This gives me the impression that
this is a flag on the transaction, but it's actually filesystem state.
Maybe "Track if a transaction abort has been reported on this
filesystem"?

> +	BTRFS_FS_STATE_TRANS_ABORTED,
> +	/*
> +	 * Indicate that replace source or target device state is changed and
> +	 * allow to block bio operations
> +	 */

Again, this makes it sound like it's device state, but it's actually
filesystem state. How about "Bio operations should be blocked on this
filesystem because a source or target device is being destroyed as part
of a device replace"?

> +	BTRFS_FS_STATE_DEV_REPLACING,
> +	/* The btrfs_fs_info created for self-tests */
> +	BTRFS_FS_STATE_DUMMY_FS_INFO,
> +};
>  
>  #define BTRFS_BACKREF_REV_MAX		256
>  #define BTRFS_BACKREF_REV_SHIFT		56
> -- 
> 2.19.1
> 

Re: [PATCH 2/9] btrfs: switch BTRFS_BLOCK_RSV_* to enums

[Omar Sandoval]

On Tue, Nov 27, 2018 at 08:53:43PM +0100, David Sterba wrote:
> We can use simple enum for values that are not part of on-disk format:
> block reserve types.

Reviewed-by: Omar Sandoval <osandov@fb.com>

> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>  fs/btrfs/ctree.h | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index f82ec5e41b0c..40c405d74a01 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -461,13 +461,18 @@ struct btrfs_space_info {
>  	struct kobject *block_group_kobjs[BTRFS_NR_RAID_TYPES];
>  };
>  
> -#define	BTRFS_BLOCK_RSV_GLOBAL		1
> -#define	BTRFS_BLOCK_RSV_DELALLOC	2
> -#define	BTRFS_BLOCK_RSV_TRANS		3
> -#define	BTRFS_BLOCK_RSV_CHUNK		4
> -#define	BTRFS_BLOCK_RSV_DELOPS		5
> -#define	BTRFS_BLOCK_RSV_EMPTY		6
> -#define	BTRFS_BLOCK_RSV_TEMP		7
> +/*
> + * Types of block reserves
> + */
> +enum {
> +	BTRFS_BLOCK_RSV_GLOBAL,
> +	BTRFS_BLOCK_RSV_DELALLOC,
> +	BTRFS_BLOCK_RSV_TRANS,
> +	BTRFS_BLOCK_RSV_CHUNK,
> +	BTRFS_BLOCK_RSV_DELOPS,
> +	BTRFS_BLOCK_RSV_EMPTY,
> +	BTRFS_BLOCK_RSV_TEMP,
> +};
>  
>  struct btrfs_block_rsv {
>  	u64 size;
> -- 
> 2.19.1
> 

Re: [PATCH 3/9] btrfs: switch BTRFS_FS_* to enums

[Omar Sandoval]

On Tue, Nov 27, 2018 at 08:53:45PM +0100, David Sterba wrote:
> We can use simple enum for values that are not part of on-disk format:
> internal filesystem states.

Hah, looks like we never had a bit 0 ;)

Reviewed-by: Omar Sandoval <osandov@fb.com>

> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>  fs/btrfs/ctree.h | 63 ++++++++++++++++++++++++------------------------
>  1 file changed, 31 insertions(+), 32 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 40c405d74a01..7176b95b40e7 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -757,38 +757,37 @@ struct btrfs_swapfile_pin {
>  
>  bool btrfs_pinned_by_swapfile(struct btrfs_fs_info *fs_info, void *ptr);
>  
> -#define BTRFS_FS_BARRIER			1
> -#define BTRFS_FS_CLOSING_START			2
> -#define BTRFS_FS_CLOSING_DONE			3
> -#define BTRFS_FS_LOG_RECOVERING			4
> -#define BTRFS_FS_OPEN				5
> -#define BTRFS_FS_QUOTA_ENABLED			6
> -#define BTRFS_FS_UPDATE_UUID_TREE_GEN		9
> -#define BTRFS_FS_CREATING_FREE_SPACE_TREE	10
> -#define BTRFS_FS_BTREE_ERR			11
> -#define BTRFS_FS_LOG1_ERR			12
> -#define BTRFS_FS_LOG2_ERR			13
> -#define BTRFS_FS_QUOTA_OVERRIDE			14
> -/* Used to record internally whether fs has been frozen */
> -#define BTRFS_FS_FROZEN				15
> -
> -/*
> - * Indicate that a whole-filesystem exclusive operation is running
> - * (device replace, resize, device add/delete, balance)
> - */
> -#define BTRFS_FS_EXCL_OP			16
> -
> -/*
> - * To info transaction_kthread we need an immediate commit so it doesn't
> - * need to wait for commit_interval
> - */
> -#define BTRFS_FS_NEED_ASYNC_COMMIT		17
> -
> -/*
> - * Indicate that balance has been set up from the ioctl and is in the main
> - * phase. The fs_info::balance_ctl is initialized.
> - */
> -#define BTRFS_FS_BALANCE_RUNNING		18
> +enum {
> +	BTRFS_FS_BARRIER,
> +	BTRFS_FS_CLOSING_START,
> +	BTRFS_FS_CLOSING_DONE,
> +	BTRFS_FS_LOG_RECOVERING,
> +	BTRFS_FS_OPEN,
> +	BTRFS_FS_QUOTA_ENABLED,
> +	BTRFS_FS_UPDATE_UUID_TREE_GEN,
> +	BTRFS_FS_CREATING_FREE_SPACE_TREE,
> +	BTRFS_FS_BTREE_ERR,
> +	BTRFS_FS_LOG1_ERR,
> +	BTRFS_FS_LOG2_ERR,
> +	BTRFS_FS_QUOTA_OVERRIDE,
> +	/* Used to record internally whether fs has been frozen */
> +	BTRFS_FS_FROZEN,
> +	/*
> +	 * Indicate that a whole-filesystem exclusive operation is running
> +	 * (device replace, resize, device add/delete, balance)
> +	 */
> +	BTRFS_FS_EXCL_OP,
> +	/*
> +	 * To info transaction_kthread we need an immediate commit so it
> +	 * doesn't need to wait for commit_interval
> +	 */
> +	BTRFS_FS_NEED_ASYNC_COMMIT,
> +	/*
> +	 * Indicate that balance has been set up from the ioctl and is in the
> +	 * main phase. The fs_info::balance_ctl is initialized.
> +	 */
> +	BTRFS_FS_BALANCE_RUNNING,
> +};
>  
>  struct btrfs_fs_info {
>  	u8 chunk_tree_uuid[BTRFS_UUID_SIZE];
> -- 
> 2.19.1
> 

Re: [PATCH 4/9] btrfs: switch BTRFS_ROOT_* to enums

[Omar Sandoval]

On Tue, Nov 27, 2018 at 08:53:48PM +0100, David Sterba wrote:
> We can use simple enum for values that are not part of on-disk format:
> root tree flags.

Reviewed-by: Omar Sandoval <osandov@fb.com>

> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>  fs/btrfs/ctree.h | 33 +++++++++++++++++----------------
>  1 file changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 7176b95b40e7..4bb0ac3050ff 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -1180,22 +1180,23 @@ struct btrfs_subvolume_writers {
>  /*
>   * The state of btrfs root
>   */
> -/*
> - * btrfs_record_root_in_trans is a multi-step process,
> - * and it can race with the balancing code.   But the
> - * race is very small, and only the first time the root
> - * is added to each transaction.  So IN_TRANS_SETUP
> - * is used to tell us when more checks are required
> - */
> -#define BTRFS_ROOT_IN_TRANS_SETUP	0
> -#define BTRFS_ROOT_REF_COWS		1
> -#define BTRFS_ROOT_TRACK_DIRTY		2
> -#define BTRFS_ROOT_IN_RADIX		3
> -#define BTRFS_ROOT_ORPHAN_ITEM_INSERTED	4
> -#define BTRFS_ROOT_DEFRAG_RUNNING	5
> -#define BTRFS_ROOT_FORCE_COW		6
> -#define BTRFS_ROOT_MULTI_LOG_TASKS	7
> -#define BTRFS_ROOT_DIRTY		8
> +enum {
> +	/*
> +	 * btrfs_record_root_in_trans is a multi-step process, and it can race
> +	 * with the balancing code.   But the race is very small, and only the
> +	 * first time the root is added to each transaction.  So IN_TRANS_SETUP
> +	 * is used to tell us when more checks are required
> +	 */
> +	BTRFS_ROOT_IN_TRANS_SETUP,
> +	BTRFS_ROOT_REF_COWS,
> +	BTRFS_ROOT_TRACK_DIRTY,
> +	BTRFS_ROOT_IN_RADIX,
> +	BTRFS_ROOT_ORPHAN_ITEM_INSERTED,
> +	BTRFS_ROOT_DEFRAG_RUNNING,
> +	BTRFS_ROOT_FORCE_COW,
> +	BTRFS_ROOT_MULTI_LOG_TASKS,
> +	BTRFS_ROOT_DIRTY,
> +};
>  
>  /*
>   * in ram representation of the tree.  extent_root is used for all allocations
> -- 
> 2.19.1
> 

Re: [PATCH 5/9] btrfs: swtich EXTENT_BUFFER_* to enums

[Omar Sandoval]

On Tue, Nov 27, 2018 at 08:53:50PM +0100, David Sterba wrote:
> We can use simple enum for values that are not part of on-disk format:
> extent buffer flags;

This one has a "swtich" typo in the subject. Otherwise,

Reviewed-by: Omar Sandoval <osandov@fb.com>

> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>  fs/btrfs/extent_io.h | 28 ++++++++++++++++------------
>  1 file changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
> index a1d3ea5a0d32..fd42492e62e5 100644
> --- a/fs/btrfs/extent_io.h
> +++ b/fs/btrfs/extent_io.h
> @@ -37,18 +37,22 @@
>  #define EXTENT_BIO_COMPRESSED 1
>  #define EXTENT_BIO_FLAG_SHIFT 16
>  
> -/* these are bit numbers for test/set bit */
> -#define EXTENT_BUFFER_UPTODATE 0
> -#define EXTENT_BUFFER_DIRTY 2
> -#define EXTENT_BUFFER_CORRUPT 3
> -#define EXTENT_BUFFER_READAHEAD 4	/* this got triggered by readahead */
> -#define EXTENT_BUFFER_TREE_REF 5
> -#define EXTENT_BUFFER_STALE 6
> -#define EXTENT_BUFFER_WRITEBACK 7
> -#define EXTENT_BUFFER_READ_ERR 8        /* read IO error */
> -#define EXTENT_BUFFER_UNMAPPED 9
> -#define EXTENT_BUFFER_IN_TREE 10
> -#define EXTENT_BUFFER_WRITE_ERR 11    /* write IO error */
> +enum {
> +	EXTENT_BUFFER_UPTODATE,
> +	EXTENT_BUFFER_DIRTY,
> +	EXTENT_BUFFER_CORRUPT,
> +	/* this got triggered by readahead */
> +	EXTENT_BUFFER_READAHEAD,
> +	EXTENT_BUFFER_TREE_REF,
> +	EXTENT_BUFFER_STALE,
> +	EXTENT_BUFFER_WRITEBACK,
> +	/* read IO error */
> +	EXTENT_BUFFER_READ_ERR,
> +	EXTENT_BUFFER_UNMAPPED,
> +	EXTENT_BUFFER_IN_TREE,
> +	/* write IO error */
> +	EXTENT_BUFFER_WRITE_ERR,
> +};
>  
>  /* these are flags for __process_pages_contig */
>  #define PAGE_UNLOCK		(1 << 0)
> -- 
> 2.19.1
> 

Re: [PATCH 6/9] btrfs: switch EXTENT_FLAG_* to enums

[Omar Sandoval]

On Tue, Nov 27, 2018 at 08:53:52PM +0100, David Sterba wrote:
> We can use simple enum for values that are not part of on-disk format:
> extent map flags.

Reviewed-by: Omar Sandoval <osandov@fb.com>

> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>  fs/btrfs/extent_map.h | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/btrfs/extent_map.h b/fs/btrfs/extent_map.h
> index 31977ffd6190..ef05a0121652 100644
> --- a/fs/btrfs/extent_map.h
> +++ b/fs/btrfs/extent_map.h
> @@ -11,13 +11,20 @@
>  #define EXTENT_MAP_INLINE ((u64)-2)
>  #define EXTENT_MAP_DELALLOC ((u64)-1)
>  
> -/* bits for the flags field */
> -#define EXTENT_FLAG_PINNED 0 /* this entry not yet on disk, don't free it */
> -#define EXTENT_FLAG_COMPRESSED 1
> -#define EXTENT_FLAG_PREALLOC 3 /* pre-allocated extent */
> -#define EXTENT_FLAG_LOGGING 4 /* Logging this extent */
> -#define EXTENT_FLAG_FILLING 5 /* Filling in a preallocated extent */
> -#define EXTENT_FLAG_FS_MAPPING 6 /* filesystem extent mapping type */
> +/* bits for the extent_map::flags field */
> +enum {
> +	/* this entry not yet on disk, don't free it */
> +	EXTENT_FLAG_PINNED,
> +	EXTENT_FLAG_COMPRESSED,
> +	/* pre-allocated extent */
> +	EXTENT_FLAG_PREALLOC,
> +	/* Logging this extent */
> +	EXTENT_FLAG_LOGGING,
> +	/* Filling in a preallocated extent */
> +	EXTENT_FLAG_FILLING,
> +	/* filesystem extent mapping type */
> +	EXTENT_FLAG_FS_MAPPING,
> +};
>  
>  struct extent_map {
>  	struct rb_node rb_node;
> -- 
> 2.19.1
> 

Re: [PATCH 7/9] btrfs: switch BTRFS_*_LOCK to enums

[Omar Sandoval]

On Tue, Nov 27, 2018 at 08:53:55PM +0100, David Sterba wrote:
> We can use simple enum for values that are not part of on-disk format:
> tree lock types.
> 
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>  fs/btrfs/locking.h | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/locking.h b/fs/btrfs/locking.h
> index 29135def468e..684d0ef4faa4 100644
> --- a/fs/btrfs/locking.h
> +++ b/fs/btrfs/locking.h
> @@ -6,10 +6,12 @@
>  #ifndef BTRFS_LOCKING_H
>  #define BTRFS_LOCKING_H
>  
> -#define BTRFS_WRITE_LOCK 1
> -#define BTRFS_READ_LOCK 2
> -#define BTRFS_WRITE_LOCK_BLOCKING 3
> -#define BTRFS_READ_LOCK_BLOCKING 4
> +enum {
> +	BTRFS_WRITE_LOCK,

See btrfs_set_path_blocking() and btrfs_release_path(); 0 means no lock,
so this needs to be BTRFS_WRITE_LOCK = 1. I imagine that lockdep would
catch this.

> +	BTRFS_READ_LOCK,
> +	BTRFS_WRITE_LOCK_BLOCKING,
> +	BTRFS_READ_LOCK_BLOCKING,
> +};
>  
>  void btrfs_tree_lock(struct extent_buffer *eb);
>  void btrfs_tree_unlock(struct extent_buffer *eb);
> -- 
> 2.19.1
> 

Re: [PATCH 8/9] btrfs: switch BTRFS_ORDERED_* to enums

[Omar Sandoval]

On Tue, Nov 27, 2018 at 08:53:57PM +0100, David Sterba wrote:
> We can use simple enum for values that are not part of on-disk format:
> ordered extent flags.

Reviewed-by: Omar Sandoval <osandov@fb.com>

> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>  fs/btrfs/ordered-data.h | 45 +++++++++++++++++++++++------------------
>  1 file changed, 25 insertions(+), 20 deletions(-)
> 
> diff --git a/fs/btrfs/ordered-data.h b/fs/btrfs/ordered-data.h
> index b10e6765d88f..fb9a161f0215 100644
> --- a/fs/btrfs/ordered-data.h
> +++ b/fs/btrfs/ordered-data.h
> @@ -37,26 +37,31 @@ struct btrfs_ordered_sum {
>   * rbtree, just before waking any waiters.  It is used to indicate the
>   * IO is done and any metadata is inserted into the tree.
>   */
> -#define BTRFS_ORDERED_IO_DONE 0 /* set when all the pages are written */
> -
> -#define BTRFS_ORDERED_COMPLETE 1 /* set when removed from the tree */
> -
> -#define BTRFS_ORDERED_NOCOW 2 /* set when we want to write in place */
> -
> -#define BTRFS_ORDERED_COMPRESSED 3 /* writing a zlib compressed extent */
> -
> -#define BTRFS_ORDERED_PREALLOC 4 /* set when writing to preallocated extent */
> -
> -#define BTRFS_ORDERED_DIRECT 5 /* set when we're doing DIO with this extent */
> -
> -#define BTRFS_ORDERED_IOERR 6 /* We had an io error when writing this out */
> -
> -#define BTRFS_ORDERED_UPDATED_ISIZE 7 /* indicates whether this ordered extent
> -				       * has done its due diligence in updating
> -				       * the isize. */
> -#define BTRFS_ORDERED_TRUNCATED 8 /* Set when we have to truncate an extent */
> -
> -#define BTRFS_ORDERED_REGULAR 10 /* Regular IO for COW */
> +enum {
> +	/* set when all the pages are written */
> +	BTRFS_ORDERED_IO_DONE,
> +	/* set when removed from the tree */
> +	BTRFS_ORDERED_COMPLETE,
> +	/* set when we want to write in place */
> +	BTRFS_ORDERED_NOCOW,
> +	/* writing a zlib compressed extent */
> +	BTRFS_ORDERED_COMPRESSED,
> +	/* set when writing to preallocated extent */
> +	BTRFS_ORDERED_PREALLOC,
> +	/* set when we're doing DIO with this extent */
> +	BTRFS_ORDERED_DIRECT,
> +	/* We had an io error when writing this out */
> +	BTRFS_ORDERED_IOERR,
> +	/*
> +	 * indicates whether this ordered extent has done its due diligence in
> +	 * updating the isize
> +	 */
> +	BTRFS_ORDERED_UPDATED_ISIZE,
> +	/* Set when we have to truncate an extent */
> +	BTRFS_ORDERED_TRUNCATED,
> +	/* Regular IO for COW */
> +	BTRFS_ORDERED_REGULAR,
> +};
>  
>  struct btrfs_ordered_extent {
>  	/* logical offset in the file */
> -- 
> 2.19.1
> 

Re: [PATCH 9/9] btrfs: drop extra enum initialization where using defaults

[Omar Sandoval]

On Tue, Nov 27, 2018 at 08:53:59PM +0100, David Sterba wrote:
> The first auto-assigned value to enum is 0, we can use that and not
> initialize all members where the auto-increment does the same. This is
> used for values that are not part of on-disk format.

Reviewed-by: Omar Sandoval <osandov@fb.com>

> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>  fs/btrfs/btrfs_inode.h |  2 +-
>  fs/btrfs/ctree.h       | 28 ++++++++++++++--------------
>  fs/btrfs/disk-io.h     | 10 +++++-----
>  fs/btrfs/qgroup.h      |  2 +-
>  fs/btrfs/sysfs.h       |  2 +-
>  fs/btrfs/transaction.h | 14 +++++++-------
>  6 files changed, 29 insertions(+), 29 deletions(-)
> 
> diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
> index 4de321aee7a5..fc25607304f2 100644
> --- a/fs/btrfs/btrfs_inode.h
> +++ b/fs/btrfs/btrfs_inode.h
> @@ -20,7 +20,7 @@
>   * new data the application may have written before commit.
>   */
>  enum {
> -	BTRFS_INODE_ORDERED_DATA_CLOSE = 0,
> +	BTRFS_INODE_ORDERED_DATA_CLOSE,
>  	BTRFS_INODE_DUMMY,
>  	BTRFS_INODE_IN_DEFRAG,
>  	BTRFS_INODE_HAS_ASYNC_EXTENT,
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 4bb0ac3050ff..f1d1c6ba3aa1 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -334,7 +334,7 @@ struct btrfs_node {
>   * The slots array records the index of the item or block pointer
>   * used while walking the tree.
>   */
> -enum { READA_NONE = 0, READA_BACK, READA_FORWARD };
> +enum { READA_NONE, READA_BACK, READA_FORWARD };
>  struct btrfs_path {
>  	struct extent_buffer *nodes[BTRFS_MAX_LEVEL];
>  	int slots[BTRFS_MAX_LEVEL];
> @@ -532,18 +532,18 @@ struct btrfs_free_cluster {
>  };
>  
>  enum btrfs_caching_type {
> -	BTRFS_CACHE_NO		= 0,
> -	BTRFS_CACHE_STARTED	= 1,
> -	BTRFS_CACHE_FAST	= 2,
> -	BTRFS_CACHE_FINISHED	= 3,
> -	BTRFS_CACHE_ERROR	= 4,
> +	BTRFS_CACHE_NO,
> +	BTRFS_CACHE_STARTED,
> +	BTRFS_CACHE_FAST,
> +	BTRFS_CACHE_FINISHED,
> +	BTRFS_CACHE_ERROR,
>  };
>  
>  enum btrfs_disk_cache_state {
> -	BTRFS_DC_WRITTEN	= 0,
> -	BTRFS_DC_ERROR		= 1,
> -	BTRFS_DC_CLEAR		= 2,
> -	BTRFS_DC_SETUP		= 3,
> +	BTRFS_DC_WRITTEN,
> +	BTRFS_DC_ERROR,
> +	BTRFS_DC_CLEAR,
> +	BTRFS_DC_SETUP,
>  };
>  
>  struct btrfs_caching_control {
> @@ -2621,10 +2621,10 @@ static inline gfp_t btrfs_alloc_write_mask(struct address_space *mapping)
>  /* extent-tree.c */
>  
>  enum btrfs_inline_ref_type {
> -	BTRFS_REF_TYPE_INVALID =	 0,
> -	BTRFS_REF_TYPE_BLOCK =		 1,
> -	BTRFS_REF_TYPE_DATA =		 2,
> -	BTRFS_REF_TYPE_ANY =		 3,
> +	BTRFS_REF_TYPE_INVALID,
> +	BTRFS_REF_TYPE_BLOCK,
> +	BTRFS_REF_TYPE_DATA,
> +	BTRFS_REF_TYPE_ANY,
>  };
>  
>  int btrfs_get_extent_inline_ref_type(const struct extent_buffer *eb,
> diff --git a/fs/btrfs/disk-io.h b/fs/btrfs/disk-io.h
> index 4cccba22640f..987a64bc0c66 100644
> --- a/fs/btrfs/disk-io.h
> +++ b/fs/btrfs/disk-io.h
> @@ -21,11 +21,11 @@
>  #define BTRFS_BDEV_BLOCKSIZE	(4096)
>  
>  enum btrfs_wq_endio_type {
> -	BTRFS_WQ_ENDIO_DATA = 0,
> -	BTRFS_WQ_ENDIO_METADATA = 1,
> -	BTRFS_WQ_ENDIO_FREE_SPACE = 2,
> -	BTRFS_WQ_ENDIO_RAID56 = 3,
> -	BTRFS_WQ_ENDIO_DIO_REPAIR = 4,
> +	BTRFS_WQ_ENDIO_DATA,
> +	BTRFS_WQ_ENDIO_METADATA,
> +	BTRFS_WQ_ENDIO_FREE_SPACE,
> +	BTRFS_WQ_ENDIO_RAID56,
> +	BTRFS_WQ_ENDIO_DIO_REPAIR,
>  };
>  
>  static inline u64 btrfs_sb_offset(int mirror)
> diff --git a/fs/btrfs/qgroup.h b/fs/btrfs/qgroup.h
> index d8f78f5ab854..e4e6ee44073a 100644
> --- a/fs/btrfs/qgroup.h
> +++ b/fs/btrfs/qgroup.h
> @@ -70,7 +70,7 @@ struct btrfs_qgroup_extent_record {
>   *	be converted into META_PERTRANS.
>   */
>  enum btrfs_qgroup_rsv_type {
> -	BTRFS_QGROUP_RSV_DATA = 0,
> +	BTRFS_QGROUP_RSV_DATA,
>  	BTRFS_QGROUP_RSV_META_PERTRANS,
>  	BTRFS_QGROUP_RSV_META_PREALLOC,
>  	BTRFS_QGROUP_RSV_LAST,
> diff --git a/fs/btrfs/sysfs.h b/fs/btrfs/sysfs.h
> index c6ee600aff89..40716b357c1d 100644
> --- a/fs/btrfs/sysfs.h
> +++ b/fs/btrfs/sysfs.h
> @@ -9,7 +9,7 @@
>  extern u64 btrfs_debugfs_test;
>  
>  enum btrfs_feature_set {
> -	FEAT_COMPAT = 0,
> +	FEAT_COMPAT,
>  	FEAT_COMPAT_RO,
>  	FEAT_INCOMPAT,
>  	FEAT_MAX
> diff --git a/fs/btrfs/transaction.h b/fs/btrfs/transaction.h
> index 703d5116a2fc..f1ba78949d1b 100644
> --- a/fs/btrfs/transaction.h
> +++ b/fs/btrfs/transaction.h
> @@ -12,13 +12,13 @@
>  #include "ctree.h"
>  
>  enum btrfs_trans_state {
> -	TRANS_STATE_RUNNING		= 0,
> -	TRANS_STATE_BLOCKED		= 1,
> -	TRANS_STATE_COMMIT_START	= 2,
> -	TRANS_STATE_COMMIT_DOING	= 3,
> -	TRANS_STATE_UNBLOCKED		= 4,
> -	TRANS_STATE_COMPLETED		= 5,
> -	TRANS_STATE_MAX			= 6,
> +	TRANS_STATE_RUNNING,
> +	TRANS_STATE_BLOCKED,
> +	TRANS_STATE_COMMIT_START,
> +	TRANS_STATE_COMMIT_DOING,
> +	TRANS_STATE_UNBLOCKED,
> +	TRANS_STATE_COMPLETED,
> +	TRANS_STATE_MAX,
>  };
>  
>  #define BTRFS_TRANS_HAVE_FREE_BGS	0
> -- 
> 2.19.1
> 

Re: [PATCH 1/9] btrfs: switch BTRFS_FS_STATE_* to enums

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 1623 bytes --]



On 2018/11/28 上午3:53, David Sterba wrote:
> We can use simple enum for values that are not part of on-disk format:
> global filesystem states.
> 
> Signed-off-by: David Sterba <dsterba@suse.com>

Good comment.

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>  fs/btrfs/ctree.h | 25 +++++++++++++++++++------
>  1 file changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index a98507fa9192..f82ec5e41b0c 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -109,13 +109,26 @@ static inline unsigned long btrfs_chunk_item_size(int num_stripes)
>  }
>  
>  /*
> - * File system states
> + * Runtime (in-memory) states of filesystem
>   */
> -#define BTRFS_FS_STATE_ERROR		0
> -#define BTRFS_FS_STATE_REMOUNTING	1
> -#define BTRFS_FS_STATE_TRANS_ABORTED	2
> -#define BTRFS_FS_STATE_DEV_REPLACING	3
> -#define BTRFS_FS_STATE_DUMMY_FS_INFO	4
> +enum {
> +	/* Global indicator of serious filesysystem errors */
> +	BTRFS_FS_STATE_ERROR,
> +	/*
> +	 * Filesystem is being remounted, allow to skip some operations, like
> +	 * defrag
> +	 */
> +	BTRFS_FS_STATE_REMOUNTING,
> +	/* Track if the transaction abort has been reported */
> +	BTRFS_FS_STATE_TRANS_ABORTED,
> +	/*
> +	 * Indicate that replace source or target device state is changed and
> +	 * allow to block bio operations
> +	 */
> +	BTRFS_FS_STATE_DEV_REPLACING,
> +	/* The btrfs_fs_info created for self-tests */
> +	BTRFS_FS_STATE_DUMMY_FS_INFO,
> +};
>  
>  #define BTRFS_BACKREF_REV_MAX		256
>  #define BTRFS_BACKREF_REV_SHIFT		56
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/9] btrfs: switch BTRFS_BLOCK_RSV_* to enums

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 1276 bytes --]



On 2018/11/28 上午3:53, David Sterba wrote:
> We can use simple enum for values that are not part of on-disk format:
> block reserve types.
> 
> Signed-off-by: David Sterba <dsterba@suse.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

However more comment will always be a good thing.

Thanks,
Qu

> ---
>  fs/btrfs/ctree.h | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index f82ec5e41b0c..40c405d74a01 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -461,13 +461,18 @@ struct btrfs_space_info {
>  	struct kobject *block_group_kobjs[BTRFS_NR_RAID_TYPES];
>  };
>  
> -#define	BTRFS_BLOCK_RSV_GLOBAL		1
> -#define	BTRFS_BLOCK_RSV_DELALLOC	2
> -#define	BTRFS_BLOCK_RSV_TRANS		3
> -#define	BTRFS_BLOCK_RSV_CHUNK		4
> -#define	BTRFS_BLOCK_RSV_DELOPS		5
> -#define	BTRFS_BLOCK_RSV_EMPTY		6
> -#define	BTRFS_BLOCK_RSV_TEMP		7
> +/*
> + * Types of block reserves
> + */
> +enum {
> +	BTRFS_BLOCK_RSV_GLOBAL,
> +	BTRFS_BLOCK_RSV_DELALLOC,
> +	BTRFS_BLOCK_RSV_TRANS,
> +	BTRFS_BLOCK_RSV_CHUNK,
> +	BTRFS_BLOCK_RSV_DELOPS,
> +	BTRFS_BLOCK_RSV_EMPTY,
> +	BTRFS_BLOCK_RSV_TEMP,
> +};
>  
>  struct btrfs_block_rsv {
>  	u64 size;
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 3/9] btrfs: switch BTRFS_FS_* to enums

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 2861 bytes --]



On 2018/11/28 上午3:53, David Sterba wrote:
> We can use simple enum for values that are not part of on-disk format:
> internal filesystem states.
> 
> Signed-off-by: David Sterba <dsterba@suse.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>  fs/btrfs/ctree.h | 63 ++++++++++++++++++++++++------------------------
>  1 file changed, 31 insertions(+), 32 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 40c405d74a01..7176b95b40e7 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -757,38 +757,37 @@ struct btrfs_swapfile_pin {
>  
>  bool btrfs_pinned_by_swapfile(struct btrfs_fs_info *fs_info, void *ptr);
>  
> -#define BTRFS_FS_BARRIER			1
> -#define BTRFS_FS_CLOSING_START			2
> -#define BTRFS_FS_CLOSING_DONE			3
> -#define BTRFS_FS_LOG_RECOVERING			4
> -#define BTRFS_FS_OPEN				5
> -#define BTRFS_FS_QUOTA_ENABLED			6
> -#define BTRFS_FS_UPDATE_UUID_TREE_GEN		9
> -#define BTRFS_FS_CREATING_FREE_SPACE_TREE	10
> -#define BTRFS_FS_BTREE_ERR			11
> -#define BTRFS_FS_LOG1_ERR			12
> -#define BTRFS_FS_LOG2_ERR			13
> -#define BTRFS_FS_QUOTA_OVERRIDE			14
> -/* Used to record internally whether fs has been frozen */
> -#define BTRFS_FS_FROZEN				15
> -
> -/*
> - * Indicate that a whole-filesystem exclusive operation is running
> - * (device replace, resize, device add/delete, balance)
> - */
> -#define BTRFS_FS_EXCL_OP			16
> -
> -/*
> - * To info transaction_kthread we need an immediate commit so it doesn't
> - * need to wait for commit_interval
> - */
> -#define BTRFS_FS_NEED_ASYNC_COMMIT		17
> -
> -/*
> - * Indicate that balance has been set up from the ioctl and is in the main
> - * phase. The fs_info::balance_ctl is initialized.
> - */
> -#define BTRFS_FS_BALANCE_RUNNING		18
> +enum {
> +	BTRFS_FS_BARRIER,
> +	BTRFS_FS_CLOSING_START,
> +	BTRFS_FS_CLOSING_DONE,
> +	BTRFS_FS_LOG_RECOVERING,
> +	BTRFS_FS_OPEN,
> +	BTRFS_FS_QUOTA_ENABLED,
> +	BTRFS_FS_UPDATE_UUID_TREE_GEN,
> +	BTRFS_FS_CREATING_FREE_SPACE_TREE,
> +	BTRFS_FS_BTREE_ERR,
> +	BTRFS_FS_LOG1_ERR,
> +	BTRFS_FS_LOG2_ERR,
> +	BTRFS_FS_QUOTA_OVERRIDE,
> +	/* Used to record internally whether fs has been frozen */
> +	BTRFS_FS_FROZEN,
> +	/*
> +	 * Indicate that a whole-filesystem exclusive operation is running
> +	 * (device replace, resize, device add/delete, balance)
> +	 */
> +	BTRFS_FS_EXCL_OP,
> +	/*
> +	 * To info transaction_kthread we need an immediate commit so it
> +	 * doesn't need to wait for commit_interval
> +	 */
> +	BTRFS_FS_NEED_ASYNC_COMMIT,
> +	/*
> +	 * Indicate that balance has been set up from the ioctl and is in the
> +	 * main phase. The fs_info::balance_ctl is initialized.
> +	 */
> +	BTRFS_FS_BALANCE_RUNNING,
> +};
>  
>  struct btrfs_fs_info {
>  	u8 chunk_tree_uuid[BTRFS_UUID_SIZE];
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 4/9] btrfs: switch BTRFS_ROOT_* to enums

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 1970 bytes --]



On 2018/11/28 上午3:53, David Sterba wrote:
> We can use simple enum for values that are not part of on-disk format:
> root tree flags.
> 
> Signed-off-by: David Sterba <dsterba@suse.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>  fs/btrfs/ctree.h | 33 +++++++++++++++++----------------
>  1 file changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 7176b95b40e7..4bb0ac3050ff 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -1180,22 +1180,23 @@ struct btrfs_subvolume_writers {
>  /*
>   * The state of btrfs root
>   */
> -/*
> - * btrfs_record_root_in_trans is a multi-step process,
> - * and it can race with the balancing code.   But the
> - * race is very small, and only the first time the root
> - * is added to each transaction.  So IN_TRANS_SETUP
> - * is used to tell us when more checks are required
> - */
> -#define BTRFS_ROOT_IN_TRANS_SETUP	0
> -#define BTRFS_ROOT_REF_COWS		1
> -#define BTRFS_ROOT_TRACK_DIRTY		2
> -#define BTRFS_ROOT_IN_RADIX		3
> -#define BTRFS_ROOT_ORPHAN_ITEM_INSERTED	4
> -#define BTRFS_ROOT_DEFRAG_RUNNING	5
> -#define BTRFS_ROOT_FORCE_COW		6
> -#define BTRFS_ROOT_MULTI_LOG_TASKS	7
> -#define BTRFS_ROOT_DIRTY		8
> +enum {
> +	/*
> +	 * btrfs_record_root_in_trans is a multi-step process, and it can race
> +	 * with the balancing code.   But the race is very small, and only the
> +	 * first time the root is added to each transaction.  So IN_TRANS_SETUP
> +	 * is used to tell us when more checks are required
> +	 */
> +	BTRFS_ROOT_IN_TRANS_SETUP,
> +	BTRFS_ROOT_REF_COWS,
> +	BTRFS_ROOT_TRACK_DIRTY,
> +	BTRFS_ROOT_IN_RADIX,
> +	BTRFS_ROOT_ORPHAN_ITEM_INSERTED,
> +	BTRFS_ROOT_DEFRAG_RUNNING,
> +	BTRFS_ROOT_FORCE_COW,
> +	BTRFS_ROOT_MULTI_LOG_TASKS,
> +	BTRFS_ROOT_DIRTY,
> +};
>  
>  /*
>   * in ram representation of the tree.  extent_root is used for all allocations
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 5/9] btrfs: swtich EXTENT_BUFFER_* to enums

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 1715 bytes --]



On 2018/11/28 上午3:53, David Sterba wrote:
> We can use simple enum for values that are not part of on-disk format:
> extent buffer flags;
> 
> Signed-off-by: David Sterba <dsterba@suse.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>  fs/btrfs/extent_io.h | 28 ++++++++++++++++------------
>  1 file changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
> index a1d3ea5a0d32..fd42492e62e5 100644
> --- a/fs/btrfs/extent_io.h
> +++ b/fs/btrfs/extent_io.h
> @@ -37,18 +37,22 @@
>  #define EXTENT_BIO_COMPRESSED 1
>  #define EXTENT_BIO_FLAG_SHIFT 16
>  
> -/* these are bit numbers for test/set bit */
> -#define EXTENT_BUFFER_UPTODATE 0
> -#define EXTENT_BUFFER_DIRTY 2
> -#define EXTENT_BUFFER_CORRUPT 3
> -#define EXTENT_BUFFER_READAHEAD 4	/* this got triggered by readahead */
> -#define EXTENT_BUFFER_TREE_REF 5
> -#define EXTENT_BUFFER_STALE 6
> -#define EXTENT_BUFFER_WRITEBACK 7
> -#define EXTENT_BUFFER_READ_ERR 8        /* read IO error */
> -#define EXTENT_BUFFER_UNMAPPED 9
> -#define EXTENT_BUFFER_IN_TREE 10
> -#define EXTENT_BUFFER_WRITE_ERR 11    /* write IO error */
> +enum {
> +	EXTENT_BUFFER_UPTODATE,
> +	EXTENT_BUFFER_DIRTY,
> +	EXTENT_BUFFER_CORRUPT,
> +	/* this got triggered by readahead */
> +	EXTENT_BUFFER_READAHEAD,
> +	EXTENT_BUFFER_TREE_REF,
> +	EXTENT_BUFFER_STALE,
> +	EXTENT_BUFFER_WRITEBACK,
> +	/* read IO error */
> +	EXTENT_BUFFER_READ_ERR,
> +	EXTENT_BUFFER_UNMAPPED,
> +	EXTENT_BUFFER_IN_TREE,
> +	/* write IO error */
> +	EXTENT_BUFFER_WRITE_ERR,
> +};
>  
>  /* these are flags for __process_pages_contig */
>  #define PAGE_UNLOCK		(1 << 0)
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 6/9] btrfs: switch EXTENT_FLAG_* to enums

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 1567 bytes --]



On 2018/11/28 上午3:53, David Sterba wrote:
> We can use simple enum for values that are not part of on-disk format:
> extent map flags.
> 
> Signed-off-by: David Sterba <dsterba@suse.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>  fs/btrfs/extent_map.h | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/btrfs/extent_map.h b/fs/btrfs/extent_map.h
> index 31977ffd6190..ef05a0121652 100644
> --- a/fs/btrfs/extent_map.h
> +++ b/fs/btrfs/extent_map.h
> @@ -11,13 +11,20 @@
>  #define EXTENT_MAP_INLINE ((u64)-2)
>  #define EXTENT_MAP_DELALLOC ((u64)-1)
>  
> -/* bits for the flags field */
> -#define EXTENT_FLAG_PINNED 0 /* this entry not yet on disk, don't free it */
> -#define EXTENT_FLAG_COMPRESSED 1
> -#define EXTENT_FLAG_PREALLOC 3 /* pre-allocated extent */
> -#define EXTENT_FLAG_LOGGING 4 /* Logging this extent */
> -#define EXTENT_FLAG_FILLING 5 /* Filling in a preallocated extent */
> -#define EXTENT_FLAG_FS_MAPPING 6 /* filesystem extent mapping type */
> +/* bits for the extent_map::flags field */
> +enum {
> +	/* this entry not yet on disk, don't free it */
> +	EXTENT_FLAG_PINNED,
> +	EXTENT_FLAG_COMPRESSED,
> +	/* pre-allocated extent */
> +	EXTENT_FLAG_PREALLOC,
> +	/* Logging this extent */
> +	EXTENT_FLAG_LOGGING,
> +	/* Filling in a preallocated extent */
> +	EXTENT_FLAG_FILLING,
> +	/* filesystem extent mapping type */
> +	EXTENT_FLAG_FS_MAPPING,
> +};
>  
>  struct extent_map {
>  	struct rb_node rb_node;
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 7/9] btrfs: switch BTRFS_*_LOCK to enums

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 979 bytes --]



On 2018/11/28 上午3:53, David Sterba wrote:
> We can use simple enum for values that are not part of on-disk format:
> tree lock types.
> 
> Signed-off-by: David Sterba <dsterba@suse.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>  fs/btrfs/locking.h | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/locking.h b/fs/btrfs/locking.h
> index 29135def468e..684d0ef4faa4 100644
> --- a/fs/btrfs/locking.h
> +++ b/fs/btrfs/locking.h
> @@ -6,10 +6,12 @@
>  #ifndef BTRFS_LOCKING_H
>  #define BTRFS_LOCKING_H
>  
> -#define BTRFS_WRITE_LOCK 1
> -#define BTRFS_READ_LOCK 2
> -#define BTRFS_WRITE_LOCK_BLOCKING 3
> -#define BTRFS_READ_LOCK_BLOCKING 4
> +enum {
> +	BTRFS_WRITE_LOCK,
> +	BTRFS_READ_LOCK,
> +	BTRFS_WRITE_LOCK_BLOCKING,
> +	BTRFS_READ_LOCK_BLOCKING,
> +};
>  
>  void btrfs_tree_lock(struct extent_buffer *eb);
>  void btrfs_tree_unlock(struct extent_buffer *eb);
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 8/9] btrfs: switch BTRFS_ORDERED_* to enums

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 2601 bytes --]



On 2018/11/28 上午3:53, David Sterba wrote:
> We can use simple enum for values that are not part of on-disk format:
> ordered extent flags.
> 
> Signed-off-by: David Sterba <dsterba@suse.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>  fs/btrfs/ordered-data.h | 45 +++++++++++++++++++++++------------------
>  1 file changed, 25 insertions(+), 20 deletions(-)
> 
> diff --git a/fs/btrfs/ordered-data.h b/fs/btrfs/ordered-data.h
> index b10e6765d88f..fb9a161f0215 100644
> --- a/fs/btrfs/ordered-data.h
> +++ b/fs/btrfs/ordered-data.h
> @@ -37,26 +37,31 @@ struct btrfs_ordered_sum {
>   * rbtree, just before waking any waiters.  It is used to indicate the
>   * IO is done and any metadata is inserted into the tree.
>   */
> -#define BTRFS_ORDERED_IO_DONE 0 /* set when all the pages are written */
> -
> -#define BTRFS_ORDERED_COMPLETE 1 /* set when removed from the tree */
> -
> -#define BTRFS_ORDERED_NOCOW 2 /* set when we want to write in place */
> -
> -#define BTRFS_ORDERED_COMPRESSED 3 /* writing a zlib compressed extent */
> -
> -#define BTRFS_ORDERED_PREALLOC 4 /* set when writing to preallocated extent */
> -
> -#define BTRFS_ORDERED_DIRECT 5 /* set when we're doing DIO with this extent */
> -
> -#define BTRFS_ORDERED_IOERR 6 /* We had an io error when writing this out */
> -
> -#define BTRFS_ORDERED_UPDATED_ISIZE 7 /* indicates whether this ordered extent
> -				       * has done its due diligence in updating
> -				       * the isize. */
> -#define BTRFS_ORDERED_TRUNCATED 8 /* Set when we have to truncate an extent */
> -
> -#define BTRFS_ORDERED_REGULAR 10 /* Regular IO for COW */
> +enum {
> +	/* set when all the pages are written */
> +	BTRFS_ORDERED_IO_DONE,
> +	/* set when removed from the tree */
> +	BTRFS_ORDERED_COMPLETE,
> +	/* set when we want to write in place */
> +	BTRFS_ORDERED_NOCOW,
> +	/* writing a zlib compressed extent */
> +	BTRFS_ORDERED_COMPRESSED,
> +	/* set when writing to preallocated extent */
> +	BTRFS_ORDERED_PREALLOC,
> +	/* set when we're doing DIO with this extent */
> +	BTRFS_ORDERED_DIRECT,
> +	/* We had an io error when writing this out */
> +	BTRFS_ORDERED_IOERR,
> +	/*
> +	 * indicates whether this ordered extent has done its due diligence in
> +	 * updating the isize
> +	 */
> +	BTRFS_ORDERED_UPDATED_ISIZE,
> +	/* Set when we have to truncate an extent */
> +	BTRFS_ORDERED_TRUNCATED,
> +	/* Regular IO for COW */
> +	BTRFS_ORDERED_REGULAR,
> +};
>  
>  struct btrfs_ordered_extent {
>  	/* logical offset in the file */
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 0/9] Switch defines to enums

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 1706 bytes --]



On 2018/11/28 上午3:53, David Sterba wrote:
> This is motivated by a merging mistake that happened a few releases ago.
> Two patches updated BTRFS_FS_* flags independently to the same value,
> git did not see any direct merge conflict. The two values got mixed at
> runtime and caused crash.
> 
> Update all #define sequential values, the above merging problem would
> not happen as there would be a conflict and the enum value
> auto-increment would prevent duplicated values anyway.

Just one small question for the bitmap usage.

For enum we won't directly know the last number is, my concern is if
we're using u16 as bitmap and there is some enum over 15, will we get a
warning at compile time or some bug would just sneak into kernel?

Thanks,
Qu

> 
> David Sterba (9):
>   btrfs: switch BTRFS_FS_STATE_* to enums
>   btrfs: switch BTRFS_BLOCK_RSV_* to enums
>   btrfs: switch BTRFS_FS_* to enums
>   btrfs: switch BTRFS_ROOT_* to enums
>   btrfs: swtich EXTENT_BUFFER_* to enums
>   btrfs: switch EXTENT_FLAG_* to enums
>   btrfs: switch BTRFS_*_LOCK to enums
>   btrfs: switch BTRFS_ORDERED_* to enums
>   btrfs: drop extra enum initialization where using defaults
> 
>  fs/btrfs/btrfs_inode.h  |   2 +-
>  fs/btrfs/ctree.h        | 168 ++++++++++++++++++++++------------------
>  fs/btrfs/disk-io.h      |  10 +--
>  fs/btrfs/extent_io.h    |  28 ++++---
>  fs/btrfs/extent_map.h   |  21 +++--
>  fs/btrfs/locking.h      |  10 ++-
>  fs/btrfs/ordered-data.h |  45 ++++++-----
>  fs/btrfs/qgroup.h       |   2 +-
>  fs/btrfs/sysfs.h        |   2 +-
>  fs/btrfs/transaction.h  |  14 ++--
>  10 files changed, 169 insertions(+), 133 deletions(-)
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 9/9] btrfs: drop extra enum initialization where using defaults

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 4913 bytes --]



On 2018/11/28 上午3:53, David Sterba wrote:
> The first auto-assigned value to enum is 0, we can use that and not
> initialize all members where the auto-increment does the same. This is
> used for values that are not part of on-disk format.
> 
> Signed-off-by: David Sterba <dsterba@suse.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>  fs/btrfs/btrfs_inode.h |  2 +-
>  fs/btrfs/ctree.h       | 28 ++++++++++++++--------------
>  fs/btrfs/disk-io.h     | 10 +++++-----
>  fs/btrfs/qgroup.h      |  2 +-
>  fs/btrfs/sysfs.h       |  2 +-
>  fs/btrfs/transaction.h | 14 +++++++-------
>  6 files changed, 29 insertions(+), 29 deletions(-)
> 
> diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
> index 4de321aee7a5..fc25607304f2 100644
> --- a/fs/btrfs/btrfs_inode.h
> +++ b/fs/btrfs/btrfs_inode.h
> @@ -20,7 +20,7 @@
>   * new data the application may have written before commit.
>   */
>  enum {
> -	BTRFS_INODE_ORDERED_DATA_CLOSE = 0,
> +	BTRFS_INODE_ORDERED_DATA_CLOSE,
>  	BTRFS_INODE_DUMMY,
>  	BTRFS_INODE_IN_DEFRAG,
>  	BTRFS_INODE_HAS_ASYNC_EXTENT,
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 4bb0ac3050ff..f1d1c6ba3aa1 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -334,7 +334,7 @@ struct btrfs_node {
>   * The slots array records the index of the item or block pointer
>   * used while walking the tree.
>   */
> -enum { READA_NONE = 0, READA_BACK, READA_FORWARD };
> +enum { READA_NONE, READA_BACK, READA_FORWARD };
>  struct btrfs_path {
>  	struct extent_buffer *nodes[BTRFS_MAX_LEVEL];
>  	int slots[BTRFS_MAX_LEVEL];
> @@ -532,18 +532,18 @@ struct btrfs_free_cluster {
>  };
>  
>  enum btrfs_caching_type {
> -	BTRFS_CACHE_NO		= 0,
> -	BTRFS_CACHE_STARTED	= 1,
> -	BTRFS_CACHE_FAST	= 2,
> -	BTRFS_CACHE_FINISHED	= 3,
> -	BTRFS_CACHE_ERROR	= 4,
> +	BTRFS_CACHE_NO,
> +	BTRFS_CACHE_STARTED,
> +	BTRFS_CACHE_FAST,
> +	BTRFS_CACHE_FINISHED,
> +	BTRFS_CACHE_ERROR,
>  };
>  
>  enum btrfs_disk_cache_state {
> -	BTRFS_DC_WRITTEN	= 0,
> -	BTRFS_DC_ERROR		= 1,
> -	BTRFS_DC_CLEAR		= 2,
> -	BTRFS_DC_SETUP		= 3,
> +	BTRFS_DC_WRITTEN,
> +	BTRFS_DC_ERROR,
> +	BTRFS_DC_CLEAR,
> +	BTRFS_DC_SETUP,
>  };
>  
>  struct btrfs_caching_control {
> @@ -2621,10 +2621,10 @@ static inline gfp_t btrfs_alloc_write_mask(struct address_space *mapping)
>  /* extent-tree.c */
>  
>  enum btrfs_inline_ref_type {
> -	BTRFS_REF_TYPE_INVALID =	 0,
> -	BTRFS_REF_TYPE_BLOCK =		 1,
> -	BTRFS_REF_TYPE_DATA =		 2,
> -	BTRFS_REF_TYPE_ANY =		 3,
> +	BTRFS_REF_TYPE_INVALID,
> +	BTRFS_REF_TYPE_BLOCK,
> +	BTRFS_REF_TYPE_DATA,
> +	BTRFS_REF_TYPE_ANY,
>  };
>  
>  int btrfs_get_extent_inline_ref_type(const struct extent_buffer *eb,
> diff --git a/fs/btrfs/disk-io.h b/fs/btrfs/disk-io.h
> index 4cccba22640f..987a64bc0c66 100644
> --- a/fs/btrfs/disk-io.h
> +++ b/fs/btrfs/disk-io.h
> @@ -21,11 +21,11 @@
>  #define BTRFS_BDEV_BLOCKSIZE	(4096)
>  
>  enum btrfs_wq_endio_type {
> -	BTRFS_WQ_ENDIO_DATA = 0,
> -	BTRFS_WQ_ENDIO_METADATA = 1,
> -	BTRFS_WQ_ENDIO_FREE_SPACE = 2,
> -	BTRFS_WQ_ENDIO_RAID56 = 3,
> -	BTRFS_WQ_ENDIO_DIO_REPAIR = 4,
> +	BTRFS_WQ_ENDIO_DATA,
> +	BTRFS_WQ_ENDIO_METADATA,
> +	BTRFS_WQ_ENDIO_FREE_SPACE,
> +	BTRFS_WQ_ENDIO_RAID56,
> +	BTRFS_WQ_ENDIO_DIO_REPAIR,
>  };
>  
>  static inline u64 btrfs_sb_offset(int mirror)
> diff --git a/fs/btrfs/qgroup.h b/fs/btrfs/qgroup.h
> index d8f78f5ab854..e4e6ee44073a 100644
> --- a/fs/btrfs/qgroup.h
> +++ b/fs/btrfs/qgroup.h
> @@ -70,7 +70,7 @@ struct btrfs_qgroup_extent_record {
>   *	be converted into META_PERTRANS.
>   */
>  enum btrfs_qgroup_rsv_type {
> -	BTRFS_QGROUP_RSV_DATA = 0,
> +	BTRFS_QGROUP_RSV_DATA,
>  	BTRFS_QGROUP_RSV_META_PERTRANS,
>  	BTRFS_QGROUP_RSV_META_PREALLOC,
>  	BTRFS_QGROUP_RSV_LAST,
> diff --git a/fs/btrfs/sysfs.h b/fs/btrfs/sysfs.h
> index c6ee600aff89..40716b357c1d 100644
> --- a/fs/btrfs/sysfs.h
> +++ b/fs/btrfs/sysfs.h
> @@ -9,7 +9,7 @@
>  extern u64 btrfs_debugfs_test;
>  
>  enum btrfs_feature_set {
> -	FEAT_COMPAT = 0,
> +	FEAT_COMPAT,
>  	FEAT_COMPAT_RO,
>  	FEAT_INCOMPAT,
>  	FEAT_MAX
> diff --git a/fs/btrfs/transaction.h b/fs/btrfs/transaction.h
> index 703d5116a2fc..f1ba78949d1b 100644
> --- a/fs/btrfs/transaction.h
> +++ b/fs/btrfs/transaction.h
> @@ -12,13 +12,13 @@
>  #include "ctree.h"
>  
>  enum btrfs_trans_state {
> -	TRANS_STATE_RUNNING		= 0,
> -	TRANS_STATE_BLOCKED		= 1,
> -	TRANS_STATE_COMMIT_START	= 2,
> -	TRANS_STATE_COMMIT_DOING	= 3,
> -	TRANS_STATE_UNBLOCKED		= 4,
> -	TRANS_STATE_COMPLETED		= 5,
> -	TRANS_STATE_MAX			= 6,
> +	TRANS_STATE_RUNNING,
> +	TRANS_STATE_BLOCKED,
> +	TRANS_STATE_COMMIT_START,
> +	TRANS_STATE_COMMIT_DOING,
> +	TRANS_STATE_UNBLOCKED,
> +	TRANS_STATE_COMPLETED,
> +	TRANS_STATE_MAX,
>  };
>  
>  #define BTRFS_TRANS_HAVE_FREE_BGS	0
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/9] btrfs: switch BTRFS_FS_STATE_* to enums

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                                        SUSE Labs
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH 2/9] btrfs: switch BTRFS_BLOCK_RSV_* to enums

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                                        SUSE Labs
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH 4/9] btrfs: switch BTRFS_ROOT_* to enums

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH 5/9] btrfs: swtich EXTENT_BUFFER_* to enums

[Johannes Thumshirn]

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH 7/9] btrfs: switch BTRFS_*_LOCK to enums

[David Sterba]

On Tue, Nov 27, 2018 at 04:37:16PM -0800, Omar Sandoval wrote:
> On Tue, Nov 27, 2018 at 08:53:55PM +0100, David Sterba wrote:
> > We can use simple enum for values that are not part of on-disk format:
> > tree lock types.
> > 
> > Signed-off-by: David Sterba <dsterba@suse.com>
> > ---
> >  fs/btrfs/locking.h | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/fs/btrfs/locking.h b/fs/btrfs/locking.h
> > index 29135def468e..684d0ef4faa4 100644
> > --- a/fs/btrfs/locking.h
> > +++ b/fs/btrfs/locking.h
> > @@ -6,10 +6,12 @@
> >  #ifndef BTRFS_LOCKING_H
> >  #define BTRFS_LOCKING_H
> >  
> > -#define BTRFS_WRITE_LOCK 1
> > -#define BTRFS_READ_LOCK 2
> > -#define BTRFS_WRITE_LOCK_BLOCKING 3
> > -#define BTRFS_READ_LOCK_BLOCKING 4
> > +enum {
> > +	BTRFS_WRITE_LOCK,
> 
> See btrfs_set_path_blocking() and btrfs_release_path(); 0 means no lock,
> so this needs to be BTRFS_WRITE_LOCK = 1. I imagine that lockdep would
> catch this.

Oh right of course, thanks for catching it. I'll drop the patch for now,
0 could be added to the set as BTRFS_NO_LOCK and replaced in the code
which is beyond what this series does.

Re: [PATCH 0/9] Switch defines to enums

[David Sterba]

On Wed, Nov 28, 2018 at 09:33:50AM +0800, Qu Wenruo wrote:
> On 2018/11/28 上午3:53, David Sterba wrote:
> > This is motivated by a merging mistake that happened a few releases ago.
> > Two patches updated BTRFS_FS_* flags independently to the same value,
> > git did not see any direct merge conflict. The two values got mixed at
> > runtime and caused crash.
> > 
> > Update all #define sequential values, the above merging problem would
> > not happen as there would be a conflict and the enum value
> > auto-increment would prevent duplicated values anyway.
> 
> Just one small question for the bitmap usage.
> 
> For enum we won't directly know the last number is, my concern is if
> we're using u16 as bitmap and there is some enum over 15, will we get a
> warning at compile time or some bug would just sneak into kernel?

Do you have a concrete example where this would happen? Most bits are
used in 'long' that should be at least 32. I'm not aware of 16bit bits
flags.

Re: [PATCH 6/9] btrfs: switch EXTENT_FLAG_* to enums

[Johannes Thumshirn]

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH 8/9] btrfs: switch BTRFS_ORDERED_* to enums

[Johannes Thumshirn]

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH 9/9] btrfs: drop extra enum initialization where using defaults

[Johannes Thumshirn]

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH 0/9] Switch defines to enums

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 1319 bytes --]



On 2018/11/28 下午9:25, David Sterba wrote:
> On Wed, Nov 28, 2018 at 09:33:50AM +0800, Qu Wenruo wrote:
>> On 2018/11/28 上午3:53, David Sterba wrote:
>>> This is motivated by a merging mistake that happened a few releases ago.
>>> Two patches updated BTRFS_FS_* flags independently to the same value,
>>> git did not see any direct merge conflict. The two values got mixed at
>>> runtime and caused crash.
>>>
>>> Update all #define sequential values, the above merging problem would
>>> not happen as there would be a conflict and the enum value
>>> auto-increment would prevent duplicated values anyway.
>>
>> Just one small question for the bitmap usage.
>>
>> For enum we won't directly know the last number is, my concern is if
>> we're using u16 as bitmap and there is some enum over 15, will we get a
>> warning at compile time or some bug would just sneak into kernel?
> 
> Do you have a concrete example where this would happen? Most bits are
> used in 'long' that should be at least 32. I'm not aware of 16bit bits
> flags.
> 

OK, I'm too paranoid here.

The set_bit() definition needs unsigned long *, and passing a u16 * will
cause compile time warning, so it shouldn't be a problem.

And for enum over 63, reviewers will definitely complain anyway.

Thanks,
Qu


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/9] btrfs: switch BTRFS_FS_STATE_* to enums

[David Sterba]

On Tue, Nov 27, 2018 at 04:24:03PM -0800, Omar Sandoval wrote:
> On Tue, Nov 27, 2018 at 08:53:41PM +0100, David Sterba wrote:
> > We can use simple enum for values that are not part of on-disk format:
> > global filesystem states.
> 
> Reviewed-by: Omar Sandoval <osandov@fb.com>
> 
> Some typos/wording suggestions below.
> 
> > Signed-off-by: David Sterba <dsterba@suse.com>
> > ---
> >  fs/btrfs/ctree.h | 25 +++++++++++++++++++------
> >  1 file changed, 19 insertions(+), 6 deletions(-)
> > 
> > diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> > index a98507fa9192..f82ec5e41b0c 100644
> > --- a/fs/btrfs/ctree.h
> > +++ b/fs/btrfs/ctree.h
> > @@ -109,13 +109,26 @@ static inline unsigned long btrfs_chunk_item_size(int num_stripes)
> >  }
> >  
> >  /*
> > - * File system states
> > + * Runtime (in-memory) states of filesystem
> >   */
> > -#define BTRFS_FS_STATE_ERROR		0
> > -#define BTRFS_FS_STATE_REMOUNTING	1
> > -#define BTRFS_FS_STATE_TRANS_ABORTED	2
> > -#define BTRFS_FS_STATE_DEV_REPLACING	3
> > -#define BTRFS_FS_STATE_DUMMY_FS_INFO	4
> > +enum {
> > +	/* Global indicator of serious filesysystem errors */
> 
> filesysystem -> filesystem
> 
> > +	BTRFS_FS_STATE_ERROR,
> > +	/*
> > +	 * Filesystem is being remounted, allow to skip some operations, like
> > +	 * defrag
> > +	 */
> > +	BTRFS_FS_STATE_REMOUNTING,
> > +	/* Track if the transaction abort has been reported */
> 
> Which one is "the" transaction abort? This gives me the impression that
> this is a flag on the transaction, but it's actually filesystem state.
> Maybe "Track if a transaction abort has been reported on this
> filesystem"?

Your wording is more clear and I've used it in the patch.

> > +	BTRFS_FS_STATE_TRANS_ABORTED,
> > +	/*
> > +	 * Indicate that replace source or target device state is changed and
> > +	 * allow to block bio operations
> > +	 */
> 
> Again, this makes it sound like it's device state, but it's actually
> filesystem state. How about "Bio operations should be blocked on this
> filesystem because a source or target device is being destroyed as part
> of a device replace"?

Same. Thanks.