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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20170317105713.a4b1564a1fa2df5c9e340d5a@kernel.org>
Date:   Fri, 17 Mar 2017 10:57:13 +0900
From:   Masami Hiramatsu <mhiramat@...nel.org>
To:     Kefeng Wang <wangkefeng.wang@...wei.com>
Cc:     Arnaldo Carvalho de Melo <acme@...nel.org>,
        <linux-kernel@...r.kernel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Jiri Olsa <jolsa@...nel.org>,
        Namhyung Kim <namhyung@...nel.org>,
        Wang Nan <wangnan0@...wei.com>, <guohanjun@...wei.com>
Subject: Re: [PATCH] perf probe: Return errno when does not hit any event

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

> 
> 
> On 2017/3/16 17:39, Masami Hiramatsu wrote:
> > On Tue, 14 Mar 2017 10:30:56 -0300
> > Arnaldo Carvalho de Melo <acme@...nel.org> 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@...nel.org>