On 04/13/2016 05:36 AM, Josh Poimboeuf wrote:
> On Thu, Feb 04, 2016 at 08:45:35PM +0100, Denys Vlasenko wrote:
>> Sometimes gcc mysteriously doesn't inline
>> very small functions we expect to be inlined. See
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66122
>>
>> With this .config:
>> http://busybox.net/~vda/kernel_config_OPTIMIZE_INLINING_and_Os,
>> the following functions get deinlined many times.
>> Examples of disassembly:
>>
>> <get_unaligned_be16> (12 copies, 51 calls):
>> 66 8b 07 mov (%rdi),%ax
>> 55 push %rbp
>> 48 89 e5 mov %rsp,%rbp
>> 86 e0 xchg %ah,%al
>> 5d pop %rbp
>> c3 retq
...
>> This patch fixes this via s/inline/__always_inline/.
>> Code size decrease after the patch is ~4.5k:
>>
>> text data bss dec hex filename
>> 92202377 20826112 36417536 149446025 8e85d89 vmlinux
>> 92197848 20826112 36417536 149441496 8e84bd8 vmlinux5_swap_after
>>
>> Signed-off-by: Denys Vlasenko <dvlas...@redhat.com>
>> Cc: Ingo Molnar <mi...@kernel.org>
>> Cc: Thomas Graf <tg...@suug.ch>
>> Cc: Peter Zijlstra <pet...@infradead.org>
>> Cc: David Rientjes <rient...@google.com>
>> Cc: Andrew Morton <a...@linux-foundation.org>
>> Cc: linux-kernel@vger.kernel.org
>> ---
>> include/uapi/linux/byteorder/big_endian.h | 24 ++++++++++++------------
>> include/uapi/linux/byteorder/little_endian.h | 24 ++++++++++++------------
>> include/uapi/linux/swab.h | 10 +++++-----
>> 3 files changed, 29 insertions(+), 29 deletions(-)
>
> Hi,
>
> This patch seems to trigger a gcc bug which can produce corrupt code. I
> discovered it when investigating an objtool warning reported by kbuild
> bot:
>
> http://www.spinics.net/lists/linux-scsi/msg95481.html
>
> From the disassembly of drivers/scsi/qla2xxx/qla_attr.o:
>
> 0000000000002f53 <qla2x00_get_host_fabric_name>:
> 2f53: 55 push %rbp
> 2f54: 48 89 e5 mov %rsp,%rbp
>
> 0000000000002f57 <qla2x00_get_fc_host_stats>:
> 2f57: 55 push %rbp
> 2f58: b9 e8 00 00 00 mov $0xe8,%ecx
> 2f5d: 48 89 e5 mov %rsp,%rbp
> ...
>
> Note that qla2x00_get_host_fabric_name() is inexplicably truncated after
> setting up the frame pointer. It falls through to the next function, which is
> very wrong.
Wow, that's ... interesting.
> I can recreate it with either gcc 5.3.1 or gcc 6.0 on linus/master with
> the .config from the above link.
>
> The call chain which appears to trigger the problem is:
>
> qla2x00_get_host_fabric_name()
> wwn_to_u64()
> get_unaligned_be64()
> be64_to_cpup()
> __be64_to_cpup() <- changed to __always_inline by this patch
>
> It occurs with the combination of the following two recent commits:
>
> - bc27fb68aaad ("include/uapi/linux/byteorder, swab: force inlining of some
> byteswap operations")
> - ef3fb2422ffe ("scsi: fc: use get/put_unaligned64 for wwn access")
>
> I can confirm that reverting either patch makes the problem go away.
> I'm planning on opening a gcc bug tomorrow.
Note that if CONFIG_OPTIMIZE_INLINING is not set, _all_ "inline"
keywords are in fact __always_inline, so the bug must be triggering
even without the patch.
