Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753222AbcCENuS (ORCPT ); Sat, 5 Mar 2016 08:50:18 -0500 Received: from mail-wm0-f65.google.com ([74.125.82.65]:35411 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751548AbcCENuL (ORCPT ); Sat, 5 Mar 2016 08:50:11 -0500 Date: Sat, 5 Mar 2016 14:50:06 +0100 From: Ingo Molnar To: Linus Torvalds Cc: Linux Kernel Mailing List , Thomas Gleixner , Dave Hansen , Peter Anvin , Dave Hansen , Andrew Morton , Peter Zijlstra , Helge Deller , Stephen Rothwell , "linux-tip-commits@vger.kernel.org" Subject: Re: [tip:mm/pkeys] mm/pkeys: Fix siginfo ABI breakage caused by new u64 field Message-ID: <20160305135006.GA15928@gmail.com> References: <20160301125451.02C7426D@viggo.jf.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3735 Lines: 88 * Linus Torvalds wrote: > On Thu, Mar 3, 2016 at 8:53 AM, tip-bot for Dave Hansen > wrote: > > > > If u64 has a natural alignment of 8 bytes (this is rare, most 32-bit > > platforms align it to 4 bytes), then the leadup to the _sifields union > > matters: > > Side note: I'm not sure that "this is rare" comment is necessarily correct. > > I think natural alignment is pretty common, even for 32-bit targets. > x86-32 is I think the exception rather than the rule. > > There is some real odd case iirc - embedded m68k, which has some > ridiculous alignment rules. I think it only ever aligns to 16-bit > boundaries. So I got curious about this, but couldn't find any good online documentation about the alignment defaults of various architectures that GCC supports. So I reverted the fix and added the new check from linux-next: Revert "mm/pkeys: Fix siginfo ABI breakage caused by new u64 field" kernel/signal.c: add compile-time check for __ARCH_SI_PREAMBLE_SIZE ... which does: void __init signals_init(void) { + /* If this check fails, the __ARCH_SI_PREAMBLE_SIZE value is wrong! */ + BUILD_BUG_ON(__ARCH_SI_PREAMBLE_SIZE + != offsetof(struct siginfo, _sifields._pad)); + and tested it on the -tip cross-arch build testing suite, which gave the following result (only 32-bit architectures listed): (warns) (warns) testing x86-32: -git: pass ( 0), -tip: pass ( 0) testing arm: -git: pass ( 1), -tip: FAIL ..... testing blackfin: -git: pass ( 0), -tip: pass ( 0) testing cris: -git: pass ( 32), -tip: pass ( 32) testing frv: -git: pass ( 1), -tip: FAIL ..... testing m32r: -git: pass ( 6), -tip: pass ( 6) testing m68k: -git: pass ( 1), -tip: pass ( 1) testing microblaze: -git: pass ( 0), -tip: pass ( 0) testing mips: -git: pass ( 1), -tip: FAIL ..... testing openrisc: -git: pass ( 2), -tip: pass ( 2) testing parisc: -git: pass ( 0), -tip: FAIL ..... testing sh: -git: pass ( 36), -tip: pass ( 36) testing sparc: -git: pass ( 0), -tip: FAIL ..... testing tile: -git: pass ( 5), -tip: pass ( 5) testing xtensa: -git: pass ( 0), -tip: FAIL ..... testing powerpc32: -git: pass ( 0), -tip: FAIL ..... so if my test is correct then it's 9 architectures that align u64 to 4 bytes, vs. 7 that align it to 8 bytes. So naturally aligned u64 is definitely not 'rare' (so the characterisation in my changelog is wrong), but it's not dominant either. FWIIW: if we only list 'major' architectures then x86-32 is indeed the odd one out... > I do keep coming back to the fact that we should *probably* just do > something like > > typedef unsigned long long __attribute__((aligned(8))) __u64; > > and then introduce a separate "u64_unaligned" type for all the legacy > cases that depended on 32-bit alignment. > > It's horrendously nasty to test, though. So in theory we could test most of it by comparing the disassembly of allyesconfig builds, but comparing disassemblies is a pretty hard to use method in practice. A more workable method would be to have a test .c file that includes all UAPI structures in existence and defines a variable out of every single one, and then generates a list of sizeof() values or so. But even that isn't perfect: a structure might shift some fields forward, into a pre-existing hole, without changing the sizeof? We'd need a list of all field offsets in all structures to be really sure, and that's nasty. Thanks, Ingo