Re: [PATCH v2 5/7] btrfs: reorder btrfs_block_group members to reduce struct size

[Sun Yangkai <sunk67188@gmail.com>]

在 2026/1/5 23:07, Filipe Manana 写道:
> On Sat, Jan 3, 2026 at 1:06 PM Sun YangKai <sunk67188@gmail.com> wrote:
>>
>> Reorder struct btrfs_block_group fields to improve packing and reduce
>> memory footprint from 624 to 600 bytes (24 bytes saved per instance).
> 
> The memory footprint is not reduced.
> 
> The structure's size is reduced, yes, but we are allocating block
> groups using kzalloc, so we end up still using the kmalloc-1k slab.
> Unless we could reduce the structure's size to 512 bytes, and
> therefore use the kmalloc-512 slab, we are not saving any memory.

Thank you for your review opinions.

Currently we have a size at ~600 bytes either with or without this patch and I'm
wondering if we can use kmem_cache for btrfs_block_group. I'm not sure but it
should also make it easier to trace the allocation.

> The number of cache lines also remains the same, so no improvements there.
> Changing the order of the fields could also have an impact in the
> caching (either for the best or the worst), but I don't think it will
> be significantly visible for any realistic workload.

I didn't take this into consideration because the fields of btrfs_block_group
seems not optimized for caching but thanks a lot for telling me this :)

Thanks.

> Thanks.
> 
> 
>>
>> Here's pahole output after this change:
>>
>> struct btrfs_block_group {
>>         struct btrfs_fs_info *     fs_info;              /*     0     8 */
>>         struct btrfs_inode *       inode;                /*     8     8 */
>>         u64                        start;                /*    16     8 */
>>         u64                        length;               /*    24     8 */
>>         u64                        pinned;               /*    32     8 */
>>         u64                        reserved;             /*    40     8 */
>>         u64                        used;                 /*    48     8 */
>>         u64                        delalloc_bytes;       /*    56     8 */
>>         /* --- cacheline 1 boundary (64 bytes) --- */
>>         u64                        bytes_super;          /*    64     8 */
>>         u64                        flags;                /*    72     8 */
>>         u64                        cache_generation;     /*    80     8 */
>>         u64                        global_root_id;       /*    88     8 */
>>         u64                        commit_used;          /*    96     8 */
>>         u32                        bitmap_high_thresh;   /*   104     4 */
>>         u32                        bitmap_low_thresh;    /*   108     4 */
>>         struct rw_semaphore        data_rwsem;           /*   112    40 */
>>         /* --- cacheline 2 boundary (128 bytes) was 24 bytes ago --- */
>>         unsigned long              full_stripe_len;      /*   152     8 */
>>         unsigned long              runtime_flags;        /*   160     8 */
>>         spinlock_t                 lock;                 /*   168     4 */
>>         unsigned int               ro;                   /*   172     4 */
>>         enum btrfs_disk_cache_state disk_cache_state;    /*   176     4 */
>>         enum btrfs_caching_type    cached;               /*   180     4 */
>>         struct btrfs_caching_control * caching_ctl;      /*   184     8 */
>>         /* --- cacheline 3 boundary (192 bytes) --- */
>>         struct btrfs_space_info *  space_info;           /*   192     8 */
>>         struct btrfs_free_space_ctl * free_space_ctl;    /*   200     8 */
>>         struct rb_node             cache_node;           /*   208    24 */
>>         struct list_head           list;                 /*   232    16 */
>>         struct list_head           cluster_list;         /*   248    16 */
>>         /* --- cacheline 4 boundary (256 bytes) was 8 bytes ago --- */
>>         struct list_head           bg_list;              /*   264    16 */
>>         struct list_head           ro_list;              /*   280    16 */
>>         refcount_t                 refs;                 /*   296     4 */
>>         atomic_t                   frozen;               /*   300     4 */
>>         struct list_head           discard_list;         /*   304    16 */
>>         /* --- cacheline 5 boundary (320 bytes) --- */
>>         enum btrfs_discard_state   discard_state;        /*   320     4 */
>>         int                        discard_index;        /*   324     4 */
>>         u64                        discard_eligible_time; /*   328     8 */
>>         u64                        discard_cursor;       /*   336     8 */
>>         struct list_head           dirty_list;           /*   344    16 */
>>         struct list_head           io_list;              /*   360    16 */
>>         struct btrfs_io_ctl        io_ctl;               /*   376    72 */
>>         /* --- cacheline 7 boundary (448 bytes) --- */
>>         atomic_t                   reservations;         /*   448     4 */
>>         atomic_t                   nocow_writers;        /*   452     4 */
>>         struct mutex               free_space_lock;      /*   456    32 */
>>         bool                       using_free_space_bitmaps; /*   488     1 */
>>         bool                       using_free_space_bitmaps_cached; /*   489     1 */
>>         bool                       reclaim_mark;         /*   490     1 */
>>
>>         /* XXX 1 byte hole, try to pack */
>>
>>         int                        swap_extents;         /*   492     4 */
>>         u64                        alloc_offset;         /*   496     8 */
>>         u64                        zone_unusable;        /*   504     8 */
>>         /* --- cacheline 8 boundary (512 bytes) --- */
>>         u64                        zone_capacity;        /*   512     8 */
>>         u64                        meta_write_pointer;   /*   520     8 */
>>         struct btrfs_chunk_map *   physical_map;         /*   528     8 */
>>         struct list_head           active_bg_list;       /*   536    16 */
>>         struct work_struct         zone_finish_work;     /*   552    32 */
>>         /* --- cacheline 9 boundary (576 bytes) was 8 bytes ago --- */
>>         struct extent_buffer *     last_eb;              /*   584     8 */
>>         enum btrfs_block_group_size_class size_class;    /*   592     4 */
>>
>>         /* size: 600, cachelines: 10, members: 56 */
>>         /* sum members: 595, holes: 1, sum holes: 1 */
>>         /* padding: 4 */
>>         /* last cacheline: 24 bytes */
>> };
>>
>> Signed-off-by: Sun YangKai <sunk67188@gmail.com>
>> ---
>>  fs/btrfs/block-group.h | 9 +++++----
>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/btrfs/block-group.h b/fs/btrfs/block-group.h
>> index 3b3c61b3af2c..88c2e3a0a5a0 100644
>> --- a/fs/btrfs/block-group.h
>> +++ b/fs/btrfs/block-group.h
>> @@ -118,7 +118,6 @@ struct btrfs_caching_control {
>>  struct btrfs_block_group {
>>         struct btrfs_fs_info *fs_info;
>>         struct btrfs_inode *inode;
>> -       spinlock_t lock;
>>         u64 start;
>>         u64 length;
>>         u64 pinned;
>> @@ -159,6 +158,8 @@ struct btrfs_block_group {
>>         unsigned long full_stripe_len;
>>         unsigned long runtime_flags;
>>
>> +       spinlock_t lock;
>> +
>>         unsigned int ro;
>>
>>         int disk_cache_state;
>> @@ -178,8 +179,6 @@ struct btrfs_block_group {
>>         /* For block groups in the same raid type */
>>         struct list_head list;
>>
>> -       refcount_t refs;
>> -
>>         /*
>>          * List of struct btrfs_free_clusters for this block group.
>>          * Today it will only have one thing on it, but that may change
>> @@ -199,6 +198,8 @@ struct btrfs_block_group {
>>         /* For read-only block groups */
>>         struct list_head ro_list;
>>
>> +       refcount_t refs;
>> +
>>         /*
>>          * When non-zero it means the block group's logical address and its
>>          * device extents can not be reused for future block group allocations
>> @@ -211,10 +212,10 @@ struct btrfs_block_group {
>>
>>         /* For discard operations */
>>         struct list_head discard_list;
>> +       enum btrfs_discard_state discard_state;
>>         int discard_index;
>>         u64 discard_eligible_time;
>>         u64 discard_cursor;
>> -       enum btrfs_discard_state discard_state;
>>
>>         /* For dirty block groups */
>>         struct list_head dirty_list;
>> --
>> 2.51.2
>>
>>