LKML Archive mirror
 help / color / mirror / Atom feed
From: Quinn Tran <quinn.tran@qlogic.com>
To: Josh Poimboeuf <jpoimboe@redhat.com>,
	James Bottomley <James.Bottomley@HansenPartnership.com>,
	"Martin K. Petersen" <martin.petersen@oracle.com>
Cc: linux-scsi <linux-scsi@vger.kernel.org>,
	linux-kernel <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" <jamborm@gcc.gnu.org>,
	"Ingo Molnar" <mingo@kernel.org>,
	Himanshu Madhani <himanshu.madhani@qlogic.com>,
	Dept-Eng QLA2xxx Upstream <qla2xxx-upstream@qlogic.com>
Subject: Re: [PATCH] scsi: fc: force inlining of wwn conversion functions
Date: Fri, 22 Apr 2016 23:17:08 +0000	[thread overview]
Message-ID: <5B57167E-EF1A-4739-BAD3-758AE3991846@qlogic.com> (raw)
In-Reply-To: <80200c53ae54f6cb34bd6fb51e9da65fdcc03004.1461073602.git.jpoimboe@redhat.com>

Current kernel (4.6.0-rc4+) + GCC 5.3.0 definitely truncated qla2x00_get_host_fabric_name() routine.  Just like Josh indicated, we’re dropping down to the next routine.


root@mars:/sys/class/fc_host/host3  2016-04-22 16:07:30
> cat fabric_name
Killed
——
static void
qla2x00_get_host_fabric_name(struct Scsi_Host *shost)
{
    32d0:       e8 00 00 00 00          callq  32d5 <qla2x00_get_host_fabric_name+0x5>
    32d5:       55                      push   %rbp
    32d6:       48 89 e5                mov    %rsp,%rbp
    32d9:       0f 1f 80 00 00 00 00    nopl   0x0(%rax)

00000000000032e0 <qla2x00_get_starget_node_name>:
qla2x00_get_starget_node_name():
/root/qt/linux.git/drivers/scsi/qla2xxx/qla_attr.c:1756
        fc_host_port_type(shost) = port_type;
}


----

Apr 22 16:07:50 mars kernel: BUG: unable to handle kernel NULL pointer dereference at 0000000000000058
Apr 22 16:07:50 mars kernel: IP: [<ffffffff813f72d7>] scsi_is_host_device+0x7/0x20
Apr 22 16:07:50 mars kernel: PGD 7fe1c8067 PUD 7f5c72067 PMD 0 
Apr 22 16:07:50 mars kernel: Oops: 0000 [#1] SMP 
Apr 22 16:07:50 mars kernel: Modules linked in: qla2xxx scsi_transport_fc ebtable_nat ebtables ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 
...
dm_mirror(E) dm_region_hash(E) dm_log(E) dm_mod(E) [last unloaded: qla2xxx]
Apr 22 16:07:50 mars kernel: CPU: 8 PID: 10452 Comm: cat Tainted: G            E   4.6.0-rc4+ #2
Apr 22 16:07:50 mars kernel: Hardware name: HP ProLiant DL380 G7, BIOS P67 05/05/2011
Apr 22 16:07:50 mars kernel: task: ffff8807fcd1a880 ti: ffff8807ff128000 task.ti: ffff8807ff128000
Apr 22 16:07:50 mars kernel: RIP: 0010:[<ffffffff813f72d7>]  [<ffffffff813f72d7>] scsi_is_host_device+0x7/0x20
Apr 22 16:07:50 mars kernel: RSP: 0018:ffff8807ff12bcf0  EFLAGS: 00010246
Apr 22 16:07:50 mars kernel: RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffff880ffe8ade88
Apr 22 16:07:50 mars kernel: RDX: ffff8807f5db9000 RSI: ffff880ffed43340 RDI: 0000000000000000
Apr 22 16:07:50 mars kernel: RBP: ffff8807ff12bd08 R08: ffff88101f45ac38 R09: ffff8807fccef280
Apr 22 16:07:50 mars kernel: R10: 0000000000000000 R11: 0000000000000000 R12: ffff88101b7e6000
Apr 22 16:07:50 mars kernel: R13: ffff8807fdaf1f00 R14: ffff8800dad379c0 R15: 0000000000000001
Apr 22 16:07:50 mars kernel: FS:  00007fd569512700(0000) GS:ffff88081fc80000(0000) knlGS:0000000000000000
Apr 22 16:07:50 mars kernel: CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
Apr 22 16:07:50 mars kernel: CR2: 0000000000000058 CR3: 00000007f23f4000 CR4: 00000000000006e0
Apr 22 16:07:50 mars kernel: Stack:
Apr 22 16:07:50 mars kernel: ffffffffa0759db5 ffff88101b7e6000 ffff8807f5db9000 ffff8807ff12bd10
Apr 22 16:07:50 mars kernel: ffff8807ff12bd30 ffffffffa065b1bb ffff880ffed43340 ffffffff8166d950
Apr 22 16:07:50 mars kernel: ffff8807ff12bd50 ffffffff813c9e30 ffffffff815ccfb2 ffff8800dad379c0
Apr 22 16:07:50 mars kernel: Call Trace:
Apr 22 16:07:50 mars kernel: [<ffffffffa0759db5>] ? qla2x00_get_starget_node_name+0x25/0x90 [qla2xxx]
Apr 22 16:07:50 mars kernel: [<ffffffffa065b1bb>] ? show_fc_host_fabric_name+0x4b/0x80 [scsi_transport_fc]
Apr 22 16:07:50 mars kernel: [<ffffffff813c9e30>] ? dev_attr_show+0x20/0x50




Regards,
Quinn Tran






-----Original Message-----
From: <linux-scsi-owner@vger.kernel.org> on behalf of Josh Poimboeuf <jpoimboe@redhat.com>
Date: Tuesday, April 19, 2016 at 6:56 AM
To: James Bottomley <James.Bottomley@HansenPartnership.com>, "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: linux-scsi <linux-scsi@vger.kernel.org>, linux-kernel <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" <jamborm@gcc.gnu.org>, Ingo Molnar <mingo@kernel.org>, Himanshu Madhani <himanshu.madhani@qlogic.com>, Dept-Eng QLA2xxx Upstream <qla2xxx-upstream@qlogic.com>
Subject: [PATCH] scsi: fc: force inlining of wwn conversion functions

>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
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2016-04-22 23:17 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 [this message]
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=5B57167E-EF1A-4739-BAD3-758AE3991846@qlogic.com \
    --to=quinn.tran@qlogic.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=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).