Re: [PATCH] perf record: Allow passing perf's own pid to '--filter'

[Wangnan (F)]

On 2015/7/6 23:40, Arnaldo Carvalho de Melo wrote:
> Em Mon, Jul 06, 2015 at 11:00:10PM +0800, pi3orama escreveu:
> > åèæç iPhone
>   
> > > å 2015å7æ6æïäå9:56ïArnaldo Carvalho de Melo <acme@xxxxxxxxxx> åéï
> > > I.e. having something in the filter expression that gets transformed
> > > into the tools' pid, I have no problem with that, just curious about
> > > what would be the best character to signal that a substitution needs to
> > > be performed, if it is really '@VAR', as my first selection would be
> > > '$VAR',
>   
> > $ has special meaning for shell. Using $ in cmdline require users use escaping or '' quoted string. Therefore I believe @ should be better. What do you think?
> Yeah, that gets in the way, as it gets in the way for '!', i.e.
> negating, and even tho, that is what is used in strace (and in 'perf
> trace'):
>
>    strace -e \!open,write ls
>
> Or:
>
>    strace -e '!open,write' ls
>
> But apart from that, it would be good if expressions used in 'perf
> probe' and here could have as much as possible the same semantics for
> those markers, i.e. 'perf probe' already uses @ for some stuff, probably
> the meaning is for "at", i.e. something at some place.
>
> '$' strongly associated with variables, so I don't think it would be a
> big problem to enclose expressions where variables (we may end having
> others, no?)  in '', i.e.
>
>    perf record -e sched:*switch --filter 'common_pid != $PERF_PID' -a
>
> Doesn't look so ugly or cumbersome :-)

But what about user want to use real shell variables also?

 perf record -e raw_syscalls:* "common_pid !="'$PERF_PID'" && 
common_pid != $X_PID"

Or

 perf record -e raw_syscalls:* "common_pid !=\$PERF_PID && common_pid 
!= $X_PID"

right?

However, since you and Steven prefer '$' than '@' and '@' has its own 
meaning 'at' in 'perf probe', I'll use '$' in my next version.

I looked your new code. You added perf_evsel__append_filter() to enable 
us append a filter expression in '(%s) <op> (%s)' manner, and also 
perf_evlist__set_filter_pid() to add 'common_pid != %d' expression. They 
are nice scaffolds if we'd like to add a new cmdline option 
'--filter-pids' and '--filter-perf'. However, I think we should let 
users who use --filter take full control of their filters, instead of 
providing many helpers which can do similar things to confuse them. So I 
decide not to use those functions you added these days in my next version.

Thank you.

> - Arnaldo


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/