Re: [PATCH] ftrace: Support full glob matching

From: Namhyung Kim
Date: Fri Oct 07 2016 - 00:20:27 EST


Hi Masami,

On Wed, Oct 05, 2016 at 08:58:15PM +0900, Masami Hiramatsu wrote:
> Use glob_match() to support flexible glob wildcards (*,?)
> and character classes ([) for ftrace.
> Since the full glob matching is slower than the current
> partial matching routines(*pat, pat*, *pat*), this leaves
> those routines and just add MATCH_GLOB for complex glob
> expression.
>
> e.g.
> ----
> [root@localhost tracing]# echo 'sched*group' > set_ftrace_filter
> [root@localhost tracing]# cat set_ftrace_filter
> sched_free_group
> sched_change_group
> sched_create_group
> sched_online_group
> sched_destroy_group
> sched_offline_group
> [root@localhost tracing]# echo '[Ss]y[Ss]_*' > set_ftrace_filter
> [root@localhost tracing]# head set_ftrace_filter
> sys_arch_prctl
> sys_rt_sigreturn
> sys_ioperm
> SyS_iopl
> sys_modify_ldt
> SyS_mmap
> SyS_set_thread_area
> SyS_get_thread_area
> SyS_set_tid_address
> sys_fork
> ----
>
> Signed-off-by: Masami Hiramatsu <mhiramat@xxxxxxxxxx>

Nice!

Acked-by: Namhyung Kim <namhyung@xxxxxxxxxx>

Thanks,
Namhyung


> ---
> Documentation/trace/events.txt | 9 +++------
> Documentation/trace/ftrace.txt | 9 +++------
> kernel/trace/Kconfig | 2 ++
> kernel/trace/ftrace.c | 4 ++++
> kernel/trace/trace.c | 2 +-
> kernel/trace/trace.h | 2 ++
> kernel/trace/trace_events_filter.c | 17 ++++++++++++++++-
> 7 files changed, 31 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/trace/events.txt b/Documentation/trace/events.txt
> index 08d74d7..2cc08d4 100644
> --- a/Documentation/trace/events.txt
> +++ b/Documentation/trace/events.txt
> @@ -189,16 +189,13 @@ And for string fields they are:
>
> ==, !=, ~
>
> -The glob (~) only accepts a wild card character (*) at the start and or
> -end of the string. For example:
> +The glob (~) accepts a wild card character (*,?) and character classes
> +([). For example:
>
> prev_comm ~ "*sh"
> prev_comm ~ "sh*"
> prev_comm ~ "*sh*"
> -
> -But does not allow for it to be within the string:
> -
> - prev_comm ~ "ba*sh" <-- is invalid
> + prev_comm ~ "ba*sh"
>
> 5.2 Setting filters
> -------------------
> diff --git a/Documentation/trace/ftrace.txt b/Documentation/trace/ftrace.txt
> index a6b3705..b26abc7 100644
> --- a/Documentation/trace/ftrace.txt
> +++ b/Documentation/trace/ftrace.txt
> @@ -2218,16 +2218,13 @@ hrtimer_interrupt
> sys_nanosleep
>
>
> -Perhaps this is not enough. The filters also allow simple wild
> -cards. Only the following are currently available
> +Perhaps this is not enough. The filters also allow glob(7) matching.
>
> <match>* - will match functions that begin with <match>
> *<match> - will match functions that end with <match>
> *<match>* - will match functions that have <match> in it
> -
> -These are the only wild cards which are supported.
> -
> - <match>*<match> will not work.
> + <match1>*<match2> - will match functions that begin with
> + <match1> and end with <match2>
>
> Note: It is better to use quotes to enclose the wild cards,
> otherwise the shell may expand the parameters into names
> diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> index ba33267..aa6eb15 100644
> --- a/kernel/trace/Kconfig
> +++ b/kernel/trace/Kconfig
> @@ -70,6 +70,7 @@ config FTRACE_NMI_ENTER
>
> config EVENT_TRACING
> select CONTEXT_SWITCH_TRACER
> + select GLOB
> bool
>
> config CONTEXT_SWITCH_TRACER
> @@ -133,6 +134,7 @@ config FUNCTION_TRACER
> select KALLSYMS
> select GENERIC_TRACER
> select CONTEXT_SWITCH_TRACER
> + select GLOB
> help
> Enable the kernel to trace every kernel function. This is done
> by using a compiler feature to insert a small, 5-byte No-Operation
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 84752c8..5741184 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -3493,6 +3493,10 @@ static int ftrace_match(char *str, struct ftrace_glob *g)
> memcmp(str + slen - g->len, g->search, g->len) == 0)
> matched = 1;
> break;
> + case MATCH_GLOB:
> + if (glob_match(g->search, str))
> + matched = 1;
> + break;
> }
>
> return matched;
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 37824d9..ae343e7 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -4065,7 +4065,7 @@ static const char readme_msg[] =
> "\n available_filter_functions - list of functions that can be filtered on\n"
> " set_ftrace_filter\t- echo function name in here to only trace these\n"
> "\t\t\t functions\n"
> - "\t accepts: func_full_name, *func_end, func_begin*, *func_middle*\n"
> + "\t accepts: func_full_name or glob-matching-pattern\n"
> "\t modules: Can select a group via module\n"
> "\t Format: :mod:<module-name>\n"
> "\t example: echo :mod:ext3 > set_ftrace_filter\n"
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index f783df4..eac2eda 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -15,6 +15,7 @@
> #include <linux/trace_events.h>
> #include <linux/compiler.h>
> #include <linux/trace_seq.h>
> +#include <linux/glob.h>
>
> #ifdef CONFIG_FTRACE_SYSCALLS
> #include <asm/unistd.h> /* For NR_SYSCALLS */
> @@ -1252,6 +1253,7 @@ enum regex_type {
> MATCH_FRONT_ONLY,
> MATCH_MIDDLE_ONLY,
> MATCH_END_ONLY,
> + MATCH_GLOB,
> };
>
> struct regex {
> diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> index 9daa9b3..e1c7e2c 100644
> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -344,6 +344,12 @@ static int regex_match_end(char *str, struct regex *r, int len)
> return 0;
> }
>
> +static int regex_match_glob(char *str, struct regex *r, int len __maybe_unused)
> +{
> + if (glob_match(r->pattern, str))
> + return 1;
> + return 0;
> +}
> /**
> * filter_parse_regex - parse a basic regex
> * @buff: the raw regex
> @@ -380,14 +386,20 @@ enum regex_type filter_parse_regex(char *buff, int len, char **search, int *not)
> if (!i) {
> *search = buff + 1;
> type = MATCH_END_ONLY;
> - } else {
> + } else if (i == len - 1) {
> if (type == MATCH_END_ONLY)
> type = MATCH_MIDDLE_ONLY;
> else
> type = MATCH_FRONT_ONLY;
> buff[i] = 0;
> break;
> + } else { /* pattern continues, use full glob */
> + type = MATCH_GLOB;
> + break;
> }
> + } else if (strchr("[?\\", buff[i])) {
> + type = MATCH_GLOB;
> + break;
> }
> }
>
> @@ -420,6 +432,9 @@ static void filter_build_regex(struct filter_pred *pred)
> case MATCH_END_ONLY:
> r->match = regex_match_end;
> break;
> + case MATCH_GLOB:
> + r->match = regex_match_glob;
> + break;
> }
>
> pred->not ^= not;
>