Messages in this thread | | | Subject | Re: [PATCH] jump_label: align jump_entry table to at least 4-bytes | From | David Daney <> | Date | Wed, 1 Mar 2017 13:12:22 -0800 |
| |
On 03/01/2017 12:02 PM, Jason Baron wrote: > > > On 03/01/2017 11:40 AM, David Daney wrote: >> On 02/28/2017 10:34 PM, Michael Ellerman wrote: >>> Jason Baron <jbaron@akamai.com> 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. >
OK, I guess I will send a proper patch e-mail with an updated changelog with the improvements you suggest.
Thanks, David.
| |