LKML Archive mirror
 help / color / mirror / Atom feed
From: Josh Poimboeuf <jpoimboe@redhat.com>
To: James Bottomley <James.Bottomley@HansenPartnership.com>,
	"Martin K. Petersen" <martin.petersen@oracle.com>
Cc: 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>,
	Arnd Bergmann <arnd@arndb.de>,
	jamborm@gcc.gnu.org, Ingo Molnar <mingo@kernel.org>,
	Himanshu Madhani <himanshu.madhani@qlogic.com>,
	qla2xxx-upstream@qlogic.com
Subject: [PATCH] scsi: fc: force inlining of wwn conversion functions
Date: Tue, 19 Apr 2016 08:56:00 -0500	[thread overview]
Message-ID: <80200c53ae54f6cb34bd6fb51e9da65fdcc03004.1461073602.git.jpoimboe@redhat.com> (raw)
In-Reply-To: <20160419085221.GA29087@gmail.com>

objtool reports [1] the following warning:

  drivers/scsi/qla2xxx/qla_attr.o: warning: objtool: qla2x00_get_host_fabric_name() falls through to next function qla2x00_get_starget_port_name()

This warning is due to a gcc bug [2] which causes corrupt code:

  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 bad.

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")

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()

The bug requires very specific conditions to trigger.  According to Martin
Jambor (from the gcc bugzilla):

  "This bug can occur when an inlineable function containing a call to
  __builtin_constant_p, which checks a parameter or a value it
  references and a (possibly indirect) caller of the function actually
  passes a constant, but stores it using a type of a different size."

There's no reliable way to avoid (or even detect) the bug.  Until it
gets fixed in released versions of gcc, the least intrusive workaround
for this particular issue is to force the wwn conversion functions to be
inlined.

[1] https://lists.01.org/pipermail/kbuild-all/2016-April/019579.html
[2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70646

Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 include/scsi/scsi_transport_fc.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/scsi/scsi_transport_fc.h b/include/scsi/scsi_transport_fc.h
index bf66ea6..1919cd4 100644
--- a/include/scsi/scsi_transport_fc.h
+++ b/include/scsi/scsi_transport_fc.h
@@ -796,12 +796,12 @@ fc_remote_port_chkready(struct fc_rport *rport)
 	return result;
 }
 
-static inline u64 wwn_to_u64(u8 *wwn)
+static __always_inline u64 wwn_to_u64(u8 *wwn)
 {
 	return get_unaligned_be64(wwn);
 }
 
-static inline void u64_to_wwn(u64 inm, u8 *wwn)
+static __always_inline void u64_to_wwn(u64 inm, u8 *wwn)
 {
 	put_unaligned_be64(inm, wwn);
 }
-- 
2.4.11

  reply	other threads:[~2016-04-19 13:57 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                                 ` Josh Poimboeuf [this message]
2016-04-22 23:17                                   ` [PATCH] scsi: fc: force inlining of wwn conversion functions 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
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=80200c53ae54f6cb34bd6fb51e9da65fdcc03004.1461073602.git.jpoimboe@redhat.com \
    --to=jpoimboe@redhat.com \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=dvlasenk@redhat.com \
    --cc=himanshu.madhani@qlogic.com \
    --cc=jamborm@gcc.gnu.org \
    --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).