[PATCH 1/2] perf probe: Fix concat_probe_trace_events

[PATCH 1/2] perf probe: Fix concat_probe_trace_events

[Ravi Bangoria]

'*ntevs' contains number of elements present in 'tevs' array. If
there are no elements in array, 'tevs2' can be directly assigned
to 'tevs' without allocating more space. So the condition should
be  '*ntevs == 0'  not  'ntevs == 0'.

Fixes: 42bba263eb58 ("perf probe: Allow wildcard for cached events")
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
---
 tools/perf/util/probe-event.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 28fb62c..4f9d6ee 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -3057,7 +3057,7 @@ concat_probe_trace_events(struct probe_trace_event **tevs, int *ntevs,
 	struct probe_trace_event *new_tevs;
 	int ret = 0;
 
-	if (ntevs == 0) {
+	if (*ntevs == 0) {
 		*tevs = *tevs2;
 		*ntevs = ntevs2;
 		*tevs2 = NULL;
-- 
2.9.3

[PATCH 2/2] perf probe: Remove stale func add_perf_probe_events

[Ravi Bangoria]

I don't see any user of this function. This function was being copied
to tools/perf/builtin-probe.c by commit b02137cc6550 ("perf probe: Move
print logic into cmd_probe()"). Since then it has became stale.

Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
---
 tools/perf/util/probe-event.c | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 4f9d6ee..9f18204 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -3361,24 +3361,6 @@ void cleanup_perf_probe_events(struct perf_probe_event *pevs, int npevs)
 	}
 }
 
-int add_perf_probe_events(struct perf_probe_event *pevs, int npevs)
-{
-	int ret;
-
-	ret = init_probe_symbol_maps(pevs->uprobes);
-	if (ret < 0)
-		return ret;
-
-	ret = convert_perf_probe_events(pevs, npevs);
-	if (ret == 0)
-		ret = apply_perf_probe_events(pevs, npevs);
-
-	cleanup_perf_probe_events(pevs, npevs);
-
-	exit_probe_symbol_maps();
-	return ret;
-}
-
 int del_perf_probe_events(struct strfilter *filter)
 {
 	int ret, ret2, ufd = -1, kfd = -1;
-- 
2.9.3

Re: [PATCH 2/2] perf probe: Remove stale func add_perf_probe_events

[Masami Hiramatsu]

On Wed,  8 Mar 2017 12:29:08 +0530
Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote:

> I don't see any user of this function. This function was being copied
> to tools/perf/builtin-probe.c by commit b02137cc6550 ("perf probe: Move
> print logic into cmd_probe()"). Since then it has became stale.

Hmm, I have intended to keep it as an library API, which allows 
user to add event silently (e.g. adding sdt event in background).

Thanks,

> 
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> ---
>  tools/perf/util/probe-event.c | 18 ------------------
>  1 file changed, 18 deletions(-)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 4f9d6ee..9f18204 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -3361,24 +3361,6 @@ void cleanup_perf_probe_events(struct perf_probe_event *pevs, int npevs)
>  	}
>  }
>  
> -int add_perf_probe_events(struct perf_probe_event *pevs, int npevs)
> -{
> -	int ret;
> -
> -	ret = init_probe_symbol_maps(pevs->uprobes);
> -	if (ret < 0)
> -		return ret;
> -
> -	ret = convert_perf_probe_events(pevs, npevs);
> -	if (ret == 0)
> -		ret = apply_perf_probe_events(pevs, npevs);
> -
> -	cleanup_perf_probe_events(pevs, npevs);
> -
> -	exit_probe_symbol_maps();
> -	return ret;
> -}
> -
>  int del_perf_probe_events(struct strfilter *filter)
>  {
>  	int ret, ret2, ufd = -1, kfd = -1;
> -- 
> 2.9.3
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH 2/2] perf probe: Remove stale func add_perf_probe_events

[Ravi Bangoria]

Thanks Masami for the review,

On Wednesday 08 March 2017 03:13 PM, Masami Hiramatsu wrote:
> On Wed,  8 Mar 2017 12:29:08 +0530
> Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote:
>
>> I don't see any user of this function. This function was being copied
>> to tools/perf/builtin-probe.c by commit b02137cc6550 ("perf probe: Move
>> print logic into cmd_probe()"). Since then it has became stale.
> Hmm, I have intended to keep it as an library API, which allows 
> user to add event silently (e.g. adding sdt event in background).

Makes sense. Dropping it.

-Ravi

Re: [PATCH 1/2] perf probe: Fix concat_probe_trace_events

[Masami Hiramatsu]

On Wed,  8 Mar 2017 12:29:07 +0530
Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote:

> '*ntevs' contains number of elements present in 'tevs' array. If
> there are no elements in array, 'tevs2' can be directly assigned
> to 'tevs' without allocating more space. So the condition should
> be  '*ntevs == 0'  not  'ntevs == 0'.

Oops, good catch!

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>

Thanks!

> 
> Fixes: 42bba263eb58 ("perf probe: Allow wildcard for cached events")
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> ---
>  tools/perf/util/probe-event.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 28fb62c..4f9d6ee 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -3057,7 +3057,7 @@ concat_probe_trace_events(struct probe_trace_event **tevs, int *ntevs,
>  	struct probe_trace_event *new_tevs;
>  	int ret = 0;
>  
> -	if (ntevs == 0) {
> +	if (*ntevs == 0) {
>  		*tevs = *tevs2;
>  		*ntevs = ntevs2;
>  		*tevs2 = NULL;
> -- 
> 2.9.3
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH 1/2] perf probe: Fix concat_probe_trace_events

[Ravi Bangoria]

On Wednesday 08 March 2017 02:07 PM, Masami Hiramatsu wrote:
> On Wed,  8 Mar 2017 12:29:07 +0530
> Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote:
>
>> '*ntevs' contains number of elements present in 'tevs' array. If
>> there are no elements in array, 'tevs2' can be directly assigned
>> to 'tevs' without allocating more space. So the condition should
>> be  '*ntevs == 0'  not  'ntevs == 0'.
> Oops, good catch!
>
> Acked-by: Masami Hiramatsu <mhiramat@kernel.org>

Hi Arnaldo,

Can you please pull this patch.

Thanks,
Ravi

Re: [PATCH 1/2] perf probe: Fix concat_probe_trace_events

[Arnaldo Carvalho de Melo]

Em Mon, Mar 20, 2017 at 03:03:05PM +0530, Ravi Bangoria escreveu:
> 
> 
> On Wednesday 08 March 2017 02:07 PM, Masami Hiramatsu wrote:
> > On Wed,  8 Mar 2017 12:29:07 +0530
> > Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote:
> >
> >> '*ntevs' contains number of elements present in 'tevs' array. If
> >> there are no elements in array, 'tevs2' can be directly assigned
> >> to 'tevs' without allocating more space. So the condition should
> >> be  '*ntevs == 0'  not  'ntevs == 0'.
> > Oops, good catch!
> >
> > Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
> 
> Hi Arnaldo,
> 
> Can you please pull this patch.

Sure, done.

- Arnaldo

[tip:perf/core] perf probe: Fix concat_probe_trace_events

[tip-bot for Ravi Bangoria]

Commit-ID:  f0a30dca5f84fe8048271799b56677ac2279de66
Gitweb:     http://git.kernel.org/tip/f0a30dca5f84fe8048271799b56677ac2279de66
Author:     Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
AuthorDate: Wed, 8 Mar 2017 12:29:07 +0530
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 20 Mar 2017 15:01:32 -0300

perf probe: Fix concat_probe_trace_events

'*ntevs' contains number of elements present in 'tevs' array. If there
are no elements in array, 'tevs2' can be directly assigned to 'tevs'
without allocating more space. So the condition should be  '*ntevs == 0'
not  'ntevs == 0'.

Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Fixes: 42bba263eb58 ("perf probe: Allow wildcard for cached events")
Link: http://lkml.kernel.org/r/20170308065908.4128-1-ravi.bangoria@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-event.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index b19d178..6740d68 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -3048,7 +3048,7 @@ concat_probe_trace_events(struct probe_trace_event **tevs, int *ntevs,
 	struct probe_trace_event *new_tevs;
 	int ret = 0;
 
-	if (ntevs == 0) {
+	if (*ntevs == 0) {
 		*tevs = *tevs2;
 		*ntevs = ntevs2;
 		*tevs2 = NULL;