Re: [PATCH 2/4] btrfs: reorder members in btrfs_delayed_root for better packing

[David Sterba <dsterba@suse.cz>]

On Sat, Jan 10, 2026 at 07:46:11AM +1030, Qu Wenruo wrote:
> 
> 
> 在 2026/1/10 03:47, David Sterba 写道:
> > There are two unnecessary 4B holes in btrfs_delayed_root;
> > 
> > struct btrfs_delayed_root {
> >          spinlock_t                 lock;                 /*     0     4 */
> > 
> >          /* XXX 4 bytes hole, try to pack */
> > 
> >          struct list_head           node_list;            /*     8    16 */
> >          struct list_head           prepare_list;         /*    24    16 */
> >          atomic_t                   items;                /*    40     4 */
> >          atomic_t                   items_seq;            /*    44     4 */
> >          int                        nodes;                /*    48     4 */
> > 
> >          /* XXX 4 bytes hole, try to pack */
> > 
> >          wait_queue_head_t          wait;                 /*    56    24 */
> > 
> >          /* size: 80, cachelines: 2, members: 7 */
> >          /* sum members: 72, holes: 2, sum holes: 8 */
> >          /* last cacheline: 16 bytes */
> > };
> > 
> > Reordering 'nodes' after 'lock' reduces size by 8B, to 72 on release
> > config.
> 
> Not a huge thing, but can we put members in descend order of their sizes 
> if they are properly aligned.

We can do that but I'm not sure about the effects. Once there are no
holes left the automatic alignment is correct with respect to the types.
The structure is compact and the next thing to consider is cacheline
grouping, so we can decouple locks, atomics or implied locks in other
structures like the wait_queue_head or semaphores.

You suggest to reorder wait from 2nd cacheline to the first, so now the
atomics are in the 2nd. This groups wait + lock, and the items +
items_seq. I can't tell just form that which one is better, this depends
on access patterns and code would need to be analyzed. In many cases
this is not important because the access is not so parallel, so eg. one
access per function is hardly measurable. And we don't care in general,
so in this case I don't think either way is better or worse.

> For this particular case, wait_queue_heat_t itself isn't properly 
> power-of-2 sized, but with 2 32bits member, it can be shrink even futher:

The final size after my patch is the same, 72 bytes, what I posted was
the structure before reordering.

> struct btrfs_delayed_root {
>          wait_queue_head_t          wait;                 /*     0    24 */
>          int                        nodes;                /*    24     4 */
>          spinlock_t                 lock;                 /*    28     4 */
>          struct list_head           node_list;            /*    32    16 */
>          struct list_head           prepare_list;         /*    48    16 */
>          /* --- cacheline 1 boundary (64 bytes) --- */
>          atomic_t                   items;                /*    64     4 */
>          atomic_t                   items_seq;            /*    68     4 */
> 
>          /* size: 72, cachelines: 2, members: 7 */
>          /* last cacheline: 8 bytes */
> };