On Tue, 2024-09-10 at 18:16 +0100, Matthew Wilcox wrote:
> On Tue, Sep 10, 2024 at 09:51:37AM -0400, Jeff Layton wrote:
> > > Before:
> > >
> > > struct uid_gid_map {
> > > u32 nr_extents; /* 0 4 */
> > >
> > > /* XXX 4 bytes hole, try to pack */
> > >
> > > union {
> > > struct uid_gid_extent extent[5]; /* 8 60 */
> > > struct {
> > > struct uid_gid_extent * forward; /* 8 8 */
> > > struct uid_gid_extent * reverse; /* 16 8 */
> > > }; /* 8 16 */
> > > }; /* 8 64 */
> > >
> > > /* size: 72, cachelines: 2, members: 2 */
> > > /* sum members: 68, holes: 1, sum holes: 4 */
> > > /* last cacheline: 8 bytes */
> > > };
> > >
> > > After:
> > >
> > > struct uid_gid_map {
> > > union {
> > > struct {
> > > struct uid_gid_extent extent[5]; /* 0 60 */
> > > u32 nr_extents; /* 60 4 */
> > > }; /* 0 64 */
> > > struct {
> > > struct uid_gid_extent * forward; /* 0 8 */
> > > struct uid_gid_extent * reverse; /* 8 8 */
> > > }; /* 0 16 */
> > > }; /* 0 64 */
> > >
> > > /* size: 64, cachelines: 1, members: 1 */
> > > };
> >
> > Is this any different from just moving nr_extents to the end of
> > struct_uid_gid_map? I don't quite get how moving it into the union
> > improves things.
>
> It's an alignment question. Look more carefully at the pahole output.
> The array of uid_gid_extent is 4-byte aligned and 60 bytes in size,
> but the two pointers must be eight bytes aligned. That forces the
> compiler to make the whole union 8-byte aligned. If the nr_extents is
> within the union, then it can pack with the array of extents. If not,
> it has to leave a 4-byte gap.
Thanks, that makes sense. With that clarified:
Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>
