Re: [PATCH] uidgid: make sure we fit into one cacheline

[Jeff Layton <jlayton@kernel.org>]

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@kernel.org>