Hi,
On 02/19/2017 09:07 AM, Sachin Sant wrote:
While booting next-20170217 on a POWER8 LPAR following
warning is displayed.
Reverting the following commit helps boot cleanly.
commit 3821fd35b5 : jump_label: Reduce the size of struct static_key
[ 11.393008] ------------[ cut here ]------------
[ 11.393031] WARNING: CPU: 5 PID: 2890 at kernel/jump_label.c:287 static_key_set_entries.isra.10+0x3c/0x50
Thanks for the report. So this is saying that the jump_entry table is
not at least 4-byte aligned. I wonder if this fixes it up?
diff --git a/arch/powerpc/include/asm/jump_label.h
b/arch/powerpc/include/asm/jump_label.h
index 9a287e0ac8b1..f870a85bac46 100644
--- a/arch/powerpc/include/asm/jump_label.h
+++ b/arch/powerpc/include/asm/jump_label.h
@@ -24,6 +24,7 @@ static __always_inline bool arch_static_branch(struct
static_key *key, bool bran
asm_volatile_goto("1:\n\t"
"nop # arch_static_branch\n\t"
".pushsection __jump_table, \"aw\"\n\t"
+ ".balign 4 \n\t"
JUMP_ENTRY_TYPE "1b, %l[l_yes], %c0\n\t"
".popsection \n\t"
: : "i" (&((char *)key)[branch]) : : l_yes);@@ -38,6 +39,7 @@ static __always_inline bool
arch_static_branch_jump(struct static_key *key, bool
asm_volatile_goto("1:\n\t"
"b %l[l_yes] # arch_static_branch_jump\n\t"
".pushsection __jump_table, \"aw\"\n\t"
+ ".balign 4 \n\t"
JUMP_ENTRY_TYPE "1b, %l[l_yes], %c0\n\t"
".popsection \n\t"
: : "i" (&((char *)key)[branch]) : : l_yes);@@ -63,6 +65,7 @@ struct jump_entry {
#define ARCH_STATIC_BRANCH(LABEL, KEY) \
1098: nop; \
.pushsection __jump_table, "aw"; \
+ .balign 4; \
FTR_ENTRY_LONG 1098b, LABEL, KEY; \
.popsection
#endif
Thanks,
-Jason
[ 11.393035] Modules linked in: nfsd(+) ip_tables x_tables autofs4
[ 11.393043] CPU: 5 PID: 2890 Comm: modprobe Not tainted 4.10.0-rc8-next-20170217-autotest #1
[ 11.393047] task: c0000003a5692500 task.stack: c0000003a7774000
[ 11.393051] NIP: c0000000017bcffc LR: c0000000017bd46c CTR: 0000000000000000
[ 11.393054] REGS: c0000003a7777800 TRAP: 0700 Not tainted (4.10.0-rc8-next-20170217-autotest)
[ 11.393058] MSR: 800000000282b033 <SF,VEC,VSX,EE,FP,ME,IR,DR,RI,LE>
[ 11.393065] CR: 48248282 XER: 00000001
[ 11.393070] CFAR: c0000000017bcfcc SOFTE: 1
GPR00: c0000000017bd42c c0000003a7777a80 c00000000262ce00 d000000003fdd580
GPR04: d000000003fe07df 0000000100000017 c0000000017bcd50 0000000000000000
GPR08: 0000000000053a09 0000000000000001 c00000000254ce00 0000000000000001
GPR12: c000000001b56c40 c00000000ea81400 0000000000000020 d000000005081098
GPR16: c0000003a7777da0 c0000003a7777dec 0000000000000000 00000000000084a8
GPR20: d000000003fef000 d000000003fe2b28 c00000000252dc90 0000000000000001
GPR24: c00000000254d314 0000000000000000 c0000000025338f8 d000000003fe089f
GPR28: 0000000000000000 d000000003fe1400 d000000003fdd578 d000000003fe07df
[ 11.393115] NIP [c0000000017bcffc] static_key_set_entries.isra.10+0x3c/0x50
[ 11.393119] LR [c0000000017bd46c] jump_label_module_notify+0x20c/0x420
[ 11.393122] Call Trace:
[ 11.393125] [c0000003a7777a80] [c0000000017bd42c] jump_label_module_notify+0x1cc/0x420 (unreliable)
[ 11.393132] [c0000003a7777b40] [c0000000016b38e0] notifier_call_chain+0x90/0x100
[ 11.393137] [c0000003a7777b90] [c0000000016b3db0] __blocking_notifier_call_chain+0x60/0x90
[ 11.393142] [c0000003a7777be0] [c0000000017357bc] load_module+0x1c1c/0x2750
[ 11.393147] [c0000003a7777d70] [c000000001736550] SyS_finit_module+0xc0/0xf0
[ 11.393152] [c0000003a7777e30] [c0000000015cb8e0] system_call+0x38/0xfc
[ 11.393156] Instruction dump:
[ 11.393158] 40c20018 e9230000 792907a0 7c844b78 f8830000 4e800020 3d42fff2 892a0514
[ 11.393166] 2f890000 40feffe0 39200001 992a0514 <0fe00000> 4bffffd0 60000000 60000000
[ 11.393173] ---[ end trace a5f8fbc5d8226aec ]---
Have attached boot log.
Thanks
-Sachin
On 02/20/2017 10:05 PM, Sachin Sant wrote:
quoted
On 20-Feb-2017, at 8:27 PM, Jason Baron <jbaron@akamai.com
<mailto:jbaron@akamai.com>> wrote:
Hi,
On 02/19/2017 09:07 AM, Sachin Sant wrote:
quoted
While booting next-20170217 on a POWER8 LPAR following
warning is displayed.
Reverting the following commit helps boot cleanly.
commit 3821fd35b5 : jump_label: Reduce the size of struct static_key
[ 11.393008] ------------[ cut here ]------------
[ 11.393031] WARNING: CPU: 5 PID: 2890 at kernel/jump_label.c:287
static_key_set_entries.isra.10+0x3c/0x50
Thanks for the report. So this is saying that the jump_entry table is
not at least 4-byte aligned. I wonder if this fixes it up?
Yes. With this patch the warning is gone.
Hi,
Thanks for testing. We probably need something like the following to
make sure we don't hit this on other arches. Steve - I will send 4
separate patches for this to get arch maintainers' acks for this?
Thanks,
-Jason
diff --git a/arch/arm/include/asm/jump_label.h
b/arch/arm/include/asm/jump_label.h
index 34f7b6980d21..9720c2f1850b 100644
--- a/arch/arm/include/asm/jump_label.h
+++ b/arch/arm/include/asm/jump_label.h
@@ -13,6 +13,7 @@ static __always_inline bool arch_static_branch(struct
static_key *key, bool bran
asm_volatile_goto("1:\n\t"
WASM(nop) "\n\t"
".pushsection __jump_table, \"aw\"\n\t"
+ ".balign 4 \n\t"
".word 1b, %l[l_yes], %c0\n\t"
".popsection\n\t"
: : "i" (&((char *)key)[branch]) : : l_yes);@@ -27,6 +28,7 @@ static __always_inline bool
arch_static_branch_jump(struct static_key *key, bool
asm_volatile_goto("1:\n\t"
WASM(b) " %l[l_yes]\n\t"
".pushsection __jump_table, \"aw\"\n\t"
+ ".balign 4 \n\t"
".word 1b, %l[l_yes], %c0\n\t"
".popsection\n\t"
: : "i" (&((char *)key)[branch]) : : l_yes);diff --git a/arch/mips/include/asm/jump_label.h
b/arch/mips/include/asm/jump_label.h
index e77672539e8e..51ce97dda3cc 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);diff --git a/arch/powerpc/include/asm/jump_label.h
b/arch/powerpc/include/asm/jump_label.h
index 9a287e0ac8b1..f870a85bac46 100644
--- a/arch/powerpc/include/asm/jump_label.h
+++ b/arch/powerpc/include/asm/jump_label.h
@@ -24,6 +24,7 @@ static __always_inline bool arch_static_branch(struct
static_key *key, bool bran
asm_volatile_goto("1:\n\t"
"nop # arch_static_branch\n\t"
".pushsection __jump_table, \"aw\"\n\t"
+ ".balign 4 \n\t"
JUMP_ENTRY_TYPE "1b, %l[l_yes], %c0\n\t"
".popsection \n\t"
: : "i" (&((char *)key)[branch]) : : l_yes);@@ -38,6 +39,7 @@ static __always_inline bool
arch_static_branch_jump(struct static_key *key, bool
asm_volatile_goto("1:\n\t"
"b %l[l_yes] # arch_static_branch_jump\n\t"
".pushsection __jump_table, \"aw\"\n\t"
+ ".balign 4 \n\t"
JUMP_ENTRY_TYPE "1b, %l[l_yes], %c0\n\t"
".popsection \n\t"
: : "i" (&((char *)key)[branch]) : : l_yes);@@ -63,6 +65,7 @@ struct jump_entry {
#define ARCH_STATIC_BRANCH(LABEL, KEY) \
1098: nop; \
.pushsection __jump_table, "aw"; \
+ .balign 4; \
FTR_ENTRY_LONG 1098b, LABEL, KEY; \
.popsection
#endifdiff --git a/arch/tile/include/asm/jump_label.h
b/arch/tile/include/asm/jump_label.h
index cde7573f397b..c9f6125c41ef 100644
--- a/arch/tile/include/asm/jump_label.h
+++ b/arch/tile/include/asm/jump_label.h
@@ -25,6 +25,7 @@ static __always_inline bool arch_static_branch(struct
static_key *key,
asm_volatile_goto("1:\n\t"
"nop" "\n\t"
".pushsection __jump_table, \"aw\"\n\t"
+ ".balign 4 \n\t"
".quad 1b, %l[l_yes], %0 + %1 \n\t"
".popsection\n\t"
: : "i" (key), "i" (branch) : : l_yes);@@ -39,6 +40,7 @@ static __always_inline bool
arch_static_branch_jump(struct static_key *key,
asm_volatile_goto("1:\n\t"
"j %l[l_yes]" "\n\t"
".pushsection __jump_table, \"aw\"\n\t"
+ ".balign 4 \n\t"
".quad 1b, %l[l_yes], %0 + %1 \n\t"
".popsection\n\t"
: : "i" (key), "i" (branch) : : l_yes);
Jason Baron [off-list ref] writes:
On 02/20/2017 10:05 PM, Sachin Sant wrote:
quoted
quoted
On 20-Feb-2017, at 8:27 PM, Jason Baron <jbaron@akamai.com
<mailto:jbaron@akamai.com>> wrote:
Hi,
On 02/19/2017 09:07 AM, Sachin Sant wrote:
quoted
While booting next-20170217 on a POWER8 LPAR following
warning is displayed.
Reverting the following commit helps boot cleanly.
commit 3821fd35b5 : jump_label: Reduce the size of struct static_key
[ 11.393008] ------------[ cut here ]------------
[ 11.393031] WARNING: CPU: 5 PID: 2890 at kernel/jump_label.c:287
static_key_set_entries.isra.10+0x3c/0x50
Thanks for the report. So this is saying that the jump_entry table is
not at least 4-byte aligned. I wonder if this fixes it up?
Yes. With this patch the warning is gone.
Hi,
Thanks for testing. We probably need something like the following to
make sure we don't hit this on other arches. Steve - I will send 4
separate patches for this to get arch maintainers' acks for this?
What's the 4 byte alignment requirement from?
On 64-bit our JUMP_ENTRY_TYPE is 8 bytes, should we be aligning to 8
bytes?
quoted hunk
diff --git a/arch/powerpc/include/asm/jump_label.h
b/arch/powerpc/include/asm/jump_label.h
index 9a287e0ac8b1..f870a85bac46 100644
--- a/arch/powerpc/include/asm/jump_label.h
+++ b/arch/powerpc/include/asm/jump_label.h
@@ -24,6 +24,7 @@ static __always_inline bool arch_static_branch(struct
static_key *key, bool bran
asm_volatile_goto("1:\n\t"
"nop # arch_static_branch\n\t"
".pushsection __jump_table, \"aw\"\n\t"
+ ".balign 4 \n\t"
Can you line those up vertically?
(That may just be an email artifact)
quoted hunk
JUMP_ENTRY_TYPE "1b, %l[l_yes], %c0\n\t"
".popsection \n\t"
: : "i" (&((char *)key)[branch]) : : l_yes);@@ -38,6 +39,7 @@ static __always_inline bool
arch_static_branch_jump(struct static_key *key, bool
asm_volatile_goto("1:\n\t"
"b %l[l_yes] # arch_static_branch_jump\n\t"
".pushsection __jump_table, \"aw\"\n\t"
+ ".balign 4 \n\t"
JUMP_ENTRY_TYPE "1b, %l[l_yes], %c0\n\t"
".popsection \n\t"
: : "i" (&((char *)key)[branch]) : : l_yes);@@ -63,6 +65,7 @@ struct jump_entry {
#define ARCH_STATIC_BRANCH(LABEL, KEY) \
1098: nop; \
.pushsection __jump_table, "aw"; \
+ .balign 4; \
FTR_ENTRY_LONG 1098b, LABEL, KEY; \
.popsection
#endif
Otherwise that looks fine assuming 4 bytes is the correct alignment.
cheers
On 02/22/2017 12:38 AM, Michael Ellerman wrote:
Jason Baron [off-list ref] writes:
quoted
On 02/20/2017 10:05 PM, Sachin Sant wrote:
quoted
quoted
On 20-Feb-2017, at 8:27 PM, Jason Baron <jbaron@akamai.com
<mailto:jbaron@akamai.com>> wrote:
Hi,
On 02/19/2017 09:07 AM, Sachin Sant wrote:
quoted
While booting next-20170217 on a POWER8 LPAR following
warning is displayed.
Reverting the following commit helps boot cleanly.
commit 3821fd35b5 : jump_label: Reduce the size of struct static_key
[ 11.393008] ------------[ cut here ]------------
[ 11.393031] WARNING: CPU: 5 PID: 2890 at kernel/jump_label.c:287
static_key_set_entries.isra.10+0x3c/0x50
Thanks for the report. So this is saying that the jump_entry table is
not at least 4-byte aligned. I wonder if this fixes it up?
Yes. With this patch the warning is gone.
Hi,
Thanks for testing. We probably need something like the following to
make sure we don't hit this on other arches. Steve - I will send 4
separate patches for this to get arch maintainers' acks for this?
What's the 4 byte alignment requirement from?
The 4 byte alignment is coming from this patch in linux-next:
https://lkml.org/lkml/2017/2/3/558
It reduces the size of 'struct static_key' by making use of the two
least significant bits of the static_key::entry pointer. Thus, the
jump_entry table needs to be 4 byte aligned to make it work. I added a
WARN_ON() to make sure the jump_entry table is in fact 4 byte aligned,
and that is what we hit here.
On 64-bit our JUMP_ENTRY_TYPE is 8 bytes, should we be aligning to 8
bytes?
4 bytes should be sufficient and apparently fixes the WARN_ON() that was
hit.
quoted
diff --git a/arch/powerpc/include/asm/jump_label.h
b/arch/powerpc/include/asm/jump_label.h
index 9a287e0ac8b1..f870a85bac46 100644
--- a/arch/powerpc/include/asm/jump_label.h
+++ b/arch/powerpc/include/asm/jump_label.h
@@ -24,6 +24,7 @@ static __always_inline bool arch_static_branch(struct
static_key *key, bool bran
asm_volatile_goto("1:\n\t"
"nop # arch_static_branch\n\t"
".pushsection __jump_table, \"aw\"\n\t"
+ ".balign 4 \n\t"
Can you line those up vertically?
(That may just be an email artifact)
sure will fix.
Thanks,
-Jason
quoted
JUMP_ENTRY_TYPE "1b, %l[l_yes], %c0\n\t"
".popsection \n\t"
: : "i" (&((char *)key)[branch]) : : l_yes);@@ -38,6 +39,7 @@ static __always_inline bool
arch_static_branch_jump(struct static_key *key, bool
asm_volatile_goto("1:\n\t"
"b %l[l_yes] # arch_static_branch_jump\n\t"
".pushsection __jump_table, \"aw\"\n\t"
+ ".balign 4 \n\t"
JUMP_ENTRY_TYPE "1b, %l[l_yes], %c0\n\t"
".popsection \n\t"
: : "i" (&((char *)key)[branch]) : : l_yes);@@ -63,6 +65,7 @@ struct jump_entry {
#define ARCH_STATIC_BRANCH(LABEL, KEY) \
1098: nop; \
.pushsection __jump_table, "aw"; \
+ .balign 4; \
FTR_ENTRY_LONG 1098b, LABEL, KEY; \
.popsection
#endif
Otherwise that looks fine assuming 4 bytes is the correct alignment.
cheers