On Thu, Jun 27, 2013 at 01:58:16PM +0900, Namhyung Kim wrote: > On Wed, 26 Jun 2013 18:25:01 -0400, Greg Price wrote: > > On Wed, Jun 26, 2013 at 10:28:56AM +0900, Namhyung Kim wrote: > >> On Sat, 22 Jun 2013 23:17:20 -0400, Greg Price wrote: > >> > @@ -1211,8 +1214,13 @@ static int > >> > machine__resolve_callchain_sample(struct machine *machine, > >> > MAP__FUNCTION, ip, &al, > >> > NULL); > >> > if (al.sym != NULL) { > >> > if (sort__has_parent && !*parent && > >> > - symbol__match_parent_regex(al.sym)) > >> > + symbol__match_regex(al.sym, &parent_regex)) > >> > *parent = al.sym; > >> > + else if (have_blackbox && root_al && > >> > + symbol__match_regex(al.sym, > >> > &blackbox_regex)) { > >> > + *root_al = al; > >> > + > >> > callchain_cursor_reset(&callchain_cursor); > >> > >> Okay, this is where the magic happens. :) > > > > Indeed! :) > > > >> So it overwrites the original 'al' in process_sample_event() to > >> blackboxed symbol and drop the callchain. Wouldn't it deserve a > >> comment? :) > > > > I could do that. Perhaps something like > > /* ignore the callchain we had so far, i.e. this symbol's callees */ > > Sound like what you had in mind? > > More important thing is, I think, it updates the original al (root_al) > so that the perf will see the new symbol as if it was sampled in the > first place and it will collect all of its callers in one place.
OK, I'll try to capture that in a comment in v2. > >> > + } > >> > if (!symbol_conf.use_callchain) > >> > break; > >> pp > >> This is unrelated to this patch, but why is this line needed? I guess > >> this check should be done before calling this function. > > > > Hmm. We actually can get into this function when > > !symbol_conf.use_callchain, if we're using say --sort=parent. But I'm > > still somewhat puzzled, because in that case it looks like we'll break > > the loop after the first address with a symbol, even if we didn't find > > the 'parent' there, which seems like it wouldn't serve the purpose. > > Right, that's what I want to say. > > We already have the check before calling this function so breaking the > loop after checking only first callchain node looks strange. If we > don't want to see callchains but only parents, it should either check > every callchain nodes or fail out as an unsupported operation IMHO. I agree. Will reply momentarily with a patch. I actually wrote a version that tries to keep the optimization, but decided it was too complicated for what it gains us. Cheers, Greg -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/