Re: [PATCH v2 1/2] trace-cmd: Optimize how pid filters are expressed

[Slavomir Kaslev <kaslevs@vmware.com>]

On Mon, 2019-04-15 at 17:59 -0400, Steven Rostedt wrote:
> On Mon, 15 Apr 2019 19:47:38 +0300
> Slavomir Kaslev <kaslevs@vmware.com> wrote:
> 
> > Express pid filters as allowed/disallowed filter ranges
> > 
> >   (pid>=100&&pid<=103)  
> > 
> > instead of specifying them per pid
> > 
> >   (pid==100||pid==101||pid==102||pid==103)
> > 
> > This makes the size of the resulting filter smaller (and faster)
> > and avoids
> > overflowing the filter size limit of one page which we can hit on
> > bigger
> > machines (say >160 CPUs).
> 
> Except it breaks if we have a split.
> 
> I ran this:
> 
>  hackbench 10 &
>  tracecmd/trace-cmd record -e sched_switch cat
> /sys/kernel/debug/tracing/events/sched/sched_switch/filter
> 
> 
> Time: 0.093
> (common_pid<6959||common_pid>6969)||(common_pid<6945||common_pid>6957
> )||(next_pid<6959||next_pid>6969)||(next_pid<6945||next_pid>6957)
> 
> This was the output. Showing that we had common_pid from 6959 - 6969
> and 6945 - 6957 (a 6958 was missing), and because of this, we now
> trace
> all processes because (common_pid < 6959 || common_pid > 6957) is
> always true.
> 
> We need an "&&" there somewhere. That should have been:
> 
> ((common_pid<6959||common_pid>6969)&&(common_pid<6945||common_pid>695
> 7))||((next_pid<6959||next_pid>6969)&&(next_pid<6945||next_pid>6957))

Good catch. I've misread how the original code worked. It appends
exclude filters with && and non-exclude with ||. I'll send a fix in v3.

-- Slavi