Re: [PATCH] perf probe: Return errno when does not hit any event

[Masami Hiramatsu]

On Thu, 16 Mar 2017 20:07:08 +0800
Kefeng Wang <wangkefeng.wang@xxxxxxxxxx> wrote:

> 
> 
> On 2017/3/16 17:39, Masami Hiramatsu wrote:
> > On Tue, 14 Mar 2017 10:30:56 -0300
> > Arnaldo Carvalho de Melo <acme@xxxxxxxxxx> wrote:
> > 
> >> Em Tue, Mar 14, 2017 at 09:19:47PM +0800, Kefeng Wang escreveu:
> >>> Hi all, any comments, thanks.
> >>
> >> For 'perf probe' make sure Masami is in the CC list, adding him, Masami?
> > 
> > Thanks Arnaldo,
> > 
> > Hi Kefeng, I've made the error ignored by design, since user might pass
> > the wildcard pattern to perf probe -d just for making sure.
> > 
> > You can check the pattern hits any event by using perf-probe --list PATTERN.
> > Would you have any use case to check the result?
> 
> There are some perf test cases from our inner tester, they will check
> the returned value for each execution, and they think return errno is
> better to cater for users when delete inexistent events, as it was before.
> And the patch only returns error when does not hit any event.
> 
> If such behavior, error ignored is more reasonableo< I will push them to
> modify the test cases.

Hmm, the commit dddc7ee32fa1 ("perf probe: Fix an error when deleting probes successfully") introduced this behavior. But I think this fix will not be
the best solution.

OK, so could you update your patch according my comment and resend it?


> >>>>> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> >>>>> index 1fcebc3..c46b41c 100644
> >>>>> --- a/tools/perf/builtin-probe.c
> >>>>> +++ b/tools/perf/builtin-probe.c
> >>>>> @@ -444,7 +444,8 @@ static int perf_del_probe_events(struct strfilter *filter)
> >>>>>  	if (ret == -ENOENT && ret2 == -ENOENT)
> >>>>>  		pr_debug("\"%s\" does not hit any event.\n", str);

This should be pr_warning if this case has an error.

> >>>>>  		/* Note that this is silently ignored */

This comment should be removed too, since this error is not ignored anymore.


> >>>>> -	ret = 0;
> >>>>> +	else
> >>>>> +		ret = 0;
> >>>>>  

With these patches, perf probe shows an error if there is no event hit
for --delete.

$ sudo ./perf probe -d hoge
"hoge" does not hit any event.
  Error: Failed to delete events.


Thanks,


> >>>>>  error:
> >>>>>  	if (kfd >= 0)
> >>>>>
> >>>>
> >>>>
> >>>> .
> >>>>
> > 
> > 
> 


-- 
Masami Hiramatsu <mhiramat@xxxxxxxxxx>