Switch defines to enums
David Sterba
maintainercover Switch defines to enums
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(-)
Signature · 1 line
2.19.1
David Sterba
maintainerPATCH 1/9 btrfs: switch BTRFS_FS_STATE_* to enums
We can use simple enum for values that are not part of on-disk format:
global filesystem states.
--- fs/btrfs/ctree.h | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-)
fs/btrfs/ctree.h +19 -6
@@ -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
Signature · 1 line
2.19.1
David Sterba
maintainerPATCH 2/9 btrfs: switch BTRFS_BLOCK_RSV_* to enums
In reply to David SterbaWe can use simple enum for values that are not part of on-disk format:
block reserve types.
--- fs/btrfs/ctree.h | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-)
fs/btrfs/ctree.h +12 -7
@@ -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;
Signature · 1 line
2.19.1
David Sterba
maintainerPATCH 3/9 btrfs: switch BTRFS_FS_* to enums
In reply to David SterbaWe can use simple enum for values that are not part of on-disk format:
internal filesystem states.
--- fs/btrfs/ctree.h | 63 ++++++++++++++++++++++++------------------------ 1 file changed, 31 insertions(+), 32 deletions(-)
fs/btrfs/ctree.h +31 -32
@@ -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 ];
Signature · 1 line
2.19.1
David Sterba
maintainerPATCH 4/9 btrfs: switch BTRFS_ROOT_* to enums
In reply to David SterbaWe can use simple enum for values that are not part of on-disk format:
root tree flags.
--- fs/btrfs/ctree.h | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-)
fs/btrfs/ctree.h +17 -16
@@ -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
Signature · 1 line
2.19.1
David Sterba
maintainerPATCH 5/9 btrfs: swtich EXTENT_BUFFER_* to enums
In reply to David SterbaWe can use simple enum for values that are not part of on-disk format:
extent buffer flags;
--- fs/btrfs/extent_io.h | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-)
fs/btrfs/extent_io.h +16 -12
@@ -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)
Signature · 1 line
2.19.1
David Sterba
maintainerPATCH 6/9 btrfs: switch EXTENT_FLAG_* to enums
In reply to David SterbaWe can use simple enum for values that are not part of on-disk format:
extent map flags.
--- fs/btrfs/extent_map.h | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)
fs/btrfs/extent_map.h +14 -7
@@ -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;
Signature · 1 line
2.19.1
David Sterba
maintainerPATCH 7/9 btrfs: switch BTRFS_*_LOCK to enums
In reply to David SterbaWe can use simple enum for values that are not part of on-disk format:
tree lock types.
--- fs/btrfs/locking.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
fs/btrfs/locking.h +6 -4
@@ -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);
Signature · 1 line
2.19.1
David Sterba
maintainerPATCH 8/9 btrfs: switch BTRFS_ORDERED_* to enums
In reply to David SterbaWe can use simple enum for values that are not part of on-disk format:
ordered extent flags.
--- fs/btrfs/ordered-data.h | 45 +++++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 20 deletions(-)
fs/btrfs/ordered-data.h +25 -20
@@ -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 */
Signature · 1 line
2.19.1
David Sterba
maintainerPATCH 9/9 btrfs: drop extra enum initialization where using defaults
In reply to David SterbaThe 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.
--- 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(-)
fs/btrfs/btrfs_inode.h +1 -1
@@ -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 ,
fs/btrfs/ctree.h +14 -14
@@ -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,
fs/btrfs/disk-io.h +5 -5
@@ -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)
fs/btrfs/qgroup.h +1 -1
@@ -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 ,
fs/btrfs/sysfs.h +1 -1
@@ -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
fs/btrfs/transaction.h +7 -7
@@ -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
Signature · 1 line
2.19.1
Re: [PATCH 1/9] btrfs: switch BTRFS_FS_STATE_* to enums
In reply to David SterbaQuoted 2 lines · David Sterba
We can use simple enum for values that are not part of on-disk format:
global filesystem states.
Some typos/wording suggestions below.
Quoted 13 lines
Signed-off-by David Sterba <[email protected]>--- fs/btrfs/ctree.h | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-)fs/btrfs/ctree.h +0
-0@@ -109,13 +109,26 @@ static inline unsigned long btrfs_chunk_item_size(int num_stripes) }/*
fs/btrfs/ctree.h static inline unsigned long btrfs_chunk_item_size(int num_stripes) · line 115
Quoted 10 lines
fs/btrfs/ctree.h +3
-6-* 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
fs/btrfs/ctree.h static inline unsigned long btrfs_chunk_item_size(int num_stripes) · line 122
Quoted 7 lines
fs/btrfs/ctree.h +7
-0+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"?
fs/btrfs/ctree.h static inline unsigned long btrfs_chunk_item_size(int num_stripes) · line 127
Quoted 5 lines
fs/btrfs/ctree.h +5
-0+ 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"?
Quoted 10 lines
fs/btrfs/ctree.h +4
-0+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 56Signature · 1 line
2.19.1
Re: [PATCH 2/9] btrfs: switch BTRFS_BLOCK_RSV_* to enums
In reply to David SterbaQuoted 2 lines · David Sterba
We can use simple enum for values that are not part of on-disk format:
block reserve types.
Quoted 38 lines
Signed-off-by David Sterba <[email protected]>--- fs/btrfs/ctree.h | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-)fs/btrfs/ctree.h +12
-7@@ -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;Signature · 1 line
2.19.1
Re: [PATCH 3/9] btrfs: switch BTRFS_FS_* to enums
In reply to David SterbaQuoted 2 lines · David Sterba
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 ;)
Quoted 82 lines
Signed-off-by David Sterba <[email protected]>--- fs/btrfs/ctree.h | 63 ++++++++++++++++++++++++------------------------ 1 file changed, 31 insertions(+), 32 deletions(-)fs/btrfs/ctree.h +31
-32@@ -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 ];Signature · 1 line
2.19.1
Re: [PATCH 4/9] btrfs: switch BTRFS_ROOT_* to enums
In reply to David SterbaQuoted 2 lines · David Sterba
We can use simple enum for values that are not part of on-disk format:
root tree flags.
Quoted 52 lines
Signed-off-by David Sterba <[email protected]>--- fs/btrfs/ctree.h | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-)fs/btrfs/ctree.h +17
-16@@ -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 Signature · 1 line
2.19.1
Re: [PATCH 5/9] btrfs: swtich EXTENT_BUFFER_* to enums
In reply to David SterbaQuoted 2 lines · David Sterba
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,
Quoted 47 lines
Signed-off-by David Sterba <[email protected]>--- fs/btrfs/extent_io.h | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-)fs/btrfs/extent_io.h +16
-12@@ -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)Signature · 1 line
2.19.1
Re: [PATCH 6/9] btrfs: switch EXTENT_FLAG_* to enums
In reply to David SterbaQuoted 2 lines · David Sterba
We can use simple enum for values that are not part of on-disk format:
extent map flags.
Quoted 40 lines
Signed-off-by David Sterba <[email protected]>--- fs/btrfs/extent_map.h | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)fs/btrfs/extent_map.h +14
-7@@ -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;Signature · 1 line
2.19.1
Re: [PATCH 7/9] btrfs: switch BTRFS_*_LOCK to enums
In reply to David SterbaQuoted 13 lines · 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 <[email protected]>--- fs/btrfs/locking.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)fs/btrfs/locking.h +0
-0
fs/btrfs/locking.h line 10
Quoted 9 lines
fs/btrfs/locking.h +2
-4@@ -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.
Quoted 10 lines
fs/btrfs/locking.h +4
-0+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);Signature · 1 line
2.19.1
Re: [PATCH 8/9] btrfs: switch BTRFS_ORDERED_* to enums
In reply to David SterbaQuoted 2 lines · David Sterba
We can use simple enum for values that are not part of on-disk format:
ordered extent flags.
Quoted 64 lines
Signed-off-by David Sterba <[email protected]>--- fs/btrfs/ordered-data.h | 45 +++++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 20 deletions(-)fs/btrfs/ordered-data.h +25
-20@@ -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 */ Signature · 1 line
2.19.1
Re: [PATCH 9/9] btrfs: drop extra enum initialization where using defaults
In reply to David SterbaQuoted 3 lines · 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.
Quoted 154 lines
Signed-off-by David Sterba <[email protected]>--- 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(-)fs/btrfs/btrfs_inode.h +1
-1@@ -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 ,fs/btrfs/ctree.h +14
-14@@ -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,fs/btrfs/disk-io.h +5
-5@@ -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)fs/btrfs/qgroup.h +1
-1@@ -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 ,fs/btrfs/sysfs.h +1
-1@@ -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 fs/btrfs/transaction.h +7
-7@@ -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 0Signature · 1 line
2.19.1
Re: [PATCH 1/9] btrfs: switch BTRFS_FS_STATE_* to enums
In reply to David SterbaQuoted 4 lines · 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 <[email protected]>
Good comment.
Thanks,
Qu
Quoted 42 lines
--- fs/btrfs/ctree.h | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-)fs/btrfs/ctree.h +19
-6@@ -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
Re: [PATCH 2/9] btrfs: switch BTRFS_BLOCK_RSV_* to enums
In reply to David SterbaQuoted 4 lines · 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 <[email protected]>
However more comment will always be a good thing.
Thanks,
Qu
Quoted 35 lines
--- fs/btrfs/ctree.h | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-)fs/btrfs/ctree.h +12
-7@@ -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;
Re: [PATCH 3/9] btrfs: switch BTRFS_FS_* to enums
In reply to David SterbaQuoted 4 lines · 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 <[email protected]>
Thanks,
Qu
Quoted 79 lines
--- fs/btrfs/ctree.h | 63 ++++++++++++++++++++++++------------------------ 1 file changed, 31 insertions(+), 32 deletions(-)fs/btrfs/ctree.h +31
-32@@ -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 ];
Re: [PATCH 4/9] btrfs: switch BTRFS_ROOT_* to enums
In reply to David SterbaQuoted 4 lines · 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 <[email protected]>
Thanks,
Qu
Quoted 49 lines
--- fs/btrfs/ctree.h | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-)fs/btrfs/ctree.h +17
-16@@ -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
Re: [PATCH 5/9] btrfs: swtich EXTENT_BUFFER_* to enums
In reply to David SterbaQuoted 4 lines · 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 <[email protected]>
Thanks,
Qu
Quoted 44 lines
--- fs/btrfs/extent_io.h | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-)fs/btrfs/extent_io.h +16
-12@@ -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)
Re: [PATCH 6/9] btrfs: switch EXTENT_FLAG_* to enums
In reply to David SterbaQuoted 4 lines · 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 <[email protected]>
Thanks,
Qu
Quoted 37 lines
--- fs/btrfs/extent_map.h | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)fs/btrfs/extent_map.h +14
-7@@ -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;
Re: [PATCH 7/9] btrfs: switch BTRFS_*_LOCK to enums
In reply to David SterbaQuoted 4 lines · 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 <[email protected]>
Thanks,
Qu
Quoted 26 lines
--- fs/btrfs/locking.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)fs/btrfs/locking.h +6
-4@@ -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);
Re: [PATCH 8/9] btrfs: switch BTRFS_ORDERED_* to enums
In reply to David SterbaQuoted 4 lines · 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 <[email protected]>
Thanks,
Qu
Quoted 61 lines
--- fs/btrfs/ordered-data.h | 45 +++++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 20 deletions(-)fs/btrfs/ordered-data.h +25
-20@@ -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 */
Quoted 8 lines · 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.
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
Quoted 24 lines
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 defaultsfs/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(-)
Re: [PATCH 9/9] btrfs: drop extra enum initialization where using defaults
In reply to David SterbaQuoted 5 lines · 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 <[email protected]>
Thanks,
Qu
Quoted 151 lines
--- 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(-)fs/btrfs/btrfs_inode.h +1
-1@@ -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 ,fs/btrfs/ctree.h +14
-14@@ -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,fs/btrfs/disk-io.h +5
-5@@ -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)fs/btrfs/qgroup.h +1
-1@@ -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 ,fs/btrfs/sysfs.h +1
-1@@ -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 fs/btrfs/transaction.h +7
-7@@ -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
Re: [PATCH 1/9] btrfs: switch BTRFS_FS_STATE_* to enums
In reply to David SterbaLooks good,
Signature · 6 lines
Johannes Thumshirn SUSE Labs [email protected] +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
In reply to David SterbaLooks good,
Signature · 6 lines
Johannes Thumshirn SUSE Labs [email protected] +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
In reply to David SterbaLooks good,
Signature · 6 lines
Johannes Thumshirn SUSE Labs Filesystems [email protected] +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
In reply to David SterbaSignature · 6 lines
Johannes Thumshirn SUSE Labs Filesystems [email protected] +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
In reply to Omar SandovalQuoted 17 lines · Omar Sandoval
Quoted 16 lines · 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 <[email protected]>--- fs/btrfs/locking.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)fs/btrfs/locking.h +0
-0@@ -6,10 +6,12 @@#ifndef BTRFS_LOCKING_H #define BTRFS_LOCKING_H
Quoted 10 lines
fs/btrfs/locking.h line 10
Quoted 6 lines
fs/btrfs/locking.h +2
-4-#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()andbtrfs_release_path(); 0 means no lock, so this needs to beBTRFS_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.
Quoted 10 lines · Qu Wenruo
Quoted 8 lines · 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.
Quoted 5 lines
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
In reply to David SterbaSignature · 6 lines
Johannes Thumshirn SUSE Labs Filesystems [email protected] +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
In reply to David SterbaSignature · 6 lines
Johannes Thumshirn SUSE Labs Filesystems [email protected] +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
In reply to David SterbaSignature · 6 lines
Johannes Thumshirn SUSE Labs Filesystems [email protected] +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
Quoted 13 lines · David Sterba
Quoted 12 lines · Qu Wenruo
Quoted 8 lines · 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.
Just one small question for the bitmap usage.
Quoted 8 lines
Quoted 3 lines
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
Re: [PATCH 1/9] btrfs: switch BTRFS_FS_STATE_* to enums
In reply to Omar SandovalQuoted 34 lines · Omar Sandoval
Quoted 2 lines · David Sterba
We can use simple enum for values that are not part of on-disk format:
global filesystem states.Reviewed-by Omar Sandoval <[email protected]>Some typos/wording suggestions below.
Quoted 13 lines
Signed-off-by David Sterba <[email protected]>--- fs/btrfs/ctree.h | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-)fs/btrfs/ctree.h +0
-0@@ -109,13 +109,26 @@ static inline unsigned long btrfs_chunk_item_size(int num_stripes) }/* fs/btrfs/ctree.h static inline unsigned long btrfs_chunk_item_size(int num_stripes) · line 115
Quoted 10 lines
fs/btrfs/ctree.h +3
-6-* 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
Quoted 12 lines
fs/btrfs/ctree.h static inline unsigned long btrfs_chunk_item_size(int num_stripes) · line 122
Quoted 7 lines
fs/btrfs/ctree.h +7
-0+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.
Quoted 10 lines
fs/btrfs/ctree.h static inline unsigned long btrfs_chunk_item_size(int num_stripes) · line 127
Quoted 5 lines
fs/btrfs/ctree.h +5
-0+ 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.