Re: [PATCH] btrfs: reorder extent buffer members for better packing

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

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



On 2020/11/4 上午5:11, David Sterba wrote:
> After the rwsem replaced the tree lock implementation, the extent buffer
> got smaller but leaving some holes behind. By changing log_index type
> and reordering, we can squeeze the size further to 240 bytes, measured on
> release config on x86_64. Log_index spans only 3 values and needs to be
> signed.
> 
> Before:
> 
> struct extent_buffer {
>         u64                        start;                /*     0     8 */
>         long unsigned int          len;                  /*     8     8 */
>         long unsigned int          bflags;               /*    16     8 */
>         struct btrfs_fs_info *     fs_info;              /*    24     8 */
>         spinlock_t                 refs_lock;            /*    32     4 */
>         atomic_t                   refs;                 /*    36     4 */
>         atomic_t                   io_pages;             /*    40     4 */
>         int                        read_mirror;          /*    44     4 */
>         struct callback_head       callback_head __attribute__((__aligned__(8))); /*    48    16 */
>         /* --- cacheline 1 boundary (64 bytes) --- */
>         pid_t                      lock_owner;           /*    64     4 */
>         bool                       lock_recursed;        /*    68     1 */
> 
>         /* XXX 3 bytes hole, try to pack */
> 
>         struct rw_semaphore        lock;                 /*    72    40 */

An off-topic question, for things like aotmic_t/spinlock_t and
rw_semaphore, wouldn't various DEBUG options change their size?

Do we need to consider such case, by moving them to the end of the
structure, or we only consider production build for pa_hole?

Thanks,
Qu

>         short int                  log_index;            /*   112     2 */
> 
>         /* XXX 6 bytes hole, try to pack */
> 
>         struct page *              pages[16];            /*   120   128 */
> 
>         /* size: 248, cachelines: 4, members: 14 */
>         /* sum members: 239, holes: 2, sum holes: 9 */
>         /* forced alignments: 1 */
>         /* last cacheline: 56 bytes */
> } __attribute__((__aligned__(8)));
> 
> After:
> 
> struct extent_buffer {
>         u64                        start;                /*     0     8 */
>         long unsigned int          len;                  /*     8     8 */
>         long unsigned int          bflags;               /*    16     8 */
>         struct btrfs_fs_info *     fs_info;              /*    24     8 */
>         spinlock_t                 refs_lock;            /*    32     4 */
>         atomic_t                   refs;                 /*    36     4 */
>         atomic_t                   io_pages;             /*    40     4 */
>         int                        read_mirror;          /*    44     4 */
>         struct callback_head       callback_head __attribute__((__aligned__(8))); /*    48    16 */
>         /* --- cacheline 1 boundary (64 bytes) --- */
>         pid_t                      lock_owner;           /*    64     4 */
>         bool                       lock_recursed;        /*    68     1 */
>         s8                         log_index;            /*    69     1 */
> 
>         /* XXX 2 bytes hole, try to pack */
> 
>         struct rw_semaphore        lock;                 /*    72    40 */
>         struct page *              pages[16];            /*   112   128 */
> 
>         /* size: 240, cachelines: 4, members: 14 */
>         /* sum members: 238, holes: 1, sum holes: 2 */
>         /* forced alignments: 1 */
>         /* last cacheline: 48 bytes */
> } __attribute__((__aligned__(8)));
> 
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>  fs/btrfs/extent_io.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
> index 5403354de0e1..3c2bf21c54eb 100644
> --- a/fs/btrfs/extent_io.h
> +++ b/fs/btrfs/extent_io.h
> @@ -88,10 +88,10 @@ struct extent_buffer {
>  	struct rcu_head rcu_head;
>  	pid_t lock_owner;
>  	bool lock_recursed;
> -	struct rw_semaphore lock;
> -
>  	/* >= 0 if eb belongs to a log tree, -1 otherwise */
> -	short log_index;
> +	s8 log_index;
> +
> +	struct rw_semaphore lock;
>  
>  	struct page *pages[INLINE_EXTENT_BUFFER_PAGES];
>  #ifdef CONFIG_BTRFS_DEBUG
> 


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