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

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

[Jiri Olsa]

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 <mpetlan@redhat.com>
Link: http://lkml.kernel.org/n/tip-dbytnk7urdnnaw7ckdu1w7lq@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
 tools/perf/builtin-mem.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/builtin-mem.c b/tools/perf/builtin-mem.c
index d608a2c9e48c..d1ce29be560e 100644
--- a/tools/perf/builtin-mem.c
+++ b/tools/perf/builtin-mem.c
@@ -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";
 
-- 
2.4.11

Re: [PATCH] perf tools mem: Fix -t store option for record command

[Arnaldo Carvalho de Melo]

Em Thu, Aug 11, 2016 at 10:50:57AM +0200, Jiri Olsa escreveu:
> 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

> Reported-by: Michael Petlan <mpetlan@redhat.com>
> Link: http://lkml.kernel.org/n/tip-dbytnk7urdnnaw7ckdu1w7lq@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@redhat.com>
> ---
>  tools/perf/builtin-mem.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tools/perf/builtin-mem.c b/tools/perf/builtin-mem.c
> index d608a2c9e48c..d1ce29be560e 100644
> --- a/tools/perf/builtin-mem.c
> +++ b/tools/perf/builtin-mem.c
> @@ -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";
>  
> -- 
> 2.4.11

Re: [PATCH] perf tools mem: Fix -t store option for record command

[Jiri Olsa]

On Thu, Aug 11, 2016 at 08:51:38AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Aug 11, 2016 at 10:50:57AM +0200, Jiri Olsa escreveu:
> > 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

> 
> > Reported-by: Michael Petlan <mpetlan@redhat.com>
> > Link: http://lkml.kernel.org/n/tip-dbytnk7urdnnaw7ckdu1w7lq@git.kernel.org
> > Signed-off-by: Jiri Olsa <jolsa@redhat.com>
> > ---
> >  tools/perf/builtin-mem.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/tools/perf/builtin-mem.c b/tools/perf/builtin-mem.c
> > index d608a2c9e48c..d1ce29be560e 100644
> > --- a/tools/perf/builtin-mem.c
> > +++ b/tools/perf/builtin-mem.c
> > @@ -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";
> >  
> > -- 
> > 2.4.11

Re: [PATCH] perf tools mem: Fix -t store option for record command

[Arnaldo Carvalho de Melo]

Em Thu, Aug 11, 2016 at 02:00:32PM +0200, Jiri Olsa escreveu:
> On Thu, Aug 11, 2016 at 08:51:38AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Thu, Aug 11, 2016 at 10:50:57AM +0200, Jiri Olsa escreveu:
> > > 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 ;-)

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

Re: [PATCH] perf tools mem: Fix -t store option for record command

[Jiri Olsa]

On Thu, Aug 11, 2016 at 11:46:46AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Aug 11, 2016 at 02:00:32PM +0200, Jiri Olsa escreveu:
> > On Thu, Aug 11, 2016 at 08:51:38AM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Thu, Aug 11, 2016 at 10:50:57AM +0200, Jiri Olsa escreveu:
> > > > 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 ;-)
> 
> 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:perf/urgent] perf tools mem: Fix -t store option for record command

[tip-bot for Jiri Olsa]

Commit-ID:  33da54fa86e29b87fe1e83bd0f15b4ef2be53ecb
Gitweb:     http://git.kernel.org/tip/33da54fa86e29b87fe1e83bd0f15b4ef2be53ecb
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Thu, 11 Aug 2016 10:50:57 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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 <mpetlan@redhat.com>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Fixes: acbe613e0c03 ("perf tools: Add monitored events array")
Link: http://lkml.kernel.org/r/1470905457-18311-1-git-send-email-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-mem.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/builtin-mem.c b/tools/perf/builtin-mem.c
index d608a2c..d1ce29b 100644
--- a/tools/perf/builtin-mem.c
+++ b/tools/perf/builtin-mem.c
@@ -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";