[kbuild-all] Re: Bogus struct page layout on 32-bit

On Sat, Apr 10, 2021 at 06:45:35AM +0800, kernel test robot wrote: > >> include/linux/mm_types.h:274:1: error: static_assert failed due to requirement '__builtin_offsetof(struct page, lru) == __builtin_offsetof(struct folio, lru)' "offsetof(struct page, lru) == offsetof(struct folio, lru)" > FOLIO_MATCH(lru, lru); > include/linux/mm_types.h:272:2: note: expanded from macro 'FOLIO_MATCH' > static_assert(offsetof(struct page, pg) == offsetof(struct folio, fl)) Well, this is interesting. pahole reports: struct page { long unsigned int flags; /* 0 4 */ /* XXX 4 bytes hole, try to pack */ union { struct { struct list_head lru; /* 8 8 */ ... struct folio { union { struct { long unsigned int flags; /* 0 4 */ struct list_head lru; /* 4 8 */ so this assert has absolutely done its job. But why has this assert triggered? Why is struct page layout not what we thought it was? Turns out it's the dma_addr added in 2019 by commit c25fff7171be ("mm: add dma_addr_t to struct page"). On this particular config, it's 64-bit, and ppc32 requires alignment to 64-bit. So the whole union gets moved out by 4 bytes.

Unfortunately, we can't just fix this by putting an 'unsigned long pad' in front of it. It still aligns the entire union to 8 bytes, and then it skips another 4 bytes after the pad. We can fix it like this ... +++ b/include/linux/mm_types.h @@ -96,11 +96,12 @@ struct page { unsigned long private; }; struct { /* page_pool used by netstack */ + unsigned long _page_pool_pad;

/** * @dma_addr: might require a 64-bit value even on * 32-bit architectures. */ - dma_addr_t dma_addr; + dma_addr_t dma_addr __packed; }; struct { /* slab, slob and slub */ union { but I don't know if GCC is smart enough to realise that dma_addr is now on an 8 byte boundary and it can use a normal instruction to access it, or whether it'll do something daft like use byte loads to access it. We could also do: + dma_addr_t dma_addr __packed __aligned(sizeof(void *)); and I see pahole, at least sees this correctly: struct { long unsigned int _page_pool_pad; /* 4 4 */ dma_addr_t dma_addr __attribute__((__aligned__(4))); /* 8 8 */ } __attribute__((__packed__)) __attribute__((__aligned__(4))); This presumably affects any 32-bit architecture with a 64-bit phys_addr_t / dma_addr_t. Advice, please?