From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932578AbbJPPFW (ORCPT ); Fri, 16 Oct 2015 11:05:22 -0400 Received: from smtprelay0242.hostedemail.com ([216.40.44.242]:36307 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S932123AbbJPPFU (ORCPT ); Fri, 16 Oct 2015 11:05:20 -0400 X-Session-Marker: 726F737465647440676F6F646D69732E6F7267 X-Spam-Summary: 2,0,0,,d41d8cd98f00b204,rostedt@goodmis.org,:::::::::,RULES_HIT:2:41:69:355:379:421:541:599:800:960:966:973:988:989:1260:1277:1311:1313:1314:1345:1359:1437:1515:1516:1518:1535:1593:1594:1605:1730:1747:1777:1792:2196:2198:2199:2200:2393:2553:2559:2562:2898:3138:3139:3140:3141:3142:3622:3865:3866:3867:3868:3870:3871:3872:3874:4050:4119:4321:4362:4385:5007:6119:6261:7514:7875:8603:10004:10848:10967:11026:11232:11473:11658:11914:12043:12291:12296:12438:12517:12519:12555:12663:12683:12740:13972:14096:14097:21080,0,RBL:none,CacheIP:none,Bayesian:0.5,0.5,0.5,Netcheck:none,DomainCache:0,MSF:not bulk,SPF:fn,MSBL:0,DNSBL:none,Custom_rules:0:0:0 X-HE-Tag: debt97_85860fcbefd37 X-Filterd-Recvd-Size: 8847 Date: Fri, 16 Oct 2015 11:05:15 -0400 From: Steven Rostedt To: Dmitry Safonov <0x7f454c46@gmail.com> Cc: mingo@redhat.com, corbet@lwn.net, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 3/5] ftrace: introduce ftrace_glob structure Message-ID: <20151016110515.511aaa52@gandalf.local.home> In-Reply-To: <1443545176-3215-3-git-send-email-0x7f454c46@gmail.com> References: <1443545176-3215-1-git-send-email-0x7f454c46@gmail.com> <1443545176-3215-3-git-send-email-0x7f454c46@gmail.com> X-Mailer: Claws Mail 3.12.0 (GTK+ 2.24.28; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 29 Sep 2015 19:46:14 +0300 Dmitry Safonov <0x7f454c46@gmail.com> wrote: > ftrace_match parameters are very related and I reduce the number of local > variables & parameters with it. > This is also preparation for module globbing as it would introduce more > realated variables & parameters. > > Signed-off-by: Dmitry Safonov <0x7f454c46@gmail.com> > --- > kernel/trace/ftrace.c | 84 ++++++++++++++++++++++++++++----------------------- > 1 file changed, 46 insertions(+), 38 deletions(-) > > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c > index a56f028..6ffc1a2 100644 > --- a/kernel/trace/ftrace.c > +++ b/kernel/trace/ftrace.c > @@ -3420,27 +3420,35 @@ ftrace_notrace_open(struct inode *inode, struct file *file) > inode, file); > } > > -static int ftrace_match(char *str, char *regex, int len, int type) > +/* Type for quick search ftrace basic regexes (globs) from filter_parse_regex */ > +struct ftrace_glob { > + char *search; > + unsigned len; > + int type; > +}; > + > +static int ftrace_match(char *str, struct ftrace_glob *g) > { > int matched = 0; > int slen; > > - switch (type) { > + switch (g->type) { > case MATCH_FULL: > - if (strcmp(str, regex) == 0) > + if (strcmp(str, g->search) == 0) > matched = 1; > break; > case MATCH_FRONT_ONLY: > - if (strncmp(str, regex, len) == 0) > + if (strncmp(str, g->search, g->len) == 0) > matched = 1; > break; > case MATCH_MIDDLE_ONLY: > - if (strstr(str, regex)) > + if (strstr(str, g->search)) > matched = 1; > break; > case MATCH_END_ONLY: > slen = strlen(str); > - if (slen >= len && memcmp(str + slen - len, regex, len) == 0) > + if (slen >= g->len && memcmp(str + slen - g->len, > + g->search, g->len) == 0) I'm going to modify this a little to: if (slen >= g->len && memcmp(str + slen - g->len, g->search, g->len) == 0) As it's a bit easier to read. > matched = 1; > break; > } > @@ -3472,8 +3480,8 @@ enter_record(struct ftrace_hash *hash, struct dyn_ftrace *rec, int clear_filter) > } > > static int > -ftrace_match_record(struct dyn_ftrace *rec, char *mod, > - char *regex, int len, int type) > +ftrace_match_record(struct dyn_ftrace *rec, > + char *mod, struct ftrace_glob *func_g) > { > char str[KSYM_SYMBOL_LEN]; > char *modname; > @@ -3486,28 +3494,27 @@ ftrace_match_record(struct dyn_ftrace *rec, char *mod, > return 0; > > /* blank search means to match all funcs in the mod */ > - if (!len) > + if (!func_g->len) > return 1; > } > > - return ftrace_match(str, regex, len, type); > + return ftrace_match(str, func_g); > } > > static int > -match_records(struct ftrace_hash *hash, char *buff, int len, char *mod) > +match_records(struct ftrace_hash *hash, char *func, int len, char *mod) > { > - unsigned search_len = 0; > struct ftrace_page *pg; > struct dyn_ftrace *rec; > - int type = MATCH_FULL; > - char *search = buff; > + struct ftrace_glob func_g = { .type = MATCH_FULL }; > int found = 0; > int ret; > int clear_filter; > > if (len) { > - type = filter_parse_regex(buff, len, &search, &clear_filter); > - search_len = strlen(search); > + func_g.type = filter_parse_regex(func, len, > + &func_g.search, &clear_filter); I'm reformatting this too. > + func_g.len = strlen(func_g.search); > } > > mutex_lock(&ftrace_lock); > @@ -3516,7 +3523,7 @@ match_records(struct ftrace_hash *hash, char *buff, int len, char *mod) > goto out_unlock; > > do_for_each_ftrace_rec(pg, rec) { > - if (ftrace_match_record(rec, mod, search, search_len, type)) { > + if (ftrace_match_record(rec, mod, &func_g)) { > ret = enter_record(hash, rec, clear_filter); > if (ret < 0) { > found = ret; > @@ -3682,14 +3689,15 @@ register_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops, > struct ftrace_hash *hash; > struct ftrace_page *pg; > struct dyn_ftrace *rec; > - int type, len, not; > + int not; > unsigned long key; > int count = 0; > - char *search; > int ret; > + struct ftrace_glob func_g; I'm moving this up to keep the "upside down x-mas tree" effect. > > - type = filter_parse_regex(glob, strlen(glob), &search, ¬); > - len = strlen(search); > + func_g.type = filter_parse_regex(glob, strlen(glob), > + &func_g.search, ¬); > + func_g.len = strlen(func_g.search); > > /* we do not support '!' for function probes */ > if (WARN_ON(not)) > @@ -3716,7 +3724,7 @@ register_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops, > > do_for_each_ftrace_rec(pg, rec) { > > - if (!ftrace_match_record(rec, NULL, search, len, type)) > + if (!ftrace_match_record(rec, NULL, &func_g)) > continue; > > entry = kmalloc(sizeof(*entry), GFP_KERNEL); > @@ -3795,18 +3803,18 @@ __unregister_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops, > struct ftrace_hash *hash; > struct hlist_node *tmp; > char str[KSYM_SYMBOL_LEN]; > - int type = MATCH_FULL; > - int i, len = 0; > - char *search; > - int ret; > + int i, ret; > + struct ftrace_glob func_g; Same here. > > if (glob && (strcmp(glob, "*") == 0 || !strlen(glob))) > - glob = NULL; > + func_g.search = NULL; > else if (glob) { > int not; > > - type = filter_parse_regex(glob, strlen(glob), &search, ¬); > - len = strlen(search); > + func_g.type = filter_parse_regex(glob, strlen(glob), > + &func_g.search, ¬); And this I'm formatting a bit differently, making the &func_g.search equal to "glob". > + func_g.len = strlen(func_g.search); > + func_g.search = glob; > > /* we do not support '!' for function probes */ > if (WARN_ON(not)) > @@ -3835,10 +3843,10 @@ __unregister_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops, > continue; > > /* do this last, since it is the most expensive */ > - if (glob) { > + if (func_g.search) { > kallsyms_lookup(entry->ip, NULL, NULL, > NULL, str); > - if (!ftrace_match(str, glob, len, type)) > + if (!ftrace_match(str, &func_g)) > continue; > } > > @@ -3867,7 +3875,7 @@ __unregister_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops, > ftrace_free_entry(entry); > } > mutex_unlock(&ftrace_lock); > - > + > out_unlock: > mutex_unlock(&trace_probe_ops.func_hash->regex_lock); > free_ftrace_hash(hash); > @@ -4585,19 +4593,19 @@ ftrace_set_func(unsigned long *array, int *idx, int size, char *buffer) > { > struct dyn_ftrace *rec; > struct ftrace_page *pg; > - int search_len; > int fail = 1; > - int type, not; > - char *search; > + int not; > bool exists; > int i; > + struct ftrace_glob func_g; Again, I'm moving that up. > > /* decode regex */ > - type = filter_parse_regex(buffer, strlen(buffer), &search, ¬); > + func_g.type = filter_parse_regex(buffer, strlen(buffer), > + &func_g.search, ¬); Format change. OK, patch looks good, but I'm going to do the formatting changes. I'm not changing the code itself, except for the few places I moved the declaration of func_g up to keep the declarations in upside-down-x-mass-tree format. Thanks, -- Steve > if (!not && *idx >= size) > return -EBUSY; > > - search_len = strlen(search); > + func_g.len = strlen(func_g.search); > > mutex_lock(&ftrace_lock); > > @@ -4608,7 +4616,7 @@ ftrace_set_func(unsigned long *array, int *idx, int size, char *buffer) > > do_for_each_ftrace_rec(pg, rec) { > > - if (ftrace_match_record(rec, NULL, search, search_len, type)) { > + if (ftrace_match_record(rec, NULL, &func_g)) { > /* if it is in the array */ > exists = false; > for (i = 0; i < *idx; i++) {