From: Arnd Bergmann <arnd@arndb.de>
To: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: James Bottomley <James.Bottomley@hansenpartnership.com>,
Josh Poimboeuf <jpoimboe@redhat.com>,
linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org,
Denys Vlasenko <dvlasenk@redhat.com>, Thomas Graf <tgraf@suug.ch>,
Peter Zijlstra <peterz@infradead.org>,
David Rientjes <rientjes@google.com>,
Andrew Morton <akpm@linux-foundation.org>,
jamborm@gcc.gnu.org, Ingo Molnar <mingo@kernel.org>,
Himanshu Madhani <himanshu.madhani@qlogic.com>,
qla2xxx-upstream@qlogic.com
Subject: Re: [PATCH] scsi: fc: force inlining of wwn conversion functions
Date: Tue, 26 Apr 2016 17:58:20 +0200 [thread overview]
Message-ID: <5298237.1Guzp0G04x@wuerfel> (raw)
In-Reply-To: <yq14maoh635.fsf@sermon.lab.mkp.net>
On Tuesday 26 April 2016 09:06:54 Martin K. Petersen wrote:
> >>>>> "Arnd" == Arnd Bergmann <arnd@arndb.de> writes:
>
> Arnd> I don't think we can realistically blacklist gcc-4.9.{0,1,2,3},
> Arnd> gcc-5.{0,1,2,3}.* and gcc-6.0 and require everyone to upgrade to
> Arnd> compilers that have not been released yet in order to build a
> Arnd> linux-4.6 kernel.
>
> I agree that compiler blacklisting is problematic and I'd like to avoid
> it. The question is how far we go in the kernel to accommodate various
> levels of brokenness.
>
> In any case. Sticking compiler workarounds in device driver code is akin
> to putting demolition orders on display on Alpha Centauri. At the very
> minimum the patch should put a fat comment in the code stating that
> these wrapper functions or #defines should not be changed in the future
> because that'll break builds using gcc XYZ. But that does not solve the
> problem for anybody else that might be doing something similar.
> Converting between u64 and $RANDOM_TYPE in an inline wrapper does not
> seem like a rare and unusual programming pattern.
It's not the driver really, it's the core scsi/fc layer, which makes
it a little dangerous that a random driver.
I agree that putting a comment in would also help. What I understand
from the bug report is that to trigger this bug you need these elements:
1. an inline function marked __always_inline
2. another inline function that is automatically inlined (not __always_inline)
3. CONFIG_OPTIMIZE_INLINING=y to guarantee 2
4. __builtin_compatible_p inside that inline function
The last point is what Denys introduced in the kernel with
bc27fb68aaad ("include/uapi/linux/byteorder, swab: force inlining of some
byteswap operations"). So maybe it's better after all to revert that
patch, to have a higher confidence in the same bug not appearing
elsewhere. It's also really a workaround for another quirk of the
compiler, but that one only results in duplicated functions in object
code rather than functions that end in the middle.
Arnd
next prev parent reply other threads:[~2016-04-26 15:59 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-04 19:45 [PATCH] asm-generic: force inlining of some atomic_long operations Denys Vlasenko
2016-02-04 19:45 ` [PATCH] force inlining of some byteswap operations Denys Vlasenko
2016-02-05 7:28 ` Ingo Molnar
2016-04-13 3:36 ` This patch triggers a bad gcc bug (was Re: [PATCH] force inlining of some byteswap operations) Josh Poimboeuf
2016-04-13 12:12 ` Denys Vlasenko
2016-04-13 12:36 ` Josh Poimboeuf
2016-04-13 15:15 ` Josh Poimboeuf
2016-04-13 16:55 ` James Bottomley
2016-04-13 17:10 ` Josh Poimboeuf
2016-04-14 15:29 ` Denys Vlasenko
2016-04-14 15:57 ` Josh Poimboeuf
2016-04-14 17:09 ` Denys Vlasenko
2016-04-15 5:45 ` Ingo Molnar
2016-04-15 13:47 ` Josh Poimboeuf
2016-04-15 22:20 ` Josh Poimboeuf
2016-04-16 9:03 ` Ingo Molnar
2016-04-18 13:39 ` Josh Poimboeuf
2016-04-18 14:07 ` Arnd Bergmann
2016-04-18 14:12 ` Josh Poimboeuf
2016-04-18 14:21 ` Arnd Bergmann
2016-04-19 8:52 ` Ingo Molnar
2016-04-19 13:56 ` [PATCH] scsi: fc: force inlining of wwn conversion functions Josh Poimboeuf
2016-04-22 23:17 ` Quinn Tran
2016-04-25 16:07 ` Josh Poimboeuf
2016-04-26 2:40 ` Martin K. Petersen
2016-04-26 3:37 ` James Bottomley
2016-04-26 7:22 ` Arnd Bergmann
2016-04-26 8:35 ` Christoph Hellwig
2016-04-26 10:05 ` Arnd Bergmann
2016-04-26 13:06 ` Martin K. Petersen
2016-04-26 15:58 ` Arnd Bergmann [this message]
2016-04-26 22:36 ` James Bottomley
2016-04-27 0:44 ` Martin K. Petersen
2016-04-27 11:05 ` Martin Jambor
2016-04-27 21:34 ` Arnd Bergmann
2016-04-28 14:58 ` Chris Metcalf
2016-04-28 15:23 ` Arnd Bergmann
2016-04-28 15:48 ` Chris Metcalf
2016-04-27 22:00 ` [PATCH, RFT] byteswap: try to avoid __builtin_constant_p gcc bug Arnd Bergmann
2016-04-27 22:11 ` Josh Poimboeuf
2016-04-28 16:27 ` Quinn Tran
2016-04-16 7:42 ` This patch triggers a bad gcc bug (was Re: [PATCH] force inlining of some byteswap operations) Arnd Bergmann
2016-04-18 13:22 ` Josh Poimboeuf
2016-02-04 19:45 ` [PATCH] force inlining of unaligned byteswap operations Denys Vlasenko
2016-02-05 7:28 ` Ingo Molnar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5298237.1Guzp0G04x@wuerfel \
--to=arnd@arndb.de \
--cc=James.Bottomley@hansenpartnership.com \
--cc=akpm@linux-foundation.org \
--cc=dvlasenk@redhat.com \
--cc=himanshu.madhani@qlogic.com \
--cc=jamborm@gcc.gnu.org \
--cc=jpoimboe@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=mingo@kernel.org \
--cc=peterz@infradead.org \
--cc=qla2xxx-upstream@qlogic.com \
--cc=rientjes@google.com \
--cc=tgraf@suug.ch \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).