| Message ID | 2ff62613189d8f58f8da90a1558ad5726172057b.1656079178.git.dsterba@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Cleanup short int types in block group reserve | expand |
On 24.06.22 16:15, David Sterba wrote: > diff --git a/fs/btrfs/block-rsv.h b/fs/btrfs/block-rsv.h > index 0702d4087ff6..bb449c75ee4c 100644 > --- a/fs/btrfs/block-rsv.h > +++ b/fs/btrfs/block-rsv.h > @@ -27,7 +27,8 @@ struct btrfs_block_rsv { > spinlock_t lock; > bool full; > bool failfast; > - unsigned short type; > + /* Block reserve type, one of BTRFS_BLOCK_RSV_* */ > + u8 type; > Is there any reason to not use the enum?
On Mon, Jun 27, 2022 at 06:51:30AM +0000, Johannes Thumshirn wrote: > On 24.06.22 16:15, David Sterba wrote: > > diff --git a/fs/btrfs/block-rsv.h b/fs/btrfs/block-rsv.h > > index 0702d4087ff6..bb449c75ee4c 100644 > > --- a/fs/btrfs/block-rsv.h > > +++ b/fs/btrfs/block-rsv.h > > @@ -27,7 +27,8 @@ struct btrfs_block_rsv { > > spinlock_t lock; > > bool full; > > bool failfast; > > - unsigned short type; > > + /* Block reserve type, one of BTRFS_BLOCK_RSV_* */ > > + u8 type; > > > > Is there any reason to not use the enum? Enum would be 'int', 4 bytes to the space optimization would be lost. Enum types can be shortened as enum btrfs_reserve type:8 but I'm not sure it's an improvement.
On 27.06.22 18:45, David Sterba wrote: > On Mon, Jun 27, 2022 at 06:51:30AM +0000, Johannes Thumshirn wrote: >> On 24.06.22 16:15, David Sterba wrote: >>> diff --git a/fs/btrfs/block-rsv.h b/fs/btrfs/block-rsv.h >>> index 0702d4087ff6..bb449c75ee4c 100644 >>> --- a/fs/btrfs/block-rsv.h >>> +++ b/fs/btrfs/block-rsv.h >>> @@ -27,7 +27,8 @@ struct btrfs_block_rsv { >>> spinlock_t lock; >>> bool full; >>> bool failfast; >>> - unsigned short type; >>> + /* Block reserve type, one of BTRFS_BLOCK_RSV_* */ >>> + u8 type; >>> >> >> Is there any reason to not use the enum? > > Enum would be 'int', 4 bytes to the space optimization would be > lost. Enum types can be shortened as > > enum btrfs_reserve type:8 > > but I'm not sure it's an improvement. > Using an enum would give some type safety (I think -Wenum-compare is on by default in the kernel). Packing that enum would give us the 1 byte size you're looking for.
On Tue, Jun 28, 2022 at 07:15:14AM +0000, Johannes Thumshirn wrote: > On 27.06.22 18:45, David Sterba wrote: > > On Mon, Jun 27, 2022 at 06:51:30AM +0000, Johannes Thumshirn wrote: > >> On 24.06.22 16:15, David Sterba wrote: > >>> diff --git a/fs/btrfs/block-rsv.h b/fs/btrfs/block-rsv.h > >>> index 0702d4087ff6..bb449c75ee4c 100644 > >>> --- a/fs/btrfs/block-rsv.h > >>> +++ b/fs/btrfs/block-rsv.h > >>> @@ -27,7 +27,8 @@ struct btrfs_block_rsv { > >>> spinlock_t lock; > >>> bool full; > >>> bool failfast; > >>> - unsigned short type; > >>> + /* Block reserve type, one of BTRFS_BLOCK_RSV_* */ > >>> + u8 type; > >>> > >> > >> Is there any reason to not use the enum? > > > > Enum would be 'int', 4 bytes to the space optimization would be > > lost. Enum types can be shortened as > > > > enum btrfs_reserve type:8 > > > > but I'm not sure it's an improvement. > > > > Using an enum would give some type safety (I think -Wenum-compare is > on by default in the kernel). Packing that enum would give us the 1 byte > size you're looking for. Yeah I'll go with the named enum and :8 in the structure.
diff --git a/fs/btrfs/block-rsv.c b/fs/btrfs/block-rsv.c index 26c43a6ef5d2..5384c6ae8fe8 100644 --- a/fs/btrfs/block-rsv.c +++ b/fs/btrfs/block-rsv.c @@ -171,7 +171,7 @@ int btrfs_block_rsv_migrate(struct btrfs_block_rsv *src, return 0; } -void btrfs_init_block_rsv(struct btrfs_block_rsv *rsv, unsigned short type) +void btrfs_init_block_rsv(struct btrfs_block_rsv *rsv, u8 type) { memset(rsv, 0, sizeof(*rsv)); spin_lock_init(&rsv->lock); @@ -179,8 +179,7 @@ void btrfs_init_block_rsv(struct btrfs_block_rsv *rsv, unsigned short type) } void btrfs_init_metadata_block_rsv(struct btrfs_fs_info *fs_info, - struct btrfs_block_rsv *rsv, - unsigned short type) + struct btrfs_block_rsv *rsv, u8 type) { btrfs_init_block_rsv(rsv, type); rsv->space_info = btrfs_find_space_info(fs_info, @@ -188,7 +187,7 @@ void btrfs_init_metadata_block_rsv(struct btrfs_fs_info *fs_info, } struct btrfs_block_rsv *btrfs_alloc_block_rsv(struct btrfs_fs_info *fs_info, - unsigned short type) + u8 type) { struct btrfs_block_rsv *block_rsv; diff --git a/fs/btrfs/block-rsv.h b/fs/btrfs/block-rsv.h index 0702d4087ff6..bb449c75ee4c 100644 --- a/fs/btrfs/block-rsv.h +++ b/fs/btrfs/block-rsv.h @@ -27,7 +27,8 @@ struct btrfs_block_rsv { spinlock_t lock; bool full; bool failfast; - unsigned short type; + /* Block reserve type, one of BTRFS_BLOCK_RSV_* */ + u8 type; /* * Qgroup equivalent for @size @reserved @@ -49,13 +50,12 @@ struct btrfs_block_rsv { u64 qgroup_rsv_reserved; }; -void btrfs_init_block_rsv(struct btrfs_block_rsv *rsv, unsigned short type); +void btrfs_init_block_rsv(struct btrfs_block_rsv *rsv, u8 type); void btrfs_init_root_block_rsv(struct btrfs_root *root); struct btrfs_block_rsv *btrfs_alloc_block_rsv(struct btrfs_fs_info *fs_info, - unsigned short type); + u8 type); void btrfs_init_metadata_block_rsv(struct btrfs_fs_info *fs_info, - struct btrfs_block_rsv *rsv, - unsigned short type); + struct btrfs_block_rsv *rsv, u8 type); void btrfs_free_block_rsv(struct btrfs_fs_info *fs_info, struct btrfs_block_rsv *rsv); int btrfs_block_rsv_add(struct btrfs_fs_info *fs_info,
The number of block group reserve types BTRFS_BLOCK_RSV_* is small and fits to u8 and there's enough left in case we want to add more. The structure size is now 48 on release build, making a slight improvement in structures where it's embedded, like btrfs_fs_info or btrfs_inode. Signed-off-by: David Sterba <dsterba@suse.com> --- fs/btrfs/block-rsv.c | 7 +++---- fs/btrfs/block-rsv.h | 10 +++++----- 2 files changed, 8 insertions(+), 9 deletions(-)