| Message ID | 1452520124-2073-12-git-send-email-wangnan0@huawei.com |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
On 01/11/2016 04:48 PM, Wang Nan wrote: > Commit 71d6de64feddd4b455555326fba2111b3006d9e0 ('perf test: Fix hist > testcases when kptr_restrict is on') solves a double free problem when You didn't run this patch thru scripts/checkpatch.pl, I guess? A certain commit citing style is enforced now, and yours doesn't quite match it... > 'perf test hist' calling setup_fake_machine(). However, the result is > still incorrect. For example: > > $ ./perf test -v 'filtering hist entries' > 25: Test filtering hist entries : > --- start --- > test child forked, pid 4186 > Cannot create kernel maps > test child finished with 0 > ---- end ---- > Test filtering hist entries: Ok > > In this case the body of this test is not get executed at all, but the > result is 'Ok'. > > Actually, in setup_fake_machine() there's no need to create real kernel > maps. What we want is the fake maps. This patch removes the > machine__create_kernel_maps() in setup_fake_machine(), so it won't be > affected by kptr_restrict setting. > > Test result: > > $ cat /proc/sys/kernel/kptr_restrict > 1 > $ ~/perf test -v hist > 15: Test matching and linking multiple hists : > --- start --- > test child forked, pid 24031 > test child finished with 0 > ---- end ---- > Test matching and linking multiple hists: Ok > [SNIP] > > Signed-off-by: Wang Nan <wangnan0@huawei.com> > Suggested-by: Namhyung Kim <namhyung@kernel.org> > Acked-by: Namhyung Kim <namhyung@kernel.org> > Cc: Arnaldo Carvalho de Melo <acme@redhat.com> > Cc: Jiri Olsa <jolsa@kernel.org> > Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> [...] MBR, Sergei
Em Mon, Jan 11, 2016 at 05:25:48PM +0300, Sergei Shtylyov escreveu: > On 01/11/2016 04:48 PM, Wang Nan wrote: > > >Commit 71d6de64feddd4b455555326fba2111b3006d9e0 ('perf test: Fix hist > >testcases when kptr_restrict is on') solves a double free problem when > > You didn't run this patch thru scripts/checkpatch.pl, I guess? A > certain commit citing style is enforced now, and yours doesn't quite > match it... Which is? /me goes to read checpatch.pl... - Arnaldo > >'perf test hist' calling setup_fake_machine(). However, the result is > >still incorrect. For example: > > > > $ ./perf test -v 'filtering hist entries' > > 25: Test filtering hist entries : > > --- start --- > > test child forked, pid 4186 > > Cannot create kernel maps > > test child finished with 0 > > ---- end ---- > > Test filtering hist entries: Ok > > > >In this case the body of this test is not get executed at all, but the > >result is 'Ok'. > > > >Actually, in setup_fake_machine() there's no need to create real kernel > >maps. What we want is the fake maps. This patch removes the > >machine__create_kernel_maps() in setup_fake_machine(), so it won't be > >affected by kptr_restrict setting. > > > >Test result: > > > > $ cat /proc/sys/kernel/kptr_restrict > > 1 > > $ ~/perf test -v hist > > 15: Test matching and linking multiple hists : > > --- start --- > > test child forked, pid 24031 > > test child finished with 0 > > ---- end ---- > > Test matching and linking multiple hists: Ok > > [SNIP] > > > >Signed-off-by: Wang Nan <wangnan0@huawei.com> > >Suggested-by: Namhyung Kim <namhyung@kernel.org> > >Acked-by: Namhyung Kim <namhyung@kernel.org> > >Cc: Arnaldo Carvalho de Melo <acme@redhat.com> > >Cc: Jiri Olsa <jolsa@kernel.org> > >Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> > [...] > > MBR, Sergei
Em Mon, Jan 11, 2016 at 12:58:37PM -0200, Arnaldo Carvalho de Melo escreveu: > Em Mon, Jan 11, 2016 at 05:25:48PM +0300, Sergei Shtylyov escreveu: > > On 01/11/2016 04:48 PM, Wang Nan wrote: > > > > >Commit 71d6de64feddd4b455555326fba2111b3006d9e0 ('perf test: Fix hist > > >testcases when kptr_restrict is on') solves a double free problem when > > > > You didn't run this patch thru scripts/checkpatch.pl, I guess? A > > certain commit citing style is enforced now, and yours doesn't quite > > match it... > > Which is? /me goes to read checpatch.pl... So, this is it: [acme@felicio linux]$ scripts/checkpatch.pl /wb/1.patch ERROR: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'Commit 71d6de64fedd ("perf test: Fix hist testcases when kptr_restrict is on")' #62: Commit 71d6de64feddd4b455555326fba2111b3006d9e0 ('perf test: Fix hist total: 1 errors, 0 warnings, 11 lines checked /wb/1.patch has style problems, please review. NOTE: If any of the errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS. [acme@felicio linux]$ Ok, matches what I use with this macro that I run in vim with ':!fixes' after selecting the long commit hash: #!/bin/bash if [ $# -eq 1 ] ; then cset=$1 else read cset fi git log --oneline $cset | head -1 | sed -r 's/([^ ]+) (.*)/Fixes: \1 \("\2\")/g' ------------------------ And I have: [acme@felicio linux]$ grep abbrev .git/config abbrev = 12 - Arnaldo
diff --git a/tools/perf/tests/hists_common.c b/tools/perf/tests/hists_common.c index bcfd081..071a8b5 100644 --- a/tools/perf/tests/hists_common.c +++ b/tools/perf/tests/hists_common.c @@ -87,11 +87,6 @@ struct machine *setup_fake_machine(struct machines *machines) return NULL; } - if (machine__create_kernel_maps(machine)) { - pr_debug("Cannot create kernel maps\n"); - return NULL; - } - for (i = 0; i < ARRAY_SIZE(fake_threads); i++) { struct thread *thread;
Commit 71d6de64feddd4b455555326fba2111b3006d9e0 ('perf test: Fix hist testcases when kptr_restrict is on') solves a double free problem when 'perf test hist' calling setup_fake_machine(). However, the result is still incorrect. For example: $ ./perf test -v 'filtering hist entries' 25: Test filtering hist entries : --- start --- test child forked, pid 4186 Cannot create kernel maps test child finished with 0 ---- end ---- Test filtering hist entries: Ok In this case the body of this test is not get executed at all, but the result is 'Ok'. Actually, in setup_fake_machine() there's no need to create real kernel maps. What we want is the fake maps. This patch removes the machine__create_kernel_maps() in setup_fake_machine(), so it won't be affected by kptr_restrict setting. Test result: $ cat /proc/sys/kernel/kptr_restrict 1 $ ~/perf test -v hist 15: Test matching and linking multiple hists : --- start --- test child forked, pid 24031 test child finished with 0 ---- end ---- Test matching and linking multiple hists: Ok [SNIP] Signed-off-by: Wang Nan <wangnan0@huawei.com> Suggested-by: Namhyung Kim <namhyung@kernel.org> Acked-by: Namhyung Kim <namhyung@kernel.org> Cc: Arnaldo Carvalho de Melo <acme@redhat.com> Cc: Jiri Olsa <jolsa@kernel.org> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> --- tools/perf/tests/hists_common.c | 5 ----- 1 file changed, 5 deletions(-)