Unintentional 24 byte hole in mm_struct reducing objs_per_slab

Unintentional 24 byte hole in mm_struct reducing objs_per_slab

[Mateusz Guzik]

The size is avoidably too big and I wrote a hack to demonstrate it
does not have to be.

stock:
# cat /sys/kernel/slab/mm_struct/slab_size
1408
# cat /sys/kernel/slab/mm_struct/objs_per_slab
23

patched:
# cat /sys/kernel/slab/mm_struct/slab_size
1344
# cat /sys/kernel/slab/mm_struct/objs_per_slab
24

At the beginning of the struct there is a field annotated with
____cacheline_aligned_in_smp (aka 64 on x86-64).

Apart from making free space up to the next field, this has a side
effect of forcing the size of the struct to be a multiple of 64. While
normally expected, it is actively harmful for mm_struct.

Allocations come from a dedicated slab:
        mm_size = sizeof(struct mm_struct) + cpumask_size() + mm_cid_size();

        mm_cachep = kmem_cache_create_usercopy("mm_struct",
                        mm_size, ARCH_MIN_MMSTRUCT_ALIGN,
                        SLAB_HWCACHE_ALIGN|SLAB_PANIC|SLAB_ACCOUNT,
                        offsetof(struct mm_struct, saved_auxv),
                        sizeof_field(struct mm_struct, saved_auxv),
                        NULL);

As in, the actual allocated size is bigger then sizeof(struct
mm_struct) and SLAB_HWCACHE_ALIGN already provides the expected round
up.

Instead, the following hack:

diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 36c5b43999e6..33de9495a8d3 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -685,7 +685,8 @@ struct mm_struct {
                         * 0, the &struct mm_struct is freed.
                         */
                        atomic_t mm_count;
-               } ____cacheline_aligned_in_smp;
+               };
+               char __padbuf[SMP_CACHE_BYTES - __alignof__(atomic_t)];

                struct maple_tree mm_mt;
 #ifdef CONFIG_MMU

.. provides the necessary padding without inducing the sizeof problem.

Now the patch looks like crap and a cheap shot failed to produce a way
to do it I would be happy with. Additionally I have epsilon interest
in arguing how to do it nicely.

That is to say I'm reporting this for an interested party to fix in a
whatever (non)hacky way they see fit.

ptype /o struct mm_struct :

stock:
/* offset      |    size */  type = struct mm_struct {
/*      0      |    1344 */    struct {
/*      0      |      64 */        struct {
/*      0      |       4 */            atomic_t mm_count;
/* XXX 60-byte padding   */

                                       /* total size (bytes):   64 */
                                   };
/*     64      |      16 */        struct maple_tree {
[snip]
/*   1288      |      32 */        struct {
/*   1288      |      16 */            struct list_head {
/*   1288      |       8 */                struct list_head *next;
/*   1296      |       8 */                struct list_head *prev;

                                           /* total size (bytes):   16 */
                                       } list;
/*   1304      |       8 */            unsigned long bitmap;
/*   1312      |       8 */            struct mem_cgroup *memcg;

                                       /* total size (bytes):   32 */
                                   } lru_gen;
/* XXX 24-byte padding   */

                                   /* total size (bytes): 1344 */
                               };
/*   1344      |       0 */    unsigned long cpu_bitmap[];

                               /* total size (bytes): 1344 */
                             }

patched:
/* offset      |    size */  type = struct mm_struct {
/*      0      |    1320 */    struct {
/*      0      |       4 */        struct {
/*      0      |       4 */            atomic_t mm_count;

                                       /* total size (bytes):    4 */
                                   };
/*      4      |      60 */        char __padbuf[60];
/*     64      |      16 */        struct maple_tree {
[snip]
/*   1288      |      32 */        struct {
/*   1288      |      16 */            struct list_head {
/*   1288      |       8 */                struct list_head *next;
/*   1296      |       8 */                struct list_head *prev;

                                           /* total size (bytes):   16 */
                                       } list;
/*   1304      |       8 */            unsigned long bitmap;
/*   1312      |       8 */            struct mem_cgroup *memcg;

                                       /* total size (bytes):   32 */
                                   } lru_gen;

                                   /* total size (bytes): 1320 */
                               };
/*   1320      |       0 */    unsigned long cpu_bitmap[];

                               /* total size (bytes): 1320 */
                             }


Cheers,
-- 
Mateusz Guzik <mjguzik gmail.com>