From: Jason Baron <jbaron@akamai.com> To: David Daney <ddaney@caviumnetworks.com>, rostedt@goodmis.org Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-mips@linux-mips.org, linuxppc-dev@lists.ozlabs.org, Ingo Molnar <mingo@kernel.org>, Benjamin Herrenschmidt <benh@kernel.crashing.org>, Paul Mackerras <paulus@samba.org>, Michael Ellerman <mpe@ellerman.id.au>, Anton Blanchard <anton@samba.org>, Rabin Vincent <rabin@rab.in>, Russell King <linux@armlinux.org.uk>, Ralf Baechle <ralf@linux-mips.org>, Chris Metcalf <cmetcalf@mellanox.com>, Zhigang Lu <zlu@ezchip.com> Subject: Re: [PATCH] jump_label: align jump_entry table to at least 4-bytes Date: Mon, 27 Feb 2017 14:18:54 -0500 [thread overview] Message-ID: <93219edf-0f6d-5cc7-309c-c998f16fe7ac@akamai.com> (raw) In-Reply-To: <f7532548-7007-1a32-f669-4520792805b3@caviumnetworks.com> On 02/27/2017 01:57 PM, David Daney wrote: > On 02/27/2017 10:49 AM, Jason Baron wrote: >> The core jump_label code makes use of the 2 lower bits of the >> static_key::[type|entries|next] field. Thus, ensure that the jump_entry >> table is at least 4-byte aligned. >> > [...] >> diff --git a/arch/mips/include/asm/jump_label.h >> b/arch/mips/include/asm/jump_label.h >> index e77672539e8e..243791f3ae71 100644 >> --- a/arch/mips/include/asm/jump_label.h >> +++ b/arch/mips/include/asm/jump_label.h >> @@ -31,6 +31,7 @@ static __always_inline bool >> arch_static_branch(struct static_key *key, bool bran >> asm_volatile_goto("1:\t" NOP_INSN "\n\t" >> "nop\n\t" >> ".pushsection __jump_table, \"aw\"\n\t" >> + ".balign 4\n\t" >> WORD_INSN " 1b, %l[l_yes], %0\n\t" >> ".popsection\n\t" >> : : "i" (&((char *)key)[branch]) : : l_yes); >> @@ -45,6 +46,7 @@ static __always_inline bool >> arch_static_branch_jump(struct static_key *key, bool >> asm_volatile_goto("1:\tj %l[l_yes]\n\t" >> "nop\n\t" >> ".pushsection __jump_table, \"aw\"\n\t" >> + ".balign 4\n\t" >> WORD_INSN " 1b, %l[l_yes], %0\n\t" >> ".popsection\n\t" >> : : "i" (&((char *)key)[branch]) : : l_yes); > > > I will speak only for the MIPS part. > > If the section is not already properly aligned, this change will add > padding, which is probably not what we want. > > Have you ever seen a problem with misalignment in the real world? > Hi, Yes, there was a WARN_ON() reported on POWER here: https://lkml.org/lkml/2017/2/19/85 The WARN_ON() triggers if either of the 2 lsb are set. More specifically, its coming from 'static_key_set_entries()' from the following patch, which was recently added to linux-next: https://lkml.org/lkml/2017/2/3/558 So this was not seen on mips, but I included all arches in this patch that I though might be affected. > If so, I think a better approach might be to set properties on the > __jump_table section to force the proper alignment, or do something in > the linker script. > So in include/asm-generic/vmlinux.lds.h, we are already setting 'ALIGN(8)', but that does not appear to be sufficient for POWER... Also, I checked the size of the vmlinux generated after this change on all 4 arches, and it was rather minimal. I think POWER increased the most, but the other arches increased by only a few bytes. Thanks, -Jason
WARNING: multiple messages have this Message-ID (diff)
From: jbaron@akamai.com (Jason Baron) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH] jump_label: align jump_entry table to at least 4-bytes Date: Mon, 27 Feb 2017 14:18:54 -0500 [thread overview] Message-ID: <93219edf-0f6d-5cc7-309c-c998f16fe7ac@akamai.com> (raw) In-Reply-To: <f7532548-7007-1a32-f669-4520792805b3@caviumnetworks.com> On 02/27/2017 01:57 PM, David Daney wrote: > On 02/27/2017 10:49 AM, Jason Baron wrote: >> The core jump_label code makes use of the 2 lower bits of the >> static_key::[type|entries|next] field. Thus, ensure that the jump_entry >> table is at least 4-byte aligned. >> > [...] >> diff --git a/arch/mips/include/asm/jump_label.h >> b/arch/mips/include/asm/jump_label.h >> index e77672539e8e..243791f3ae71 100644 >> --- a/arch/mips/include/asm/jump_label.h >> +++ b/arch/mips/include/asm/jump_label.h >> @@ -31,6 +31,7 @@ static __always_inline bool >> arch_static_branch(struct static_key *key, bool bran >> asm_volatile_goto("1:\t" NOP_INSN "\n\t" >> "nop\n\t" >> ".pushsection __jump_table, \"aw\"\n\t" >> + ".balign 4\n\t" >> WORD_INSN " 1b, %l[l_yes], %0\n\t" >> ".popsection\n\t" >> : : "i" (&((char *)key)[branch]) : : l_yes); >> @@ -45,6 +46,7 @@ static __always_inline bool >> arch_static_branch_jump(struct static_key *key, bool >> asm_volatile_goto("1:\tj %l[l_yes]\n\t" >> "nop\n\t" >> ".pushsection __jump_table, \"aw\"\n\t" >> + ".balign 4\n\t" >> WORD_INSN " 1b, %l[l_yes], %0\n\t" >> ".popsection\n\t" >> : : "i" (&((char *)key)[branch]) : : l_yes); > > > I will speak only for the MIPS part. > > If the section is not already properly aligned, this change will add > padding, which is probably not what we want. > > Have you ever seen a problem with misalignment in the real world? > Hi, Yes, there was a WARN_ON() reported on POWER here: https://lkml.org/lkml/2017/2/19/85 The WARN_ON() triggers if either of the 2 lsb are set. More specifically, its coming from 'static_key_set_entries()' from the following patch, which was recently added to linux-next: https://lkml.org/lkml/2017/2/3/558 So this was not seen on mips, but I included all arches in this patch that I though might be affected. > If so, I think a better approach might be to set properties on the > __jump_table section to force the proper alignment, or do something in > the linker script. > So in include/asm-generic/vmlinux.lds.h, we are already setting 'ALIGN(8)', but that does not appear to be sufficient for POWER... Also, I checked the size of the vmlinux generated after this change on all 4 arches, and it was rather minimal. I think POWER increased the most, but the other arches increased by only a few bytes. Thanks, -Jason
next prev parent reply other threads:[~2017-02-27 19:20 UTC|newest] Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-02-27 18:49 [PATCH] jump_label: align jump_entry table to at least 4-bytes Jason Baron 2017-02-27 18:49 ` Jason Baron 2017-02-27 18:57 ` David Daney 2017-02-27 18:57 ` David Daney 2017-02-27 19:18 ` Jason Baron [this message] 2017-02-27 19:18 ` Jason Baron 2017-02-27 19:59 ` David Daney 2017-02-27 19:59 ` David Daney 2017-02-27 21:06 ` Steven Rostedt 2017-02-27 21:06 ` Steven Rostedt 2017-02-27 21:41 ` David Daney 2017-02-27 21:41 ` David Daney 2017-02-27 22:09 ` Steven Rostedt 2017-02-27 22:09 ` Steven Rostedt 2017-02-27 22:21 ` David Daney 2017-02-27 22:21 ` David Daney 2017-02-27 22:21 ` David Daney 2017-02-27 22:36 ` Steven Rostedt 2017-02-27 22:36 ` Steven Rostedt 2017-02-27 22:45 ` David Daney 2017-02-27 22:45 ` David Daney 2017-02-27 22:50 ` Jason Baron 2017-02-27 22:50 ` Jason Baron 2017-02-27 23:34 ` David Daney 2017-02-27 23:34 ` David Daney 2017-02-28 4:55 ` Sachin Sant 2017-02-28 16:21 ` Steven Rostedt 2017-02-28 16:21 ` Steven Rostedt 2017-02-28 18:16 ` David Daney 2017-02-28 18:16 ` David Daney 2017-02-28 18:39 ` Jason Baron 2017-02-28 18:39 ` Jason Baron 2017-02-28 19:05 ` David Daney 2017-02-28 19:05 ` David Daney 2017-02-28 19:22 ` David Daney 2017-02-28 19:22 ` David Daney 2017-02-28 19:34 ` Jason Baron 2017-02-28 19:34 ` Jason Baron 2017-02-28 20:15 ` David Daney 2017-02-28 20:15 ` David Daney 2017-02-28 20:15 ` David Daney 2017-02-28 22:41 ` Jason Baron 2017-02-28 22:41 ` Jason Baron 2017-03-01 6:34 ` Michael Ellerman 2017-03-01 6:34 ` Michael Ellerman 2017-03-01 16:40 ` David Daney 2017-03-01 16:40 ` David Daney 2017-03-01 20:02 ` Jason Baron 2017-03-01 20:02 ` Jason Baron 2017-03-01 21:12 ` David Daney 2017-03-01 21:12 ` David Daney 2017-03-01 7:12 ` Sachin Sant 2017-03-01 7:12 ` Sachin Sant 2017-02-27 22:58 ` Steven Rostedt 2017-02-27 22:58 ` Steven Rostedt 2017-03-06 2:16 ` [PATCH] MIPS: jump_lable: Give __jump_table elements an entsize kbuild test robot 2017-03-06 2:16 ` kbuild test robot
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=93219edf-0f6d-5cc7-309c-c998f16fe7ac@akamai.com \ --to=jbaron@akamai.com \ --cc=anton@samba.org \ --cc=benh@kernel.crashing.org \ --cc=cmetcalf@mellanox.com \ --cc=ddaney@caviumnetworks.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mips@linux-mips.org \ --cc=linux@armlinux.org.uk \ --cc=linuxppc-dev@lists.ozlabs.org \ --cc=mingo@kernel.org \ --cc=mpe@ellerman.id.au \ --cc=paulus@samba.org \ --cc=rabin@rab.in \ --cc=ralf@linux-mips.org \ --cc=rostedt@goodmis.org \ --cc=zlu@ezchip.com \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.