Those look good to me 18/18 = 1 with no remainder. The odd numbers are
the offset of the section in the ELF file.
If you look at the stack trace, it seems that it is during module loading.
Are the primitives for generating the tables doing something different
for the module case? I am not familiar enough with the powerpc ABIs to
know.
Try this:
$ perl -n -e 's/\[ /\[/; my @f = split " "; print hex($f[5]) % 0x18 if
$#f > 5; print $_' <~/jump_table.log
There are no entries with size that is not a multiple of 0x18.
I think my patch to add the ENTSIZE is not doing anything here.
I suspect that the alignment of the __jump_table section in the .ko
files is not correct, and you are seeing some sort of problem due to that.
Those look good to me 18/18 = 1 with no remainder. The odd numbers are
the offset of the section in the ELF file.
If you look at the stack trace, it seems that it is during module loading.
Are the primitives for generating the tables doing something different
for the module case? I am not familiar enough with the powerpc ABIs to
know.
Try this:
$ perl -n -e 's/\[ /\[/; my @f = split " "; print hex($f[5]) % 0x18 if
$#f > 5; print $_' <~/jump_table.log
There are no entries with size that is not a multiple of 0x18.
I think my patch to add the ENTSIZE is not doing anything here.
I suspect that the alignment of the __jump_table section in the .ko
files is not correct, and you are seeing some sort of problem due to that.
Hi,
Yes, if you look at the trace that Sachin sent the module being loaded
that does the WARN_ON() is nfsd.ko.
That module from Sachin's trace has:
[31] __jump_table PROGBITS 0000000000000000 03fd77 0000c0
18 WAM 0 0 1
So its not the size but rather the start offset '03fd77', that is the
problem here. That is what the WARN_ON triggers on, that the start of
the table is not 4-byte aligned.
Using a ppc cross-compiler and the ENTSIZE patch that line does not
change, however if I use the initial patch posted in this thread, the
start does align to 4-bytes and thus the warning goes away, as Sachin
verified. In fact, without the patch I found several modules that don't
start at the proper alignment, however with the patch that started this
thread they were all properly aligned.
In terms of the '.balign' causing holes, we originally added the
'_ASM_ALIGN' to x86 for precisely this reason. See commit:
ef64789 jump label: Add _ASM_ALIGN for x86 and x86_64 and discussion.
In addition, we have a lot of runtime with the .balign in the tree and
I'm not aware of any holes in the table. I think the code would blow up
pretty badly if there were.
A number of arches were already using the '.balign', and the patch I
proposed simply added it to remaining ones, now that we added a
WARN_ON() to catch this condition.
Thanks,
-Jason
Those look good to me 18/18 = 1 with no remainder. The odd numbers are
the offset of the section in the ELF file.
If you look at the stack trace, it seems that it is during module
loading.
Are the primitives for generating the tables doing something different
for the module case? I am not familiar enough with the powerpc ABIs to
know.
Try this:
$ perl -n -e 's/\[ /\[/; my @f = split " "; print hex($f[5]) % 0x18 if
$#f > 5; print $_' <~/jump_table.log
There are no entries with size that is not a multiple of 0x18.
I think my patch to add the ENTSIZE is not doing anything here.
I suspect that the alignment of the __jump_table section in the .ko
files is not correct, and you are seeing some sort of problem due to
that.
Hi,
Yes, if you look at the trace that Sachin sent the module being loaded
that does the WARN_ON() is nfsd.ko.
That module from Sachin's trace has:
[31] __jump_table PROGBITS 0000000000000000 03fd77 0000c0
18 WAM 0 0 1
The problem is then the section alignment (last column) for power.
On mips with no patches applied, we get:
[17] __jump_table PROGBITS 0000000000000000 00d2c0 000048
00 WA 0 0 8
Look, proper alignment!
The question I have is why do the power ".llong" and ".long" assembler
directives not force section alignment? Is there an alternative that
could be used that would result in the proper alignment? Would ".word"
work?
If not, then I would say patch only power with your balign thing.
8-byte alignment for 64-bit kernel, 4-byte alignment for 32-bit kernel
So its not the size but rather the start offset '03fd77', that is the
problem here. That is what the WARN_ON triggers on, that the start of
the table is not 4-byte aligned.
Using a ppc cross-compiler and the ENTSIZE patch that line does not
change, however if I use the initial patch posted in this thread, the
start does align to 4-bytes and thus the warning goes away, as Sachin
verified. In fact, without the patch I found several modules that don't
start at the proper alignment, however with the patch that started this
thread they were all properly aligned.
In terms of the '.balign' causing holes, we originally added the
'_ASM_ALIGN' to x86 for precisely this reason. See commit:
ef64789 jump label: Add _ASM_ALIGN for x86 and x86_64 and discussion.
In addition, we have a lot of runtime with the .balign in the tree and
I'm not aware of any holes in the table. I think the code would blow up
pretty badly if there were.
A number of arches were already using the '.balign', and the patch I
proposed simply added it to remaining ones, now that we added a
WARN_ON() to catch this condition.
Thanks,
-Jason
From: David Daney <hidden> Date: 2017-03-01 00:07:15
On 02/28/2017 11:05 AM, David Daney wrote:
On 02/28/2017 10:39 AM, Jason Baron wrote:
quoted
[...]
quoted
quoted
I suspect that the alignment of the __jump_table section in the .ko
files is not correct, and you are seeing some sort of problem due to
that.
Hi,
Yes, if you look at the trace that Sachin sent the module being loaded
that does the WARN_ON() is nfsd.ko.
That module from Sachin's trace has:
[31] __jump_table PROGBITS 0000000000000000 03fd77 0000c0
18 WAM 0 0 1
The problem is then the section alignment (last column) for power.
On mips with no patches applied, we get:
[17] __jump_table PROGBITS 0000000000000000 00d2c0 000048
00 WA 0 0 8
Look, proper alignment!
The question I have is why do the power ".llong" and ".long" assembler
directives not force section alignment? Is there an alternative that
could be used that would result in the proper alignment? Would ".word"
work?
If not, then I would say patch only power with your balign thing. 8-byte
alignment for 64-bit kernel, 4-byte alignment for 32-bit kernel
I think the proper fix is either:
A) Modify scripts/module-common.lds to force __jump_table alignment for
all architectures.
B) Add arch/powerpc/kernel/module.lds to force __jump_table alignment
for powerpc only.
David.
quoted
So its not the size but rather the start offset '03fd77', that is the
problem here. That is what the WARN_ON triggers on, that the start of
the table is not 4-byte aligned.
Using a ppc cross-compiler and the ENTSIZE patch that line does not
change, however if I use the initial patch posted in this thread, the
start does align to 4-bytes and thus the warning goes away, as Sachin
verified. In fact, without the patch I found several modules that don't
start at the proper alignment, however with the patch that started this
thread they were all properly aligned.
In terms of the '.balign' causing holes, we originally added the
'_ASM_ALIGN' to x86 for precisely this reason. See commit:
ef64789 jump label: Add _ASM_ALIGN for x86 and x86_64 and discussion.
In addition, we have a lot of runtime with the .balign in the tree and
I'm not aware of any holes in the table. I think the code would blow up
pretty badly if there were.
A number of arches were already using the '.balign', and the patch I
proposed simply added it to remaining ones, now that we added a
WARN_ON() to catch this condition.
Thanks,
-Jason
From: Jason Baron <jbaron@akamai.com> Date: 2017-02-28 19:35:48
On 02/28/2017 02:22 PM, David Daney wrote:
On 02/28/2017 11:05 AM, David Daney wrote:
quoted
On 02/28/2017 10:39 AM, Jason Baron wrote:
quoted
[...]
quoted
quoted
quoted
I suspect that the alignment of the __jump_table section in the .ko
files is not correct, and you are seeing some sort of problem due to
that.
Hi,
Yes, if you look at the trace that Sachin sent the module being loaded
that does the WARN_ON() is nfsd.ko.
That module from Sachin's trace has:
[31] __jump_table PROGBITS 0000000000000000 03fd77 0000c0
18 WAM 0 0 1
The problem is then the section alignment (last column) for power.
On mips with no patches applied, we get:
[17] __jump_table PROGBITS 0000000000000000 00d2c0 000048
00 WA 0 0 8
Look, proper alignment!
The question I have is why do the power ".llong" and ".long" assembler
directives not force section alignment? Is there an alternative that
could be used that would result in the proper alignment? Would ".word"
work?
If not, then I would say patch only power with your balign thing. 8-byte
alignment for 64-bit kernel, 4-byte alignment for 32-bit kernel
I think the proper fix is either:
A) Modify scripts/module-common.lds to force __jump_table alignment for
all architectures.
B) Add arch/powerpc/kernel/module.lds to force __jump_table alignment
for powerpc only.
David.
Ok, I can try adding it to the linger script.
FWIW, here is my before and after with the .balign thing for the nfsd.ko
module on powperc (using a cross-compiler):
before:
[31] __jump_table PROGBITS 0000000000000000 03ee3e 0000f0
00 WA 0 0 1
after:
[31] __jump_table PROGBITS 0000000000000000 03ee40 0000f0
00 WA 0 0 4
Thanks,
-Jason
quoted
quoted
So its not the size but rather the start offset '03fd77', that is the
problem here. That is what the WARN_ON triggers on, that the start of
the table is not 4-byte aligned.
Using a ppc cross-compiler and the ENTSIZE patch that line does not
change, however if I use the initial patch posted in this thread, the
start does align to 4-bytes and thus the warning goes away, as Sachin
verified. In fact, without the patch I found several modules that don't
start at the proper alignment, however with the patch that started this
thread they were all properly aligned.
In terms of the '.balign' causing holes, we originally added the
'_ASM_ALIGN' to x86 for precisely this reason. See commit:
ef64789 jump label: Add _ASM_ALIGN for x86 and x86_64 and discussion.
In addition, we have a lot of runtime with the .balign in the tree and
I'm not aware of any holes in the table. I think the code would blow up
pretty badly if there were.
A number of arches were already using the '.balign', and the patch I
proposed simply added it to remaining ones, now that we added a
WARN_ON() to catch this condition.
Thanks,
-Jason
From: David Daney <hidden> Date: 2017-02-28 20:17:01
On 02/28/2017 11:34 AM, Jason Baron wrote:
On 02/28/2017 02:22 PM, David Daney wrote:
quoted
On 02/28/2017 11:05 AM, David Daney wrote:
quoted
On 02/28/2017 10:39 AM, Jason Baron wrote:
quoted
[...]
quoted
quoted
quoted
I suspect that the alignment of the __jump_table section in the .ko
files is not correct, and you are seeing some sort of problem due to
that.
Hi,
Yes, if you look at the trace that Sachin sent the module being loaded
that does the WARN_ON() is nfsd.ko.
That module from Sachin's trace has:
[31] __jump_table PROGBITS 0000000000000000 03fd77 0000c0
18 WAM 0 0 1
The problem is then the section alignment (last column) for power.
On mips with no patches applied, we get:
[17] __jump_table PROGBITS 0000000000000000 00d2c0 000048
00 WA 0 0 8
Look, proper alignment!
The question I have is why do the power ".llong" and ".long" assembler
directives not force section alignment? Is there an alternative that
could be used that would result in the proper alignment? Would ".word"
work?
If not, then I would say patch only power with your balign thing. 8-byte
alignment for 64-bit kernel, 4-byte alignment for 32-bit kernel
I think the proper fix is either:
A) Modify scripts/module-common.lds to force __jump_table alignment for
all architectures.
B) Add arch/powerpc/kernel/module.lds to force __jump_table alignment
for powerpc only.
David.
Ok, I can try adding it to the linger script.
FWIW, here is my before and after with the .balign thing for the nfsd.ko
module on powperc (using a cross-compiler):
before:
[31] __jump_table PROGBITS 0000000000000000 03ee3e 0000f0
00 WA 0 0 1
after:
[31] __jump_table PROGBITS 0000000000000000 03ee40 0000f0
00 WA 0 0 4
Try the (lightly tested) attached.
If it works and Steven likes it, perhaps someone can merge it.
David.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-module-set-__jump_table-alignment-to-8.patch
Type: text/x-patch
Size: 916 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170228/f4a8f907/attachment.bin>
From: Jason Baron <jbaron@akamai.com> Date: 2017-02-28 22:50:18
On 02/28/2017 03:15 PM, David Daney wrote:
On 02/28/2017 11:34 AM, Jason Baron wrote:
quoted
On 02/28/2017 02:22 PM, David Daney wrote:
quoted
On 02/28/2017 11:05 AM, David Daney wrote:
quoted
On 02/28/2017 10:39 AM, Jason Baron wrote:
quoted
[...]
quoted
quoted
quoted
I suspect that the alignment of the __jump_table section in the .ko
files is not correct, and you are seeing some sort of problem due to
that.
Hi,
Yes, if you look at the trace that Sachin sent the module being loaded
that does the WARN_ON() is nfsd.ko.
That module from Sachin's trace has:
[31] __jump_table PROGBITS 0000000000000000 03fd77
0000c0
18 WAM 0 0 1
The problem is then the section alignment (last column) for power.
On mips with no patches applied, we get:
[17] __jump_table PROGBITS 0000000000000000 00d2c0 000048
00 WA 0 0 8
Look, proper alignment!
The question I have is why do the power ".llong" and ".long" assembler
directives not force section alignment? Is there an alternative that
could be used that would result in the proper alignment? Would ".word"
work?
If not, then I would say patch only power with your balign thing.
8-byte
alignment for 64-bit kernel, 4-byte alignment for 32-bit kernel
I think the proper fix is either:
A) Modify scripts/module-common.lds to force __jump_table alignment for
all architectures.
B) Add arch/powerpc/kernel/module.lds to force __jump_table alignment
for powerpc only.
David.
Ok, I can try adding it to the linger script.
FWIW, here is my before and after with the .balign thing for the nfsd.ko
module on powperc (using a cross-compiler):
before:
[31] __jump_table PROGBITS 0000000000000000 03ee3e 0000f0
00 WA 0 0 1
after:
[31] __jump_table PROGBITS 0000000000000000 03ee40 0000f0
00 WA 0 0 4
Try the (lightly tested) attached.
If it works and Steven likes it, perhaps someone can merge it.
David.
So before your module.lds script:
# powerpc64-linux-readelf -eW fs/nfsd/nfsd.o | grep jump
[31] __jump_table PROGBITS 0000000000000000 03edfe 0000f0
00 WA 0 0 1
# powerpc64-linux-readelf -eW fs/nfsd/nfsd.ko | grep jump
[32] __jump_table PROGBITS 0000000000000000 044046 0000f0
00 WA 0 0 1
With your patch:
# powerpc64-linux-readelf -eW fs/nfsd/nfsd.o | grep jump
[31] __jump_table PROGBITS 0000000000000000 03edfe 0000f0
00 WA 0 0 1
# powerpc64-linux-readelf -eW fs/nfsd/nfsd.ko | grep jump
[18] __jump_table PROGBITS 0000000000000000 03e358 0000f0
00 WA 0 0 8
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.
Thanks,
-Jason
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2017-03-01 06:44:34
Jason Baron [off-list ref] 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.
cheers
From: David Daney <hidden> Date: 2017-03-01 17:16:08
On 02/28/2017 10:34 PM, Michael Ellerman wrote:
Jason Baron [off-list ref] writes:
...
quoted
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.
From: Jason Baron <jbaron@akamai.com> Date: 2017-03-01 20:55:05
On 03/01/2017 11:40 AM, David Daney wrote:
On 02/28/2017 10:34 PM, Michael Ellerman wrote:
quoted
Jason Baron [off-list ref] writes:
...
quoted
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: David Daney <hidden> Date: 2017-03-01 21:27:39
On 03/01/2017 12:02 PM, Jason Baron wrote:
On 03/01/2017 11:40 AM, David Daney wrote:
quoted
On 02/28/2017 10:34 PM, Michael Ellerman wrote:
quoted
Jason Baron [off-list ref] writes:
...
quoted
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.