* [PATCH perf 0/4] Build fixes for gcc 6
@ 2016-01-19 21:32 Ben Hutchings
2016-01-19 21:32 ` [PATCH perf 1/4] perf tools: Fix wrong indentation and build failure with " Ben Hutchings
` (4 more replies)
0 siblings, 5 replies; 22+ messages in thread
From: Ben Hutchings @ 2016-01-19 21:32 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo; +Cc: linux-kernel
[-- Attachment #1: Type: text/plain, Size: 752 bytes --]
gcc 6 warns about various things in tools/perf and with -Werror
these turn into build failures. One of them is a real though not
very serious bug.
Ben.
Ben Hutchings (4):
perf tools: Fix wrong indentation and build failure with gcc 6
perf top: Fix behaviour of Shift-Tab in annotated view with nothing
focussed
perf tools: Fix unused variables: x86_{32,64}_regoffset_table
perf tests: Delete mis-indented dead code that causes build failure
with gcc 6
tools/perf/arch/x86/tests/intel-cqm.c | 5 +++--
tools/perf/arch/x86/util/dwarf-regs.c | 38 ++++++++++++++++-------------------
tools/perf/ui/browsers/annotate.c | 4 ++--
tools/perf/util/pmu.c | 2 +-
4 files changed, 23 insertions(+), 26 deletions(-)
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH perf 1/4] perf tools: Fix wrong indentation and build failure with gcc 6 2016-01-19 21:32 [PATCH perf 0/4] Build fixes for gcc 6 Ben Hutchings @ 2016-01-19 21:32 ` Ben Hutchings 2016-01-19 21:32 ` [PATCH perf 2/4] perf top: Fix behaviour of Shift-Tab in annotated view with nothing focussed Ben Hutchings ` (3 subsequent siblings) 4 siblings, 0 replies; 22+ messages in thread From: Ben Hutchings @ 2016-01-19 21:32 UTC (permalink / raw) To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo; +Cc: linux-kernel [-- Attachment #1: Type: text/plain, Size: 989 bytes --] One line in perf_pmu__parse_unit() is indented wrongly, leading to a warning (=> error) from gcc 6: util/pmu.c:156:3: error: statement is indented as if it were guarded by... [-Werror=misleading-indentation] sret = read(fd, alias->unit, UNIT_MAX_LEN); ^~~~ util/pmu.c:153:2: note: ...this 'if' clause, but it is not if (fd == -1) ^~ Fixes: 410136f5dd96 ("tools/perf/stat: Add event unit and scale support") Signed-off-by: Ben Hutchings <ben@decadent.org.uk> --- tools/perf/util/pmu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c index e4b173d..c900b66 100644 --- a/tools/perf/util/pmu.c +++ b/tools/perf/util/pmu.c @@ -153,7 +153,7 @@ static int perf_pmu__parse_unit(struct perf_pmu_alias *alias, char *dir, char *n if (fd == -1) return -1; - sret = read(fd, alias->unit, UNIT_MAX_LEN); + sret = read(fd, alias->unit, UNIT_MAX_LEN); if (sret < 0) goto error; [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 811 bytes --] ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH perf 2/4] perf top: Fix behaviour of Shift-Tab in annotated view with nothing focussed 2016-01-19 21:32 [PATCH perf 0/4] Build fixes for gcc 6 Ben Hutchings 2016-01-19 21:32 ` [PATCH perf 1/4] perf tools: Fix wrong indentation and build failure with " Ben Hutchings @ 2016-01-19 21:32 ` Ben Hutchings 2016-01-19 21:33 ` [PATCH perf 3/4] perf tools: Fix unused variables: x86_{32,64}_regoffset_table Ben Hutchings ` (2 subsequent siblings) 4 siblings, 0 replies; 22+ messages in thread From: Ben Hutchings @ 2016-01-19 21:32 UTC (permalink / raw) To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo; +Cc: linux-kernel [-- Attachment #1: Type: text/plain, Size: 1094 bytes --] gcc 6 catches a case of missing braces: ui/browsers/annotate.c: In function 'annotate_browser__run': ui/browsers/annotate.c:760:5: error: statement is indented as if it were guarded by... [-Werror=misleading-indentation] if (nd == NULL) ^~ ui/browsers/annotate.c:758:4: note: ...this 'if' clause, but it is not if (nd != NULL) ^~ Fixes: c97cf42219b7 ("perf top: Live TUI Annotation") Signed-off-by: Ben Hutchings <ben@decadent.org.uk> --- tools/perf/ui/browsers/annotate.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c index d4d7cc2..718bd46 100644 --- a/tools/perf/ui/browsers/annotate.c +++ b/tools/perf/ui/browsers/annotate.c @@ -755,11 +755,11 @@ static int annotate_browser__run(struct annotate_browser *browser, nd = browser->curr_hot; break; case K_UNTAB: - if (nd != NULL) + if (nd != NULL) { nd = rb_next(nd); if (nd == NULL) nd = rb_first(&browser->entries); - else + } else nd = browser->curr_hot; break; case K_F1: [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 811 bytes --] ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH perf 3/4] perf tools: Fix unused variables: x86_{32,64}_regoffset_table 2016-01-19 21:32 [PATCH perf 0/4] Build fixes for gcc 6 Ben Hutchings 2016-01-19 21:32 ` [PATCH perf 1/4] perf tools: Fix wrong indentation and build failure with " Ben Hutchings 2016-01-19 21:32 ` [PATCH perf 2/4] perf top: Fix behaviour of Shift-Tab in annotated view with nothing focussed Ben Hutchings @ 2016-01-19 21:33 ` Ben Hutchings 2016-01-20 13:59 ` Arnaldo Carvalho de Melo 2016-01-19 21:33 ` [PATCH perf 4/4] perf tests: Delete mis-indented dead code that causes build failure with gcc 6 Ben Hutchings 2016-01-19 21:40 ` [PATCH perf 0/4] Build fixes for " Markus Trippelsdorf 4 siblings, 1 reply; 22+ messages in thread From: Ben Hutchings @ 2016-01-19 21:33 UTC (permalink / raw) To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo; +Cc: linux-kernel [-- Attachment #1: Type: text/plain, Size: 2585 bytes --] gcc 5 doesn't seem to care about these, but gcc 6 does and that results in a build failure. Fixes: bbbe6bf6037d ("perf tools: Introduce regs_query_register_offset() ...") Signed-off-by: Ben Hutchings <ben@decadent.org.uk> --- tools/perf/arch/x86/util/dwarf-regs.c | 38 ++++++++++++++++------------------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/tools/perf/arch/x86/util/dwarf-regs.c b/tools/perf/arch/x86/util/dwarf-regs.c index 9223c16..fe1e516 100644 --- a/tools/perf/arch/x86/util/dwarf-regs.c +++ b/tools/perf/arch/x86/util/dwarf-regs.c @@ -55,26 +55,10 @@ struct pt_regs_offset { #define REG_OFFSET_END {.name = NULL, .offset = 0} +/* TODO: switching by dwarf address size */ #ifdef __x86_64__ -# define REG_OFFSET_NAME_64(n, r) {.name = n, .offset = offsetof(struct pt_regs, r)} -# define REG_OFFSET_NAME_32(n, r) {.name = n, .offset = -1} -#else -# define REG_OFFSET_NAME_64(n, r) {.name = n, .offset = -1} -# define REG_OFFSET_NAME_32(n, r) {.name = n, .offset = offsetof(struct pt_regs, r)} -#endif - -static const struct pt_regs_offset x86_32_regoffset_table[] = { - REG_OFFSET_NAME_32("%ax", eax), - REG_OFFSET_NAME_32("%cx", ecx), - REG_OFFSET_NAME_32("%dx", edx), - REG_OFFSET_NAME_32("%bx", ebx), - REG_OFFSET_NAME_32("$stack", esp), /* Stack address instead of %sp */ - REG_OFFSET_NAME_32("%bp", ebp), - REG_OFFSET_NAME_32("%si", esi), - REG_OFFSET_NAME_32("%di", edi), - REG_OFFSET_END, -}; +#define REG_OFFSET_NAME_64(n, r) {.name = n, .offset = offsetof(struct pt_regs, r)} static const struct pt_regs_offset x86_64_regoffset_table[] = { REG_OFFSET_NAME_64("%ax", rax), REG_OFFSET_NAME_64("%dx", rdx), @@ -94,12 +78,24 @@ static const struct pt_regs_offset x86_64_regoffset_table[] = { REG_OFFSET_NAME_64("%r15", r15), REG_OFFSET_END, }; - -/* TODO: switching by dwarf address size */ -#ifdef __x86_64__ #define regoffset_table x86_64_regoffset_table + #else + +#define REG_OFFSET_NAME_32(n, r) {.name = n, .offset = offsetof(struct pt_regs, r)} +static const struct pt_regs_offset x86_32_regoffset_table[] = { + REG_OFFSET_NAME_32("%ax", eax), + REG_OFFSET_NAME_32("%cx", ecx), + REG_OFFSET_NAME_32("%dx", edx), + REG_OFFSET_NAME_32("%bx", ebx), + REG_OFFSET_NAME_32("$stack", esp), /* Stack address instead of %sp */ + REG_OFFSET_NAME_32("%bp", ebp), + REG_OFFSET_NAME_32("%si", esi), + REG_OFFSET_NAME_32("%di", edi), + REG_OFFSET_END, +}; #define regoffset_table x86_32_regoffset_table + #endif /* Minus 1 for the ending REG_OFFSET_END */ [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 811 bytes --] ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH perf 3/4] perf tools: Fix unused variables: x86_{32,64}_regoffset_table 2016-01-19 21:33 ` [PATCH perf 3/4] perf tools: Fix unused variables: x86_{32,64}_regoffset_table Ben Hutchings @ 2016-01-20 13:59 ` Arnaldo Carvalho de Melo 2016-01-21 4:43 ` Wangnan (F) 0 siblings, 1 reply; 22+ messages in thread From: Arnaldo Carvalho de Melo @ 2016-01-20 13:59 UTC (permalink / raw) To: Ben Hutchings, Masami Hiramatsu, Wang Nan, He Kuang Cc: Peter Zijlstra, Ingo Molnar, linux-kernel, Jiri Olsa, Kaixu Xia, Zefan Li Em Tue, Jan 19, 2016 at 09:33:06PM +0000, Ben Hutchings escreveu: > gcc 5 doesn't seem to care about these, but gcc 6 does and that > results in a build failure. Ben, please CC the people on the CC list for the patch that introduces the problem, Wang, He, can I have your Acked-by? - Arnaldo > Fixes: bbbe6bf6037d ("perf tools: Introduce regs_query_register_offset() ...") > Signed-off-by: Ben Hutchings <ben@decadent.org.uk> > --- > tools/perf/arch/x86/util/dwarf-regs.c | 38 ++++++++++++++++------------------- > 1 file changed, 17 insertions(+), 21 deletions(-) > > diff --git a/tools/perf/arch/x86/util/dwarf-regs.c b/tools/perf/arch/x86/util/dwarf-regs.c > index 9223c16..fe1e516 100644 > --- a/tools/perf/arch/x86/util/dwarf-regs.c > +++ b/tools/perf/arch/x86/util/dwarf-regs.c > @@ -55,26 +55,10 @@ struct pt_regs_offset { > > #define REG_OFFSET_END {.name = NULL, .offset = 0} > > +/* TODO: switching by dwarf address size */ > #ifdef __x86_64__ > -# define REG_OFFSET_NAME_64(n, r) {.name = n, .offset = offsetof(struct pt_regs, r)} > -# define REG_OFFSET_NAME_32(n, r) {.name = n, .offset = -1} > -#else > -# define REG_OFFSET_NAME_64(n, r) {.name = n, .offset = -1} > -# define REG_OFFSET_NAME_32(n, r) {.name = n, .offset = offsetof(struct pt_regs, r)} > -#endif > - > -static const struct pt_regs_offset x86_32_regoffset_table[] = { > - REG_OFFSET_NAME_32("%ax", eax), > - REG_OFFSET_NAME_32("%cx", ecx), > - REG_OFFSET_NAME_32("%dx", edx), > - REG_OFFSET_NAME_32("%bx", ebx), > - REG_OFFSET_NAME_32("$stack", esp), /* Stack address instead of %sp */ > - REG_OFFSET_NAME_32("%bp", ebp), > - REG_OFFSET_NAME_32("%si", esi), > - REG_OFFSET_NAME_32("%di", edi), > - REG_OFFSET_END, > -}; > > +#define REG_OFFSET_NAME_64(n, r) {.name = n, .offset = offsetof(struct pt_regs, r)} > static const struct pt_regs_offset x86_64_regoffset_table[] = { > REG_OFFSET_NAME_64("%ax", rax), > REG_OFFSET_NAME_64("%dx", rdx), > @@ -94,12 +78,24 @@ static const struct pt_regs_offset x86_64_regoffset_table[] = { > REG_OFFSET_NAME_64("%r15", r15), > REG_OFFSET_END, > }; > - > -/* TODO: switching by dwarf address size */ > -#ifdef __x86_64__ > #define regoffset_table x86_64_regoffset_table > + > #else > + > +#define REG_OFFSET_NAME_32(n, r) {.name = n, .offset = offsetof(struct pt_regs, r)} > +static const struct pt_regs_offset x86_32_regoffset_table[] = { > + REG_OFFSET_NAME_32("%ax", eax), > + REG_OFFSET_NAME_32("%cx", ecx), > + REG_OFFSET_NAME_32("%dx", edx), > + REG_OFFSET_NAME_32("%bx", ebx), > + REG_OFFSET_NAME_32("$stack", esp), /* Stack address instead of %sp */ > + REG_OFFSET_NAME_32("%bp", ebp), > + REG_OFFSET_NAME_32("%si", esi), > + REG_OFFSET_NAME_32("%di", edi), > + REG_OFFSET_END, > +}; > #define regoffset_table x86_32_regoffset_table > + > #endif > > /* Minus 1 for the ending REG_OFFSET_END */ > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH perf 3/4] perf tools: Fix unused variables: x86_{32,64}_regoffset_table 2016-01-20 13:59 ` Arnaldo Carvalho de Melo @ 2016-01-21 4:43 ` Wangnan (F) 2016-01-21 15:38 ` Arnaldo Carvalho de Melo 2016-01-22 5:56 ` 平松雅巳 / HIRAMATU，MASAMI 0 siblings, 2 replies; 22+ messages in thread From: Wangnan (F) @ 2016-01-21 4:43 UTC (permalink / raw) To: Arnaldo Carvalho de Melo, Ben Hutchings, Masami Hiramatsu, He Kuang Cc: Peter Zijlstra, Ingo Molnar, linux-kernel, Jiri Olsa, Kaixu Xia, Zefan Li On 2016/1/20 21:59, Arnaldo Carvalho de Melo wrote: > Em Tue, Jan 19, 2016 at 09:33:06PM +0000, Ben Hutchings escreveu: >> gcc 5 doesn't seem to care about these, but gcc 6 does and that >> results in a build failure. > Ben, please CC the people on the CC list for the patch that introduces > the problem, Wang, He, can I have your Acked-by? > > - Arnaldo > This patch lead me find a bug in original code. If both perf and target ELF binary is x86_64, following command works okay: # perf probe -v -n --exec /tmp/oxygen_root/lib64/libc.so.6 pselect data exceptfds readfds writefds nfds sigmask tval timeout <SNIP> Opening /sys/kernel/debug/tracing//uprobe_events write=1 Writing event: p:probe_libc/pselect /home/w00229757/oxygen_root-w00229757/lib64/libc-2.18.so:0xdfef0 data=-216(%sp):u64 exceptfds=%cx:u64 readfds=%si:u64 writefds=%dx:u64 nfds=%di:s32 sigmask=%r9:u64 tval=-232(%sp):u64 timeout=%r8:u64 <SNIP> But if the library is x86_32, result is incorrect: # perf probe -v -n --exec /tmp/oxygen_root/lib32/libc.so.6 pselect data exceptfds readfds writefds nfds sigmask tval <SNIP> Writing event: p:probe_libc/pselect /tmp/oxygen_root-w00229757/lib32/libc-2.18.so:0xd1330 data=-172(%si):u64 exceptfds=+16(%si):u32 readfds=+8(%si):u32 writefds=+12(%si):u32 nfds=+4(%si):s32 sigmask=+24(%si):u32 tval=-180(%si):u64 timeout=+20(%si):u32 <SNIP> We know that (%si) is used to passing arguments. Here we should see '%sp' or '$stack'. Use a x86_32 perf we get currect result: # ~/perf probe -v -n --exec /tmp/oxygen_root/lib32/libc.so.6 pselect data exceptfds readfds writefds nfds sigmask tval <SNIP> Writing event: p:probe_libc/pselect /tmp/oxygen_root-w00229757/lib32/libc-2.18.so:0xd1330 data=-172($stack):u64 exceptfds=+16($stack):u32 readfds=+8($stack):u32 writefds=+12($stack):u32 nfds=+4($stack):s32 sigmask=+24($stack):u32 tval=-180($stack):u64 <SNIP> Use a small test program to check the result: #include <sys/time.h> #include <sys/types.h> #include <unistd.h> #include <memory.h> static struct { fd_set r, w, e; struct timespec ts; sigset_t m; } s; int main() { memset(&s, '\0', sizeof(s)); pselect(0, &s.r, &s.w, &s.e, &s.ts, &s.m); return 0; } # gcc -m32 -g ./test_pselect.c Use x86_32 perf: # ./perf probe -v --exec /tmp/oxygen_root/lib32/libc.so.6 pselect data exceptfds readfds writefds nfds sigmask tval Writing event: p:probe_libc/pselect /tmp/oxygen_root-w00229757/lib32/libc-2.18.so:0xd1330 data=-172($stack):u64 exceptfds=+16($stack):u32 readfds=+8($stack):u32 writefds=+12($stack):u32 nfds=+4($stack):s32 sigmask=+24($stack):u32 tval=-180($stack):u64 Added new event: probe_libc:pselect (on pselect in /tmp/oxygen_root-w00229757/lib32/libc-2.18.so with data exceptfds readfds writefds nfds sigmask tval) You can now use it in all perf tools, such as: perf record -e probe_libc:pselect -aR sleep 1 # ./perf record -e probe_libc:pselect ./a.out [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.011 MB perf.data (1 samples) ] # ./perf script a.out 25336 [006] 64588.457597: probe_libc:pselect: (f7663330) data=0xf772e00000000000 exceptfds=0x8049880 readfds=0x8049780 writefds=0x8049800 nfds=0 sigmask=0x8049908 tval=0x0 Switch to x86_64 perf: # ./perf probe -v --exec /tmp/oxygen_root/lib32/libc.so.6 pselect data exceptfds readfds writefds nfds sigmask tval <SNIP> Opening /sys/kernel/debug/tracing//uprobe_events write=1 Writing event: p:probe_libc/pselect /tmp/oxygen_root-w00229757/lib32/libc-2.18.so:0xd1330 data=-172(%si):u64 exceptfds=+16(%si):u32 readfds=+8(%si):u32 writefds=+12(%si):u32 nfds=+4(%si):s32 sigmask=+24(%si):u32 tval=-180(%si):u64 Added new event: probe_libc:pselect (on pselect in /tmp/oxygen_root-w00229757/lib32/libc-2.18.so with data exceptfds readfds writefds nfds sigmask tval) You can now use it in all perf tools, such as: perf record -e probe_libc:pselect -aR sleep 1 # ./perf record -e probe_libc:pselect ./a.out [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.011 MB perf.data (1 samples) ] # ./perf script a.out 25599 [002] 64759.743554: probe_libc:pselect: (f76e7330) data=0x0 exceptfds=0x0 readfds=0x0 writefds=0x0 nfds=0 sigmask=0x0 tval=0x0 Sad... I think this problem is not introduced by my patch. In fact there's a fundamental problem in get_arch_regstr() that it is impossible to switch sub ISA. Not only x86_64 and x86_32, I think on arm64 we also have this problem when we try to setup uprobes on arm32 code. For me the later problem is more important because there are many legacy arm32 applications on Android platform (and I have already seen the buggy unwind result in this case. It is another problem though). So I suggest us to solve this problem first before considering gcc 6 Werror. At least x86_32_regoffset_table and x86_64_regoffset_table should both be compiled no matter which ISA we select for perf. Thank you. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH perf 3/4] perf tools: Fix unused variables: x86_{32,64}_regoffset_table 2016-01-21 4:43 ` Wangnan (F) @ 2016-01-21 15:38 ` Arnaldo Carvalho de Melo 2016-01-21 15:41 ` Arnaldo Carvalho de Melo 2016-01-22 5:56 ` 平松雅巳 / HIRAMATU，MASAMI 1 sibling, 1 reply; 22+ messages in thread From: Arnaldo Carvalho de Melo @ 2016-01-21 15:38 UTC (permalink / raw) To: Wangnan (F) Cc: Ben Hutchings, Masami Hiramatsu, He Kuang, Peter Zijlstra, Ingo Molnar, linux-kernel, Jiri Olsa, Kaixu Xia, Zefan Li Em Thu, Jan 21, 2016 at 12:43:04PM +0800, Wangnan (F) escreveu: > I think this problem is not introduced by my patch. In fact > there's a fundamental problem in get_arch_regstr() that it is > impossible to switch sub ISA. > Not only x86_64 and x86_32, I think on arm64 we also have this > problem when we try to setup uprobes on arm32 code. For me the > later problem is more important because there are many legacy arm32 > applications on Android platform (and I have already seen the buggy > unwind result in this case. It is another problem though). Humm, and possibly to do something with arm code on a x86 workstation, even if just analysis, yeah, I think these functions should take as an argument the desired architecture instead of assuming it is the one in the machine issuing the commands. > So I suggest us to solve this problem first before considering > gcc 6 Werror. At least x86_32_regoffset_table and x86_64_regoffset_table > should both be compiled no matter which ISA we select for perf. > > Thank you. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH perf 3/4] perf tools: Fix unused variables: x86_{32,64}_regoffset_table 2016-01-21 15:38 ` Arnaldo Carvalho de Melo @ 2016-01-21 15:41 ` Arnaldo Carvalho de Melo 2016-01-22 1:26 ` Wangnan (F) 0 siblings, 1 reply; 22+ messages in thread From: Arnaldo Carvalho de Melo @ 2016-01-21 15:41 UTC (permalink / raw) To: Wangnan (F) Cc: Ben Hutchings, Masami Hiramatsu, He Kuang, Peter Zijlstra, Ingo Molnar, linux-kernel, Jiri Olsa, Kaixu Xia, Zefan Li Em Thu, Jan 21, 2016 at 12:38:48PM -0300, Arnaldo Carvalho de Melo escreveu: > Em Thu, Jan 21, 2016 at 12:43:04PM +0800, Wangnan (F) escreveu: > > I think this problem is not introduced by my patch. In fact > > there's a fundamental problem in get_arch_regstr() that it is > > impossible to switch sub ISA. > > > Not only x86_64 and x86_32, I think on arm64 we also have this > > problem when we try to setup uprobes on arm32 code. For me the > > later problem is more important because there are many legacy arm32 > > applications on Android platform (and I have already seen the buggy > > unwind result in this case. It is another problem though). > > Humm, and possibly to do something with arm code on a x86 workstation, > even if just analysis, yeah, I think these functions should take as an > argument the desired architecture instead of assuming it is the one in > the machine issuing the commands. > > > So I suggest us to solve this problem first before considering > > gcc 6 Werror. At least x86_32_regoffset_table and x86_64_regoffset_table But... I think that the unflexible original code has a bug, one that makes it not work when using gcc6 :-\ So I think we should make it build in gcc6, using that patch (or does it have some other problem?) so that at least doing what we can do now can be done for those using gcc6. Then fix these shortcomings you detected. - Arnaldo > > should both be compiled no matter which ISA we select for perf. > > > > Thank you. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH perf 3/4] perf tools: Fix unused variables: x86_{32,64}_regoffset_table 2016-01-21 15:41 ` Arnaldo Carvalho de Melo @ 2016-01-22 1:26 ` Wangnan (F) 0 siblings, 0 replies; 22+ messages in thread From: Wangnan (F) @ 2016-01-22 1:26 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Ben Hutchings, Masami Hiramatsu, He Kuang, Peter Zijlstra, Ingo Molnar, linux-kernel, Jiri Olsa, Kaixu Xia, Zefan Li On 2016/1/21 23:41, Arnaldo Carvalho de Melo wrote: > > But... I think that the unflexible original code has a bug, one that makes it > not work when using gcc6 :-\ > > So I think we should make it build in gcc6, using that patch (or does it > have some other problem?) so that at least doing what we can do now can > be done for those using gcc6. > > Then fix these shortcomings you detected. OK. His patch does what it claims to do. Please merge it first, then let's look into my problem. Thank you. ^ permalink raw reply [flat|nested] 22+ messages in thread
* RE: [PATCH perf 3/4] perf tools: Fix unused variables: x86_{32,64}_regoffset_table 2016-01-21 4:43 ` Wangnan (F) 2016-01-21 15:38 ` Arnaldo Carvalho de Melo @ 2016-01-22 5:56 ` 平松雅巳 / HIRAMATU，MASAMI 2016-01-22 6:19 ` Wangnan (F) 1 sibling, 1 reply; 22+ messages in thread From: 平松雅巳 / HIRAMATU，MASAMI @ 2016-01-22 5:56 UTC (permalink / raw) To: 'Wangnan (F)', Arnaldo Carvalho de Melo, Ben Hutchings, He Kuang Cc: Peter Zijlstra, Ingo Molnar, linux-kernel, Jiri Olsa, Kaixu Xia, Zefan Li >From: Wangnan (F) [mailto:wangnan0@huawei.com] > >On 2016/1/20 21:59, Arnaldo Carvalho de Melo wrote: >> Em Tue, Jan 19, 2016 at 09:33:06PM +0000, Ben Hutchings escreveu: >>> gcc 5 doesn't seem to care about these, but gcc 6 does and that >>> results in a build failure. >> Ben, please CC the people on the CC list for the patch that introduces >> the problem, Wang, He, can I have your Acked-by? >> >> - Arnaldo >> > >This patch lead me find a bug in original code. > >If both perf and target ELF binary is x86_64, following command works okay: > > # perf probe -v -n --exec /tmp/oxygen_root/lib64/libc.so.6 pselect >data exceptfds readfds writefds nfds sigmask tval timeout > <SNIP> > Opening /sys/kernel/debug/tracing//uprobe_events write=1 > Writing event: p:probe_libc/pselect >/home/w00229757/oxygen_root-w00229757/lib64/libc-2.18.so:0xdfef0 >data=-216(%sp):u64 exceptfds=%cx:u64 readfds=%si:u64 writefds=%dx:u64 >nfds=%di:s32 sigmask=%r9:u64 tval=-232(%sp):u64 timeout=%r8:u64 > <SNIP> > >But if the library is x86_32, result is incorrect: > > # perf probe -v -n --exec /tmp/oxygen_root/lib32/libc.so.6 pselect >data exceptfds readfds writefds nfds sigmask tval > <SNIP> > Writing event: p:probe_libc/pselect >/tmp/oxygen_root-w00229757/lib32/libc-2.18.so:0xd1330 data=-172(%si):u64 >exceptfds=+16(%si):u32 readfds=+8(%si):u32 writefds=+12(%si):u32 >nfds=+4(%si):s32 sigmask=+24(%si):u32 tval=-180(%si):u64 >timeout=+20(%si):u32 > <SNIP> > >We know that (%si) is used to passing arguments. Here we should see >'%sp' or '$stack'. > >Use a x86_32 perf we get currect result: > > # ~/perf probe -v -n --exec /tmp/oxygen_root/lib32/libc.so.6 pselect >data exceptfds readfds writefds nfds sigmask tval > <SNIP> > Writing event: p:probe_libc/pselect >/tmp/oxygen_root-w00229757/lib32/libc-2.18.so:0xd1330 >data=-172($stack):u64 exceptfds=+16($stack):u32 readfds=+8($stack):u32 >writefds=+12($stack):u32 nfds=+4($stack):s32 sigmask=+24($stack):u32 >tval=-180($stack):u64 > <SNIP> Ah, I see. Uprobes may not check the target binary is in 32bit mode. Since the stack of x86-64 and x86-32 on pt_regs are different, (regs->sp points stack on x86-64, &(regs->pt) points stack on x86-32) uprobes would better checking and change the behavior. But anyway, it is also fixed by changing perf's register table. > > >Use a small test program to check the result: > > #include <sys/time.h> > #include <sys/types.h> > #include <unistd.h> > #include <memory.h> > > static struct { > fd_set r, w, e; > struct timespec ts; > sigset_t m; > } s; > > int main() > { > memset(&s, '\0', sizeof(s)); > > pselect(0, &s.r, &s.w, &s.e, &s.ts, &s.m); > return 0; > } > ># gcc -m32 -g ./test_pselect.c > >Use x86_32 perf: > ># ./perf probe -v --exec /tmp/oxygen_root/lib32/libc.so.6 pselect data >exceptfds readfds writefds nfds sigmask tval >Writing event: p:probe_libc/pselect >/tmp/oxygen_root-w00229757/lib32/libc-2.18.so:0xd1330 >data=-172($stack):u64 exceptfds=+16($stack):u32 readfds=+8($stack):u32 >writefds=+12($stack):u32 nfds=+4($stack):s32 sigmask=+24($stack):u32 >tval=-180($stack):u64 >Added new event: > probe_libc:pselect (on pselect in >/tmp/oxygen_root-w00229757/lib32/libc-2.18.so with data exceptfds >readfds writefds nfds sigmask tval) > >You can now use it in all perf tools, such as: > > perf record -e probe_libc:pselect -aR sleep 1 > ># ./perf record -e probe_libc:pselect ./a.out >[ perf record: Woken up 1 times to write data ] >[ perf record: Captured and wrote 0.011 MB perf.data (1 samples) ] ># ./perf script > a.out 25336 [006] 64588.457597: probe_libc:pselect: >(f7663330) data=0xf772e00000000000 exceptfds=0x8049880 readfds=0x8049780 >writefds=0x8049800 nfds=0 sigmask=0x8049908 tval=0x0 > >Switch to x86_64 perf: > > # ./perf probe -v --exec /tmp/oxygen_root/lib32/libc.so.6 pselect >data exceptfds readfds writefds nfds sigmask tval > <SNIP> > Opening /sys/kernel/debug/tracing//uprobe_events write=1 >Writing event: p:probe_libc/pselect >/tmp/oxygen_root-w00229757/lib32/libc-2.18.so:0xd1330 data=-172(%si):u64 >exceptfds=+16(%si):u32 readfds=+8(%si):u32 writefds=+12(%si):u32 >nfds=+4(%si):s32 sigmask=+24(%si):u32 tval=-180(%si):u64 >Added new event: > probe_libc:pselect (on pselect in >/tmp/oxygen_root-w00229757/lib32/libc-2.18.so with data exceptfds >readfds writefds nfds sigmask tval) > >You can now use it in all perf tools, such as: > > perf record -e probe_libc:pselect -aR sleep 1 > ># ./perf record -e probe_libc:pselect ./a.out >[ perf record: Woken up 1 times to write data ] >[ perf record: Captured and wrote 0.011 MB perf.data (1 samples) ] ># ./perf script > a.out 25599 [002] 64759.743554: probe_libc:pselect: >(f76e7330) data=0x0 exceptfds=0x0 readfds=0x0 writefds=0x0 nfds=0 >sigmask=0x0 tval=0x0 > >Sad... > >I think this problem is not introduced by my patch. In fact >there's a fundamental problem in get_arch_regstr() that it is >impossible to switch sub ISA. Right, but I guess this can fixed by switching %sp (for x86-64) and +0(%sp) (for x86-32) instead of $stack. Thanks! > >Not only x86_64 and x86_32, I think on arm64 we also have this >problem when we try to setup uprobes on arm32 code. For me the >later problem is more important because there are many legacy arm32 >applications on Android platform (and I have already seen the buggy >unwind result in this case. It is another problem though). > >So I suggest us to solve this problem first before considering >gcc 6 Werror. At least x86_32_regoffset_table and x86_64_regoffset_table >should both be compiled no matter which ISA we select for perf. > >Thank you. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH perf 3/4] perf tools: Fix unused variables: x86_{32,64}_regoffset_table 2016-01-22 5:56 ` 平松雅巳 / HIRAMATU，MASAMI @ 2016-01-22 6:19 ` Wangnan (F) 2016-01-22 7:59 ` 平松雅巳 / HIRAMATU，MASAMI 0 siblings, 1 reply; 22+ messages in thread From: Wangnan (F) @ 2016-01-22 6:19 UTC (permalink / raw) To: 平松雅巳 / HIRAMATU，MASAMI, Arnaldo Carvalho de Melo, Ben Hutchings, He Kuang Cc: Peter Zijlstra, Ingo Molnar, linux-kernel, Jiri Olsa, Kaixu Xia, Zefan Li On 2016/1/22 13:56, 平松雅巳 / HIRAMATU，MASAMI wrote: >> From: Wangnan (F) [mailto:wangnan0@huawei.com] >> >> On 2016/1/20 21:59, Arnaldo Carvalho de Melo wrote: >>> Em Tue, Jan 19, 2016 at 09:33:06PM +0000, Ben Hutchings escreveu: >>>> gcc 5 doesn't seem to care about these, but gcc 6 does and that >>>> results in a build failure. >>> Ben, please CC the people on the CC list for the patch that introduces >>> the problem, Wang, He, can I have your Acked-by? >>> >>> - Arnaldo >>> >> This patch lead me find a bug in original code. >> >> If both perf and target ELF binary is x86_64, following command works okay: >> >> # perf probe -v -n --exec /tmp/oxygen_root/lib64/libc.so.6 pselect >> data exceptfds readfds writefds nfds sigmask tval timeout >> <SNIP> >> Opening /sys/kernel/debug/tracing//uprobe_events write=1 >> Writing event: p:probe_libc/pselect >> /home/w00229757/oxygen_root-w00229757/lib64/libc-2.18.so:0xdfef0 >> data=-216(%sp):u64 exceptfds=%cx:u64 readfds=%si:u64 writefds=%dx:u64 >> nfds=%di:s32 sigmask=%r9:u64 tval=-232(%sp):u64 timeout=%r8:u64 >> <SNIP> >> >> But if the library is x86_32, result is incorrect: >> >> # perf probe -v -n --exec /tmp/oxygen_root/lib32/libc.so.6 pselect >> data exceptfds readfds writefds nfds sigmask tval >> <SNIP> >> Writing event: p:probe_libc/pselect >> /tmp/oxygen_root-w00229757/lib32/libc-2.18.so:0xd1330 data=-172(%si):u64 >> exceptfds=+16(%si):u32 readfds=+8(%si):u32 writefds=+12(%si):u32 >> nfds=+4(%si):s32 sigmask=+24(%si):u32 tval=-180(%si):u64 >> timeout=+20(%si):u32 >> <SNIP> >> >> We know that (%si) is used to passing arguments. Here we should see >> '%sp' or '$stack'. >> >> Use a x86_32 perf we get currect result: >> >> # ~/perf probe -v -n --exec /tmp/oxygen_root/lib32/libc.so.6 pselect >> data exceptfds readfds writefds nfds sigmask tval >> <SNIP> >> Writing event: p:probe_libc/pselect >> /tmp/oxygen_root-w00229757/lib32/libc-2.18.so:0xd1330 >> data=-172($stack):u64 exceptfds=+16($stack):u32 readfds=+8($stack):u32 >> writefds=+12($stack):u32 nfds=+4($stack):s32 sigmask=+24($stack):u32 >> tval=-180($stack):u64 >> <SNIP> > Ah, I see. Uprobes may not check the target binary is in 32bit mode. > Since the stack of x86-64 and x86-32 on pt_regs are different, > (regs->sp points stack on x86-64, &(regs->pt) points stack on x86-32) > uprobes would better checking and change the behavior. > > But anyway, it is also fixed by changing perf's register table. > >> >> Use a small test program to check the result: >> >> #include <sys/time.h> >> #include <sys/types.h> >> #include <unistd.h> >> #include <memory.h> >> >> static struct { >> fd_set r, w, e; >> struct timespec ts; >> sigset_t m; >> } s; >> >> int main() >> { >> memset(&s, '\0', sizeof(s)); >> >> pselect(0, &s.r, &s.w, &s.e, &s.ts, &s.m); >> return 0; >> } >> >> # gcc -m32 -g ./test_pselect.c >> >> Use x86_32 perf: >> >> # ./perf probe -v --exec /tmp/oxygen_root/lib32/libc.so.6 pselect data >> exceptfds readfds writefds nfds sigmask tval >> Writing event: p:probe_libc/pselect >> /tmp/oxygen_root-w00229757/lib32/libc-2.18.so:0xd1330 >> data=-172($stack):u64 exceptfds=+16($stack):u32 readfds=+8($stack):u32 >> writefds=+12($stack):u32 nfds=+4($stack):s32 sigmask=+24($stack):u32 >> tval=-180($stack):u64 >> Added new event: >> probe_libc:pselect (on pselect in >> /tmp/oxygen_root-w00229757/lib32/libc-2.18.so with data exceptfds >> readfds writefds nfds sigmask tval) >> >> You can now use it in all perf tools, such as: >> >> perf record -e probe_libc:pselect -aR sleep 1 >> >> # ./perf record -e probe_libc:pselect ./a.out >> [ perf record: Woken up 1 times to write data ] >> [ perf record: Captured and wrote 0.011 MB perf.data (1 samples) ] >> # ./perf script >> a.out 25336 [006] 64588.457597: probe_libc:pselect: >> (f7663330) data=0xf772e00000000000 exceptfds=0x8049880 readfds=0x8049780 >> writefds=0x8049800 nfds=0 sigmask=0x8049908 tval=0x0 >> >> Switch to x86_64 perf: >> >> # ./perf probe -v --exec /tmp/oxygen_root/lib32/libc.so.6 pselect >> data exceptfds readfds writefds nfds sigmask tval >> <SNIP> >> Opening /sys/kernel/debug/tracing//uprobe_events write=1 >> Writing event: p:probe_libc/pselect >> /tmp/oxygen_root-w00229757/lib32/libc-2.18.so:0xd1330 data=-172(%si):u64 >> exceptfds=+16(%si):u32 readfds=+8(%si):u32 writefds=+12(%si):u32 >> nfds=+4(%si):s32 sigmask=+24(%si):u32 tval=-180(%si):u64 >> Added new event: >> probe_libc:pselect (on pselect in >> /tmp/oxygen_root-w00229757/lib32/libc-2.18.so with data exceptfds >> readfds writefds nfds sigmask tval) >> >> You can now use it in all perf tools, such as: >> >> perf record -e probe_libc:pselect -aR sleep 1 >> >> # ./perf record -e probe_libc:pselect ./a.out >> [ perf record: Woken up 1 times to write data ] >> [ perf record: Captured and wrote 0.011 MB perf.data (1 samples) ] >> # ./perf script >> a.out 25599 [002] 64759.743554: probe_libc:pselect: >> (f76e7330) data=0x0 exceptfds=0x0 readfds=0x0 writefds=0x0 nfds=0 >> sigmask=0x0 tval=0x0 >> >> Sad... >> >> I think this problem is not introduced by my patch. In fact >> there's a fundamental problem in get_arch_regstr() that it is >> impossible to switch sub ISA. > Right, but I guess this can fixed by switching %sp (for x86-64) > and +0(%sp) (for x86-32) instead of $stack. > It may not work. No matter how we change regoffset_table, when get_arch_regstr() get a register number 4, how can it know whether we are looking for x86_32 register and return $stack or return %si for x86_64? The fundamental problem is: we need a way to map dwarf's register number to register names so uprobe know which register we are looking for, but the API we designed for this can't distinguish the sub ISAs dwarf are using. Thanks. ^ permalink raw reply [flat|nested] 22+ messages in thread
* RE: [PATCH perf 3/4] perf tools: Fix unused variables: x86_{32,64}_regoffset_table 2016-01-22 6:19 ` Wangnan (F) @ 2016-01-22 7:59 ` 平松雅巳 / HIRAMATU，MASAMI 0 siblings, 0 replies; 22+ messages in thread From: 平松雅巳 / HIRAMATU，MASAMI @ 2016-01-22 7:59 UTC (permalink / raw) To: 'Wangnan (F)', Arnaldo Carvalho de Melo, Ben Hutchings, He Kuang Cc: Peter Zijlstra, Ingo Molnar, linux-kernel, Jiri Olsa, Kaixu Xia, Zefan Li >From: Wangnan (F) [mailto:wangnan0@huawei.com] >On 2016/1/22 13:56, 平松雅巳 / HIRAMATU，MASAMI wrote: >>> From: Wangnan (F) [mailto:wangnan0@huawei.com] >>> I think this problem is not introduced by my patch. In fact >>> there's a fundamental problem in get_arch_regstr() that it is >>> impossible to switch sub ISA. >> Right, but I guess this can fixed by switching %sp (for x86-64) >> and +0(%sp) (for x86-32) instead of $stack. >> > >It may not work. > >No matter how we change regoffset_table, when get_arch_regstr() >get a register number 4, how can it know whether we are looking >for x86_32 register and return $stack or return %si for x86_64? I think we can also use elf header to get the ISA of the target binary. >The fundamental problem is: we need a way to map dwarf's register >number to register names so uprobe know which register we are >looking for, but the API we designed for this can't distinguish >the sub ISAs dwarf are using. As I said, we can read the elf header and get the Class or Machine and switch the table according to it. Can't it? Thanks, ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH perf 4/4] perf tests: Delete mis-indented dead code that causes build failure with gcc 6 2016-01-19 21:32 [PATCH perf 0/4] Build fixes for gcc 6 Ben Hutchings ` (2 preceding siblings ...) 2016-01-19 21:33 ` [PATCH perf 3/4] perf tools: Fix unused variables: x86_{32,64}_regoffset_table Ben Hutchings @ 2016-01-19 21:33 ` Ben Hutchings 2016-01-19 21:40 ` [PATCH perf 0/4] Build fixes for " Markus Trippelsdorf 4 siblings, 0 replies; 22+ messages in thread From: Ben Hutchings @ 2016-01-19 21:33 UTC (permalink / raw) To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo; +Cc: linux-kernel [-- Attachment #1: Type: text/plain, Size: 894 bytes --] gcc 6 complains: arch/x86/tests/intel-cqm.c: In function 'spawn': arch/x86/tests/intel-cqm.c:21:3: error: statement is indented as if it were guarded by... [-Werror=misleading-indentation] sleep(5); ^~~~~ arch/x86/tests/intel-cqm.c:20:2: note: ...this 'while' clause, but it is not while(1); ^~~~~ Fixes: 035827e9f2bd ("perf tests: Add Intel CQM test") Signed-off-by: Ben Hutchings <ben@decadent.org.uk> --- tools/perf/arch/x86/tests/intel-cqm.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/perf/arch/x86/tests/intel-cqm.c b/tools/perf/arch/x86/tests/intel-cqm.c index d28c1b6..bd123a1 100644 --- a/tools/perf/arch/x86/tests/intel-cqm.c +++ b/tools/perf/arch/x86/tests/intel-cqm.c @@ -17,8 +17,9 @@ static pid_t spawn(void) if (pid) return pid; - while(1); - sleep(5); + while (1) + ; + return 0; } [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 811 bytes --] ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH perf 0/4] Build fixes for gcc 6 2016-01-19 21:32 [PATCH perf 0/4] Build fixes for gcc 6 Ben Hutchings ` (3 preceding siblings ...) 2016-01-19 21:33 ` [PATCH perf 4/4] perf tests: Delete mis-indented dead code that causes build failure with gcc 6 Ben Hutchings @ 2016-01-19 21:40 ` Markus Trippelsdorf 2016-01-19 21:58 ` Ben Hutchings 4 siblings, 1 reply; 22+ messages in thread From: Markus Trippelsdorf @ 2016-01-19 21:40 UTC (permalink / raw) To: Ben Hutchings Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, linux-kernel On 2016.01.19 at 21:32 +0000, Ben Hutchings wrote: > gcc 6 warns about various things in tools/perf and with -Werror > these turn into build failures. One of them is a real though not > very serious bug. I've already send patches for 1,2 and 4. See: https://lkml.org/lkml/2015/12/14/460 Not sure what happened with them. Also your patch number 4 is wrong, you should just delete the semicolon. -- Markus ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH perf 0/4] Build fixes for gcc 6 2016-01-19 21:40 ` [PATCH perf 0/4] Build fixes for " Markus Trippelsdorf @ 2016-01-19 21:58 ` Ben Hutchings 2016-01-19 22:00 ` Markus Trippelsdorf 0 siblings, 1 reply; 22+ messages in thread From: Ben Hutchings @ 2016-01-19 21:58 UTC (permalink / raw) To: Markus Trippelsdorf Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, linux-kernel [-- Attachment #1: Type: text/plain, Size: 670 bytes --] On Tue, Jan 19, 2016 at 10:40:18PM +0100, Markus Trippelsdorf wrote: > On 2016.01.19 at 21:32 +0000, Ben Hutchings wrote: > > gcc 6 warns about various things in tools/perf and with -Werror > > these turn into build failures. One of them is a real though not > > very serious bug. > > I've already send patches for 1,2 and 4. See: > https://lkml.org/lkml/2015/12/14/460 > > Not sure what happened with them. Also your patch number 4 is wrong, you > should just delete the semicolon. I think that the busy-wait, intentional or not, may be a necessary part of the test case. Ben. -- Ben Hutchings When in doubt, use brute force. - Ken Thompson [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 811 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH perf 0/4] Build fixes for gcc 6 2016-01-19 21:58 ` Ben Hutchings @ 2016-01-19 22:00 ` Markus Trippelsdorf 2016-01-19 22:04 ` Ben Hutchings 2016-01-19 22:28 ` Arnaldo Carvalho de Melo 0 siblings, 2 replies; 22+ messages in thread From: Markus Trippelsdorf @ 2016-01-19 22:00 UTC (permalink / raw) To: Ben Hutchings Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, linux-kernel On 2016.01.19 at 21:58 +0000, Ben Hutchings wrote: > On Tue, Jan 19, 2016 at 10:40:18PM +0100, Markus Trippelsdorf wrote: > > On 2016.01.19 at 21:32 +0000, Ben Hutchings wrote: > > > gcc 6 warns about various things in tools/perf and with -Werror > > > these turn into build failures. One of them is a real though not > > > very serious bug. > > > > I've already send patches for 1,2 and 4. See: > > https://lkml.org/lkml/2015/12/14/460 > > > > Not sure what happened with them. Also your patch number 4 is wrong, you > > should just delete the semicolon. > > I think that the busy-wait, intentional or not, may be a necessary > part of the test case. Well, the author of the code thinks otherwise: https://lkml.org/lkml/2015/12/14/269 -- Markus ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH perf 0/4] Build fixes for gcc 6 2016-01-19 22:00 ` Markus Trippelsdorf @ 2016-01-19 22:04 ` Ben Hutchings 2016-01-19 22:28 ` Arnaldo Carvalho de Melo 1 sibling, 0 replies; 22+ messages in thread From: Ben Hutchings @ 2016-01-19 22:04 UTC (permalink / raw) To: Markus Trippelsdorf Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1019 bytes --] On Tue, 2016-01-19 at 23:00 +0100, Markus Trippelsdorf wrote: > On 2016.01.19 at 21:58 +0000, Ben Hutchings wrote: > > On Tue, Jan 19, 2016 at 10:40:18PM +0100, Markus Trippelsdorf wrote: > > > On 2016.01.19 at 21:32 +0000, Ben Hutchings wrote: > > > > gcc 6 warns about various things in tools/perf and with -Werror > > > > these turn into build failures. One of them is a real though not > > > > very serious bug. > > > > > > I've already send patches for 1,2 and 4. See: > > > https://lkml.org/lkml/2015/12/14/460 > > > > > > Not sure what happened with them. Also your patch number 4 is wrong, you > > > should just delete the semicolon. > > > > I think that the busy-wait, intentional or not, may be a necessary > > part of the test case. > > Well, the author of the code thinks otherwise: > > https://lkml.org/lkml/2015/12/14/269 Oh I see, thanks. Ben. -- Ben Hutchings Horngren's Observation: Among economists, the real world is often a special case. [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 811 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH perf 0/4] Build fixes for gcc 6 2016-01-19 22:00 ` Markus Trippelsdorf 2016-01-19 22:04 ` Ben Hutchings @ 2016-01-19 22:28 ` Arnaldo Carvalho de Melo 2016-01-19 22:30 ` Arnaldo Carvalho de Melo 2016-01-19 22:31 ` Markus Trippelsdorf 1 sibling, 2 replies; 22+ messages in thread From: Arnaldo Carvalho de Melo @ 2016-01-19 22:28 UTC (permalink / raw) To: Markus Trippelsdorf Cc: Ben Hutchings, Peter Zijlstra, Ingo Molnar, linux-kernel Em Tue, Jan 19, 2016 at 11:00:50PM +0100, Markus Trippelsdorf escreveu: > On 2016.01.19 at 21:58 +0000, Ben Hutchings wrote: > > On Tue, Jan 19, 2016 at 10:40:18PM +0100, Markus Trippelsdorf wrote: > > > On 2016.01.19 at 21:32 +0000, Ben Hutchings wrote: > > > > gcc 6 warns about various things in tools/perf and with -Werror > > > > these turn into build failures. One of them is a real though not > > > > very serious bug. > > > > > > I've already send patches for 1,2 and 4. See: > > > https://lkml.org/lkml/2015/12/14/460 > > > > > > Not sure what happened with them. Also your patch number 4 is wrong, you > > > should just delete the semicolon. > > > > I think that the busy-wait, intentional or not, may be a necessary > > part of the test case. > > Well, the author of the code thinks otherwise: > > https://lkml.org/lkml/2015/12/14/269 Right, I saw those and I think I haven't processed them because I was waiting for those to be broken up in separate patches after I read Ingo's comment about one of them fixing up a real bug, a part that the original autor, mfleming even acked, could you please break it down into multiple patches? - Arnaldo ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH perf 0/4] Build fixes for gcc 6 2016-01-19 22:28 ` Arnaldo Carvalho de Melo @ 2016-01-19 22:30 ` Arnaldo Carvalho de Melo 2016-01-19 22:43 ` Markus Trippelsdorf 2016-01-19 22:31 ` Markus Trippelsdorf 1 sibling, 1 reply; 22+ messages in thread From: Arnaldo Carvalho de Melo @ 2016-01-19 22:30 UTC (permalink / raw) To: Markus Trippelsdorf Cc: Ben Hutchings, Peter Zijlstra, Ingo Molnar, linux-kernel Em Tue, Jan 19, 2016 at 07:28:51PM -0300, Arnaldo Carvalho de Melo escreveu: > Em Tue, Jan 19, 2016 at 11:00:50PM +0100, Markus Trippelsdorf escreveu: > > On 2016.01.19 at 21:58 +0000, Ben Hutchings wrote: > > > On Tue, Jan 19, 2016 at 10:40:18PM +0100, Markus Trippelsdorf wrote: > > > > On 2016.01.19 at 21:32 +0000, Ben Hutchings wrote: > > > > > gcc 6 warns about various things in tools/perf and with -Werror > > > > > these turn into build failures. One of them is a real though not > > > > > very serious bug. > > > > > > > > I've already send patches for 1,2 and 4. See: > > > > https://lkml.org/lkml/2015/12/14/460 > > > > > > > > Not sure what happened with them. Also your patch number 4 is wrong, you > > > > should just delete the semicolon. > > > > > > I think that the busy-wait, intentional or not, may be a necessary > > > part of the test case. > > > > Well, the author of the code thinks otherwise: > > > > https://lkml.org/lkml/2015/12/14/269 > > Right, I saw those and I think I haven't processed them because I was > waiting for those to be broken up in separate patches after I read > Ingo's comment about one of them fixing up a real bug, a part that the > original autor, mfleming even acked, could you please break it down into > multiple patches? Alternatively I can do it for the patch acked by Matt and use the other patches from Ben, that even have the Fixes: tags (yay, those are appreciated!). - Arnaldo ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH perf 0/4] Build fixes for gcc 6 2016-01-19 22:30 ` Arnaldo Carvalho de Melo @ 2016-01-19 22:43 ` Markus Trippelsdorf 0 siblings, 0 replies; 22+ messages in thread From: Markus Trippelsdorf @ 2016-01-19 22:43 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Ben Hutchings, Peter Zijlstra, Ingo Molnar, linux-kernel On 2016.01.19 at 19:30 -0300, Arnaldo Carvalho de Melo wrote: > Em Tue, Jan 19, 2016 at 07:28:51PM -0300, Arnaldo Carvalho de Melo escreveu: > > Em Tue, Jan 19, 2016 at 11:00:50PM +0100, Markus Trippelsdorf escreveu: > > > On 2016.01.19 at 21:58 +0000, Ben Hutchings wrote: > > > > On Tue, Jan 19, 2016 at 10:40:18PM +0100, Markus Trippelsdorf wrote: > > > > > On 2016.01.19 at 21:32 +0000, Ben Hutchings wrote: > > > > > > gcc 6 warns about various things in tools/perf and with -Werror > > > > > > these turn into build failures. One of them is a real though not > > > > > > very serious bug. > > > > > > > > > > I've already send patches for 1,2 and 4. See: > > > > > https://lkml.org/lkml/2015/12/14/460 > > > > > > > > > > Not sure what happened with them. Also your patch number 4 is wrong, you > > > > > should just delete the semicolon. > > > > > > > > I think that the busy-wait, intentional or not, may be a necessary > > > > part of the test case. > > > > > > Well, the author of the code thinks otherwise: > > > > > > https://lkml.org/lkml/2015/12/14/269 > > > > Right, I saw those and I think I haven't processed them because I was > > waiting for those to be broken up in separate patches after I read > > Ingo's comment about one of them fixing up a real bug, a part that the > > original autor, mfleming even acked, could you please break it down into > > multiple patches? > > Alternatively I can do it for the patch acked by Matt and use the other > patches from Ben, that even have the Fixes: tags (yay, those are > appreciated!). Just use what is easiest for you to work with. I don't really mind. -- Markus ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH perf 0/4] Build fixes for gcc 6 2016-01-19 22:28 ` Arnaldo Carvalho de Melo 2016-01-19 22:30 ` Arnaldo Carvalho de Melo @ 2016-01-19 22:31 ` Markus Trippelsdorf 2016-01-20 13:20 ` Arnaldo Carvalho de Melo 1 sibling, 1 reply; 22+ messages in thread From: Markus Trippelsdorf @ 2016-01-19 22:31 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Ben Hutchings, Peter Zijlstra, Ingo Molnar, linux-kernel On 2016.01.19 at 19:28 -0300, Arnaldo Carvalho de Melo wrote: > Em Tue, Jan 19, 2016 at 11:00:50PM +0100, Markus Trippelsdorf escreveu: > > On 2016.01.19 at 21:58 +0000, Ben Hutchings wrote: > > > On Tue, Jan 19, 2016 at 10:40:18PM +0100, Markus Trippelsdorf wrote: > > > > On 2016.01.19 at 21:32 +0000, Ben Hutchings wrote: > > > > > gcc 6 warns about various things in tools/perf and with -Werror > > > > > these turn into build failures. One of them is a real though not > > > > > very serious bug. > > > > > > > > I've already send patches for 1,2 and 4. See: > > > > https://lkml.org/lkml/2015/12/14/460 > > > > > > > > Not sure what happened with them. Also your patch number 4 is wrong, you > > > > should just delete the semicolon. > > > > > > I think that the busy-wait, intentional or not, may be a necessary > > > part of the test case. > > > > Well, the author of the code thinks otherwise: > > > > https://lkml.org/lkml/2015/12/14/269 > > Right, I saw those and I think I haven't processed them because I was > waiting for those to be broken up in separate patches after I read > Ingo's comment about one of them fixing up a real bug, a part that the > original autor, mfleming even acked, could you please break it down into > multiple patches? https://lkml.org/lkml/2015/12/14/460 https://lkml.org/lkml/2015/12/14/461 https://lkml.org/lkml/2015/12/14/465 -- Markus ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH perf 0/4] Build fixes for gcc 6 2016-01-19 22:31 ` Markus Trippelsdorf @ 2016-01-20 13:20 ` Arnaldo Carvalho de Melo 0 siblings, 0 replies; 22+ messages in thread From: Arnaldo Carvalho de Melo @ 2016-01-20 13:20 UTC (permalink / raw) To: Markus Trippelsdorf Cc: Ben Hutchings, Peter Zijlstra, Ingo Molnar, linux-kernel Em Tue, Jan 19, 2016 at 11:31:01PM +0100, Markus Trippelsdorf escreveu: > On 2016.01.19 at 19:28 -0300, Arnaldo Carvalho de Melo wrote: > > Em Tue, Jan 19, 2016 at 11:00:50PM +0100, Markus Trippelsdorf escreveu: > > > On 2016.01.19 at 21:58 +0000, Ben Hutchings wrote: > > > > On Tue, Jan 19, 2016 at 10:40:18PM +0100, Markus Trippelsdorf wrote: > > > > > On 2016.01.19 at 21:32 +0000, Ben Hutchings wrote: > > > > > > gcc 6 warns about various things in tools/perf and with -Werror > > > > > > these turn into build failures. One of them is a real though not > > > > > > very serious bug. > > > > > > > > > > I've already send patches for 1,2 and 4. See: > > > > > https://lkml.org/lkml/2015/12/14/460 > > > > > > > > > > Not sure what happened with them. Also your patch number 4 is wrong, you > > > > > should just delete the semicolon. > > > > > > > > I think that the busy-wait, intentional or not, may be a necessary > > > > part of the test case. > > > > > > Well, the author of the code thinks otherwise: > > > > > > https://lkml.org/lkml/2015/12/14/269 > > > > Right, I saw those and I think I haven't processed them because I was > > waiting for those to be broken up in separate patches after I read > > Ingo's comment about one of them fixing up a real bug, a part that the > > original autor, mfleming even acked, could you please break it down into > > multiple patches? > > https://lkml.org/lkml/2015/12/14/460 > https://lkml.org/lkml/2015/12/14/461 > https://lkml.org/lkml/2015/12/14/465 And you had done that already, my bad, going thru them now. - Arnaldo ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2016-01-22 8:00 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-19 21:32 [PATCH perf 0/4] Build fixes for gcc 6 Ben Hutchings
2016-01-19 21:32 ` [PATCH perf 1/4] perf tools: Fix wrong indentation and build failure with " Ben Hutchings
2016-01-19 21:32 ` [PATCH perf 2/4] perf top: Fix behaviour of Shift-Tab in annotated view with nothing focussed Ben Hutchings
2016-01-19 21:33 ` [PATCH perf 3/4] perf tools: Fix unused variables: x86_{32,64}_regoffset_table Ben Hutchings
2016-01-20 13:59 ` Arnaldo Carvalho de Melo
2016-01-21 4:43 ` Wangnan (F)
2016-01-21 15:38 ` Arnaldo Carvalho de Melo
2016-01-21 15:41 ` Arnaldo Carvalho de Melo
2016-01-22 1:26 ` Wangnan (F)
2016-01-22 5:56 ` 平松雅巳 / HIRAMATU，MASAMI
2016-01-22 6:19 ` Wangnan (F)
2016-01-22 7:59 ` 平松雅巳 / HIRAMATU，MASAMI
2016-01-19 21:33 ` [PATCH perf 4/4] perf tests: Delete mis-indented dead code that causes build failure with gcc 6 Ben Hutchings
2016-01-19 21:40 ` [PATCH perf 0/4] Build fixes for " Markus Trippelsdorf
2016-01-19 21:58 ` Ben Hutchings
2016-01-19 22:00 ` Markus Trippelsdorf
2016-01-19 22:04 ` Ben Hutchings
2016-01-19 22:28 ` Arnaldo Carvalho de Melo
2016-01-19 22:30 ` Arnaldo Carvalho de Melo
2016-01-19 22:43 ` Markus Trippelsdorf
2016-01-19 22:31 ` Markus Trippelsdorf
2016-01-20 13:20 ` Arnaldo Carvalho de Melo
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.