| Message ID | cover.1543348078.git.dsterba@suse.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Switch defines to enums | expand |
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(-) >
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.
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