* [RFC,PATCH] kprobes - optimized kprobes might crash before setting kernel stack @ 2011-02-14 15:12 Jiri Olsa 2011-02-15 9:41 ` Masami Hiramatsu 0 siblings, 1 reply; 26+ messages in thread From: Jiri Olsa @ 2011-02-14 15:12 UTC (permalink / raw) To: masami.hiramatsu.pt; +Cc: linux-kernel, Jiri Olsa hi, you can crash the kernel using kprobe tracer via: echo "p system_call_after_swapgs" > ./kprobe_events echo 1 > ./events/kprobes/enable The reason is that at the system_call_after_swapgs label, the kernel stack is not set up. If optimized kprobes are enabled, the user space stack is being used in this case (see optimized kprobe template) and this might result in a crash. Looks like there are several places like this over the entry_$(BIT) code. First I thought it'd be ok to localize those places, but I haven't found any reasonable/maintainable way to disable only those places. So I switched off the whole entry code from optimizing, but this also switch many safe places (attached patch - tested on x86_64). Also not sure this crash falls in to the area of that once such probe is used, user should know consequences.. any ideas? wbr, jirka Signed-off-by: Jiri Olsa <jolsa@redhat.com> --- arch/x86/ia32/ia32entry.S | 2 ++ arch/x86/kernel/entry_32.S | 6 ++++-- arch/x86/kernel/entry_64.S | 6 ++++-- arch/x86/kernel/kprobes.c | 8 ++++++++ arch/x86/kernel/vmlinux.lds.S | 1 + include/asm-generic/sections.h | 1 + include/asm-generic/vmlinux.lds.h | 6 ++++++ 7 files changed, 26 insertions(+), 4 deletions(-) diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S index 0ed7896..50f1630 100644 --- a/arch/x86/ia32/ia32entry.S +++ b/arch/x86/ia32/ia32entry.S @@ -25,6 +25,8 @@ #define sysretl_audit ia32_ret_from_sys_call #endif + .section .entry.text, "ax" + #define IA32_NR_syscalls ((ia32_syscall_end - ia32_sys_call_table)/8) .macro IA32_ARG_FIXUP noebp=0 diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S index c8b4efa..051b4e2 100644 --- a/arch/x86/kernel/entry_32.S +++ b/arch/x86/kernel/entry_32.S @@ -65,6 +65,8 @@ #define sysexit_audit syscall_exit_work #endif + .section .entry.text, "ax" + /* * We use macros for low-level operations which need to be overridden * for paravirtualization. The following will never clobber any registers: @@ -788,7 +790,7 @@ ENDPROC(ptregs_clone) */ .section .init.rodata,"a" ENTRY(interrupt) -.text +.entry.text .p2align 5 .p2align CONFIG_X86_L1_CACHE_SHIFT ENTRY(irq_entries_start) @@ -807,7 +809,7 @@ vector=FIRST_EXTERNAL_VECTOR .endif .previous .long 1b - .text + .entry.text vector=vector+1 .endif .endr diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S index aed1ffb..0a0ed79 100644 --- a/arch/x86/kernel/entry_64.S +++ b/arch/x86/kernel/entry_64.S @@ -61,6 +61,8 @@ #define __AUDIT_ARCH_LE 0x40000000 .code64 + .section .entry.text, "ax" + #ifdef CONFIG_FUNCTION_TRACER #ifdef CONFIG_DYNAMIC_FTRACE ENTRY(mcount) @@ -744,7 +746,7 @@ END(stub_rt_sigreturn) */ .section .init.rodata,"a" ENTRY(interrupt) - .text + .section .entry.text .p2align 5 .p2align CONFIG_X86_L1_CACHE_SHIFT ENTRY(irq_entries_start) @@ -763,7 +765,7 @@ vector=FIRST_EXTERNAL_VECTOR .endif .previous .quad 1b - .text + .section .entry.text vector=vector+1 .endif .endr diff --git a/arch/x86/kernel/kprobes.c b/arch/x86/kernel/kprobes.c index d91c477..d03bc1e 100644 --- a/arch/x86/kernel/kprobes.c +++ b/arch/x86/kernel/kprobes.c @@ -1276,6 +1276,14 @@ static int __kprobes can_optimize(unsigned long paddr) if (!kallsyms_lookup_size_offset(paddr, &size, &offset)) return 0; + /* + * Do not optimize in the entry code due to the unstable + * stack handling. + */ + if ((paddr >= (unsigned long ) __entry_text_start) && + (paddr < (unsigned long ) __entry_text_end)) + return 0; + /* Check there is enough space for a relative jump. */ if (size - offset < RELATIVEJUMP_SIZE) return 0; diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S index e9f7a3c..0381e1f 100644 --- a/arch/x86/kernel/vmlinux.lds.S +++ b/arch/x86/kernel/vmlinux.lds.S @@ -105,6 +105,7 @@ SECTIONS SCHED_TEXT LOCK_TEXT KPROBES_TEXT + ENTRY_TEXT IRQENTRY_TEXT *(.fixup) *(.gnu.warning) diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h index b3bfabc..c1a1216 100644 --- a/include/asm-generic/sections.h +++ b/include/asm-generic/sections.h @@ -11,6 +11,7 @@ extern char _sinittext[], _einittext[]; extern char _end[]; extern char __per_cpu_load[], __per_cpu_start[], __per_cpu_end[]; extern char __kprobes_text_start[], __kprobes_text_end[]; +extern char __entry_text_start[], __entry_text_end[]; extern char __initdata_begin[], __initdata_end[]; extern char __start_rodata[], __end_rodata[]; diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index fe77e33..906c3ce 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -424,6 +424,12 @@ *(.kprobes.text) \ VMLINUX_SYMBOL(__kprobes_text_end) = .; +#define ENTRY_TEXT \ + ALIGN_FUNCTION(); \ + VMLINUX_SYMBOL(__entry_text_start) = .; \ + *(.entry.text) \ + VMLINUX_SYMBOL(__entry_text_end) = .; + #ifdef CONFIG_FUNCTION_GRAPH_TRACER #define IRQENTRY_TEXT \ ALIGN_FUNCTION(); \ -- 1.7.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [RFC,PATCH] kprobes - optimized kprobes might crash before setting kernel stack 2011-02-14 15:12 [RFC,PATCH] kprobes - optimized kprobes might crash before setting kernel stack Jiri Olsa @ 2011-02-15 9:41 ` Masami Hiramatsu 2011-02-15 12:30 ` Jiri Olsa 0 siblings, 1 reply; 26+ messages in thread From: Masami Hiramatsu @ 2011-02-15 9:41 UTC (permalink / raw) To: Jiri Olsa; +Cc: linux-kernel (2011/02/15 0:12), Jiri Olsa wrote: > hi, > > you can crash the kernel using kprobe tracer via: > > echo "p system_call_after_swapgs" > ./kprobe_events > echo 1 > ./events/kprobes/enable Ah, thank you very much! > The reason is that at the system_call_after_swapgs label, > the kernel stack is not set up. If optimized kprobes are > enabled, the user space stack is being used in this case > (see optimized kprobe template) and this might result in a crash. Verified here, and also it didn't occur when turning optimization off by sysctl. So this is a bug of kprobe jump optimization, not kprobes itself. > Looks like there are several places like this over the entry_$(BIT) > code. First I thought it'd be ok to localize those places, but > I haven't found any reasonable/maintainable way to disable only those > places. Hmm, agreed. > So I switched off the whole entry code from optimizing, but this > also switch many safe places (attached patch - tested on x86_64). I'm OK for this solution. I think possible another solution is using interrupt stack in optprobe template too. Anyway in short term, this solution will be good. > Also not sure this crash falls in to the area of that once such > probe is used, user should know consequences.. User can see that those probe is not optimized via sysfs. Thanks again, > > any ideas? > > wbr, > jirka > > > Signed-off-by: Jiri Olsa <jolsa@redhat.com> > --- > arch/x86/ia32/ia32entry.S | 2 ++ > arch/x86/kernel/entry_32.S | 6 ++++-- > arch/x86/kernel/entry_64.S | 6 ++++-- > arch/x86/kernel/kprobes.c | 8 ++++++++ > arch/x86/kernel/vmlinux.lds.S | 1 + > include/asm-generic/sections.h | 1 + > include/asm-generic/vmlinux.lds.h | 6 ++++++ > 7 files changed, 26 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S > index 0ed7896..50f1630 100644 > --- a/arch/x86/ia32/ia32entry.S > +++ b/arch/x86/ia32/ia32entry.S > @@ -25,6 +25,8 @@ > #define sysretl_audit ia32_ret_from_sys_call > #endif > > + .section .entry.text, "ax" > + > #define IA32_NR_syscalls ((ia32_syscall_end - ia32_sys_call_table)/8) > > .macro IA32_ARG_FIXUP noebp=0 > diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S > index c8b4efa..051b4e2 100644 > --- a/arch/x86/kernel/entry_32.S > +++ b/arch/x86/kernel/entry_32.S > @@ -65,6 +65,8 @@ > #define sysexit_audit syscall_exit_work > #endif > > + .section .entry.text, "ax" > + > /* > * We use macros for low-level operations which need to be overridden > * for paravirtualization. The following will never clobber any registers: > @@ -788,7 +790,7 @@ ENDPROC(ptregs_clone) > */ > .section .init.rodata,"a" > ENTRY(interrupt) > -.text > +.entry.text > .p2align 5 > .p2align CONFIG_X86_L1_CACHE_SHIFT > ENTRY(irq_entries_start) > @@ -807,7 +809,7 @@ vector=FIRST_EXTERNAL_VECTOR > .endif > .previous > .long 1b > - .text > + .entry.text > vector=vector+1 > .endif > .endr > diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S > index aed1ffb..0a0ed79 100644 > --- a/arch/x86/kernel/entry_64.S > +++ b/arch/x86/kernel/entry_64.S > @@ -61,6 +61,8 @@ > #define __AUDIT_ARCH_LE 0x40000000 > > .code64 > + .section .entry.text, "ax" > + > #ifdef CONFIG_FUNCTION_TRACER > #ifdef CONFIG_DYNAMIC_FTRACE > ENTRY(mcount) > @@ -744,7 +746,7 @@ END(stub_rt_sigreturn) > */ > .section .init.rodata,"a" > ENTRY(interrupt) > - .text > + .section .entry.text > .p2align 5 > .p2align CONFIG_X86_L1_CACHE_SHIFT > ENTRY(irq_entries_start) > @@ -763,7 +765,7 @@ vector=FIRST_EXTERNAL_VECTOR > .endif > .previous > .quad 1b > - .text > + .section .entry.text > vector=vector+1 > .endif > .endr > diff --git a/arch/x86/kernel/kprobes.c b/arch/x86/kernel/kprobes.c > index d91c477..d03bc1e 100644 > --- a/arch/x86/kernel/kprobes.c > +++ b/arch/x86/kernel/kprobes.c > @@ -1276,6 +1276,14 @@ static int __kprobes can_optimize(unsigned long paddr) > if (!kallsyms_lookup_size_offset(paddr, &size, &offset)) > return 0; > > + /* > + * Do not optimize in the entry code due to the unstable > + * stack handling. > + */ > + if ((paddr >= (unsigned long ) __entry_text_start) && > + (paddr < (unsigned long ) __entry_text_end)) > + return 0; > + > /* Check there is enough space for a relative jump. */ > if (size - offset < RELATIVEJUMP_SIZE) > return 0; > diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S > index e9f7a3c..0381e1f 100644 > --- a/arch/x86/kernel/vmlinux.lds.S > +++ b/arch/x86/kernel/vmlinux.lds.S > @@ -105,6 +105,7 @@ SECTIONS > SCHED_TEXT > LOCK_TEXT > KPROBES_TEXT > + ENTRY_TEXT > IRQENTRY_TEXT > *(.fixup) > *(.gnu.warning) > diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h > index b3bfabc..c1a1216 100644 > --- a/include/asm-generic/sections.h > +++ b/include/asm-generic/sections.h > @@ -11,6 +11,7 @@ extern char _sinittext[], _einittext[]; > extern char _end[]; > extern char __per_cpu_load[], __per_cpu_start[], __per_cpu_end[]; > extern char __kprobes_text_start[], __kprobes_text_end[]; > +extern char __entry_text_start[], __entry_text_end[]; > extern char __initdata_begin[], __initdata_end[]; > extern char __start_rodata[], __end_rodata[]; > > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h > index fe77e33..906c3ce 100644 > --- a/include/asm-generic/vmlinux.lds.h > +++ b/include/asm-generic/vmlinux.lds.h > @@ -424,6 +424,12 @@ > *(.kprobes.text) \ > VMLINUX_SYMBOL(__kprobes_text_end) = .; > > +#define ENTRY_TEXT \ > + ALIGN_FUNCTION(); \ > + VMLINUX_SYMBOL(__entry_text_start) = .; \ > + *(.entry.text) \ > + VMLINUX_SYMBOL(__entry_text_end) = .; > + > #ifdef CONFIG_FUNCTION_GRAPH_TRACER > #define IRQENTRY_TEXT \ > ALIGN_FUNCTION(); \ -- Masami HIRAMATSU 2nd Dept. Linux Technology Center Hitachi, Ltd., Systems Development Laboratory E-mail: masami.hiramatsu.pt@hitachi.com ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFC,PATCH] kprobes - optimized kprobes might crash before setting kernel stack 2011-02-15 9:41 ` Masami Hiramatsu @ 2011-02-15 12:30 ` Jiri Olsa 2011-02-15 15:55 ` Masami Hiramatsu 0 siblings, 1 reply; 26+ messages in thread From: Jiri Olsa @ 2011-02-15 12:30 UTC (permalink / raw) To: Masami Hiramatsu; +Cc: linux-kernel On Tue, Feb 15, 2011 at 06:41:58PM +0900, Masami Hiramatsu wrote: > (2011/02/15 0:12), Jiri Olsa wrote: > > hi, > > > > you can crash the kernel using kprobe tracer via: > > > > echo "p system_call_after_swapgs" > ./kprobe_events > > echo 1 > ./events/kprobes/enable > > Ah, thank you very much! > > > The reason is that at the system_call_after_swapgs label, > > the kernel stack is not set up. If optimized kprobes are > > enabled, the user space stack is being used in this case > > (see optimized kprobe template) and this might result in a crash. > > Verified here, and also it didn't occur when turning optimization > off by sysctl. So this is a bug of kprobe jump optimization, not > kprobes itself. > > > Looks like there are several places like this over the entry_$(BIT) > > code. First I thought it'd be ok to localize those places, but > > I haven't found any reasonable/maintainable way to disable only those > > places. > > Hmm, agreed. > > > So I switched off the whole entry code from optimizing, but this > > also switch many safe places (attached patch - tested on x86_64). > > I'm OK for this solution. I think possible another solution is using > interrupt stack in optprobe template too. Anyway in short term, this > solution will be good. ok, I'll test on 32 bits and resend to Ingo > > > Also not sure this crash falls in to the area of that once such > > probe is used, user should know consequences.. > > User can see that those probe is not optimized via sysfs. I cannot find this, where can I see this info? thanks, jirka > > Thanks again, > > > > > any ideas? > > > > wbr, > > jirka > > > > > > Signed-off-by: Jiri Olsa <jolsa@redhat.com> > > --- > > arch/x86/ia32/ia32entry.S | 2 ++ > > arch/x86/kernel/entry_32.S | 6 ++++-- > > arch/x86/kernel/entry_64.S | 6 ++++-- > > arch/x86/kernel/kprobes.c | 8 ++++++++ > > arch/x86/kernel/vmlinux.lds.S | 1 + > > include/asm-generic/sections.h | 1 + > > include/asm-generic/vmlinux.lds.h | 6 ++++++ > > 7 files changed, 26 insertions(+), 4 deletions(-) > > > > diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S > > index 0ed7896..50f1630 100644 > > --- a/arch/x86/ia32/ia32entry.S > > +++ b/arch/x86/ia32/ia32entry.S > > @@ -25,6 +25,8 @@ > > #define sysretl_audit ia32_ret_from_sys_call > > #endif > > > > + .section .entry.text, "ax" > > + > > #define IA32_NR_syscalls ((ia32_syscall_end - ia32_sys_call_table)/8) > > > > .macro IA32_ARG_FIXUP noebp=0 > > diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S > > index c8b4efa..051b4e2 100644 > > --- a/arch/x86/kernel/entry_32.S > > +++ b/arch/x86/kernel/entry_32.S > > @@ -65,6 +65,8 @@ > > #define sysexit_audit syscall_exit_work > > #endif > > > > + .section .entry.text, "ax" > > + > > /* > > * We use macros for low-level operations which need to be overridden > > * for paravirtualization. The following will never clobber any registers: > > @@ -788,7 +790,7 @@ ENDPROC(ptregs_clone) > > */ > > .section .init.rodata,"a" > > ENTRY(interrupt) > > -.text > > +.entry.text > > .p2align 5 > > .p2align CONFIG_X86_L1_CACHE_SHIFT > > ENTRY(irq_entries_start) > > @@ -807,7 +809,7 @@ vector=FIRST_EXTERNAL_VECTOR > > .endif > > .previous > > .long 1b > > - .text > > + .entry.text > > vector=vector+1 > > .endif > > .endr > > diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S > > index aed1ffb..0a0ed79 100644 > > --- a/arch/x86/kernel/entry_64.S > > +++ b/arch/x86/kernel/entry_64.S > > @@ -61,6 +61,8 @@ > > #define __AUDIT_ARCH_LE 0x40000000 > > > > .code64 > > + .section .entry.text, "ax" > > + > > #ifdef CONFIG_FUNCTION_TRACER > > #ifdef CONFIG_DYNAMIC_FTRACE > > ENTRY(mcount) > > @@ -744,7 +746,7 @@ END(stub_rt_sigreturn) > > */ > > .section .init.rodata,"a" > > ENTRY(interrupt) > > - .text > > + .section .entry.text > > .p2align 5 > > .p2align CONFIG_X86_L1_CACHE_SHIFT > > ENTRY(irq_entries_start) > > @@ -763,7 +765,7 @@ vector=FIRST_EXTERNAL_VECTOR > > .endif > > .previous > > .quad 1b > > - .text > > + .section .entry.text > > vector=vector+1 > > .endif > > .endr > > diff --git a/arch/x86/kernel/kprobes.c b/arch/x86/kernel/kprobes.c > > index d91c477..d03bc1e 100644 > > --- a/arch/x86/kernel/kprobes.c > > +++ b/arch/x86/kernel/kprobes.c > > @@ -1276,6 +1276,14 @@ static int __kprobes can_optimize(unsigned long paddr) > > if (!kallsyms_lookup_size_offset(paddr, &size, &offset)) > > return 0; > > > > + /* > > + * Do not optimize in the entry code due to the unstable > > + * stack handling. > > + */ > > + if ((paddr >= (unsigned long ) __entry_text_start) && > > + (paddr < (unsigned long ) __entry_text_end)) > > + return 0; > > + > > /* Check there is enough space for a relative jump. */ > > if (size - offset < RELATIVEJUMP_SIZE) > > return 0; > > diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S > > index e9f7a3c..0381e1f 100644 > > --- a/arch/x86/kernel/vmlinux.lds.S > > +++ b/arch/x86/kernel/vmlinux.lds.S > > @@ -105,6 +105,7 @@ SECTIONS > > SCHED_TEXT > > LOCK_TEXT > > KPROBES_TEXT > > + ENTRY_TEXT > > IRQENTRY_TEXT > > *(.fixup) > > *(.gnu.warning) > > diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h > > index b3bfabc..c1a1216 100644 > > --- a/include/asm-generic/sections.h > > +++ b/include/asm-generic/sections.h > > @@ -11,6 +11,7 @@ extern char _sinittext[], _einittext[]; > > extern char _end[]; > > extern char __per_cpu_load[], __per_cpu_start[], __per_cpu_end[]; > > extern char __kprobes_text_start[], __kprobes_text_end[]; > > +extern char __entry_text_start[], __entry_text_end[]; > > extern char __initdata_begin[], __initdata_end[]; > > extern char __start_rodata[], __end_rodata[]; > > > > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h > > index fe77e33..906c3ce 100644 > > --- a/include/asm-generic/vmlinux.lds.h > > +++ b/include/asm-generic/vmlinux.lds.h > > @@ -424,6 +424,12 @@ > > *(.kprobes.text) \ > > VMLINUX_SYMBOL(__kprobes_text_end) = .; > > > > +#define ENTRY_TEXT \ > > + ALIGN_FUNCTION(); \ > > + VMLINUX_SYMBOL(__entry_text_start) = .; \ > > + *(.entry.text) \ > > + VMLINUX_SYMBOL(__entry_text_end) = .; > > + > > #ifdef CONFIG_FUNCTION_GRAPH_TRACER > > #define IRQENTRY_TEXT \ > > ALIGN_FUNCTION(); \ > > > -- > Masami HIRAMATSU > 2nd Dept. Linux Technology Center > Hitachi, Ltd., Systems Development Laboratory > E-mail: masami.hiramatsu.pt@hitachi.com ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFC,PATCH] kprobes - optimized kprobes might crash before setting kernel stack 2011-02-15 12:30 ` Jiri Olsa @ 2011-02-15 15:55 ` Masami Hiramatsu 2011-02-15 16:54 ` Jiri Olsa 2011-02-15 17:05 ` [PATCH] kprobes - do not allow optimized kprobes in entry code Jiri Olsa 0 siblings, 2 replies; 26+ messages in thread From: Masami Hiramatsu @ 2011-02-15 15:55 UTC (permalink / raw) To: Jiri Olsa; +Cc: linux-kernel (2011/02/15 21:30), Jiri Olsa wrote: > On Tue, Feb 15, 2011 at 06:41:58PM +0900, Masami Hiramatsu wrote: >> (2011/02/15 0:12), Jiri Olsa wrote: >>> hi, >>> >>> you can crash the kernel using kprobe tracer via: >>> >>> echo "p system_call_after_swapgs" > ./kprobe_events >>> echo 1 > ./events/kprobes/enable >> >> Ah, thank you very much! >> >>> The reason is that at the system_call_after_swapgs label, >>> the kernel stack is not set up. If optimized kprobes are >>> enabled, the user space stack is being used in this case >>> (see optimized kprobe template) and this might result in a crash. >> >> Verified here, and also it didn't occur when turning optimization >> off by sysctl. So this is a bug of kprobe jump optimization, not >> kprobes itself. >> >>> Looks like there are several places like this over the entry_$(BIT) >>> code. First I thought it'd be ok to localize those places, but >>> I haven't found any reasonable/maintainable way to disable only those >>> places. >> >> Hmm, agreed. >> >>> So I switched off the whole entry code from optimizing, but this >>> also switch many safe places (attached patch - tested on x86_64). >> >> I'm OK for this solution. I think possible another solution is using >> interrupt stack in optprobe template too. Anyway in short term, this >> solution will be good. > > ok, I'll test on 32 bits and resend to Ingo Thanks! And also, with deeply thinking about this problem, it seems that your idea could be the best way to fix, because kprobes can not know where the kernel stack is ready without those text section. >>> Also not sure this crash falls in to the area of that once such >>> probe is used, user should know consequences.. >> >> User can see that those probe is not optimized via sysfs. > > I cannot find this, where can I see this info? Ah, actually, that is under debugfs, which is usually mounted on /sys/kernel/debug. You can read "/sys/kernel/debug/kprobes/list" for getting a list of currently registered probes. Thank you, -- Masami HIRAMATSU 2nd Dept. Linux Technology Center Hitachi, Ltd., Systems Development Laboratory E-mail: masami.hiramatsu.pt@hitachi.com ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFC,PATCH] kprobes - optimized kprobes might crash before setting kernel stack 2011-02-15 15:55 ` Masami Hiramatsu @ 2011-02-15 16:54 ` Jiri Olsa 2011-02-15 17:05 ` [PATCH] kprobes - do not allow optimized kprobes in entry code Jiri Olsa 1 sibling, 0 replies; 26+ messages in thread From: Jiri Olsa @ 2011-02-15 16:54 UTC (permalink / raw) To: Masami Hiramatsu; +Cc: linux-kernel On Wed, Feb 16, 2011 at 12:55:53AM +0900, Masami Hiramatsu wrote: > (2011/02/15 21:30), Jiri Olsa wrote: > > On Tue, Feb 15, 2011 at 06:41:58PM +0900, Masami Hiramatsu wrote: > >> (2011/02/15 0:12), Jiri Olsa wrote: > >>> hi, > >>> > >>> you can crash the kernel using kprobe tracer via: > >>> > >>> echo "p system_call_after_swapgs" > ./kprobe_events > >>> echo 1 > ./events/kprobes/enable > >> > >> Ah, thank you very much! > >> > >>> The reason is that at the system_call_after_swapgs label, > >>> the kernel stack is not set up. If optimized kprobes are > >>> enabled, the user space stack is being used in this case > >>> (see optimized kprobe template) and this might result in a crash. > >> > >> Verified here, and also it didn't occur when turning optimization > >> off by sysctl. So this is a bug of kprobe jump optimization, not > >> kprobes itself. > >> > >>> Looks like there are several places like this over the entry_$(BIT) > >>> code. First I thought it'd be ok to localize those places, but > >>> I haven't found any reasonable/maintainable way to disable only those > >>> places. > >> > >> Hmm, agreed. > >> > >>> So I switched off the whole entry code from optimizing, but this > >>> also switch many safe places (attached patch - tested on x86_64). > >> > >> I'm OK for this solution. I think possible another solution is using > >> interrupt stack in optprobe template too. Anyway in short term, this > >> solution will be good. > > > > ok, I'll test on 32 bits and resend to Ingo > > Thanks! > And also, with deeply thinking about this problem, it seems that > your idea could be the best way to fix, because kprobes can not > know where the kernel stack is ready without those text section. > > > >>> Also not sure this crash falls in to the area of that once such > >>> probe is used, user should know consequences.. > >> > >> User can see that those probe is not optimized via sysfs. > > > > I cannot find this, where can I see this info? > > Ah, actually, that is under debugfs, which is usually mounted on > /sys/kernel/debug. You can read "/sys/kernel/debug/kprobes/list" > for getting a list of currently registered probes. I see, thanks jirka > > Thank you, > -- > Masami HIRAMATSU > 2nd Dept. Linux Technology Center > Hitachi, Ltd., Systems Development Laboratory > E-mail: masami.hiramatsu.pt@hitachi.com ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH] kprobes - do not allow optimized kprobes in entry code 2011-02-15 15:55 ` Masami Hiramatsu 2011-02-15 16:54 ` Jiri Olsa @ 2011-02-15 17:05 ` Jiri Olsa 2011-02-16 3:36 ` Masami Hiramatsu 1 sibling, 1 reply; 26+ messages in thread From: Jiri Olsa @ 2011-02-15 17:05 UTC (permalink / raw) To: Masami Hiramatsu, mingo, ananth, davem; +Cc: linux-kernel You can crash the kernel using kprobe tracer by running: echo "p system_call_after_swapgs" > ./kprobe_events echo 1 > ./events/kprobes/enable The reason is that at the system_call_after_swapgs label, the kernel stack is not set up. If optimized kprobes are enabled, the user space stack is being used in this case (see optimized kprobe template) and this might result in a crash. There are several places like this over the entry code (entry_$BIT). As it seems there's no any reasonable/maintainable way to disable only those places where the stack is not ready, I switched off the whole entry code from kprobe optimizing. wbr, jirka Signed-off-by: Jiri Olsa <jolsa@redhat.com> --- arch/x86/ia32/ia32entry.S | 2 ++ arch/x86/kernel/entry_32.S | 6 ++++-- arch/x86/kernel/entry_64.S | 6 ++++-- arch/x86/kernel/kprobes.c | 8 ++++++++ arch/x86/kernel/vmlinux.lds.S | 1 + include/asm-generic/sections.h | 1 + include/asm-generic/vmlinux.lds.h | 6 ++++++ 7 files changed, 26 insertions(+), 4 deletions(-) diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S index 0ed7896..50f1630 100644 --- a/arch/x86/ia32/ia32entry.S +++ b/arch/x86/ia32/ia32entry.S @@ -25,6 +25,8 @@ #define sysretl_audit ia32_ret_from_sys_call #endif + .section .entry.text, "ax" + #define IA32_NR_syscalls ((ia32_syscall_end - ia32_sys_call_table)/8) .macro IA32_ARG_FIXUP noebp=0 diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S index c8b4efa..f5accf8 100644 --- a/arch/x86/kernel/entry_32.S +++ b/arch/x86/kernel/entry_32.S @@ -65,6 +65,8 @@ #define sysexit_audit syscall_exit_work #endif + .section .entry.text, "ax" + /* * We use macros for low-level operations which need to be overridden * for paravirtualization. The following will never clobber any registers: @@ -788,7 +790,7 @@ ENDPROC(ptregs_clone) */ .section .init.rodata,"a" ENTRY(interrupt) -.text +.section .entry.text, "ax" .p2align 5 .p2align CONFIG_X86_L1_CACHE_SHIFT ENTRY(irq_entries_start) @@ -807,7 +809,7 @@ vector=FIRST_EXTERNAL_VECTOR .endif .previous .long 1b - .text + .section .entry.text, "ax" vector=vector+1 .endif .endr diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S index 891268c..39f8d21 100644 --- a/arch/x86/kernel/entry_64.S +++ b/arch/x86/kernel/entry_64.S @@ -61,6 +61,8 @@ #define __AUDIT_ARCH_LE 0x40000000 .code64 + .section .entry.text, "ax" + #ifdef CONFIG_FUNCTION_TRACER #ifdef CONFIG_DYNAMIC_FTRACE ENTRY(mcount) @@ -744,7 +746,7 @@ END(stub_rt_sigreturn) */ .section .init.rodata,"a" ENTRY(interrupt) - .text + .section .entry.text .p2align 5 .p2align CONFIG_X86_L1_CACHE_SHIFT ENTRY(irq_entries_start) @@ -763,7 +765,7 @@ vector=FIRST_EXTERNAL_VECTOR .endif .previous .quad 1b - .text + .section .entry.text vector=vector+1 .endif .endr diff --git a/arch/x86/kernel/kprobes.c b/arch/x86/kernel/kprobes.c index d91c477..d03bc1e 100644 --- a/arch/x86/kernel/kprobes.c +++ b/arch/x86/kernel/kprobes.c @@ -1276,6 +1276,14 @@ static int __kprobes can_optimize(unsigned long paddr) if (!kallsyms_lookup_size_offset(paddr, &size, &offset)) return 0; + /* + * Do not optimize in the entry code due to the unstable + * stack handling. + */ + if ((paddr >= (unsigned long ) __entry_text_start) && + (paddr < (unsigned long ) __entry_text_end)) + return 0; + /* Check there is enough space for a relative jump. */ if (size - offset < RELATIVEJUMP_SIZE) return 0; diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S index e9f7a3c..0381e1f 100644 --- a/arch/x86/kernel/vmlinux.lds.S +++ b/arch/x86/kernel/vmlinux.lds.S @@ -105,6 +105,7 @@ SECTIONS SCHED_TEXT LOCK_TEXT KPROBES_TEXT + ENTRY_TEXT IRQENTRY_TEXT *(.fixup) *(.gnu.warning) diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h index b3bfabc..c1a1216 100644 --- a/include/asm-generic/sections.h +++ b/include/asm-generic/sections.h @@ -11,6 +11,7 @@ extern char _sinittext[], _einittext[]; extern char _end[]; extern char __per_cpu_load[], __per_cpu_start[], __per_cpu_end[]; extern char __kprobes_text_start[], __kprobes_text_end[]; +extern char __entry_text_start[], __entry_text_end[]; extern char __initdata_begin[], __initdata_end[]; extern char __start_rodata[], __end_rodata[]; diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index fe77e33..906c3ce 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -424,6 +424,12 @@ *(.kprobes.text) \ VMLINUX_SYMBOL(__kprobes_text_end) = .; +#define ENTRY_TEXT \ + ALIGN_FUNCTION(); \ + VMLINUX_SYMBOL(__entry_text_start) = .; \ + *(.entry.text) \ + VMLINUX_SYMBOL(__entry_text_end) = .; + #ifdef CONFIG_FUNCTION_GRAPH_TRACER #define IRQENTRY_TEXT \ ALIGN_FUNCTION(); \ -- 1.7.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH] kprobes - do not allow optimized kprobes in entry code 2011-02-15 17:05 ` [PATCH] kprobes - do not allow optimized kprobes in entry code Jiri Olsa @ 2011-02-16 3:36 ` Masami Hiramatsu 2011-02-17 15:11 ` Ingo Molnar 0 siblings, 1 reply; 26+ messages in thread From: Masami Hiramatsu @ 2011-02-16 3:36 UTC (permalink / raw) To: Jiri Olsa; +Cc: mingo, ananth, davem, linux-kernel (2011/02/16 2:05), Jiri Olsa wrote: > You can crash the kernel using kprobe tracer by running: > > echo "p system_call_after_swapgs" > ./kprobe_events > echo 1 > ./events/kprobes/enable > > The reason is that at the system_call_after_swapgs label, the kernel > stack is not set up. If optimized kprobes are enabled, the user space > stack is being used in this case (see optimized kprobe template) and > this might result in a crash. > > There are several places like this over the entry code (entry_$BIT). > As it seems there's no any reasonable/maintainable way to disable only > those places where the stack is not ready, I switched off the whole > entry code from kprobe optimizing. Agreed, and this could be the best way, because kprobes can not know where the kernel stack is ready without this text section. > > wbr, > jirka > > > Signed-off-by: Jiri Olsa <jolsa@redhat.com> Looks good for me :) Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> > --- > arch/x86/ia32/ia32entry.S | 2 ++ > arch/x86/kernel/entry_32.S | 6 ++++-- > arch/x86/kernel/entry_64.S | 6 ++++-- > arch/x86/kernel/kprobes.c | 8 ++++++++ > arch/x86/kernel/vmlinux.lds.S | 1 + > include/asm-generic/sections.h | 1 + > include/asm-generic/vmlinux.lds.h | 6 ++++++ > 7 files changed, 26 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S > index 0ed7896..50f1630 100644 > --- a/arch/x86/ia32/ia32entry.S > +++ b/arch/x86/ia32/ia32entry.S > @@ -25,6 +25,8 @@ > #define sysretl_audit ia32_ret_from_sys_call > #endif > > + .section .entry.text, "ax" > + > #define IA32_NR_syscalls ((ia32_syscall_end - ia32_sys_call_table)/8) > > .macro IA32_ARG_FIXUP noebp=0 > diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S > index c8b4efa..f5accf8 100644 > --- a/arch/x86/kernel/entry_32.S > +++ b/arch/x86/kernel/entry_32.S > @@ -65,6 +65,8 @@ > #define sysexit_audit syscall_exit_work > #endif > > + .section .entry.text, "ax" > + > /* > * We use macros for low-level operations which need to be overridden > * for paravirtualization. The following will never clobber any registers: > @@ -788,7 +790,7 @@ ENDPROC(ptregs_clone) > */ > .section .init.rodata,"a" > ENTRY(interrupt) > -.text > +.section .entry.text, "ax" > .p2align 5 > .p2align CONFIG_X86_L1_CACHE_SHIFT > ENTRY(irq_entries_start) > @@ -807,7 +809,7 @@ vector=FIRST_EXTERNAL_VECTOR > .endif > .previous > .long 1b > - .text > + .section .entry.text, "ax" > vector=vector+1 > .endif > .endr > diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S > index 891268c..39f8d21 100644 > --- a/arch/x86/kernel/entry_64.S > +++ b/arch/x86/kernel/entry_64.S > @@ -61,6 +61,8 @@ > #define __AUDIT_ARCH_LE 0x40000000 > > .code64 > + .section .entry.text, "ax" > + > #ifdef CONFIG_FUNCTION_TRACER > #ifdef CONFIG_DYNAMIC_FTRACE > ENTRY(mcount) > @@ -744,7 +746,7 @@ END(stub_rt_sigreturn) > */ > .section .init.rodata,"a" > ENTRY(interrupt) > - .text > + .section .entry.text > .p2align 5 > .p2align CONFIG_X86_L1_CACHE_SHIFT > ENTRY(irq_entries_start) > @@ -763,7 +765,7 @@ vector=FIRST_EXTERNAL_VECTOR > .endif > .previous > .quad 1b > - .text > + .section .entry.text > vector=vector+1 > .endif > .endr > diff --git a/arch/x86/kernel/kprobes.c b/arch/x86/kernel/kprobes.c > index d91c477..d03bc1e 100644 > --- a/arch/x86/kernel/kprobes.c > +++ b/arch/x86/kernel/kprobes.c > @@ -1276,6 +1276,14 @@ static int __kprobes can_optimize(unsigned long paddr) > if (!kallsyms_lookup_size_offset(paddr, &size, &offset)) > return 0; > > + /* > + * Do not optimize in the entry code due to the unstable > + * stack handling. > + */ > + if ((paddr >= (unsigned long ) __entry_text_start) && > + (paddr < (unsigned long ) __entry_text_end)) > + return 0; > + > /* Check there is enough space for a relative jump. */ > if (size - offset < RELATIVEJUMP_SIZE) > return 0; > diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S > index e9f7a3c..0381e1f 100644 > --- a/arch/x86/kernel/vmlinux.lds.S > +++ b/arch/x86/kernel/vmlinux.lds.S > @@ -105,6 +105,7 @@ SECTIONS > SCHED_TEXT > LOCK_TEXT > KPROBES_TEXT > + ENTRY_TEXT > IRQENTRY_TEXT > *(.fixup) > *(.gnu.warning) > diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h > index b3bfabc..c1a1216 100644 > --- a/include/asm-generic/sections.h > +++ b/include/asm-generic/sections.h > @@ -11,6 +11,7 @@ extern char _sinittext[], _einittext[]; > extern char _end[]; > extern char __per_cpu_load[], __per_cpu_start[], __per_cpu_end[]; > extern char __kprobes_text_start[], __kprobes_text_end[]; > +extern char __entry_text_start[], __entry_text_end[]; > extern char __initdata_begin[], __initdata_end[]; > extern char __start_rodata[], __end_rodata[]; > > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h > index fe77e33..906c3ce 100644 > --- a/include/asm-generic/vmlinux.lds.h > +++ b/include/asm-generic/vmlinux.lds.h > @@ -424,6 +424,12 @@ > *(.kprobes.text) \ > VMLINUX_SYMBOL(__kprobes_text_end) = .; > > +#define ENTRY_TEXT \ > + ALIGN_FUNCTION(); \ > + VMLINUX_SYMBOL(__entry_text_start) = .; \ > + *(.entry.text) \ > + VMLINUX_SYMBOL(__entry_text_end) = .; > + > #ifdef CONFIG_FUNCTION_GRAPH_TRACER > #define IRQENTRY_TEXT \ > ALIGN_FUNCTION(); \ -- Masami HIRAMATSU 2nd Dept. Linux Technology Center Hitachi, Ltd., Systems Development Laboratory E-mail: masami.hiramatsu.pt@hitachi.com ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] kprobes - do not allow optimized kprobes in entry code 2011-02-16 3:36 ` Masami Hiramatsu @ 2011-02-17 15:11 ` Ingo Molnar 2011-02-17 15:20 ` Jiri Olsa 2011-02-18 16:26 ` Jiri Olsa 0 siblings, 2 replies; 26+ messages in thread From: Ingo Molnar @ 2011-02-17 15:11 UTC (permalink / raw) To: Masami Hiramatsu, H. Peter Anvin Cc: Jiri Olsa, ananth, davem, linux-kernel, Thomas Gleixner, Peter Zijlstra, Eric Dumazet * Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote: > (2011/02/16 2:05), Jiri Olsa wrote: > > You can crash the kernel using kprobe tracer by running: > > > > echo "p system_call_after_swapgs" > ./kprobe_events > > echo 1 > ./events/kprobes/enable > > > > The reason is that at the system_call_after_swapgs label, the kernel > > stack is not set up. If optimized kprobes are enabled, the user space > > stack is being used in this case (see optimized kprobe template) and > > this might result in a crash. > > > > There are several places like this over the entry code (entry_$BIT). > > As it seems there's no any reasonable/maintainable way to disable only > > those places where the stack is not ready, I switched off the whole > > entry code from kprobe optimizing. > > Agreed, and this could be the best way, because kprobes can not > know where the kernel stack is ready without this text section. The only worry would be that if we move the syscall entry code out of the regular text section fragments the icache layout a tiny bit, possibly hurting performance. It's probably not measurable, but we need to measure it: Testing could be done of some syscall but also cache-intense workload, like 'hackbench 10', via perf 'stat --repeat 30' and have a very close look at instruction cache eviction differences. Perhaps also explicitly enable measure one of these: L1-icache-loads [Hardware cache event] L1-icache-load-misses [Hardware cache event] L1-icache-prefetches [Hardware cache event] L1-icache-prefetch-misses [Hardware cache event] iTLB-loads [Hardware cache event] iTLB-load-misses [Hardware cache event] to see whether there's any statistically significant difference in icache/iTLB evictions, with and without the patch. If such stats are included in the changelog - even if just to show that any change is within measurement accuracy, it would make it easier to apply this change. Thanks, Ingo ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] kprobes - do not allow optimized kprobes in entry code 2011-02-17 15:11 ` Ingo Molnar @ 2011-02-17 15:20 ` Jiri Olsa 2011-02-18 16:26 ` Jiri Olsa 1 sibling, 0 replies; 26+ messages in thread From: Jiri Olsa @ 2011-02-17 15:20 UTC (permalink / raw) To: Ingo Molnar Cc: Masami Hiramatsu, H. Peter Anvin, ananth, davem, linux-kernel, Thomas Gleixner, Peter Zijlstra, Eric Dumazet On Thu, Feb 17, 2011 at 04:11:03PM +0100, Ingo Molnar wrote: > > * Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote: > > > (2011/02/16 2:05), Jiri Olsa wrote: > > > You can crash the kernel using kprobe tracer by running: > > > > > > echo "p system_call_after_swapgs" > ./kprobe_events > > > echo 1 > ./events/kprobes/enable > > > > > > The reason is that at the system_call_after_swapgs label, the kernel > > > stack is not set up. If optimized kprobes are enabled, the user space > > > stack is being used in this case (see optimized kprobe template) and > > > this might result in a crash. > > > > > > There are several places like this over the entry code (entry_$BIT). > > > As it seems there's no any reasonable/maintainable way to disable only > > > those places where the stack is not ready, I switched off the whole > > > entry code from kprobe optimizing. > > > > Agreed, and this could be the best way, because kprobes can not > > know where the kernel stack is ready without this text section. > > The only worry would be that if we move the syscall entry code out of the regular > text section fragments the icache layout a tiny bit, possibly hurting performance. > > It's probably not measurable, but we need to measure it: > > Testing could be done of some syscall but also cache-intense workload, like > 'hackbench 10', via perf 'stat --repeat 30' and have a very close look at > instruction cache eviction differences. > > Perhaps also explicitly enable measure one of these: > > L1-icache-loads [Hardware cache event] > L1-icache-load-misses [Hardware cache event] > L1-icache-prefetches [Hardware cache event] > L1-icache-prefetch-misses [Hardware cache event] > > iTLB-loads [Hardware cache event] > iTLB-load-misses [Hardware cache event] > > to see whether there's any statistically significant difference in icache/iTLB > evictions, with and without the patch. > > If such stats are included in the changelog - even if just to show that any change > is within measurement accuracy, it would make it easier to apply this change. ok, I'll run it thanks, jirka ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] kprobes - do not allow optimized kprobes in entry code 2011-02-17 15:11 ` Ingo Molnar 2011-02-17 15:20 ` Jiri Olsa @ 2011-02-18 16:26 ` Jiri Olsa 2011-02-19 14:14 ` Masami Hiramatsu 1 sibling, 1 reply; 26+ messages in thread From: Jiri Olsa @ 2011-02-18 16:26 UTC (permalink / raw) To: Ingo Molnar Cc: Masami Hiramatsu, H. Peter Anvin, ananth, davem, linux-kernel, Thomas Gleixner, Peter Zijlstra, Eric Dumazet On Thu, Feb 17, 2011 at 04:11:03PM +0100, Ingo Molnar wrote: > > * Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote: > > > (2011/02/16 2:05), Jiri Olsa wrote: > > > You can crash the kernel using kprobe tracer by running: > > > > > > echo "p system_call_after_swapgs" > ./kprobe_events > > > echo 1 > ./events/kprobes/enable > > > > > > The reason is that at the system_call_after_swapgs label, the kernel > > > stack is not set up. If optimized kprobes are enabled, the user space > > > stack is being used in this case (see optimized kprobe template) and > > > this might result in a crash. > > > > > > There are several places like this over the entry code (entry_$BIT). > > > As it seems there's no any reasonable/maintainable way to disable only > > > those places where the stack is not ready, I switched off the whole > > > entry code from kprobe optimizing. > > > > Agreed, and this could be the best way, because kprobes can not > > know where the kernel stack is ready without this text section. > > The only worry would be that if we move the syscall entry code out of the regular > text section fragments the icache layout a tiny bit, possibly hurting performance. > > It's probably not measurable, but we need to measure it: > > Testing could be done of some syscall but also cache-intense workload, like > 'hackbench 10', via perf 'stat --repeat 30' and have a very close look at > instruction cache eviction differences. > > Perhaps also explicitly enable measure one of these: > > L1-icache-loads [Hardware cache event] > L1-icache-load-misses [Hardware cache event] > L1-icache-prefetches [Hardware cache event] > L1-icache-prefetch-misses [Hardware cache event] > > iTLB-loads [Hardware cache event] > iTLB-load-misses [Hardware cache event] > > to see whether there's any statistically significant difference in icache/iTLB > evictions, with and without the patch. > > If such stats are included in the changelog - even if just to show that any change > is within measurement accuracy, it would make it easier to apply this change. > > Thanks, > > Ingo hi, I have some results, but need help with interpretation.. ;) I ran following command (with repeat 100 and 500) perf stat --repeat 100 -e L1-icache-load -e L1-icache-load-misses -e L1-icache-prefetches -e L1-icache-prefetch-misses -e iTLB-loads -e iTLB-load-misses ./hackbench/hackbench 10 I can tell just the obvious: - the cache load count is higher for the patched kernel, but the cache misses count is lower - patched kernel has also lower count of prefetches, other counts are bigger for patched kernel there's still some variability in counter values each time I run the perf please let me know what you think, I can run other tests if needed thanks, jirka -------------------------------------------------------------------------- the results for current tip tree are: Performance counter stats for './hackbench/hackbench 10' (100 runs): 815008015 L1-icache-loads ( +- 0.316% ) (scaled from 81.00%) 26267361 L1-icache-load-misses ( +- 0.210% ) (scaled from 81.00%) 204143 L1-icache-prefetches ( +- 1.291% ) (scaled from 81.01%) <not counted> L1-icache-prefetch-misses 814902708 iTLB-loads ( +- 0.315% ) (scaled from 80.99%) 82082 iTLB-load-misses ( +- 0.931% ) (scaled from 80.98%) 0.205850655 seconds time elapsed ( +- 0.333% ) Performance counter stats for './hackbench/hackbench 10' (500 runs): 817646684 L1-icache-loads ( +- 0.150% ) (scaled from 80.99%) 26282174 L1-icache-load-misses ( +- 0.099% ) (scaled from 81.00%) 211864 L1-icache-prefetches ( +- 0.616% ) (scaled from 80.99%) <not counted> L1-icache-prefetch-misses 817646737 iTLB-loads ( +- 0.151% ) (scaled from 80.98%) 82368 iTLB-load-misses ( +- 0.451% ) (scaled from 80.98%) 0.206651959 seconds time elapsed ( +- 0.152% ) -------------------------------------------------------------------------- the results for tip tree with the patch applied are: Performance counter stats for './hackbench/hackbench 10' (100 runs): 959206624 L1-icache-loads ( +- 0.320% ) (scaled from 80.98%) 24322357 L1-icache-load-misses ( +- 0.334% ) (scaled from 80.93%) 177970 L1-icache-prefetches ( +- 1.240% ) (scaled from 80.97%) <not counted> L1-icache-prefetch-misses 959349089 iTLB-loads ( +- 0.320% ) (scaled from 80.93%) 85535 iTLB-load-misses ( +- 1.329% ) (scaled from 80.92%) 0.209696972 seconds time elapsed ( +- 0.352% ) Performance counter stats for './hackbench/hackbench 10' (500 runs): 960162049 L1-icache-loads ( +- 0.114% ) (scaled from 80.95%) 24237651 L1-icache-load-misses ( +- 0.117% ) (scaled from 80.96%) 179800 L1-icache-prefetches ( +- 0.530% ) (scaled from 80.95%) <not counted> L1-icache-prefetch-misses 960352725 iTLB-loads ( +- 0.114% ) (scaled from 80.93%) 84410 iTLB-load-misses ( +- 0.491% ) (scaled from 80.92%) 0.210509948 seconds time elapsed ( +- 0.140% ) ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] kprobes - do not allow optimized kprobes in entry code 2011-02-18 16:26 ` Jiri Olsa @ 2011-02-19 14:14 ` Masami Hiramatsu 2011-02-20 12:59 ` Ingo Molnar 0 siblings, 1 reply; 26+ messages in thread From: Masami Hiramatsu @ 2011-02-19 14:14 UTC (permalink / raw) To: Jiri Olsa Cc: Ingo Molnar, H. Peter Anvin, ananth, davem, linux-kernel, Thomas Gleixner, Peter Zijlstra, Eric Dumazet, 2nddept-manager@sdl.hitachi.co.jp (2011/02/19 1:26), Jiri Olsa wrote: [...] >> The only worry would be that if we move the syscall entry code out of the regular >> text section fragments the icache layout a tiny bit, possibly hurting performance. >> >> It's probably not measurable, but we need to measure it: >> >> Testing could be done of some syscall but also cache-intense workload, like >> 'hackbench 10', via perf 'stat --repeat 30' and have a very close look at >> instruction cache eviction differences. >> >> Perhaps also explicitly enable measure one of these: >> >> L1-icache-loads [Hardware cache event] >> L1-icache-load-misses [Hardware cache event] >> L1-icache-prefetches [Hardware cache event] >> L1-icache-prefetch-misses [Hardware cache event] >> >> iTLB-loads [Hardware cache event] >> iTLB-load-misses [Hardware cache event] >> >> to see whether there's any statistically significant difference in icache/iTLB >> evictions, with and without the patch. >> >> If such stats are included in the changelog - even if just to show that any change >> is within measurement accuracy, it would make it easier to apply this change. >> >> Thanks, >> >> Ingo > > > hi, > > I have some results, but need help with interpretation.. ;) > > I ran following command (with repeat 100 and 500) > > perf stat --repeat 100 -e L1-icache-load -e L1-icache-load-misses -e > L1-icache-prefetches -e L1-icache-prefetch-misses -e iTLB-loads -e > iTLB-load-misses ./hackbench/hackbench 10 > > I can tell just the obvious: > - the cache load count is higher for the patched kernel, > but the cache misses count is lower > - patched kernel has also lower count of prefetches, > other counts are bigger for patched kernel > > there's still some variability in counter values each time I run the perf Thanks, I've also tested. (But my machine has no L1-icache-prefetches* support) What I can tell is both of L1-icache-load and L1-icache-load-misses is reduced by the patch. ;-) Thank you, -------------------------------------------------------------------------- the results for current tip tree are: $ ./perf stat --repeat 100 -e L1-icache-load -e L1-icache- load-misses -e iTLB-loads -e iTLB-load-misses hackbench 10 Performance counter stats for 'hackbench 10' (100 runs): 16,949,055 L1-icache-load ( +- 0.303% ) 1,237,453 L1-icache-load-misses ( +- 0.254% ) 40,000,357 iTLB-loads ( +- 0.257% ) 14,545 iTLB-load-misses ( +- 0.306% ) 0.171622060 seconds time elapsed ( +- 0.196% ) $ ./perf stat --repeat 500 -e L1-icache-load -e L1-icache- load-misses -e iTLB-loads -e iTLB-load-misses hackbench 10 Performance counter stats for 'hackbench 10' (500 runs): 16,896,081 L1-icache-load ( +- 0.146% ) 1,234,272 L1-icache-load-misses ( +- 0.105% ) 39,850,899 iTLB-loads ( +- 0.116% ) 14,455 iTLB-load-misses ( +- 0.119% ) 0.171901412 seconds time elapsed ( +- 0.083% ) -------------------------------------------------------------------------- the results for tip tree with the patch applied are: $ ./perf stat --repeat 100 -e L1-icache-load -e L1-icache- load-misses -e iTLB-loads -e iTLB-load-misses hackbench 10 Performance counter stats for 'hackbench 10' (100 runs): 16,819,190 L1-icache-load ( +- 0.288% ) 1,162,386 L1-icache-load-misses ( +- 0.269% ) 40,020,154 iTLB-loads ( +- 0.254% ) 14,440 iTLB-load-misses ( +- 0.220% ) 0.169014989 seconds time elapsed ( +- 0.361% ) $ ./perf stat --repeat 500 -e L1-icache-load -e L1-icache- load-misses -e iTLB-loads -e iTLB-load-misses hackbench 10 Performance counter stats for 'hackbench 10' (500 runs): 16,783,970 L1-icache-load ( +- 0.144% ) 1,155,816 L1-icache-load-misses ( +- 0.113% ) 39,958,292 iTLB-loads ( +- 0.122% ) 14,462 iTLB-load-misses ( +- 0.138% ) 0.168279115 seconds time elapsed ( +- 0.089% ) -------------------------------------------------------------------------- Here is an entry of the /proc/cpuinfo. processor : 0 vendor_id : GenuineIntel cpu family : 6 model : 26 model name : Intel(R) Core(TM) i7 CPU 920 @ 2.67GHz stepping : 4 cpu MHz : 2673.700 cache size : 8192 KB physical id : 0 siblings : 8 core id : 0 cpu cores : 4 apicid : 0 initial apicid : 0 fpu : yes fpu_exception : yes cpuid level : 11 wp : yes flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl xtopology nonstop_tsc aperfmperf pni dtes64 monitor ds_cpl vmx est tm2 ssse3 cx16 xtpr pdcm sse4_1 sse4_2 popcnt lahf_lm ida dts tpr_shadow vnmi flexpriority ept vpid bogomips : 5347.40 clflush size : 64 cache_alignment : 64 address sizes : 36 bits physical, 48 bits virtual power management: -- Masami HIRAMATSU 2nd Dept. Linux Technology Center Hitachi, Ltd., Systems Development Laboratory E-mail: masami.hiramatsu.pt@hitachi.com ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] kprobes - do not allow optimized kprobes in entry code 2011-02-19 14:14 ` Masami Hiramatsu @ 2011-02-20 12:59 ` Ingo Molnar 2011-02-21 11:54 ` Jiri Olsa 2011-02-21 14:25 ` [PATCH 0/2] x86: separating entry text section + kprobes fix Jiri Olsa 0 siblings, 2 replies; 26+ messages in thread From: Ingo Molnar @ 2011-02-20 12:59 UTC (permalink / raw) To: Masami Hiramatsu, Arnaldo Carvalho de Melo, Frédéric Weisbecker Cc: Jiri Olsa, H. Peter Anvin, ananth, davem, linux-kernel, Thomas Gleixner, Peter Zijlstra, Eric Dumazet, 2nddept-manager@sdl.hitachi.co.jp * Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote: > Thanks, I've also tested. (But my machine has no L1-icache-prefetches* support) > What I can tell is both of L1-icache-load and L1-icache-load-misses is reduced by > the patch. ;-) That's actually a pretty interesting result: it means that compressing entry code into a single section compresses the icache footprint in a measurable way. The icache miss rate went down by about 6%: > 1,234,272 L1-icache-load-misses ( +- 0.105% ) > 1,155,816 L1-icache-load-misses ( +- 0.113% ) Which, assuming that there's no kernel build and bootup related skew effect that is larger than 2-3% means that this is an improvement. perf feature request: would be nice if it was able to do: perf stat --record ... perf diff and it would show a comparison of the two runs. In hindsight it makes sense: the patch probably reduced the fragmentation of the icache for this workload. But it's still surprising :-) Mind splitting the patch up into two parts, the first one that does the performance optimization intentionally (with numbers, explanation, etc.), the second one that uses the new section for kprobes exclusion? Thanks, Ingo ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] kprobes - do not allow optimized kprobes in entry code 2011-02-20 12:59 ` Ingo Molnar @ 2011-02-21 11:54 ` Jiri Olsa 2011-02-21 14:25 ` [PATCH 0/2] x86: separating entry text section + kprobes fix Jiri Olsa 1 sibling, 0 replies; 26+ messages in thread From: Jiri Olsa @ 2011-02-21 11:54 UTC (permalink / raw) To: Ingo Molnar Cc: Masami Hiramatsu, Arnaldo Carvalho de Melo, Frédéric Weisbecker, H. Peter Anvin, ananth, davem, linux-kernel, Thomas Gleixner, Peter Zijlstra, Eric Dumazet, 2nddept-manager@sdl.hitachi.co.jp On Sun, Feb 20, 2011 at 01:59:48PM +0100, Ingo Molnar wrote: > > * Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote: > > > Thanks, I've also tested. (But my machine has no L1-icache-prefetches* support) > > What I can tell is both of L1-icache-load and L1-icache-load-misses is reduced by > > the patch. ;-) > > That's actually a pretty interesting result: it means that compressing entry code > into a single section compresses the icache footprint in a measurable way. The > icache miss rate went down by about 6%: > > > 1,234,272 L1-icache-load-misses ( +- 0.105% ) > > 1,155,816 L1-icache-load-misses ( +- 0.113% ) > > Which, assuming that there's no kernel build and bootup related skew effect that is > larger than 2-3% means that this is an improvement. > > perf feature request: would be nice if it was able to do: > > perf stat --record ... > perf diff > > and it would show a comparison of the two runs. > > In hindsight it makes sense: the patch probably reduced the fragmentation of the > icache for this workload. > > But it's still surprising :-) > > Mind splitting the patch up into two parts, the first one that does the performance > optimization intentionally (with numbers, explanation, etc.), the second one that > uses the new section for kprobes exclusion? np, I'll send v2 shortly thanks, jirka > > Thanks, > > Ingo ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 0/2] x86: separating entry text section + kprobes fix 2011-02-20 12:59 ` Ingo Molnar 2011-02-21 11:54 ` Jiri Olsa @ 2011-02-21 14:25 ` Jiri Olsa 2011-02-21 14:25 ` [PATCH 1/2] x86: separating entry text section Jiri Olsa 2011-02-21 14:25 ` [PATCH 2/2] kprobes: disabling optimized kprobes for " Jiri Olsa 1 sibling, 2 replies; 26+ messages in thread From: Jiri Olsa @ 2011-02-21 14:25 UTC (permalink / raw) To: mingo Cc: masami.hiramatsu.pt, acme, fweisbec, hpa, ananth, davem, linux-kernel, tglx, a.p.zijlstra, eric.dumazet, 2nddept-manager hi, while fixing the optimized kprobes issue, we found out that separating the entry text section might have performance benefits with regards to the instruction cache usage. attached patches: 1/2 - x86: separating entry text section 2/2 - kprobes: disabling optimized kprobes for entry text section wbr, jirka --- arch/x86/ia32/ia32entry.S | 2 ++ arch/x86/kernel/entry_32.S | 6 ++++-- arch/x86/kernel/entry_64.S | 6 ++++-- arch/x86/kernel/kprobes.c | 8 ++++++++ arch/x86/kernel/vmlinux.lds.S | 1 + include/asm-generic/sections.h | 1 + include/asm-generic/vmlinux.lds.h | 6 ++++++ 7 files changed, 26 insertions(+), 4 deletions(-) ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 1/2] x86: separating entry text section 2011-02-21 14:25 ` [PATCH 0/2] x86: separating entry text section + kprobes fix Jiri Olsa @ 2011-02-21 14:25 ` Jiri Olsa 2011-02-22 3:22 ` Masami Hiramatsu 2011-02-22 8:09 ` Ingo Molnar 2011-02-21 14:25 ` [PATCH 2/2] kprobes: disabling optimized kprobes for " Jiri Olsa 1 sibling, 2 replies; 26+ messages in thread From: Jiri Olsa @ 2011-02-21 14:25 UTC (permalink / raw) To: mingo Cc: masami.hiramatsu.pt, acme, fweisbec, hpa, ananth, davem, linux-kernel, tglx, a.p.zijlstra, eric.dumazet, 2nddept-manager, Jiri Olsa Putting x86 entry code to the separate section: .entry.text. Separating the entry text section seems to have performance benefits with regards to the instruction cache usage. Running hackbench showed that the change compresses the icache footprint. The icache miss rate went down by about 8%: before patch: 26282174 L1-icache-load-misses ( +- 0.099% ) (scaled from 81.00%) after patch: 24237651 L1-icache-load-misses ( +- 0.117% ) (scaled from 80.96%) Whole perf output follows. - results for current tip tree: Performance counter stats for './hackbench/hackbench 10' (500 runs): 817646684 L1-icache-loads ( +- 0.150% ) (scaled from 80.99%) 26282174 L1-icache-load-misses ( +- 0.099% ) (scaled from 81.00%) 211864 L1-icache-prefetches ( +- 0.616% ) (scaled from 80.99%) <not counted> L1-icache-prefetch-misses 817646737 iTLB-loads ( +- 0.151% ) (scaled from 80.98%) 82368 iTLB-load-misses ( +- 0.451% ) (scaled from 80.98%) 0.206651959 seconds time elapsed ( +- 0.152% ) - results for current tip tree with the patch applied are: Performance counter stats for './hackbench/hackbench 10' (500 runs): 960162049 L1-icache-loads ( +- 0.114% ) (scaled from 80.95%) 24237651 L1-icache-load-misses ( +- 0.117% ) (scaled from 80.96%) 179800 L1-icache-prefetches ( +- 0.530% ) (scaled from 80.95%) <not counted> L1-icache-prefetch-misses 960352725 iTLB-loads ( +- 0.114% ) (scaled from 80.93%) 84410 iTLB-load-misses ( +- 0.491% ) (scaled from 80.92%) 0.210509948 seconds time elapsed ( +- 0.140% ) wbr, jirka Signed-off-by: Jiri Olsa <jolsa@redhat.com> --- arch/x86/ia32/ia32entry.S | 2 ++ arch/x86/kernel/entry_32.S | 6 ++++-- arch/x86/kernel/entry_64.S | 6 ++++-- arch/x86/kernel/vmlinux.lds.S | 1 + include/asm-generic/sections.h | 1 + include/asm-generic/vmlinux.lds.h | 6 ++++++ 6 files changed, 18 insertions(+), 4 deletions(-) diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S index 0ed7896..50f1630 100644 --- a/arch/x86/ia32/ia32entry.S +++ b/arch/x86/ia32/ia32entry.S @@ -25,6 +25,8 @@ #define sysretl_audit ia32_ret_from_sys_call #endif + .section .entry.text, "ax" + #define IA32_NR_syscalls ((ia32_syscall_end - ia32_sys_call_table)/8) .macro IA32_ARG_FIXUP noebp=0 diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S index c8b4efa..f5accf8 100644 --- a/arch/x86/kernel/entry_32.S +++ b/arch/x86/kernel/entry_32.S @@ -65,6 +65,8 @@ #define sysexit_audit syscall_exit_work #endif + .section .entry.text, "ax" + /* * We use macros for low-level operations which need to be overridden * for paravirtualization. The following will never clobber any registers: @@ -788,7 +790,7 @@ ENDPROC(ptregs_clone) */ .section .init.rodata,"a" ENTRY(interrupt) -.text +.section .entry.text, "ax" .p2align 5 .p2align CONFIG_X86_L1_CACHE_SHIFT ENTRY(irq_entries_start) @@ -807,7 +809,7 @@ vector=FIRST_EXTERNAL_VECTOR .endif .previous .long 1b - .text + .section .entry.text, "ax" vector=vector+1 .endif .endr diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S index 891268c..39f8d21 100644 --- a/arch/x86/kernel/entry_64.S +++ b/arch/x86/kernel/entry_64.S @@ -61,6 +61,8 @@ #define __AUDIT_ARCH_LE 0x40000000 .code64 + .section .entry.text, "ax" + #ifdef CONFIG_FUNCTION_TRACER #ifdef CONFIG_DYNAMIC_FTRACE ENTRY(mcount) @@ -744,7 +746,7 @@ END(stub_rt_sigreturn) */ .section .init.rodata,"a" ENTRY(interrupt) - .text + .section .entry.text .p2align 5 .p2align CONFIG_X86_L1_CACHE_SHIFT ENTRY(irq_entries_start) @@ -763,7 +765,7 @@ vector=FIRST_EXTERNAL_VECTOR .endif .previous .quad 1b - .text + .section .entry.text vector=vector+1 .endif .endr diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S index e70cc3d..459dce2 100644 --- a/arch/x86/kernel/vmlinux.lds.S +++ b/arch/x86/kernel/vmlinux.lds.S @@ -105,6 +105,7 @@ SECTIONS SCHED_TEXT LOCK_TEXT KPROBES_TEXT + ENTRY_TEXT IRQENTRY_TEXT *(.fixup) *(.gnu.warning) diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h index b3bfabc..c1a1216 100644 --- a/include/asm-generic/sections.h +++ b/include/asm-generic/sections.h @@ -11,6 +11,7 @@ extern char _sinittext[], _einittext[]; extern char _end[]; extern char __per_cpu_load[], __per_cpu_start[], __per_cpu_end[]; extern char __kprobes_text_start[], __kprobes_text_end[]; +extern char __entry_text_start[], __entry_text_end[]; extern char __initdata_begin[], __initdata_end[]; extern char __start_rodata[], __end_rodata[]; diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index fe77e33..906c3ce 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -424,6 +424,12 @@ *(.kprobes.text) \ VMLINUX_SYMBOL(__kprobes_text_end) = .; +#define ENTRY_TEXT \ + ALIGN_FUNCTION(); \ + VMLINUX_SYMBOL(__entry_text_start) = .; \ + *(.entry.text) \ + VMLINUX_SYMBOL(__entry_text_end) = .; + #ifdef CONFIG_FUNCTION_GRAPH_TRACER #define IRQENTRY_TEXT \ ALIGN_FUNCTION(); \ -- 1.7.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] x86: separating entry text section 2011-02-21 14:25 ` [PATCH 1/2] x86: separating entry text section Jiri Olsa @ 2011-02-22 3:22 ` Masami Hiramatsu 2011-02-22 8:09 ` Ingo Molnar 1 sibling, 0 replies; 26+ messages in thread From: Masami Hiramatsu @ 2011-02-22 3:22 UTC (permalink / raw) To: Jiri Olsa Cc: mingo, acme, fweisbec, hpa, ananth, davem, linux-kernel, tglx, a.p.zijlstra, eric.dumazet, 2nddept-manager, 2nddept-manager@sdl.hitachi.co.jp (2011/02/21 23:25), Jiri Olsa wrote: > Putting x86 entry code to the separate section: .entry.text. > > Separating the entry text section seems to have performance > benefits with regards to the instruction cache usage. > > Running hackbench showed that the change compresses the icache > footprint. The icache miss rate went down by about 8%: > > before patch: > 26282174 L1-icache-load-misses ( +- 0.099% ) (scaled from 81.00%) > > after patch: > 24237651 L1-icache-load-misses ( +- 0.117% ) (scaled from 80.96%) > > > Whole perf output follows. > > - results for current tip tree: > > Performance counter stats for './hackbench/hackbench 10' (500 runs): > > 817646684 L1-icache-loads ( +- 0.150% ) (scaled from 80.99%) > 26282174 L1-icache-load-misses ( +- 0.099% ) (scaled from 81.00%) > 211864 L1-icache-prefetches ( +- 0.616% ) (scaled from 80.99%) > <not counted> L1-icache-prefetch-misses > 817646737 iTLB-loads ( +- 0.151% ) (scaled from 80.98%) > 82368 iTLB-load-misses ( +- 0.451% ) (scaled from 80.98%) > > 0.206651959 seconds time elapsed ( +- 0.152% ) > > > - results for current tip tree with the patch applied are: > > Performance counter stats for './hackbench/hackbench 10' (500 runs): > > 960162049 L1-icache-loads ( +- 0.114% ) (scaled from 80.95%) > 24237651 L1-icache-load-misses ( +- 0.117% ) (scaled from 80.96%) > 179800 L1-icache-prefetches ( +- 0.530% ) (scaled from 80.95%) > <not counted> L1-icache-prefetch-misses > 960352725 iTLB-loads ( +- 0.114% ) (scaled from 80.93%) > 84410 iTLB-load-misses ( +- 0.491% ) (scaled from 80.92%) > > 0.210509948 seconds time elapsed ( +- 0.140% ) > > > wbr, > jirka > > > Signed-off-by: Jiri Olsa <jolsa@redhat.com> Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> Thanks! > --- > arch/x86/ia32/ia32entry.S | 2 ++ > arch/x86/kernel/entry_32.S | 6 ++++-- > arch/x86/kernel/entry_64.S | 6 ++++-- > arch/x86/kernel/vmlinux.lds.S | 1 + > include/asm-generic/sections.h | 1 + > include/asm-generic/vmlinux.lds.h | 6 ++++++ > 6 files changed, 18 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S > index 0ed7896..50f1630 100644 > --- a/arch/x86/ia32/ia32entry.S > +++ b/arch/x86/ia32/ia32entry.S > @@ -25,6 +25,8 @@ > #define sysretl_audit ia32_ret_from_sys_call > #endif > > + .section .entry.text, "ax" > + > #define IA32_NR_syscalls ((ia32_syscall_end - ia32_sys_call_table)/8) > > .macro IA32_ARG_FIXUP noebp=0 > diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S > index c8b4efa..f5accf8 100644 > --- a/arch/x86/kernel/entry_32.S > +++ b/arch/x86/kernel/entry_32.S > @@ -65,6 +65,8 @@ > #define sysexit_audit syscall_exit_work > #endif > > + .section .entry.text, "ax" > + > /* > * We use macros for low-level operations which need to be overridden > * for paravirtualization. The following will never clobber any registers: > @@ -788,7 +790,7 @@ ENDPROC(ptregs_clone) > */ > .section .init.rodata,"a" > ENTRY(interrupt) > -.text > +.section .entry.text, "ax" > .p2align 5 > .p2align CONFIG_X86_L1_CACHE_SHIFT > ENTRY(irq_entries_start) > @@ -807,7 +809,7 @@ vector=FIRST_EXTERNAL_VECTOR > .endif > .previous > .long 1b > - .text > + .section .entry.text, "ax" > vector=vector+1 > .endif > .endr > diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S > index 891268c..39f8d21 100644 > --- a/arch/x86/kernel/entry_64.S > +++ b/arch/x86/kernel/entry_64.S > @@ -61,6 +61,8 @@ > #define __AUDIT_ARCH_LE 0x40000000 > > .code64 > + .section .entry.text, "ax" > + > #ifdef CONFIG_FUNCTION_TRACER > #ifdef CONFIG_DYNAMIC_FTRACE > ENTRY(mcount) > @@ -744,7 +746,7 @@ END(stub_rt_sigreturn) > */ > .section .init.rodata,"a" > ENTRY(interrupt) > - .text > + .section .entry.text > .p2align 5 > .p2align CONFIG_X86_L1_CACHE_SHIFT > ENTRY(irq_entries_start) > @@ -763,7 +765,7 @@ vector=FIRST_EXTERNAL_VECTOR > .endif > .previous > .quad 1b > - .text > + .section .entry.text > vector=vector+1 > .endif > .endr > diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S > index e70cc3d..459dce2 100644 > --- a/arch/x86/kernel/vmlinux.lds.S > +++ b/arch/x86/kernel/vmlinux.lds.S > @@ -105,6 +105,7 @@ SECTIONS > SCHED_TEXT > LOCK_TEXT > KPROBES_TEXT > + ENTRY_TEXT > IRQENTRY_TEXT > *(.fixup) > *(.gnu.warning) > diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h > index b3bfabc..c1a1216 100644 > --- a/include/asm-generic/sections.h > +++ b/include/asm-generic/sections.h > @@ -11,6 +11,7 @@ extern char _sinittext[], _einittext[]; > extern char _end[]; > extern char __per_cpu_load[], __per_cpu_start[], __per_cpu_end[]; > extern char __kprobes_text_start[], __kprobes_text_end[]; > +extern char __entry_text_start[], __entry_text_end[]; > extern char __initdata_begin[], __initdata_end[]; > extern char __start_rodata[], __end_rodata[]; > > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h > index fe77e33..906c3ce 100644 > --- a/include/asm-generic/vmlinux.lds.h > +++ b/include/asm-generic/vmlinux.lds.h > @@ -424,6 +424,12 @@ > *(.kprobes.text) \ > VMLINUX_SYMBOL(__kprobes_text_end) = .; > > +#define ENTRY_TEXT \ > + ALIGN_FUNCTION(); \ > + VMLINUX_SYMBOL(__entry_text_start) = .; \ > + *(.entry.text) \ > + VMLINUX_SYMBOL(__entry_text_end) = .; > + > #ifdef CONFIG_FUNCTION_GRAPH_TRACER > #define IRQENTRY_TEXT \ > ALIGN_FUNCTION(); \ -- Masami HIRAMATSU 2nd Dept. Linux Technology Center Hitachi, Ltd., Systems Development Laboratory E-mail: masami.hiramatsu.pt@hitachi.com ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] x86: separating entry text section 2011-02-21 14:25 ` [PATCH 1/2] x86: separating entry text section Jiri Olsa 2011-02-22 3:22 ` Masami Hiramatsu @ 2011-02-22 8:09 ` Ingo Molnar 2011-02-22 12:52 ` Jiri Olsa 1 sibling, 1 reply; 26+ messages in thread From: Ingo Molnar @ 2011-02-22 8:09 UTC (permalink / raw) To: Jiri Olsa, Arnaldo Carvalho de Melo, Frédéric Weisbecker, Peter Zijlstra Cc: masami.hiramatsu.pt, acme, fweisbec, hpa, ananth, davem, linux-kernel, tglx, a.p.zijlstra, eric.dumazet, 2nddept-manager * Jiri Olsa <jolsa@redhat.com> wrote: > Putting x86 entry code to the separate section: .entry.text. Trying to apply your patch i noticed one detail: > before patch: > 26282174 L1-icache-load-misses ( +- 0.099% ) (scaled from 81.00%) > 0.206651959 seconds time elapsed ( +- 0.152% ) > > after patch: > 24237651 L1-icache-load-misses ( +- 0.117% ) (scaled from 80.96%) > 0.210509948 seconds time elapsed ( +- 0.140% ) So time elapsed actually went up. hackbench is notoriously unstable when it comes to runtime - and increasing the --repeat value only has limited effects on that. Dropping all system caches: echo 1 > /proc/sys/vm/drop_caches Seems to do a better job of 'resetting' system state, but if we put that into the measured workload then the results are all over the place (as we now depend on IO being done): # cat hb10 echo 1 > /proc/sys/vm/drop_caches ./hackbench 10 # perf stat --repeat 3 ./hb10 Time: 0.097 Time: 0.095 Time: 0.101 Performance counter stats for './hb10' (3 runs): 21.351257 task-clock-msecs # 0.044 CPUs ( +- 27.165% ) 6 context-switches # 0.000 M/sec ( +- 34.694% ) 1 CPU-migrations # 0.000 M/sec ( +- 25.000% ) 410 page-faults # 0.019 M/sec ( +- 0.081% ) 25,407,650 cycles # 1189.984 M/sec ( +- 49.154% ) 25,407,650 instructions # 1.000 IPC ( +- 49.154% ) 5,126,580 branches # 240.107 M/sec ( +- 46.012% ) 192,272 branch-misses # 3.750 % ( +- 44.911% ) 901,701 cache-references # 42.232 M/sec ( +- 12.857% ) 802,767 cache-misses # 37.598 M/sec ( +- 9.282% ) 0.483297792 seconds time elapsed ( +- 31.152% ) So here's a perf stat feature suggestion to solve such measurement problems: a new 'pre-run' 'dry' command could be specified that is executed before the real 'hot' run is executed. Something like this: perf stat --pre-run-script ./hb10 --repeat 10 ./hackbench 10 Would do the cache-clearing before each run, it would run hackbench once (dry run) and then would run hackbench 10 for real - and would repeat the whole thing 10 times. Only the 'hot' portion of the run would be measured and displayed in the perf stat output event counts. Another observation: > 24237651 L1-icache-load-misses ( +- 0.117% ) (scaled from 80.96%) Could you please do runs that do not display 'scaled from' messages? Since we are measuring a relatively small effect here, and scaling adds noise, it would be nice to ensure that the effect persists with non-scaled events as well: You can do that by reducing the number of events that are measured. The PMU can not measure all those L1 cache events you listed - so only use the most important one and add cycles and instructions to make sure the measurements are comparable: -e L1-icache-load-misses -e instructions -e cycles Btw., there's another 'perf stat' feature suggestion: it would be nice if it was possible to 'record' a perf stat run, and do a 'perf diff' over it. That would compare the two runs all automatically, without you having to do the comparison manually. Thanks, Ingo ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] x86: separating entry text section 2011-02-22 8:09 ` Ingo Molnar @ 2011-02-22 12:52 ` Jiri Olsa 2011-03-07 10:44 ` Jiri Olsa 0 siblings, 1 reply; 26+ messages in thread From: Jiri Olsa @ 2011-02-22 12:52 UTC (permalink / raw) To: Ingo Molnar Cc: Arnaldo Carvalho de Melo, Frédéric Weisbecker, Peter Zijlstra, masami.hiramatsu.pt, hpa, ananth, davem, linux-kernel, tglx, eric.dumazet, 2nddept-manager On Tue, Feb 22, 2011 at 09:09:34AM +0100, Ingo Molnar wrote: > > * Jiri Olsa <jolsa@redhat.com> wrote: > > > Putting x86 entry code to the separate section: .entry.text. > > Trying to apply your patch i noticed one detail: > > > before patch: > > 26282174 L1-icache-load-misses ( +- 0.099% ) (scaled from 81.00%) > > 0.206651959 seconds time elapsed ( +- 0.152% ) > > > > after patch: > > 24237651 L1-icache-load-misses ( +- 0.117% ) (scaled from 80.96%) > > 0.210509948 seconds time elapsed ( +- 0.140% ) > > So time elapsed actually went up. > > hackbench is notoriously unstable when it comes to runtime - and increasing the > --repeat value only has limited effects on that. > > Dropping all system caches: > > echo 1 > /proc/sys/vm/drop_caches > > Seems to do a better job of 'resetting' system state, but if we put that into the > measured workload then the results are all over the place (as we now depend on IO > being done): > > # cat hb10 > > echo 1 > /proc/sys/vm/drop_caches > ./hackbench 10 > > # perf stat --repeat 3 ./hb10 > > Time: 0.097 > Time: 0.095 > Time: 0.101 > > Performance counter stats for './hb10' (3 runs): > > 21.351257 task-clock-msecs # 0.044 CPUs ( +- 27.165% ) > 6 context-switches # 0.000 M/sec ( +- 34.694% ) > 1 CPU-migrations # 0.000 M/sec ( +- 25.000% ) > 410 page-faults # 0.019 M/sec ( +- 0.081% ) > 25,407,650 cycles # 1189.984 M/sec ( +- 49.154% ) > 25,407,650 instructions # 1.000 IPC ( +- 49.154% ) > 5,126,580 branches # 240.107 M/sec ( +- 46.012% ) > 192,272 branch-misses # 3.750 % ( +- 44.911% ) > 901,701 cache-references # 42.232 M/sec ( +- 12.857% ) > 802,767 cache-misses # 37.598 M/sec ( +- 9.282% ) > > 0.483297792 seconds time elapsed ( +- 31.152% ) > > So here's a perf stat feature suggestion to solve such measurement problems: a new > 'pre-run' 'dry' command could be specified that is executed before the real 'hot' > run is executed. Something like this: > > perf stat --pre-run-script ./hb10 --repeat 10 ./hackbench 10 > > Would do the cache-clearing before each run, it would run hackbench once (dry run) > and then would run hackbench 10 for real - and would repeat the whole thing 10 > times. Only the 'hot' portion of the run would be measured and displayed in the perf > stat output event counts. > > Another observation: > > > 24237651 L1-icache-load-misses ( +- 0.117% ) (scaled from 80.96%) > > Could you please do runs that do not display 'scaled from' messages? Since we are > measuring a relatively small effect here, and scaling adds noise, it would be nice > to ensure that the effect persists with non-scaled events as well: > > You can do that by reducing the number of events that are measured. The PMU can not > measure all those L1 cache events you listed - so only use the most important one > and add cycles and instructions to make sure the measurements are comparable: > > -e L1-icache-load-misses -e instructions -e cycles > > Btw., there's another 'perf stat' feature suggestion: it would be nice if it was > possible to 'record' a perf stat run, and do a 'perf diff' over it. That would > compare the two runs all automatically, without you having to do the comparison > manually. hi, I made another test with "reseting" the system state as suggested and only for cache-misses together with instructions and cycles events. I can see even bigger drop of icache load misses than before from 19359739 to 16448709 (about 15%). The instruction/cycles count is slightly bigger in the patched kernel run though.. perf stat --repeat 100 -e L1-icache-load-misses -e instructions -e cycles ./hackbench/hackbench 10 ------------------------------------------------------------------------------- before patch: Performance counter stats for './hackbench/hackbench 10' (100 runs): 19359739 L1-icache-load-misses ( +- 0.313% ) 2667528936 instructions # 0.498 IPC ( +- 0.165% ) 5352849800 cycles ( +- 0.303% ) 0.205402048 seconds time elapsed ( +- 0.299% ) Performance counter stats for './hackbench/hackbench 10' (500 runs): 19417627 L1-icache-load-misses ( +- 0.147% ) 2676914223 instructions # 0.497 IPC ( +- 0.079% ) 5389516026 cycles ( +- 0.144% ) 0.206267711 seconds time elapsed ( +- 0.138% ) ------------------------------------------------------------------------------- after patch: Performance counter stats for './hackbench/hackbench 10' (100 runs): 16448709 L1-icache-load-misses ( +- 0.426% ) 2698406306 instructions # 0.500 IPC ( +- 0.177% ) 5393976267 cycles ( +- 0.321% ) 0.206072845 seconds time elapsed ( +- 0.276% ) Performance counter stats for './hackbench/hackbench 10' (500 runs): 16490788 L1-icache-load-misses ( +- 0.180% ) 2717734941 instructions # 0.502 IPC ( +- 0.079% ) 5414756975 cycles ( +- 0.148% ) 0.206747566 seconds time elapsed ( +- 0.137% ) Attaching patch with above numbers in comment. thanks, jirka --- Putting x86 entry code to the separate section: .entry.text. Separating the entry text section seems to have performance benefits with regards to the instruction cache usage. Running hackbench showed that the change compresses the icache footprint. The icache load miss rate went down by about 15%: before patch: 19417627 L1-icache-load-misses ( +- 0.147% ) after patch: 16490788 L1-icache-load-misses ( +- 0.180% ) Whole perf output follows. - results for current tip tree: Performance counter stats for './hackbench/hackbench 10' (500 runs): 19417627 L1-icache-load-misses ( +- 0.147% ) 2676914223 instructions # 0.497 IPC ( +- 0.079% ) 5389516026 cycles ( +- 0.144% ) 0.206267711 seconds time elapsed ( +- 0.138% ) - results for current tip tree with the patch applied are: Performance counter stats for './hackbench/hackbench 10' (500 runs): 16490788 L1-icache-load-misses ( +- 0.180% ) 2717734941 instructions # 0.502 IPC ( +- 0.079% ) 5414756975 cycles ( +- 0.148% ) 0.206747566 seconds time elapsed ( +- 0.137% ) wbr, jirka Signed-off-by: Jiri Olsa <jolsa@redhat.com> --- arch/x86/ia32/ia32entry.S | 2 ++ arch/x86/kernel/entry_32.S | 6 ++++-- arch/x86/kernel/entry_64.S | 6 ++++-- arch/x86/kernel/vmlinux.lds.S | 1 + include/asm-generic/sections.h | 1 + include/asm-generic/vmlinux.lds.h | 6 ++++++ 6 files changed, 18 insertions(+), 4 deletions(-) diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S index 0ed7896..50f1630 100644 --- a/arch/x86/ia32/ia32entry.S +++ b/arch/x86/ia32/ia32entry.S @@ -25,6 +25,8 @@ #define sysretl_audit ia32_ret_from_sys_call #endif + .section .entry.text, "ax" + #define IA32_NR_syscalls ((ia32_syscall_end - ia32_sys_call_table)/8) .macro IA32_ARG_FIXUP noebp=0 diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S index c8b4efa..f5accf8 100644 --- a/arch/x86/kernel/entry_32.S +++ b/arch/x86/kernel/entry_32.S @@ -65,6 +65,8 @@ #define sysexit_audit syscall_exit_work #endif + .section .entry.text, "ax" + /* * We use macros for low-level operations which need to be overridden * for paravirtualization. The following will never clobber any registers: @@ -788,7 +790,7 @@ ENDPROC(ptregs_clone) */ .section .init.rodata,"a" ENTRY(interrupt) -.text +.section .entry.text, "ax" .p2align 5 .p2align CONFIG_X86_L1_CACHE_SHIFT ENTRY(irq_entries_start) @@ -807,7 +809,7 @@ vector=FIRST_EXTERNAL_VECTOR .endif .previous .long 1b - .text + .section .entry.text, "ax" vector=vector+1 .endif .endr diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S index 891268c..39f8d21 100644 --- a/arch/x86/kernel/entry_64.S +++ b/arch/x86/kernel/entry_64.S @@ -61,6 +61,8 @@ #define __AUDIT_ARCH_LE 0x40000000 .code64 + .section .entry.text, "ax" + #ifdef CONFIG_FUNCTION_TRACER #ifdef CONFIG_DYNAMIC_FTRACE ENTRY(mcount) @@ -744,7 +746,7 @@ END(stub_rt_sigreturn) */ .section .init.rodata,"a" ENTRY(interrupt) - .text + .section .entry.text .p2align 5 .p2align CONFIG_X86_L1_CACHE_SHIFT ENTRY(irq_entries_start) @@ -763,7 +765,7 @@ vector=FIRST_EXTERNAL_VECTOR .endif .previous .quad 1b - .text + .section .entry.text vector=vector+1 .endif .endr diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S index e70cc3d..459dce2 100644 --- a/arch/x86/kernel/vmlinux.lds.S +++ b/arch/x86/kernel/vmlinux.lds.S @@ -105,6 +105,7 @@ SECTIONS SCHED_TEXT LOCK_TEXT KPROBES_TEXT + ENTRY_TEXT IRQENTRY_TEXT *(.fixup) *(.gnu.warning) diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h index b3bfabc..c1a1216 100644 --- a/include/asm-generic/sections.h +++ b/include/asm-generic/sections.h @@ -11,6 +11,7 @@ extern char _sinittext[], _einittext[]; extern char _end[]; extern char __per_cpu_load[], __per_cpu_start[], __per_cpu_end[]; extern char __kprobes_text_start[], __kprobes_text_end[]; +extern char __entry_text_start[], __entry_text_end[]; extern char __initdata_begin[], __initdata_end[]; extern char __start_rodata[], __end_rodata[]; diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index fe77e33..906c3ce 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -424,6 +424,12 @@ *(.kprobes.text) \ VMLINUX_SYMBOL(__kprobes_text_end) = .; +#define ENTRY_TEXT \ + ALIGN_FUNCTION(); \ + VMLINUX_SYMBOL(__entry_text_start) = .; \ + *(.entry.text) \ + VMLINUX_SYMBOL(__entry_text_end) = .; + #ifdef CONFIG_FUNCTION_GRAPH_TRACER #define IRQENTRY_TEXT \ ALIGN_FUNCTION(); \ -- 1.7.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] x86: separating entry text section 2011-02-22 12:52 ` Jiri Olsa @ 2011-03-07 10:44 ` Jiri Olsa 2011-03-07 15:29 ` Ingo Molnar 0 siblings, 1 reply; 26+ messages in thread From: Jiri Olsa @ 2011-03-07 10:44 UTC (permalink / raw) To: Ingo Molnar Cc: Arnaldo Carvalho de Melo, Frédéric Weisbecker, Peter Zijlstra, masami.hiramatsu.pt, hpa, ananth, davem, linux-kernel, tglx, eric.dumazet, 2nddept-manager hi, any feedback? thanks, jirka On Tue, Feb 22, 2011 at 01:52:01PM +0100, Jiri Olsa wrote: > On Tue, Feb 22, 2011 at 09:09:34AM +0100, Ingo Molnar wrote: > > > > * Jiri Olsa <jolsa@redhat.com> wrote: > > > > > Putting x86 entry code to the separate section: .entry.text. > > > > Trying to apply your patch i noticed one detail: > > > > > before patch: > > > 26282174 L1-icache-load-misses ( +- 0.099% ) (scaled from 81.00%) > > > 0.206651959 seconds time elapsed ( +- 0.152% ) > > > > > > after patch: > > > 24237651 L1-icache-load-misses ( +- 0.117% ) (scaled from 80.96%) > > > 0.210509948 seconds time elapsed ( +- 0.140% ) > > > > So time elapsed actually went up. > > > > hackbench is notoriously unstable when it comes to runtime - and increasing the > > --repeat value only has limited effects on that. > > > > Dropping all system caches: > > > > echo 1 > /proc/sys/vm/drop_caches > > > > Seems to do a better job of 'resetting' system state, but if we put that into the > > measured workload then the results are all over the place (as we now depend on IO > > being done): > > > > # cat hb10 > > > > echo 1 > /proc/sys/vm/drop_caches > > ./hackbench 10 > > > > # perf stat --repeat 3 ./hb10 > > > > Time: 0.097 > > Time: 0.095 > > Time: 0.101 > > > > Performance counter stats for './hb10' (3 runs): > > > > 21.351257 task-clock-msecs # 0.044 CPUs ( +- 27.165% ) > > 6 context-switches # 0.000 M/sec ( +- 34.694% ) > > 1 CPU-migrations # 0.000 M/sec ( +- 25.000% ) > > 410 page-faults # 0.019 M/sec ( +- 0.081% ) > > 25,407,650 cycles # 1189.984 M/sec ( +- 49.154% ) > > 25,407,650 instructions # 1.000 IPC ( +- 49.154% ) > > 5,126,580 branches # 240.107 M/sec ( +- 46.012% ) > > 192,272 branch-misses # 3.750 % ( +- 44.911% ) > > 901,701 cache-references # 42.232 M/sec ( +- 12.857% ) > > 802,767 cache-misses # 37.598 M/sec ( +- 9.282% ) > > > > 0.483297792 seconds time elapsed ( +- 31.152% ) > > > > So here's a perf stat feature suggestion to solve such measurement problems: a new > > 'pre-run' 'dry' command could be specified that is executed before the real 'hot' > > run is executed. Something like this: > > > > perf stat --pre-run-script ./hb10 --repeat 10 ./hackbench 10 > > > > Would do the cache-clearing before each run, it would run hackbench once (dry run) > > and then would run hackbench 10 for real - and would repeat the whole thing 10 > > times. Only the 'hot' portion of the run would be measured and displayed in the perf > > stat output event counts. > > > > Another observation: > > > > > 24237651 L1-icache-load-misses ( +- 0.117% ) (scaled from 80.96%) > > > > Could you please do runs that do not display 'scaled from' messages? Since we are > > measuring a relatively small effect here, and scaling adds noise, it would be nice > > to ensure that the effect persists with non-scaled events as well: > > > > You can do that by reducing the number of events that are measured. The PMU can not > > measure all those L1 cache events you listed - so only use the most important one > > and add cycles and instructions to make sure the measurements are comparable: > > > > -e L1-icache-load-misses -e instructions -e cycles > > > > Btw., there's another 'perf stat' feature suggestion: it would be nice if it was > > possible to 'record' a perf stat run, and do a 'perf diff' over it. That would > > compare the two runs all automatically, without you having to do the comparison > > manually. > > hi, > > I made another test with "reseting" the system state as suggested and > only for cache-misses together with instructions and cycles events. > > I can see even bigger drop of icache load misses than before > from 19359739 to 16448709 (about 15%). > > The instruction/cycles count is slightly bigger in the patched > kernel run though.. > > perf stat --repeat 100 -e L1-icache-load-misses -e instructions -e cycles ./hackbench/hackbench 10 > > ------------------------------------------------------------------------------- > before patch: > > Performance counter stats for './hackbench/hackbench 10' (100 runs): > > 19359739 L1-icache-load-misses ( +- 0.313% ) > 2667528936 instructions # 0.498 IPC ( +- 0.165% ) > 5352849800 cycles ( +- 0.303% ) > > 0.205402048 seconds time elapsed ( +- 0.299% ) > > Performance counter stats for './hackbench/hackbench 10' (500 runs): > > 19417627 L1-icache-load-misses ( +- 0.147% ) > 2676914223 instructions # 0.497 IPC ( +- 0.079% ) > 5389516026 cycles ( +- 0.144% ) > > 0.206267711 seconds time elapsed ( +- 0.138% ) > > > ------------------------------------------------------------------------------- > after patch: > > Performance counter stats for './hackbench/hackbench 10' (100 runs): > > 16448709 L1-icache-load-misses ( +- 0.426% ) > 2698406306 instructions # 0.500 IPC ( +- 0.177% ) > 5393976267 cycles ( +- 0.321% ) > > 0.206072845 seconds time elapsed ( +- 0.276% ) > > Performance counter stats for './hackbench/hackbench 10' (500 runs): > > 16490788 L1-icache-load-misses ( +- 0.180% ) > 2717734941 instructions # 0.502 IPC ( +- 0.079% ) > 5414756975 cycles ( +- 0.148% ) > > 0.206747566 seconds time elapsed ( +- 0.137% ) > > > Attaching patch with above numbers in comment. > > thanks, > jirka > > > --- > Putting x86 entry code to the separate section: .entry.text. > > Separating the entry text section seems to have performance > benefits with regards to the instruction cache usage. > > Running hackbench showed that the change compresses the icache > footprint. The icache load miss rate went down by about 15%: > > before patch: > 19417627 L1-icache-load-misses ( +- 0.147% ) > > after patch: > 16490788 L1-icache-load-misses ( +- 0.180% ) > > > Whole perf output follows. > > - results for current tip tree: > Performance counter stats for './hackbench/hackbench 10' (500 runs): > > 19417627 L1-icache-load-misses ( +- 0.147% ) > 2676914223 instructions # 0.497 IPC ( +- 0.079% ) > 5389516026 cycles ( +- 0.144% ) > > 0.206267711 seconds time elapsed ( +- 0.138% ) > > - results for current tip tree with the patch applied are: > Performance counter stats for './hackbench/hackbench 10' (500 runs): > > 16490788 L1-icache-load-misses ( +- 0.180% ) > 2717734941 instructions # 0.502 IPC ( +- 0.079% ) > 5414756975 cycles ( +- 0.148% ) > > 0.206747566 seconds time elapsed ( +- 0.137% ) > > > wbr, > jirka > > > Signed-off-by: Jiri Olsa <jolsa@redhat.com> > --- > arch/x86/ia32/ia32entry.S | 2 ++ > arch/x86/kernel/entry_32.S | 6 ++++-- > arch/x86/kernel/entry_64.S | 6 ++++-- > arch/x86/kernel/vmlinux.lds.S | 1 + > include/asm-generic/sections.h | 1 + > include/asm-generic/vmlinux.lds.h | 6 ++++++ > 6 files changed, 18 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S > index 0ed7896..50f1630 100644 > --- a/arch/x86/ia32/ia32entry.S > +++ b/arch/x86/ia32/ia32entry.S > @@ -25,6 +25,8 @@ > #define sysretl_audit ia32_ret_from_sys_call > #endif > > + .section .entry.text, "ax" > + > #define IA32_NR_syscalls ((ia32_syscall_end - ia32_sys_call_table)/8) > > .macro IA32_ARG_FIXUP noebp=0 > diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S > index c8b4efa..f5accf8 100644 > --- a/arch/x86/kernel/entry_32.S > +++ b/arch/x86/kernel/entry_32.S > @@ -65,6 +65,8 @@ > #define sysexit_audit syscall_exit_work > #endif > > + .section .entry.text, "ax" > + > /* > * We use macros for low-level operations which need to be overridden > * for paravirtualization. The following will never clobber any registers: > @@ -788,7 +790,7 @@ ENDPROC(ptregs_clone) > */ > .section .init.rodata,"a" > ENTRY(interrupt) > -.text > +.section .entry.text, "ax" > .p2align 5 > .p2align CONFIG_X86_L1_CACHE_SHIFT > ENTRY(irq_entries_start) > @@ -807,7 +809,7 @@ vector=FIRST_EXTERNAL_VECTOR > .endif > .previous > .long 1b > - .text > + .section .entry.text, "ax" > vector=vector+1 > .endif > .endr > diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S > index 891268c..39f8d21 100644 > --- a/arch/x86/kernel/entry_64.S > +++ b/arch/x86/kernel/entry_64.S > @@ -61,6 +61,8 @@ > #define __AUDIT_ARCH_LE 0x40000000 > > .code64 > + .section .entry.text, "ax" > + > #ifdef CONFIG_FUNCTION_TRACER > #ifdef CONFIG_DYNAMIC_FTRACE > ENTRY(mcount) > @@ -744,7 +746,7 @@ END(stub_rt_sigreturn) > */ > .section .init.rodata,"a" > ENTRY(interrupt) > - .text > + .section .entry.text > .p2align 5 > .p2align CONFIG_X86_L1_CACHE_SHIFT > ENTRY(irq_entries_start) > @@ -763,7 +765,7 @@ vector=FIRST_EXTERNAL_VECTOR > .endif > .previous > .quad 1b > - .text > + .section .entry.text > vector=vector+1 > .endif > .endr > diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S > index e70cc3d..459dce2 100644 > --- a/arch/x86/kernel/vmlinux.lds.S > +++ b/arch/x86/kernel/vmlinux.lds.S > @@ -105,6 +105,7 @@ SECTIONS > SCHED_TEXT > LOCK_TEXT > KPROBES_TEXT > + ENTRY_TEXT > IRQENTRY_TEXT > *(.fixup) > *(.gnu.warning) > diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h > index b3bfabc..c1a1216 100644 > --- a/include/asm-generic/sections.h > +++ b/include/asm-generic/sections.h > @@ -11,6 +11,7 @@ extern char _sinittext[], _einittext[]; > extern char _end[]; > extern char __per_cpu_load[], __per_cpu_start[], __per_cpu_end[]; > extern char __kprobes_text_start[], __kprobes_text_end[]; > +extern char __entry_text_start[], __entry_text_end[]; > extern char __initdata_begin[], __initdata_end[]; > extern char __start_rodata[], __end_rodata[]; > > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h > index fe77e33..906c3ce 100644 > --- a/include/asm-generic/vmlinux.lds.h > +++ b/include/asm-generic/vmlinux.lds.h > @@ -424,6 +424,12 @@ > *(.kprobes.text) \ > VMLINUX_SYMBOL(__kprobes_text_end) = .; > > +#define ENTRY_TEXT \ > + ALIGN_FUNCTION(); \ > + VMLINUX_SYMBOL(__entry_text_start) = .; \ > + *(.entry.text) \ > + VMLINUX_SYMBOL(__entry_text_end) = .; > + > #ifdef CONFIG_FUNCTION_GRAPH_TRACER > #define IRQENTRY_TEXT \ > ALIGN_FUNCTION(); \ > -- > 1.7.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] x86: separating entry text section 2011-03-07 10:44 ` Jiri Olsa @ 2011-03-07 15:29 ` Ingo Molnar 2011-03-07 18:10 ` Jiri Olsa 0 siblings, 1 reply; 26+ messages in thread From: Ingo Molnar @ 2011-03-07 15:29 UTC (permalink / raw) To: Jiri Olsa Cc: Arnaldo Carvalho de Melo, Frédéric Weisbecker, Peter Zijlstra, masami.hiramatsu.pt, hpa, ananth, davem, linux-kernel, tglx, eric.dumazet, 2nddept-manager * Jiri Olsa <jolsa@redhat.com> wrote: > hi, > any feedback? Did you get my reply below? Thanks, Ingo ----- Forwarded message from Ingo Molnar <mingo@elte.hu> ----- Date: Tue, 22 Feb 2011 09:09:34 +0100 From: Ingo Molnar <mingo@elte.hu> To: Jiri Olsa <jolsa@redhat.com>, Arnaldo Carvalho de Melo <acme@redhat.com>, Frédéric Weisbecker <fweisbec@gmail.com>, Peter Zijlstra <a.p.zijlstra@chello.nl> Cc: masami.hiramatsu.pt@hitachi.com, acme@redhat.com, fweisbec@gmail.com, hpa@zytor.com, ananth@in.ibm.com, davem@davemloft.net, linux-kernel@vger.kernel.org, tglx@linutronix.de, a.p.zijlstra@chello.nl, eric.dumazet@gmail.com, 2nddept-manager@sdl.hitachi.co.jp Subject: Re: [PATCH 1/2] x86: separating entry text section * Jiri Olsa <jolsa@redhat.com> wrote: > Putting x86 entry code to the separate section: .entry.text. Trying to apply your patch i noticed one detail: > before patch: > 26282174 L1-icache-load-misses ( +- 0.099% ) (scaled from 81.00%) > 0.206651959 seconds time elapsed ( +- 0.152% ) > > after patch: > 24237651 L1-icache-load-misses ( +- 0.117% ) (scaled from 80.96%) > 0.210509948 seconds time elapsed ( +- 0.140% ) So time elapsed actually went up. hackbench is notoriously unstable when it comes to runtime - and increasing the --repeat value only has limited effects on that. Dropping all system caches: echo 1 > /proc/sys/vm/drop_caches Seems to do a better job of 'resetting' system state, but if we put that into the measured workload then the results are all over the place (as we now depend on IO being done): # cat hb10 echo 1 > /proc/sys/vm/drop_caches ./hackbench 10 # perf stat --repeat 3 ./hb10 Time: 0.097 Time: 0.095 Time: 0.101 Performance counter stats for './hb10' (3 runs): 21.351257 task-clock-msecs # 0.044 CPUs ( +- 27.165% ) 6 context-switches # 0.000 M/sec ( +- 34.694% ) 1 CPU-migrations # 0.000 M/sec ( +- 25.000% ) 410 page-faults # 0.019 M/sec ( +- 0.081% ) 25,407,650 cycles # 1189.984 M/sec ( +- 49.154% ) 25,407,650 instructions # 1.000 IPC ( +- 49.154% ) 5,126,580 branches # 240.107 M/sec ( +- 46.012% ) 192,272 branch-misses # 3.750 % ( +- 44.911% ) 901,701 cache-references # 42.232 M/sec ( +- 12.857% ) 802,767 cache-misses # 37.598 M/sec ( +- 9.282% ) 0.483297792 seconds time elapsed ( +- 31.152% ) So here's a perf stat feature suggestion to solve such measurement problems: a new 'pre-run' 'dry' command could be specified that is executed before the real 'hot' run is executed. Something like this: perf stat --pre-run-script ./hb10 --repeat 10 ./hackbench 10 Would do the cache-clearing before each run, it would run hackbench once (dry run) and then would run hackbench 10 for real - and would repeat the whole thing 10 times. Only the 'hot' portion of the run would be measured and displayed in the perf stat output event counts. Another observation: > 24237651 L1-icache-load-misses ( +- 0.117% ) (scaled from 80.96%) Could you please do runs that do not display 'scaled from' messages? Since we are measuring a relatively small effect here, and scaling adds noise, it would be nice to ensure that the effect persists with non-scaled events as well: You can do that by reducing the number of events that are measured. The PMU can not measure all those L1 cache events you listed - so only use the most important one and add cycles and instructions to make sure the measurements are comparable: -e L1-icache-load-misses -e instructions -e cycles Btw., there's another 'perf stat' feature suggestion: it would be nice if it was possible to 'record' a perf stat run, and do a 'perf diff' over it. That would compare the two runs all automatically, without you having to do the comparison manually. Thanks, Ingo ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] x86: separating entry text section 2011-03-07 15:29 ` Ingo Molnar @ 2011-03-07 18:10 ` Jiri Olsa 2011-03-08 16:15 ` Ingo Molnar 2011-03-08 20:15 ` [tip:perf/core] x86: Separate out " tip-bot for Jiri Olsa 0 siblings, 2 replies; 26+ messages in thread From: Jiri Olsa @ 2011-03-07 18:10 UTC (permalink / raw) To: Ingo Molnar Cc: Arnaldo Carvalho de Melo, Frédéric Weisbecker, Peter Zijlstra, masami.hiramatsu.pt, hpa, ananth, davem, linux-kernel, tglx, eric.dumazet, 2nddept-manager On Mon, Mar 07, 2011 at 04:29:21PM +0100, Ingo Molnar wrote: > > * Jiri Olsa <jolsa@redhat.com> wrote: > > > hi, > > any feedback? > > Did you get my reply below? > > Thanks, > > Ingo > hi, strange, I believe I did reply on this.. got missed somehow or I'm missing simething :) reattached.. thanks, jirka --- hi, I made another test with "reseting" the system state as suggested and only for cache-misses together with instructions and cycles events. I can see even bigger drop of icache load misses than before from 19359739 to 16448709 (about 15%). The instruction/cycles count is slightly bigger in the patched kernel run though.. perf stat --repeat 100 -e L1-icache-load-misses -e instructions -e cycles ./hackbench/hackbench 10 ------------------------------------------------------------------------------- before patch: Performance counter stats for './hackbench/hackbench 10' (100 runs): 19359739 L1-icache-load-misses ( +- 0.313% ) 2667528936 instructions # 0.498 IPC ( +- 0.165% ) 5352849800 cycles ( +- 0.303% ) 0.205402048 seconds time elapsed ( +- 0.299% ) Performance counter stats for './hackbench/hackbench 10' (500 runs): 19417627 L1-icache-load-misses ( +- 0.147% ) 2676914223 instructions # 0.497 IPC ( +- 0.079% ) 5389516026 cycles ( +- 0.144% ) 0.206267711 seconds time elapsed ( +- 0.138% ) ------------------------------------------------------------------------------- after patch: Performance counter stats for './hackbench/hackbench 10' (100 runs): 16448709 L1-icache-load-misses ( +- 0.426% ) 2698406306 instructions # 0.500 IPC ( +- 0.177% ) 5393976267 cycles ( +- 0.321% ) 0.206072845 seconds time elapsed ( +- 0.276% ) Performance counter stats for './hackbench/hackbench 10' (500 runs): 16490788 L1-icache-load-misses ( +- 0.180% ) 2717734941 instructions # 0.502 IPC ( +- 0.079% ) 5414756975 cycles ( +- 0.148% ) 0.206747566 seconds time elapsed ( +- 0.137% ) Attaching patch with above numbers in comment. thanks, jirka --- Putting x86 entry code to the separate section: .entry.text. Separating the entry text section seems to have performance benefits with regards to the instruction cache usage. Running hackbench showed that the change compresses the icache footprint. The icache load miss rate went down by about 15%: before patch: 19417627 L1-icache-load-misses ( +- 0.147% ) after patch: 16490788 L1-icache-load-misses ( +- 0.180% ) Whole perf output follows. - results for current tip tree: Performance counter stats for './hackbench/hackbench 10' (500 runs): 19417627 L1-icache-load-misses ( +- 0.147% ) 2676914223 instructions # 0.497 IPC ( +- 0.079% ) 5389516026 cycles ( +- 0.144% ) 0.206267711 seconds time elapsed ( +- 0.138% ) - results for current tip tree with the patch applied are: Performance counter stats for './hackbench/hackbench 10' (500 runs): 16490788 L1-icache-load-misses ( +- 0.180% ) 2717734941 instructions # 0.502 IPC ( +- 0.079% ) 5414756975 cycles ( +- 0.148% ) 0.206747566 seconds time elapsed ( +- 0.137% ) wbr, jirka Signed-off-by: Jiri Olsa <jolsa@redhat.com> --- arch/x86/ia32/ia32entry.S | 2 ++ arch/x86/kernel/entry_32.S | 6 ++++-- arch/x86/kernel/entry_64.S | 6 ++++-- arch/x86/kernel/vmlinux.lds.S | 1 + include/asm-generic/sections.h | 1 + include/asm-generic/vmlinux.lds.h | 6 ++++++ 6 files changed, 18 insertions(+), 4 deletions(-) diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S index 0ed7896..50f1630 100644 --- a/arch/x86/ia32/ia32entry.S +++ b/arch/x86/ia32/ia32entry.S @@ -25,6 +25,8 @@ #define sysretl_audit ia32_ret_from_sys_call #endif + .section .entry.text, "ax" + #define IA32_NR_syscalls ((ia32_syscall_end - ia32_sys_call_table)/8) .macro IA32_ARG_FIXUP noebp=0 diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S index c8b4efa..f5accf8 100644 --- a/arch/x86/kernel/entry_32.S +++ b/arch/x86/kernel/entry_32.S @@ -65,6 +65,8 @@ #define sysexit_audit syscall_exit_work #endif + .section .entry.text, "ax" + /* * We use macros for low-level operations which need to be overridden * for paravirtualization. The following will never clobber any registers: @@ -788,7 +790,7 @@ ENDPROC(ptregs_clone) */ .section .init.rodata,"a" ENTRY(interrupt) -.text +.section .entry.text, "ax" .p2align 5 .p2align CONFIG_X86_L1_CACHE_SHIFT ENTRY(irq_entries_start) @@ -807,7 +809,7 @@ vector=FIRST_EXTERNAL_VECTOR .endif .previous .long 1b - .text + .section .entry.text, "ax" vector=vector+1 .endif .endr diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S index 891268c..39f8d21 100644 --- a/arch/x86/kernel/entry_64.S +++ b/arch/x86/kernel/entry_64.S @@ -61,6 +61,8 @@ #define __AUDIT_ARCH_LE 0x40000000 .code64 + .section .entry.text, "ax" + #ifdef CONFIG_FUNCTION_TRACER #ifdef CONFIG_DYNAMIC_FTRACE ENTRY(mcount) @@ -744,7 +746,7 @@ END(stub_rt_sigreturn) */ .section .init.rodata,"a" ENTRY(interrupt) - .text + .section .entry.text .p2align 5 .p2align CONFIG_X86_L1_CACHE_SHIFT ENTRY(irq_entries_start) @@ -763,7 +765,7 @@ vector=FIRST_EXTERNAL_VECTOR .endif .previous .quad 1b - .text + .section .entry.text vector=vector+1 .endif .endr diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S index e70cc3d..459dce2 100644 --- a/arch/x86/kernel/vmlinux.lds.S +++ b/arch/x86/kernel/vmlinux.lds.S @@ -105,6 +105,7 @@ SECTIONS SCHED_TEXT LOCK_TEXT KPROBES_TEXT + ENTRY_TEXT IRQENTRY_TEXT *(.fixup) *(.gnu.warning) diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h index b3bfabc..c1a1216 100644 --- a/include/asm-generic/sections.h +++ b/include/asm-generic/sections.h @@ -11,6 +11,7 @@ extern char _sinittext[], _einittext[]; extern char _end[]; extern char __per_cpu_load[], __per_cpu_start[], __per_cpu_end[]; extern char __kprobes_text_start[], __kprobes_text_end[]; +extern char __entry_text_start[], __entry_text_end[]; extern char __initdata_begin[], __initdata_end[]; extern char __start_rodata[], __end_rodata[]; diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index fe77e33..906c3ce 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -424,6 +424,12 @@ *(.kprobes.text) \ VMLINUX_SYMBOL(__kprobes_text_end) = .; +#define ENTRY_TEXT \ + ALIGN_FUNCTION(); \ + VMLINUX_SYMBOL(__entry_text_start) = .; \ + *(.entry.text) \ + VMLINUX_SYMBOL(__entry_text_end) = .; + #ifdef CONFIG_FUNCTION_GRAPH_TRACER #define IRQENTRY_TEXT \ ALIGN_FUNCTION(); \ -- 1.7.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 1/2] x86: separating entry text section 2011-03-07 18:10 ` Jiri Olsa @ 2011-03-08 16:15 ` Ingo Molnar 2011-03-08 20:15 ` [tip:perf/core] x86: Separate out " tip-bot for Jiri Olsa 1 sibling, 0 replies; 26+ messages in thread From: Ingo Molnar @ 2011-03-08 16:15 UTC (permalink / raw) To: Jiri Olsa Cc: Arnaldo Carvalho de Melo, Frédéric Weisbecker, Peter Zijlstra, masami.hiramatsu.pt, hpa, ananth, davem, linux-kernel, tglx, eric.dumazet, 2nddept-manager * Jiri Olsa <jolsa@redhat.com> wrote: > On Mon, Mar 07, 2011 at 04:29:21PM +0100, Ingo Molnar wrote: > > > > * Jiri Olsa <jolsa@redhat.com> wrote: > > > > > hi, > > > any feedback? > > > > Did you get my reply below? > > > > Thanks, > > > > Ingo > > > > hi, > > strange, I believe I did reply on this.. got missed > somehow or I'm missing simething :) reattached.. hm, the direct mail to me got lost, but i found your reply in the lkml folder. That's how i missed your reply. > I made another test with "reseting" the system state as suggested and only for > cache-misses together with instructions and cycles events. > > I can see even bigger drop of icache load misses than before > from 19359739 to 16448709 (about 15%). > > The instruction/cycles count is slightly bigger in the patched > kernel run though.. Ok, elapsed time is more stable, it went from: > 0.205402048 seconds time elapsed ( +- 0.299% ) > 0.206267711 seconds time elapsed ( +- 0.138% ) to: > 0.206072845 seconds time elapsed ( +- 0.276% ) > 0.206747566 seconds time elapsed ( +- 0.137% ) which is 0.1% slower, within the noise of the measurement. Will apply the patches, thanks Jiri! Thanks, Ingo ^ permalink raw reply [flat|nested] 26+ messages in thread
* [tip:perf/core] x86: Separate out entry text section 2011-03-07 18:10 ` Jiri Olsa 2011-03-08 16:15 ` Ingo Molnar @ 2011-03-08 20:15 ` tip-bot for Jiri Olsa 1 sibling, 0 replies; 26+ messages in thread From: tip-bot for Jiri Olsa @ 2011-03-08 20:15 UTC (permalink / raw) To: linux-tip-commits Cc: linux-kernel, acme, hpa, mingo, eric.dumazet, torvalds, a.p.zijlstra, jolsa, npiggin, fweisbec, akpm, tglx, mingo Commit-ID: ea7145477a461e09d8d194cac4b996dc4f449107 Gitweb: http://git.kernel.org/tip/ea7145477a461e09d8d194cac4b996dc4f449107 Author: Jiri Olsa <jolsa@redhat.com> AuthorDate: Mon, 7 Mar 2011 19:10:39 +0100 Committer: Ingo Molnar <mingo@elte.hu> CommitDate: Tue, 8 Mar 2011 17:22:11 +0100 x86: Separate out entry text section Put x86 entry code into a separate link section: .entry.text. Separating the entry text section seems to have performance benefits - caused by more efficient instruction cache usage. Running hackbench with perf stat --repeat showed that the change compresses the icache footprint. The icache load miss rate went down by about 15%: before patch: 19417627 L1-icache-load-misses ( +- 0.147% ) after patch: 16490788 L1-icache-load-misses ( +- 0.180% ) The motivation of the patch was to fix a particular kprobes bug that relates to the entry text section, the performance advantage was discovered accidentally. Whole perf output follows: - results for current tip tree: Performance counter stats for './hackbench/hackbench 10' (500 runs): 19417627 L1-icache-load-misses ( +- 0.147% ) 2676914223 instructions # 0.497 IPC ( +- 0.079% ) 5389516026 cycles ( +- 0.144% ) 0.206267711 seconds time elapsed ( +- 0.138% ) - results for current tip tree with the patch applied: Performance counter stats for './hackbench/hackbench 10' (500 runs): 16490788 L1-icache-load-misses ( +- 0.180% ) 2717734941 instructions # 0.502 IPC ( +- 0.079% ) 5414756975 cycles ( +- 0.148% ) 0.206747566 seconds time elapsed ( +- 0.137% ) Signed-off-by: Jiri Olsa <jolsa@redhat.com> Cc: Arnaldo Carvalho de Melo <acme@redhat.com> Cc: Frederic Weisbecker <fweisbec@gmail.com> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Nick Piggin <npiggin@kernel.dk> Cc: Eric Dumazet <eric.dumazet@gmail.com> Cc: masami.hiramatsu.pt@hitachi.com Cc: ananth@in.ibm.com Cc: davem@davemloft.net Cc: 2nddept-manager@sdl.hitachi.co.jp LKML-Reference: <20110307181039.GB15197@jolsa.redhat.com> Signed-off-by: Ingo Molnar <mingo@elte.hu> --- arch/x86/ia32/ia32entry.S | 2 ++ arch/x86/kernel/entry_32.S | 6 ++++-- arch/x86/kernel/entry_64.S | 6 ++++-- arch/x86/kernel/vmlinux.lds.S | 1 + include/asm-generic/sections.h | 1 + include/asm-generic/vmlinux.lds.h | 6 ++++++ 6 files changed, 18 insertions(+), 4 deletions(-) diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S index 518bb99..f729b2e 100644 --- a/arch/x86/ia32/ia32entry.S +++ b/arch/x86/ia32/ia32entry.S @@ -25,6 +25,8 @@ #define sysretl_audit ia32_ret_from_sys_call #endif + .section .entry.text, "ax" + #define IA32_NR_syscalls ((ia32_syscall_end - ia32_sys_call_table)/8) .macro IA32_ARG_FIXUP noebp=0 diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S index c8b4efa..f5accf8 100644 --- a/arch/x86/kernel/entry_32.S +++ b/arch/x86/kernel/entry_32.S @@ -65,6 +65,8 @@ #define sysexit_audit syscall_exit_work #endif + .section .entry.text, "ax" + /* * We use macros for low-level operations which need to be overridden * for paravirtualization. The following will never clobber any registers: @@ -788,7 +790,7 @@ ENDPROC(ptregs_clone) */ .section .init.rodata,"a" ENTRY(interrupt) -.text +.section .entry.text, "ax" .p2align 5 .p2align CONFIG_X86_L1_CACHE_SHIFT ENTRY(irq_entries_start) @@ -807,7 +809,7 @@ vector=FIRST_EXTERNAL_VECTOR .endif .previous .long 1b - .text + .section .entry.text, "ax" vector=vector+1 .endif .endr diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S index aed1ffb..0a0ed79 100644 --- a/arch/x86/kernel/entry_64.S +++ b/arch/x86/kernel/entry_64.S @@ -61,6 +61,8 @@ #define __AUDIT_ARCH_LE 0x40000000 .code64 + .section .entry.text, "ax" + #ifdef CONFIG_FUNCTION_TRACER #ifdef CONFIG_DYNAMIC_FTRACE ENTRY(mcount) @@ -744,7 +746,7 @@ END(stub_rt_sigreturn) */ .section .init.rodata,"a" ENTRY(interrupt) - .text + .section .entry.text .p2align 5 .p2align CONFIG_X86_L1_CACHE_SHIFT ENTRY(irq_entries_start) @@ -763,7 +765,7 @@ vector=FIRST_EXTERNAL_VECTOR .endif .previous .quad 1b - .text + .section .entry.text vector=vector+1 .endif .endr diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S index bf47007..6d4341d 100644 --- a/arch/x86/kernel/vmlinux.lds.S +++ b/arch/x86/kernel/vmlinux.lds.S @@ -105,6 +105,7 @@ SECTIONS SCHED_TEXT LOCK_TEXT KPROBES_TEXT + ENTRY_TEXT IRQENTRY_TEXT *(.fixup) *(.gnu.warning) diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h index b3bfabc..c1a1216 100644 --- a/include/asm-generic/sections.h +++ b/include/asm-generic/sections.h @@ -11,6 +11,7 @@ extern char _sinittext[], _einittext[]; extern char _end[]; extern char __per_cpu_load[], __per_cpu_start[], __per_cpu_end[]; extern char __kprobes_text_start[], __kprobes_text_end[]; +extern char __entry_text_start[], __entry_text_end[]; extern char __initdata_begin[], __initdata_end[]; extern char __start_rodata[], __end_rodata[]; diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index fe77e33..906c3ce 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -424,6 +424,12 @@ *(.kprobes.text) \ VMLINUX_SYMBOL(__kprobes_text_end) = .; +#define ENTRY_TEXT \ + ALIGN_FUNCTION(); \ + VMLINUX_SYMBOL(__entry_text_start) = .; \ + *(.entry.text) \ + VMLINUX_SYMBOL(__entry_text_end) = .; + #ifdef CONFIG_FUNCTION_GRAPH_TRACER #define IRQENTRY_TEXT \ ALIGN_FUNCTION(); \ ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 2/2] kprobes: disabling optimized kprobes for entry text section 2011-02-21 14:25 ` [PATCH 0/2] x86: separating entry text section + kprobes fix Jiri Olsa 2011-02-21 14:25 ` [PATCH 1/2] x86: separating entry text section Jiri Olsa @ 2011-02-21 14:25 ` Jiri Olsa 2011-02-22 3:22 ` Masami Hiramatsu 2011-03-08 20:16 ` [tip:perf/core] kprobes: Disabling " tip-bot for Jiri Olsa 1 sibling, 2 replies; 26+ messages in thread From: Jiri Olsa @ 2011-02-21 14:25 UTC (permalink / raw) To: mingo Cc: masami.hiramatsu.pt, acme, fweisbec, hpa, ananth, davem, linux-kernel, tglx, a.p.zijlstra, eric.dumazet, 2nddept-manager, Jiri Olsa You can crash the kernel using kprobe tracer by running: echo "p system_call_after_swapgs" > ./kprobe_events echo 1 > ./events/kprobes/enable The reason is that at the system_call_after_swapgs label, the kernel stack is not set up. If optimized kprobes are enabled, the user space stack is being used in this case (see optimized kprobe template) and this might result in a crash. There are several places like this over the entry code (entry_$BIT). As it seems there's no any reasonable/maintainable way to disable only those places where the stack is not ready, I switched off the whole entry code from kprobe optimizing. wbr, jirka Signed-off-by: Jiri Olsa <jolsa@redhat.com> --- arch/x86/kernel/kprobes.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/arch/x86/kernel/kprobes.c b/arch/x86/kernel/kprobes.c index d91c477..d03bc1e 100644 --- a/arch/x86/kernel/kprobes.c +++ b/arch/x86/kernel/kprobes.c @@ -1276,6 +1276,14 @@ static int __kprobes can_optimize(unsigned long paddr) if (!kallsyms_lookup_size_offset(paddr, &size, &offset)) return 0; + /* + * Do not optimize in the entry code due to the unstable + * stack handling. + */ + if ((paddr >= (unsigned long ) __entry_text_start) && + (paddr < (unsigned long ) __entry_text_end)) + return 0; + /* Check there is enough space for a relative jump. */ if (size - offset < RELATIVEJUMP_SIZE) return 0; -- 1.7.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] kprobes: disabling optimized kprobes for entry text section 2011-02-21 14:25 ` [PATCH 2/2] kprobes: disabling optimized kprobes for " Jiri Olsa @ 2011-02-22 3:22 ` Masami Hiramatsu 2011-03-08 20:16 ` [tip:perf/core] kprobes: Disabling " tip-bot for Jiri Olsa 1 sibling, 0 replies; 26+ messages in thread From: Masami Hiramatsu @ 2011-02-22 3:22 UTC (permalink / raw) To: Jiri Olsa Cc: mingo, acme, fweisbec, hpa, ananth, davem, linux-kernel, tglx, a.p.zijlstra, eric.dumazet, 2nddept-manager, 2nddept-manager@sdl.hitachi.co.jp (2011/02/21 23:25), Jiri Olsa wrote: > You can crash the kernel using kprobe tracer by running: > > echo "p system_call_after_swapgs" > ./kprobe_events > echo 1 > ./events/kprobes/enable > > The reason is that at the system_call_after_swapgs label, the kernel > stack is not set up. If optimized kprobes are enabled, the user space > stack is being used in this case (see optimized kprobe template) and > this might result in a crash. > > There are several places like this over the entry code (entry_$BIT). > As it seems there's no any reasonable/maintainable way to disable only > those places where the stack is not ready, I switched off the whole > entry code from kprobe optimizing. Thank you very much! > > wbr, > jirka > > > Signed-off-by: Jiri Olsa <jolsa@redhat.com> Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> > --- > arch/x86/kernel/kprobes.c | 8 ++++++++ > 1 files changed, 8 insertions(+), 0 deletions(-) > > diff --git a/arch/x86/kernel/kprobes.c b/arch/x86/kernel/kprobes.c > index d91c477..d03bc1e 100644 > --- a/arch/x86/kernel/kprobes.c > +++ b/arch/x86/kernel/kprobes.c > @@ -1276,6 +1276,14 @@ static int __kprobes can_optimize(unsigned long paddr) > if (!kallsyms_lookup_size_offset(paddr, &size, &offset)) > return 0; > > + /* > + * Do not optimize in the entry code due to the unstable > + * stack handling. > + */ > + if ((paddr >= (unsigned long ) __entry_text_start) && > + (paddr < (unsigned long ) __entry_text_end)) > + return 0; > + > /* Check there is enough space for a relative jump. */ > if (size - offset < RELATIVEJUMP_SIZE) > return 0; -- Masami HIRAMATSU 2nd Dept. Linux Technology Center Hitachi, Ltd., Systems Development Laboratory E-mail: masami.hiramatsu.pt@hitachi.com ^ permalink raw reply [flat|nested] 26+ messages in thread
* [tip:perf/core] kprobes: Disabling optimized kprobes for entry text section 2011-02-21 14:25 ` [PATCH 2/2] kprobes: disabling optimized kprobes for " Jiri Olsa 2011-02-22 3:22 ` Masami Hiramatsu @ 2011-03-08 20:16 ` tip-bot for Jiri Olsa 1 sibling, 0 replies; 26+ messages in thread From: tip-bot for Jiri Olsa @ 2011-03-08 20:16 UTC (permalink / raw) To: linux-tip-commits Cc: linux-kernel, hpa, mingo, masami.hiramatsu.pt, tglx, mingo, jolsa Commit-ID: 2a8247a2600c3e087a568fc68a6ec4eedac27ef1 Gitweb: http://git.kernel.org/tip/2a8247a2600c3e087a568fc68a6ec4eedac27ef1 Author: Jiri Olsa <jolsa@redhat.com> AuthorDate: Mon, 21 Feb 2011 15:25:13 +0100 Committer: Ingo Molnar <mingo@elte.hu> CommitDate: Tue, 8 Mar 2011 17:22:12 +0100 kprobes: Disabling optimized kprobes for entry text section You can crash the kernel (with root/admin privileges) using kprobe tracer by running: echo "p system_call_after_swapgs" > ./kprobe_events echo 1 > ./events/kprobes/enable The reason is that at the system_call_after_swapgs label, the kernel stack is not set up. If optimized kprobes are enabled, the user space stack is being used in this case (see optimized kprobe template) and this might result in a crash. There are several places like this over the entry code (entry_$BIT). As it seems there's no any reasonable/maintainable way to disable only those places where the stack is not ready, I switched off the whole entry code from kprobe optimizing. Signed-off-by: Jiri Olsa <jolsa@redhat.com> Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> Cc: acme@redhat.com Cc: fweisbec@gmail.com Cc: ananth@in.ibm.com Cc: davem@davemloft.net Cc: a.p.zijlstra@chello.nl Cc: eric.dumazet@gmail.com Cc: 2nddept-manager@sdl.hitachi.co.jp LKML-Reference: <1298298313-5980-3-git-send-email-jolsa@redhat.com> Signed-off-by: Ingo Molnar <mingo@elte.hu> --- arch/x86/kernel/kprobes.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/arch/x86/kernel/kprobes.c b/arch/x86/kernel/kprobes.c index d91c477..c969fd9 100644 --- a/arch/x86/kernel/kprobes.c +++ b/arch/x86/kernel/kprobes.c @@ -1276,6 +1276,14 @@ static int __kprobes can_optimize(unsigned long paddr) if (!kallsyms_lookup_size_offset(paddr, &size, &offset)) return 0; + /* + * Do not optimize in the entry code due to the unstable + * stack handling. + */ + if ((paddr >= (unsigned long )__entry_text_start) && + (paddr < (unsigned long )__entry_text_end)) + return 0; + /* Check there is enough space for a relative jump. */ if (size - offset < RELATIVEJUMP_SIZE) return 0; ^ permalink raw reply related [flat|nested] 26+ messages in thread
end of thread, other threads:[~2011-03-08 20:16 UTC | newest] Thread overview: 26+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-02-14 15:12 [RFC,PATCH] kprobes - optimized kprobes might crash before setting kernel stack Jiri Olsa 2011-02-15 9:41 ` Masami Hiramatsu 2011-02-15 12:30 ` Jiri Olsa 2011-02-15 15:55 ` Masami Hiramatsu 2011-02-15 16:54 ` Jiri Olsa 2011-02-15 17:05 ` [PATCH] kprobes - do not allow optimized kprobes in entry code Jiri Olsa 2011-02-16 3:36 ` Masami Hiramatsu 2011-02-17 15:11 ` Ingo Molnar 2011-02-17 15:20 ` Jiri Olsa 2011-02-18 16:26 ` Jiri Olsa 2011-02-19 14:14 ` Masami Hiramatsu 2011-02-20 12:59 ` Ingo Molnar 2011-02-21 11:54 ` Jiri Olsa 2011-02-21 14:25 ` [PATCH 0/2] x86: separating entry text section + kprobes fix Jiri Olsa 2011-02-21 14:25 ` [PATCH 1/2] x86: separating entry text section Jiri Olsa 2011-02-22 3:22 ` Masami Hiramatsu 2011-02-22 8:09 ` Ingo Molnar 2011-02-22 12:52 ` Jiri Olsa 2011-03-07 10:44 ` Jiri Olsa 2011-03-07 15:29 ` Ingo Molnar 2011-03-07 18:10 ` Jiri Olsa 2011-03-08 16:15 ` Ingo Molnar 2011-03-08 20:15 ` [tip:perf/core] x86: Separate out " tip-bot for Jiri Olsa 2011-02-21 14:25 ` [PATCH 2/2] kprobes: disabling optimized kprobes for " Jiri Olsa 2011-02-22 3:22 ` Masami Hiramatsu 2011-03-08 20:16 ` [tip:perf/core] kprobes: Disabling " tip-bot for Jiri Olsa
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox