* [PATCH v4 0/2] Optimize pid filters @ 2019-04-17 13:09 Slavomir Kaslev 2019-04-17 13:09 ` [PATCH v4 1/2] trace-cmd: Optimize how pid filters are expressed Slavomir Kaslev 2019-04-17 13:09 ` [PATCH v4 2/2] trace-cmd: Document record --no-filter option in record's man page Slavomir Kaslev 0 siblings, 2 replies; 12+ messages in thread From: Slavomir Kaslev @ 2019-04-17 13:09 UTC (permalink / raw) To: rostedt Cc: linux-trace-devel, pauld, ykaradzhov, jbacik, tstoyanov, slavomir.kaslev This patchset optimizes how pid filters are expressed and makes it less likely that we overflow ftrace filters' size limit of one page. Changes since v3: Addressed Steven's review feedback Add section about --no-filter to `trace-cmd record`'s man page Dropped the patch adding --no-filter since Steven has already applied it Changes since v2: Append exclude rules with && Changes since v1: Add missing tags Fix append_filter_pid_range() callers to pass valid range as [min,max] Slavomir Kaslev (2): trace-cmd: Optimize how pid filters are expressed trace-cmd: Document record --no-filter option in record's man page Documentation/trace-cmd-record.1.txt | 4 + tracecmd/trace-record.c | 117 ++++++++++++++++++--------- 2 files changed, 85 insertions(+), 36 deletions(-) -- 2.19.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v4 1/2] trace-cmd: Optimize how pid filters are expressed 2019-04-17 13:09 [PATCH v4 0/2] Optimize pid filters Slavomir Kaslev @ 2019-04-17 13:09 ` Slavomir Kaslev 2019-04-17 13:58 ` Phil Auld 2019-04-17 13:09 ` [PATCH v4 2/2] trace-cmd: Document record --no-filter option in record's man page Slavomir Kaslev 1 sibling, 1 reply; 12+ messages in thread From: Slavomir Kaslev @ 2019-04-17 13:09 UTC (permalink / raw) To: rostedt Cc: linux-trace-devel, pauld, ykaradzhov, jbacik, tstoyanov, slavomir.kaslev 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). Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com> Reported-by: Phil Auld <pauld@redhat.com> Suggested-by: Steven Rostedt (VMware) <rostedt@goodmis.org> --- tracecmd/trace-record.c | 117 +++++++++++++++++++++++++++------------- 1 file changed, 81 insertions(+), 36 deletions(-) diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c index a3a34f1..4523128 100644 --- a/tracecmd/trace-record.c +++ b/tracecmd/trace-record.c @@ -951,10 +951,63 @@ static void update_ftrace_pids(int reset) static void update_event_filters(struct buffer_instance *instance); static void update_pid_event_filters(struct buffer_instance *instance); +static void append_filter_pid_range(char **filter, int *curr_len, + const char *field, + int start_pid, int end_pid, bool exclude) +{ + const char *op = "", *op1, *op2, *op3; + int len; + + if (*filter && **filter) + op = exclude ? "&&" : "||"; + + /* Handle thus case explicitly so that we get `pid==3` instead of + * `pid>=3&&pid<=3` for singleton ranges + */ + if (start_pid == end_pid) { +#define FMT "%s(%s%s%d)" + len = snprintf(NULL, 0, FMT, op, + field, exclude ? "!=" : "==", start_pid); + *filter = realloc(*filter, *curr_len + len + 1); + if (!*filter) + die("realloc"); + + len = snprintf(*filter + *curr_len, len + 1, FMT, op, + field, exclude ? "!=" : "==", start_pid); + *curr_len += len; + + return; +#undef FMT + } + + if (exclude) { + op1 = "<"; + op2 = "||"; + op3 = ">"; + } else { + op1 = ">="; + op2 = "&&"; + op3 = "<="; + } + +#define FMT "%s(%s%s%d%s%s%s%d)" + len = snprintf(NULL, 0, FMT, op, + field, op1, start_pid, op2, + field, op3, end_pid); + *filter = realloc(*filter, *curr_len + len + 1); + if (!*filter) + die("realloc"); + + len = snprintf(*filter + *curr_len, len + 1, FMT, op, + field, op1, start_pid, op2, + field, op3, end_pid); + *curr_len += len; +} + /** * make_pid_filter - create a filter string to all pids against @field * @curr_filter: Append to a previous filter (may realloc). Can be NULL - * @field: The fild to compare the pids against + * @field: The field to compare the pids against * * Creates a new string or appends to an existing one if @curr_filter * is not NULL. The new string will contain a filter with all pids @@ -964,54 +1017,46 @@ static void update_pid_event_filters(struct buffer_instance *instance); */ static char *make_pid_filter(char *curr_filter, const char *field) { + int start_pid = -1, last_pid = -1; + int last_exclude = -1; struct filter_pids *p; - char *filter; - char *orit; - char *match; - char *str; + char *filter = NULL; int curr_len = 0; - int len; /* Use the new method if possible */ if (have_set_event_pid) return NULL; - len = len_filter_pids + (strlen(field) + strlen("(==)||")) * nr_filter_pids; - - if (curr_filter) { - curr_len = strlen(curr_filter); - filter = realloc(curr_filter, curr_len + len + strlen("(&&())")); - if (!filter) - die("realloc"); - memmove(filter+1, curr_filter, curr_len); - filter[0] = '('; - strcat(filter, ")&&("); - curr_len = strlen(filter); - } else - filter = malloc(len); - if (!filter) - die("Failed to allocate pid filter"); - - /* Last '||' that is not used will cover the \0 */ - str = filter + curr_len; + if (!filter_pids) + return curr_filter; for (p = filter_pids; p; p = p->next) { - if (p->exclude) { - match = "!="; - orit = "&&"; - } else { - match = "=="; - orit = "||"; + /* + * PIDs are inserted in `filter_pids` from the front and that's + * why we expect them in descending order here. + */ + if (p->pid == last_pid - 1 && p->exclude == last_exclude) { + last_pid = p->pid; + continue; } - if (p == filter_pids) - orit = ""; - len = sprintf(str, "%s(%s%s%d)", orit, field, match, p->pid); - str += len; + if (start_pid != -1) + append_filter_pid_range(&filter, &curr_len, field, + last_pid, start_pid, + last_exclude); + + start_pid = last_pid = p->pid; + last_exclude = p->exclude; + } + append_filter_pid_range(&filter, &curr_len, field, + last_pid, start_pid, last_exclude); - if (curr_len) - sprintf(str, ")"); + if (curr_filter) { + char *save = filter; + asprintf(&filter, "(%s)&&(%s)", curr_filter, filter); + free(save); + } return filter; } -- 2.19.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v4 1/2] trace-cmd: Optimize how pid filters are expressed 2019-04-17 13:09 ` [PATCH v4 1/2] trace-cmd: Optimize how pid filters are expressed Slavomir Kaslev @ 2019-04-17 13:58 ` Phil Auld 2019-04-17 14:18 ` John Kacur ` (2 more replies) 0 siblings, 3 replies; 12+ messages in thread From: Phil Auld @ 2019-04-17 13:58 UTC (permalink / raw) To: Slavomir Kaslev Cc: rostedt, linux-trace-devel, ykaradzhov, jbacik, tstoyanov, slavomir.kaslev On Wed, Apr 17, 2019 at 04:09:58PM +0300 Slavomir Kaslev 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). This one works as well :) I finally hit a case where my trace-cmd pids were non-contiguous and this split the range up correctly. FILTER write /sys/kernel/debug/tracing/events/sched/sched_kthread_stop/filter (len 74) value "(common_pid<21420||common_pid>21425)&&(common_pid<21265||common_pid>21418)" FILTER write /sys/kernel/debug/tracing/events/sched/sched_kthread_stop_ret/filter (len 74) value "(common_pid<21420||common_pid>21425)&&(common_pid<21265||common_pid>21418)" ... FILTER write /sys/kernel/debug/tracing/events/sched/sched_switch/filter (len 142) value "(common_pid<21420||common_pid>21425)&&(common_pid<21265||common_pid>21418)||(next_pid<21420||next_pid>21425)&&(next_pid<21265||next_pid>21418)" The latter is correct given precendce of && before || but I wonder if () don't make sense? I always have to look that one up :) If I were writing that in code I'd probably put in the extra ()s, but since it's generated and no one actually sees it, probably okay and simpler as is. Having seen that and having tried it on a few other machines I'd be more willing to have a Tested-by: Phil Auld <pauld@redhat.com> on it, if you want it. Cheers, Phil > > Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com> > Reported-by: Phil Auld <pauld@redhat.com> > Suggested-by: Steven Rostedt (VMware) <rostedt@goodmis.org> > --- > tracecmd/trace-record.c | 117 +++++++++++++++++++++++++++------------- > 1 file changed, 81 insertions(+), 36 deletions(-) > > diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c > index a3a34f1..4523128 100644 > --- a/tracecmd/trace-record.c > +++ b/tracecmd/trace-record.c > @@ -951,10 +951,63 @@ static void update_ftrace_pids(int reset) > static void update_event_filters(struct buffer_instance *instance); > static void update_pid_event_filters(struct buffer_instance *instance); > > +static void append_filter_pid_range(char **filter, int *curr_len, > + const char *field, > + int start_pid, int end_pid, bool exclude) > +{ > + const char *op = "", *op1, *op2, *op3; > + int len; > + > + if (*filter && **filter) > + op = exclude ? "&&" : "||"; > + > + /* Handle thus case explicitly so that we get `pid==3` instead of > + * `pid>=3&&pid<=3` for singleton ranges > + */ > + if (start_pid == end_pid) { > +#define FMT "%s(%s%s%d)" > + len = snprintf(NULL, 0, FMT, op, > + field, exclude ? "!=" : "==", start_pid); > + *filter = realloc(*filter, *curr_len + len + 1); > + if (!*filter) > + die("realloc"); > + > + len = snprintf(*filter + *curr_len, len + 1, FMT, op, > + field, exclude ? "!=" : "==", start_pid); > + *curr_len += len; > + > + return; > +#undef FMT > + } > + > + if (exclude) { > + op1 = "<"; > + op2 = "||"; > + op3 = ">"; > + } else { > + op1 = ">="; > + op2 = "&&"; > + op3 = "<="; > + } > + > +#define FMT "%s(%s%s%d%s%s%s%d)" > + len = snprintf(NULL, 0, FMT, op, > + field, op1, start_pid, op2, > + field, op3, end_pid); > + *filter = realloc(*filter, *curr_len + len + 1); > + if (!*filter) > + die("realloc"); > + > + len = snprintf(*filter + *curr_len, len + 1, FMT, op, > + field, op1, start_pid, op2, > + field, op3, end_pid); > + *curr_len += len; > +} > + > /** > * make_pid_filter - create a filter string to all pids against @field > * @curr_filter: Append to a previous filter (may realloc). Can be NULL > - * @field: The fild to compare the pids against > + * @field: The field to compare the pids against > * > * Creates a new string or appends to an existing one if @curr_filter > * is not NULL. The new string will contain a filter with all pids > @@ -964,54 +1017,46 @@ static void update_pid_event_filters(struct buffer_instance *instance); > */ > static char *make_pid_filter(char *curr_filter, const char *field) > { > + int start_pid = -1, last_pid = -1; > + int last_exclude = -1; > struct filter_pids *p; > - char *filter; > - char *orit; > - char *match; > - char *str; > + char *filter = NULL; > int curr_len = 0; > - int len; > > /* Use the new method if possible */ > if (have_set_event_pid) > return NULL; > > - len = len_filter_pids + (strlen(field) + strlen("(==)||")) * nr_filter_pids; > - > - if (curr_filter) { > - curr_len = strlen(curr_filter); > - filter = realloc(curr_filter, curr_len + len + strlen("(&&())")); > - if (!filter) > - die("realloc"); > - memmove(filter+1, curr_filter, curr_len); > - filter[0] = '('; > - strcat(filter, ")&&("); > - curr_len = strlen(filter); > - } else > - filter = malloc(len); > - if (!filter) > - die("Failed to allocate pid filter"); > - > - /* Last '||' that is not used will cover the \0 */ > - str = filter + curr_len; > + if (!filter_pids) > + return curr_filter; > > for (p = filter_pids; p; p = p->next) { > - if (p->exclude) { > - match = "!="; > - orit = "&&"; > - } else { > - match = "=="; > - orit = "||"; > + /* > + * PIDs are inserted in `filter_pids` from the front and that's > + * why we expect them in descending order here. > + */ > + if (p->pid == last_pid - 1 && p->exclude == last_exclude) { > + last_pid = p->pid; > + continue; > } > - if (p == filter_pids) > - orit = ""; > > - len = sprintf(str, "%s(%s%s%d)", orit, field, match, p->pid); > - str += len; > + if (start_pid != -1) > + append_filter_pid_range(&filter, &curr_len, field, > + last_pid, start_pid, > + last_exclude); > + > + start_pid = last_pid = p->pid; > + last_exclude = p->exclude; > + > } > + append_filter_pid_range(&filter, &curr_len, field, > + last_pid, start_pid, last_exclude); > > - if (curr_len) > - sprintf(str, ")"); > + if (curr_filter) { > + char *save = filter; > + asprintf(&filter, "(%s)&&(%s)", curr_filter, filter); > + free(save); > + } > > return filter; > } > -- > 2.19.1 > -- ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 1/2] trace-cmd: Optimize how pid filters are expressed 2019-04-17 13:58 ` Phil Auld @ 2019-04-17 14:18 ` John Kacur 2019-04-17 14:25 ` Phil Auld 2019-04-17 14:52 ` Steven Rostedt 2019-04-17 14:54 ` Steven Rostedt 2 siblings, 1 reply; 12+ messages in thread From: John Kacur @ 2019-04-17 14:18 UTC (permalink / raw) To: Phil Auld Cc: Slavomir Kaslev, rostedt, linux-trace-devel, ykaradzhov, jbacik, tstoyanov, slavomir.kaslev On Wed, 17 Apr 2019, Phil Auld wrote: > On Wed, Apr 17, 2019 at 04:09:58PM +0300 Slavomir Kaslev 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). > > This one works as well :) > > I finally hit a case where my trace-cmd pids were non-contiguous and > this split the range up correctly. > > > FILTER write /sys/kernel/debug/tracing/events/sched/sched_kthread_stop/filter (len 74) value "(common_pid<21420||common_pid>21425)&&(common_pid<21265||common_pid>21418)" > FILTER write /sys/kernel/debug/tracing/events/sched/sched_kthread_stop_ret/filter (len 74) value "(common_pid<21420||common_pid>21425)&&(common_pid<21265||common_pid>21418)" > ... > FILTER write /sys/kernel/debug/tracing/events/sched/sched_switch/filter (len 142) value "(common_pid<21420||common_pid>21425)&&(common_pid<21265||common_pid>21418)||(next_pid<21420||next_pid>21425)&&(next_pid<21265||next_pid>21418)" It seems crazy that we write "common_pid", instead of "pid" or "cpid", or something like that. > > > The latter is correct given precendce of && before || but I wonder if () don't make sense? I always have to look > that one up :) > > If I were writing that in code I'd probably put in the extra ()s, but since it's generated and no > one actually sees it, probably okay and simpler as is. > > > Having seen that and having tried it on a few other machines I'd be more willing to have a > > Tested-by: Phil Auld <pauld@redhat.com> > > on it, if you want it. > > Cheers, > Phil > > > > > > Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com> > > Reported-by: Phil Auld <pauld@redhat.com> > > Suggested-by: Steven Rostedt (VMware) <rostedt@goodmis.org> > > --- > > tracecmd/trace-record.c | 117 +++++++++++++++++++++++++++------------- > > 1 file changed, 81 insertions(+), 36 deletions(-) > > > > diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c > > index a3a34f1..4523128 100644 > > --- a/tracecmd/trace-record.c > > +++ b/tracecmd/trace-record.c > > @@ -951,10 +951,63 @@ static void update_ftrace_pids(int reset) > > static void update_event_filters(struct buffer_instance *instance); > > static void update_pid_event_filters(struct buffer_instance *instance); > > > > +static void append_filter_pid_range(char **filter, int *curr_len, > > + const char *field, > > + int start_pid, int end_pid, bool exclude) > > +{ > > + const char *op = "", *op1, *op2, *op3; > > + int len; > > + > > + if (*filter && **filter) > > + op = exclude ? "&&" : "||"; > > + > > + /* Handle thus case explicitly so that we get `pid==3` instead of > > + * `pid>=3&&pid<=3` for singleton ranges > > + */ > > + if (start_pid == end_pid) { > > +#define FMT "%s(%s%s%d)" > > + len = snprintf(NULL, 0, FMT, op, > > + field, exclude ? "!=" : "==", start_pid); > > + *filter = realloc(*filter, *curr_len + len + 1); > > + if (!*filter) > > + die("realloc"); > > + > > + len = snprintf(*filter + *curr_len, len + 1, FMT, op, > > + field, exclude ? "!=" : "==", start_pid); > > + *curr_len += len; > > + > > + return; > > +#undef FMT > > + } > > + > > + if (exclude) { > > + op1 = "<"; > > + op2 = "||"; > > + op3 = ">"; > > + } else { > > + op1 = ">="; > > + op2 = "&&"; > > + op3 = "<="; > > + } > > + > > +#define FMT "%s(%s%s%d%s%s%s%d)" > > + len = snprintf(NULL, 0, FMT, op, > > + field, op1, start_pid, op2, > > + field, op3, end_pid); > > + *filter = realloc(*filter, *curr_len + len + 1); > > + if (!*filter) > > + die("realloc"); > > + > > + len = snprintf(*filter + *curr_len, len + 1, FMT, op, > > + field, op1, start_pid, op2, > > + field, op3, end_pid); > > + *curr_len += len; > > +} > > + > > /** > > * make_pid_filter - create a filter string to all pids against @field > > * @curr_filter: Append to a previous filter (may realloc). Can be NULL > > - * @field: The fild to compare the pids against > > + * @field: The field to compare the pids against > > * > > * Creates a new string or appends to an existing one if @curr_filter > > * is not NULL. The new string will contain a filter with all pids > > @@ -964,54 +1017,46 @@ static void update_pid_event_filters(struct buffer_instance *instance); > > */ > > static char *make_pid_filter(char *curr_filter, const char *field) > > { > > + int start_pid = -1, last_pid = -1; > > + int last_exclude = -1; > > struct filter_pids *p; > > - char *filter; > > - char *orit; > > - char *match; > > - char *str; > > + char *filter = NULL; > > int curr_len = 0; > > - int len; > > > > /* Use the new method if possible */ > > if (have_set_event_pid) > > return NULL; > > > > - len = len_filter_pids + (strlen(field) + strlen("(==)||")) * nr_filter_pids; > > - > > - if (curr_filter) { > > - curr_len = strlen(curr_filter); > > - filter = realloc(curr_filter, curr_len + len + strlen("(&&())")); > > - if (!filter) > > - die("realloc"); > > - memmove(filter+1, curr_filter, curr_len); > > - filter[0] = '('; > > - strcat(filter, ")&&("); > > - curr_len = strlen(filter); > > - } else > > - filter = malloc(len); > > - if (!filter) > > - die("Failed to allocate pid filter"); > > - > > - /* Last '||' that is not used will cover the \0 */ > > - str = filter + curr_len; > > + if (!filter_pids) > > + return curr_filter; > > > > for (p = filter_pids; p; p = p->next) { > > - if (p->exclude) { > > - match = "!="; > > - orit = "&&"; > > - } else { > > - match = "=="; > > - orit = "||"; > > + /* > > + * PIDs are inserted in `filter_pids` from the front and that's > > + * why we expect them in descending order here. > > + */ > > + if (p->pid == last_pid - 1 && p->exclude == last_exclude) { > > + last_pid = p->pid; > > + continue; > > } > > - if (p == filter_pids) > > - orit = ""; > > > > - len = sprintf(str, "%s(%s%s%d)", orit, field, match, p->pid); > > - str += len; > > + if (start_pid != -1) > > + append_filter_pid_range(&filter, &curr_len, field, > > + last_pid, start_pid, > > + last_exclude); > > + > > + start_pid = last_pid = p->pid; > > + last_exclude = p->exclude; > > + > > } > > + append_filter_pid_range(&filter, &curr_len, field, > > + last_pid, start_pid, last_exclude); > > > > - if (curr_len) > > - sprintf(str, ")"); > > + if (curr_filter) { > > + char *save = filter; > > + asprintf(&filter, "(%s)&&(%s)", curr_filter, filter); > > + free(save); > > + } > > > > return filter; > > } > > -- > > 2.19.1 > > > > -- > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 1/2] trace-cmd: Optimize how pid filters are expressed 2019-04-17 14:18 ` John Kacur @ 2019-04-17 14:25 ` Phil Auld 2019-04-17 14:52 ` Steven Rostedt 0 siblings, 1 reply; 12+ messages in thread From: Phil Auld @ 2019-04-17 14:25 UTC (permalink / raw) To: John Kacur Cc: Slavomir Kaslev, rostedt, linux-trace-devel, ykaradzhov, jbacik, tstoyanov, slavomir.kaslev On Wed, Apr 17, 2019 at 04:18:50PM +0200 John Kacur wrote: > > > On Wed, 17 Apr 2019, Phil Auld wrote: > > > On Wed, Apr 17, 2019 at 04:09:58PM +0300 Slavomir Kaslev 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). > > > > This one works as well :) > > > > I finally hit a case where my trace-cmd pids were non-contiguous and > > this split the range up correctly. > > > > > > FILTER write /sys/kernel/debug/tracing/events/sched/sched_kthread_stop/filter (len 74) value "(common_pid<21420||common_pid>21425)&&(common_pid<21265||common_pid>21418)" > > FILTER write /sys/kernel/debug/tracing/events/sched/sched_kthread_stop_ret/filter (len 74) value "(common_pid<21420||common_pid>21425)&&(common_pid<21265||common_pid>21418)" > > ... > > FILTER write /sys/kernel/debug/tracing/events/sched/sched_switch/filter (len 142) value "(common_pid<21420||common_pid>21425)&&(common_pid<21265||common_pid>21418)||(next_pid<21420||next_pid>21425)&&(next_pid<21265||next_pid>21418)" > > It seems crazy that we write "common_pid", instead of "pid" or "cpid", or > something like that. > > > I assume those have to match fields in the trace event mechanism, but I don't know a lot about it. "pid" is used in the wakeup event filters: FILTER write /sys/kernel/debug/tracing/events/sched/sched_wakeup_new/filter (len 122) value "(common_pid<21420||common_pid>21425)&&(common_pid<21265||common_pid>21418)||(pid<21420||pid>21425)&&(pid<21265||pid>21418)" Cheers, Phil ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 1/2] trace-cmd: Optimize how pid filters are expressed 2019-04-17 14:25 ` Phil Auld @ 2019-04-17 14:52 ` Steven Rostedt 0 siblings, 0 replies; 12+ messages in thread From: Steven Rostedt @ 2019-04-17 14:52 UTC (permalink / raw) To: Phil Auld Cc: John Kacur, Slavomir Kaslev, linux-trace-devel, ykaradzhov, jbacik, tstoyanov, slavomir.kaslev On Wed, 17 Apr 2019 10:25:15 -0400 Phil Auld <pauld@redhat.com> wrote: > > > FILTER write /sys/kernel/debug/tracing/events/sched/sched_kthread_stop/filter (len 74) value "(common_pid<21420||common_pid>21425)&&(common_pid<21265||common_pid>21418)" > > > FILTER write /sys/kernel/debug/tracing/events/sched/sched_kthread_stop_ret/filter (len 74) value "(common_pid<21420||common_pid>21425)&&(common_pid<21265||common_pid>21418)" > > > ... > > > FILTER write /sys/kernel/debug/tracing/events/sched/sched_switch/filter (len 142) value "(common_pid<21420||common_pid>21425)&&(common_pid<21265||common_pid>21418)||(next_pid<21420||next_pid>21425)&&(next_pid<21265||next_pid>21418)" > > > > It seems crazy that we write "common_pid", instead of "pid" or "cpid", or > > something like that. > > > > > > > > I assume those have to match fields in the trace event mechanism, but I don't know a lot > about it. > > "pid" is used in the wakeup event filters: > > FILTER write /sys/kernel/debug/tracing/events/sched/sched_wakeup_new/filter (len 122) value "(common_pid<21420||common_pid>21425)&&(common_pid<21265||common_pid>21418)||(pid<21420||pid>21425)&&(pid<21265||pid>21418)" > Correct, the names have to match the fields listed in the format file. The "common_*" was added way back in the beginning so that they differ from the field names in the events. -- Steve ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 1/2] trace-cmd: Optimize how pid filters are expressed 2019-04-17 13:58 ` Phil Auld 2019-04-17 14:18 ` John Kacur @ 2019-04-17 14:52 ` Steven Rostedt 2019-04-17 14:54 ` Steven Rostedt 2 siblings, 0 replies; 12+ messages in thread From: Steven Rostedt @ 2019-04-17 14:52 UTC (permalink / raw) To: Phil Auld Cc: Slavomir Kaslev, linux-trace-devel, ykaradzhov, jbacik, tstoyanov, slavomir.kaslev On Wed, 17 Apr 2019 09:58:58 -0400 Phil Auld <pauld@redhat.com> wrote: > Having seen that and having tried it on a few other machines I'd be more willing to have a > > Tested-by: Phil Auld <pauld@redhat.com> > > on it, if you want it. Thanks! I will add it. -- Steve ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 1/2] trace-cmd: Optimize how pid filters are expressed 2019-04-17 13:58 ` Phil Auld 2019-04-17 14:18 ` John Kacur 2019-04-17 14:52 ` Steven Rostedt @ 2019-04-17 14:54 ` Steven Rostedt 2 siblings, 0 replies; 12+ messages in thread From: Steven Rostedt @ 2019-04-17 14:54 UTC (permalink / raw) To: Phil Auld Cc: Slavomir Kaslev, linux-trace-devel, ykaradzhov, jbacik, tstoyanov, slavomir.kaslev On Wed, 17 Apr 2019 09:58:58 -0400 Phil Auld <pauld@redhat.com> wrote: > FILTER write /sys/kernel/debug/tracing/events/sched/sched_switch/filter (len 142) value "(common_pid<21420||common_pid>21425)&&(common_pid<21265||common_pid>21418)||(next_pid<21420||next_pid>21425)&&(next_pid<21265||next_pid>21418)" > > > The latter is correct given precendce of && before || but I wonder if () don't make sense? I always have to look > that one up :) Yes, as the one who wrote the parsing code (and was lectured by Al Viro about it), I know for a fact that && has a higher precedence than the || and it should work. I was about to comment about that, but then remembered my "lesson" :-) -- Steve > > If I were writing that in code I'd probably put in the extra ()s, but since it's generated and no > one actually sees it, probably okay and simpler as is. ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v4 2/2] trace-cmd: Document record --no-filter option in record's man page 2019-04-17 13:09 [PATCH v4 0/2] Optimize pid filters Slavomir Kaslev 2019-04-17 13:09 ` [PATCH v4 1/2] trace-cmd: Optimize how pid filters are expressed Slavomir Kaslev @ 2019-04-17 13:09 ` Slavomir Kaslev 2019-04-17 14:05 ` Phil Auld 2019-04-17 16:19 ` Steven Rostedt 1 sibling, 2 replies; 12+ messages in thread From: Slavomir Kaslev @ 2019-04-17 13:09 UTC (permalink / raw) To: rostedt Cc: linux-trace-devel, pauld, ykaradzhov, jbacik, tstoyanov, slavomir.kaslev Add a section about the new --no-filter argument of `trace-cmd record` to its man page. Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com> --- Documentation/trace-cmd-record.1.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Documentation/trace-cmd-record.1.txt b/Documentation/trace-cmd-record.1.txt index 68afa16..b230aa8 100644 --- a/Documentation/trace-cmd-record.1.txt +++ b/Documentation/trace-cmd-record.1.txt @@ -83,6 +83,10 @@ OPTIONS The above are usually safe to use to compare fields. +*--no-filter*:: + Do not set any event filters, including the default one which ignores events + caused by trace-cmd itself. + *-R* 'trigger':: Specify a trigger for the previous event. This must come after a *-e*. This will add a given trigger to the given event. To only enable the trigger -- 2.19.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v4 2/2] trace-cmd: Document record --no-filter option in record's man page 2019-04-17 13:09 ` [PATCH v4 2/2] trace-cmd: Document record --no-filter option in record's man page Slavomir Kaslev @ 2019-04-17 14:05 ` Phil Auld 2019-04-17 16:19 ` Steven Rostedt 1 sibling, 0 replies; 12+ messages in thread From: Phil Auld @ 2019-04-17 14:05 UTC (permalink / raw) To: Slavomir Kaslev Cc: rostedt, linux-trace-devel, ykaradzhov, jbacik, tstoyanov, slavomir.kaslev On Wed, Apr 17, 2019 at 04:09:59PM +0300 Slavomir Kaslev wrote: > Add a section about the new --no-filter argument of `trace-cmd record` > to its man page. > > Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com> > --- > Documentation/trace-cmd-record.1.txt | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/Documentation/trace-cmd-record.1.txt b/Documentation/trace-cmd-record.1.txt > index 68afa16..b230aa8 100644 > --- a/Documentation/trace-cmd-record.1.txt > +++ b/Documentation/trace-cmd-record.1.txt > @@ -83,6 +83,10 @@ OPTIONS > > The above are usually safe to use to compare fields. > > +*--no-filter*:: > + Do not set any event filters, including the default one which ignores events > + caused by trace-cmd itself. > + > *-R* 'trigger':: > Specify a trigger for the previous event. This must come after a *-e*. > This will add a given trigger to the given event. To only enable the trigger > -- > 2.19.1 > Acked-by: Phil Auld <pauld@redhat.com> and/or Tested-by: Phil Auld <pauld@redhat.com> Cheers, Phil -- ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 2/2] trace-cmd: Document record --no-filter option in record's man page 2019-04-17 13:09 ` [PATCH v4 2/2] trace-cmd: Document record --no-filter option in record's man page Slavomir Kaslev 2019-04-17 14:05 ` Phil Auld @ 2019-04-17 16:19 ` Steven Rostedt 2019-04-17 16:24 ` Steven Rostedt 1 sibling, 1 reply; 12+ messages in thread From: Steven Rostedt @ 2019-04-17 16:19 UTC (permalink / raw) To: Slavomir Kaslev Cc: linux-trace-devel, pauld, ykaradzhov, jbacik, tstoyanov, slavomir.kaslev On Wed, 17 Apr 2019 16:09:59 +0300 Slavomir Kaslev <kaslevs@vmware.com> wrote: > Add a section about the new --no-filter argument of `trace-cmd record` > to its man page. > > Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com> > --- > Documentation/trace-cmd-record.1.txt | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/Documentation/trace-cmd-record.1.txt b/Documentation/trace-cmd-record.1.txt > index 68afa16..b230aa8 100644 > --- a/Documentation/trace-cmd-record.1.txt > +++ b/Documentation/trace-cmd-record.1.txt > @@ -83,6 +83,10 @@ OPTIONS > > The above are usually safe to use to compare fields. > > +*--no-filter*:: > + Do not set any event filters, including the default one which ignores events > + caused by trace-cmd itself. Actually, this is wrong and so is the help message. It should be: *--no-filter*:: Do not filter out the trace-cmd threads. By default, the threads are filtered out to not be traced by events. This option will have the trace-cmd threads also be traced. And the help message should be: # trace-cmd record -h [..] --no-filter include trace-cmd threads in the trace Want to resend this patch and also fix the help message (assume it is applied). Thanks! -- Steve > + > *-R* 'trigger':: > Specify a trigger for the previous event. This must come after a *-e*. > This will add a given trigger to the given event. To only enable the trigger ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 2/2] trace-cmd: Document record --no-filter option in record's man page 2019-04-17 16:19 ` Steven Rostedt @ 2019-04-17 16:24 ` Steven Rostedt 0 siblings, 0 replies; 12+ messages in thread From: Steven Rostedt @ 2019-04-17 16:24 UTC (permalink / raw) To: Slavomir Kaslev Cc: linux-trace-devel, pauld, ykaradzhov, jbacik, tstoyanov, slavomir.kaslev On Wed, 17 Apr 2019 12:19:08 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > Actually, this is wrong and so is the help message. > > It should be: > > *--no-filter*:: > Do not filter out the trace-cmd threads. By default, the > threads are filtered out to not be traced by events. This > option will have the trace-cmd threads also be traced. > > And the help message should be: > > # trace-cmd record -h > [..] > --no-filter include trace-cmd threads in the trace > > Want to resend this patch and also fix the help message (assume it is > applied). Actually, I just pushed an update to upstream. Could you just combine this and the --no-filter patch, and update both help messages. That is, I didn't apply the --no-filter patch. Thanks! -- Steve ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-04-17 16:24 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-04-17 13:09 [PATCH v4 0/2] Optimize pid filters Slavomir Kaslev 2019-04-17 13:09 ` [PATCH v4 1/2] trace-cmd: Optimize how pid filters are expressed Slavomir Kaslev 2019-04-17 13:58 ` Phil Auld 2019-04-17 14:18 ` John Kacur 2019-04-17 14:25 ` Phil Auld 2019-04-17 14:52 ` Steven Rostedt 2019-04-17 14:52 ` Steven Rostedt 2019-04-17 14:54 ` Steven Rostedt 2019-04-17 13:09 ` [PATCH v4 2/2] trace-cmd: Document record --no-filter option in record's man page Slavomir Kaslev 2019-04-17 14:05 ` Phil Auld 2019-04-17 16:19 ` Steven Rostedt 2019-04-17 16:24 ` Steven Rostedt
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).