Re: [PATCH] ftrace: Support full glob matching

[Steven Rostedt]

Shuah,

Hi! I'm going to be pulling these patches into my tree, but since they
touch selftests, can you take a look at them. They were posted to LKML,
but if you need me to bounce them to you too, I can do that as well.

Here's the links of the patches with the selftests:

http://lkml.kernel.org/r/147685282642.8760.554191165644931816.stgit@devbox
http://lkml.kernel.org/r/147685283587.8760.2601950190140526395.stgit@devbox
http://lkml.kernel.org/r/147685284503.8760.3870499047839781698.stgit@devbox
http://lkml.kernel.org/r/147685285432.8760.9114124487704649863.stgit@devbox

I'm still in the process of reviewing them myself.

-- Steve


On Wed,  5 Oct 2016 20:58:15 +0900
Masami Hiramatsu <mhiramat@xxxxxxxxxx> 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>
> ---
>  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;