perf tools mem: Fix -t store option for record command

PATCH In review Jiri Olsa 4 participants 6 messages Aug 11, 2016 – Aug 16, 2016 perf events

Share & tools

Jiri Olsa

author reviewer

2016-08-11 08:52 UTC #0 lore raw

PATCH 1/1 perf tools mem: Fix -t store option for record command

Michael reported 'perf mem -t store record' being broken. The reason is latest rework of this area:
commit acbe613e0c03 ("perf tools: Add monitored events array")

We don't mark perf_mem_events store record when -t store option is specified.

Fixes commit acbe613e0c03 ("perf tools: Add monitored events array")

Reported-by Michael Petlan <[email protected]>

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Jiri Olsa <[email protected]>

---
 tools/perf/builtin-mem.c | 3 +++
 1 file changed, 3 insertions(+)

tools/perf/builtin-mem.c +3 -0

index d608a2c9e48c..d1ce29be560e 100644
@@ -88,6 +88,9 @@ static int __cmd_record(int argc, const char **argv, struct perf_mem *mem)
 	if (mem->operation & MEM_OPERATION_LOAD)
 		perf_mem_events[PERF_MEM_EVENTS__LOAD].record = true;
 
+	if (mem->operation & MEM_OPERATION_STORE)
+		perf_mem_events[PERF_MEM_EVENTS__STORE].record = true;
+
 	if (perf_mem_events[PERF_MEM_EVENTS__LOAD].record)
 		rec_argv[i++] = "-W";

Signature · 1 line

2.4.11

Arnaldo Carvalho de Melo

maintainer

2016-08-11 11:51 UTC #1 lore raw

Em Thu, Aug 11, 2016 at 10:50:57AM +0200, Jiri Olsa escreveu:

Quoted 8 lines

Michael reported 'perf mem -t store record' being broken. The reason is latest rework of this area:
commit acbe613e0c03 ("perf tools: Add monitored events array")
We don't mark perf_mem_events store record when -t store option is specified.
Fixes commit acbe613e0c03 ("perf tools: Add monitored events array")

         ^^^^^^

I'll remove this extraneous bit

Quoted 23 lines

Reported-by Michael Petlan <[email protected]>

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Jiri Olsa <[email protected]>

---
 tools/perf/builtin-mem.c | 3 +++
 1 file changed, 3 insertions(+)

tools/perf/builtin-mem.c +3 -0

index d608a2c9e48c..d1ce29be560e 100644
@@ -88,6 +88,9 @@ static int __cmd_record(int argc, const char **argv, struct perf_mem *mem)
 	if (mem->operation & MEM_OPERATION_LOAD)
 		perf_mem_events[PERF_MEM_EVENTS__LOAD].record = true;
 
+	if (mem->operation & MEM_OPERATION_STORE)
+		perf_mem_events[PERF_MEM_EVENTS__STORE].record = true;
+
 	if (perf_mem_events[PERF_MEM_EVENTS__LOAD].record)
 		rec_argv[i++] = "-W";

Signature · 1 line

2.4.11

Jiri Olsa

2016-08-11 12:00 UTC #2 lore raw

Quoted 11 lines · Arnaldo Carvalho de Melo

Em Thu, Aug 11, 2016 at 10:50:57AM +0200, Jiri Olsa escreveu:
Quoted 8 lines
Michael reported 'perf mem -t store record' being broken. The reason is latest rework of this area:
commit acbe613e0c03 ("perf tools: Add monitored events array")
We don't mark perf_mem_events store record when -t store option is specified.
Fixes commit acbe613e0c03 ("perf tools: Add monitored events array")
         ^^^^^^
I'll remove this extraneous bit

well, checkpatch script complained about that when I did not put it there.. but I don't mind ;-)

jirka

Quoted 24 lines

Reported-by Michael Petlan <[email protected]>

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Jiri Olsa <[email protected]>

---
 tools/perf/builtin-mem.c | 3 +++
 1 file changed, 3 insertions(+)

tools/perf/builtin-mem.c +3 -0

index d608a2c9e48c..d1ce29be560e 100644
@@ -88,6 +88,9 @@ static int __cmd_record(int argc, const char **argv, struct perf_mem *mem)
 	if (mem->operation & MEM_OPERATION_LOAD)
 		perf_mem_events[PERF_MEM_EVENTS__LOAD].record = true;
 
+	if (mem->operation & MEM_OPERATION_STORE)
+		perf_mem_events[PERF_MEM_EVENTS__STORE].record = true;
+
 	if (perf_mem_events[PERF_MEM_EVENTS__LOAD].record)
 		rec_argv[i++] = "-W";

Signature · 1 line

2.4.11

Arnaldo Carvalho de Melo

maintainer

2016-08-11 14:47 UTC #3 lore raw

Em Thu, Aug 11, 2016 at 02:00:32PM +0200, Jiri Olsa escreveu:

Quoted 9 lines · Arnaldo Carvalho de Melo

Quoted 8 lines
Em Thu, Aug 11, 2016 at 10:50:57AM +0200, Jiri Olsa escreveu:
Quoted 7 lines
Michael reported 'perf mem -t store record' being broken. The reason is latest rework of this area:
commit acbe613e0c03 ("perf tools: Add monitored events array")
We don't mark perf_mem_events store record when -t store option is specified.

Quoted 6 lines

Quoted 3 lines
Quoted 1 line
Fixes commit acbe613e0c03 ("perf tools: Add monitored events array")
         ^^^^^^
I'll remove this extraneous bit
well, checkpatch script complained about that when I did not put it there.. but I don't mind ;-)

Huh?

[acme@jouet linux]$ grep Fixes /wb/1.patch

Fixes commit acbe613e0c03 ("perf tools: Add monitored events array")

[acme@jouet linux]$ scripts/checkpatch.pl /wb/1.patch
total: 0 errors, 0 warnings, 9 lines checked

/wb/1.patch has no obvious style problems and is ready for submission. [acme@jouet linux]$ vim /wb/1.patch
[acme@jouet linux]$ grep Fixes /wb/1.patch

Fixes acbe613e0c03 ("perf tools: Add monitored events array")

[acme@jouet linux]$ scripts/checkpatch.pl /wb/1.patch
total: 0 errors, 0 warnings, 9 lines checked

/wb/1.patch has no obvious style problems and is ready for submission. [acme@jouet linux]$

- Arnaldo

Jiri Olsa

2016-08-11 14:58 UTC #4 lore raw

Quoted 10 lines · Arnaldo Carvalho de Melo

Em Thu, Aug 11, 2016 at 02:00:32PM +0200, Jiri Olsa escreveu:
Quoted 9 lines · Arnaldo Carvalho de Melo
Quoted 8 lines
Em Thu, Aug 11, 2016 at 10:50:57AM +0200, Jiri Olsa escreveu:
Quoted 7 lines
Michael reported 'perf mem -t store record' being broken. The reason is latest rework of this area:
commit acbe613e0c03 ("perf tools: Add monitored events array")
We don't mark perf_mem_events store record when -t store option is specified.

Quoted 8 lines

Quoted 6 lines
Quoted 3 lines
Quoted 1 line
Fixes commit acbe613e0c03 ("perf tools: Add monitored events array")
         ^^^^^^
I'll remove this extraneous bit
well, checkpatch script complained about that when I did not put it there.. but I don't mind ;-)
Huh?

ah that was the other line with the commit number, the one with Fixes: tag is ok.. nevermind, sry for noise ;-)

jirka

---
[jolsa@krava linux-perf]$ ./scripts/checkpatch.pl 0001-perf-tools-mem-Fix-t-store-option-for-record-command.patch
ERROR: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit acbe613e0c03 ("perf tools: Add monitored events array")' #8:
acbe613e0c03 ("perf tools: Add monitored events array")

total: 1 errors, 0 warnings, 9 lines checked

tip-bot for Jiri Olsa

merge bot

2016-08-16 18:14 UTC #5 lore raw

In reply to Jiri Olsa

perf tools mem: Fix -t store option for record command

Commit-ID:  33da54fa86e29b87fe1e83bd0f15b4ef2be53ecb
Gitweb:     http://git.kernel.org/tip/33da54fa86e29b87fe1e83bd0f15b4ef2be53ecb
Author:     Jiri Olsa <[email protected]>
AuthorDate: Thu, 11 Aug 2016 10:50:57 +0200
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Fri, 12 Aug 2016 14:39:48 -0300

perf tools mem: Fix -t store option for record command

Michael reported 'perf mem -t store record' being broken. The reason is latest rework of this area:

commit acbe613e0c03 ("perf tools: Add monitored events array")

We don't mark perf_mem_events store record when -t store option is specified.

Committer notes:

Before:

  # perf mem -t store record usleep 1
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.020 MB perf.data (7 samples) ]
  # perf evlist
  cycles:ppp
  #

After:

  # perf mem -t store record usleep 1
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.020 MB perf.data (7 samples) ]
  # perf evlist
  cpu/mem-stores/P
  #

Reported-by Michael Petlan <[email protected]>

Signed-off-by Jiri Olsa <[email protected]>

Tested-by Arnaldo Carvalho de Melo <[email protected]>

Cc David Ahern <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Peter Zijlstra <[email protected]>

Fixes acbe613e0c03 ("perf tools: Add monitored events array")

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/builtin-mem.c | 3 +++
 1 file changed, 3 insertions(+)

tools/perf/builtin-mem.c +3 -0

index d608a2c..d1ce29b 100644
@@ -88,6 +88,9 @@ static int __cmd_record(int argc, const char **argv, struct perf_mem *mem)
 	if (mem->operation & MEM_OPERATION_LOAD)
 		perf_mem_events[PERF_MEM_EVENTS__LOAD].record = true;
 
+	if (mem->operation & MEM_OPERATION_STORE)
+		perf_mem_events[PERF_MEM_EVENTS__STORE].record = true;
+
 	if (perf_mem_events[PERF_MEM_EVENTS__LOAD].record)
 		rec_argv[i++] = "-W";