From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753080AbdCAUzF (ORCPT ); Wed, 1 Mar 2017 15:55:05 -0500 Received: from prod-mail-xrelay07.akamai.com ([23.79.238.175]:33330 "EHLO prod-mail-xrelay07.akamai.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750734AbdCAUzC (ORCPT ); Wed, 1 Mar 2017 15:55:02 -0500 Subject: Re: [PATCH] jump_label: align jump_entry table to at least 4-bytes To: David Daney , Michael Ellerman , Steven Rostedt , Sachin Sant References: <1488221364-13905-1-git-send-email-jbaron@akamai.com> <6db89a8d-6053-51d1-5fd4-bae0179a5ebd@caviumnetworks.com> <20170227170911.2280ca3e@gandalf.local.home> <7fa95eea-20be-611c-2b63-fca600779465@caviumnetworks.com> <20170227173630.57fff459@gandalf.local.home> <7bd72716-feea-073f-741c-04212ebd0802@caviumnetworks.com> <68fe24ea-7795-24d8-211b-9d8a50affe9f@akamai.com> <510FF566-011D-4199-86F7-2BB4DBF36434@linux.vnet.ibm.com> <20170228112144.65455de5@gandalf.local.home> <1de00727-de97-f887-78bd-dd49131cdf61@akamai.com> <999e2c3f-698c-703c-67a9-26aea3c97dc0@caviumnetworks.com> <542488db-5c59-afa5-6d1d-a437c87bc613@akamai.com> <912fa97a-aa1d-c0e4-dc83-fc5c745db1c1@caviumnetworks.com> <23989c10-7b47-3fda-f790-25b53 9704bec@akamai.com> <871suh5wtw.fsf@concordia.ellerman.id.au> <7e05bd3d-08da-00ba-1b79-c9be8c659524@caviumnetworks.com> Cc: linux-mips@linux-mips.org, Chris Metcalf , LKML , Ralf Baechle , Russell King , Rabin Vincent , Paul Mackerras , Anton Blanchard , linuxppc-dev@lists.ozlabs.org, Ingo Molnar , linux-arm-kernel@lists.infradead.org, Zhigang Lu From: Jason Baron Message-ID: Date: Wed, 1 Mar 2017 15:02:52 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: <7e05bd3d-08da-00ba-1b79-c9be8c659524@caviumnetworks.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 03/01/2017 11:40 AM, David Daney wrote: > On 02/28/2017 10:34 PM, Michael Ellerman wrote: >> Jason Baron writes: >> ... >>> I also checked all the other .ko files and they were properly aligned. >>> So I think this should hopefully work, and I like that its not a >>> per-arch fix. >>> >>> Sachin, sorry to bother you again, but I'm hoping you can try David's >>> latest patch to scripts/module-common.lds, just to test in your setup. >> >> It does fix the problem. >> >> I was reproducing with crc_t10dif: >> >> [ 695.890552] ------------[ cut here ]------------ >> [ 695.890709] WARNING: CPU: 15 PID: 3019 at >> ../kernel/jump_label.c:287 static_key_set_entries+0x74/0xa0 >> [ 695.890710] Modules linked in: crc_t10dif(+) crct10dif_generic >> crct10dif_common ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat >> nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 xt_addrtype >> iptable_filter ip_tables xt_conntrack x_tables nf_nat nf_conntrack >> bridge stp llc dm_thin_pool dm_persistent_data dm_bio_prison dm_bufio >> libcrc32c kvm virtio_balloon binfmt_misc autofs4 virtio_net virtio_pci >> virtio_ring virtio >> >> Which had: >> >> [21] __jump_table PROGBITS 0000000000000000 0004e8 >> 000018 00 WA 0 0 1 >> >> >> And now has: >> >> [18] __jump_table PROGBITS 0000000000000000 0004d0 >> 000018 00 WA 0 0 8 >> >> And all other modules have an alignment of 8 on __jump_table, as >> expected. >> >> I'm inclined to merge a version of the balign patch for powerpc anyway, >> just to be on the safe side. I guess the old code was coping fine with >> the unaligned keys, but it still makes me nervous. > > > The original "balign patch" has a couple of problems: > > 1) 4-byte alignment is not sufficient for 64-bit kernels > > 2) It is redundant if the linker script patch is accepted. > > The linker script patch seems reasonable to me. Maybe its worth adding a comment that the alignment is necessary because the core jump_label makes use of the 2 lsb bits of its __jump_table pointer due to commit: 3821fd3 jump_label: Reduce the size of struct static_key Also, in the comment it says that it fixes an oops. We hit a WARN_ON() not an oops, although bad things are likely to happen when the branch is updated. Thanks, -Jason From mboxrd@z Thu Jan 1 00:00:00 1970 From: jbaron@akamai.com (Jason Baron) Date: Wed, 1 Mar 2017 15:02:52 -0500 Subject: [PATCH] jump_label: align jump_entry table to at least 4-bytes In-Reply-To: <7e05bd3d-08da-00ba-1b79-c9be8c659524@caviumnetworks.com> References: <1488221364-13905-1-git-send-email-jbaron@akamai.com> <6db89a8d-6053-51d1-5fd4-bae0179a5ebd@caviumnetworks.com> <20170227170911.2280ca3e@gandalf.local.home> <7fa95eea-20be-611c-2b63-fca600779465@caviumnetworks.com> <20170227173630.57fff459@gandalf.local.home> <7bd72716-feea-073f-741c-04212ebd0802@caviumnetworks.com> <68fe24ea-7795-24d8-211b-9d8a50affe9f@akamai.com> <510FF566-011D-4199-86F7-2BB4DBF36434@linux.vnet.ibm.com> <20170228112144.65455de5@gandalf.local.home> <1de00727-de97-f887-78bd-dd49131cdf61@akamai.com> <999e2c3f-698c-703c-67a9-26aea3c97dc0@caviumnetworks.com> <542488db-5c59-afa5-6d1d-a437c87bc613@akamai.com> <912fa97a-aa1d-c0e4-dc83-fc5c745db1c1@caviumnetworks.com> <23989c10-7b47-3fda-f790-25b53 9704bec@akamai.com> <871suh5wtw.fsf@concordia.ellerman.id.au> <7e05bd3d-08da-00ba-1b79-c9be8c659524@caviumnetworks.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 03/01/2017 11:40 AM, David Daney wrote: > On 02/28/2017 10:34 PM, Michael Ellerman wrote: >> Jason Baron writes: >> ... >>> I also checked all the other .ko files and they were properly aligned. >>> So I think this should hopefully work, and I like that its not a >>> per-arch fix. >>> >>> Sachin, sorry to bother you again, but I'm hoping you can try David's >>> latest patch to scripts/module-common.lds, just to test in your setup. >> >> It does fix the problem. >> >> I was reproducing with crc_t10dif: >> >> [ 695.890552] ------------[ cut here ]------------ >> [ 695.890709] WARNING: CPU: 15 PID: 3019 at >> ../kernel/jump_label.c:287 static_key_set_entries+0x74/0xa0 >> [ 695.890710] Modules linked in: crc_t10dif(+) crct10dif_generic >> crct10dif_common ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat >> nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 xt_addrtype >> iptable_filter ip_tables xt_conntrack x_tables nf_nat nf_conntrack >> bridge stp llc dm_thin_pool dm_persistent_data dm_bio_prison dm_bufio >> libcrc32c kvm virtio_balloon binfmt_misc autofs4 virtio_net virtio_pci >> virtio_ring virtio >> >> Which had: >> >> [21] __jump_table PROGBITS 0000000000000000 0004e8 >> 000018 00 WA 0 0 1 >> >> >> And now has: >> >> [18] __jump_table PROGBITS 0000000000000000 0004d0 >> 000018 00 WA 0 0 8 >> >> And all other modules have an alignment of 8 on __jump_table, as >> expected. >> >> I'm inclined to merge a version of the balign patch for powerpc anyway, >> just to be on the safe side. I guess the old code was coping fine with >> the unaligned keys, but it still makes me nervous. > > > The original "balign patch" has a couple of problems: > > 1) 4-byte alignment is not sufficient for 64-bit kernels > > 2) It is redundant if the linker script patch is accepted. > > The linker script patch seems reasonable to me. Maybe its worth adding a comment that the alignment is necessary because the core jump_label makes use of the 2 lsb bits of its __jump_table pointer due to commit: 3821fd3 jump_label: Reduce the size of struct static_key Also, in the comment it says that it fixes an oops. We hit a WARN_ON() not an oops, although bad things are likely to happen when the branch is updated. Thanks, -Jason