On Tue, Sep 10, 2024 at 10:16:39AM +0200, Christian Brauner wrote:
> When I expanded uidgid mappings I intended for a struct uid_gid_map to
> fit into a single cacheline on x86 as they tend to be pretty
> performance sensitive (idmapped mounts etc). But a 4 byte hole was added
> that brought it over 64 bytes. Fix that by adding the static extent
> array and the extent counter into a substruct. C's type punning for
> unions guarantees that we can access ->nr_extents even if the last
> written to member wasn't within the same object. This is also what we
> rely on in struct_group() and friends. This of course relies on
> non-strict aliasing which we don't do.
>
> 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 */
> };
May I suggest adding a compile-time assert on the size? While it may be
growing it will be unavoidable at some point, it at least wont happen
unintentionally.
This is not the first time something unintentionally passes a threshold
of the sort and I would argue someone(tm) should do a sweep.
