RE: [PATCH V4 1/3] perf tools: enable LBR call stack support

[Liang, Kan]

> On Tue, 18 Nov 2014 16:36:55 -0500, kan liang wrote:
> > From: Kan Liang <kan.liang@xxxxxxxxx>
> >
> > Currently, there are two call chain recording options, fp and dwarf.
> > Haswell has a new feature that utilizes the existing LBR facility to
> > record call chains. So it provides the third options to record call
> > chain. This patch enables the lbr call stack support.
> >
> > LBR call stack has some limitations. It reuses current LBR facility,
> > so LBR call stack and branch record can not be enabled at the same
> > time. It is only available for user callchain.
> > However, LBR call stack can work on the user app which doesn't have
> > frame-pointer or dwarf debug info compiled. It is a good alternative
> > when nothing else works.
> 
> 
> [SNIP]
> >  static void
> > -perf_evsel__config_callgraph(struct perf_evsel *evsel)
> > +perf_evsel__config_callgraph(struct perf_evsel *evsel,
> > +			     struct record_opts *opts)
> >  {
> >  	bool function = perf_evsel__is_function_event(evsel);
> >  	struct perf_event_attr *attr = &evsel->attr;
> >
> >  	perf_evsel__set_sample_bit(evsel, CALLCHAIN);
> >
> > +	if (callchain_param.record_mode == CALLCHAIN_LBR) {
> > +		if (!opts->branch_stack) {
> > +			perf_evsel__set_sample_bit(evsel,
> BRANCH_STACK);
> > +			attr->branch_sample_type =
> PERF_SAMPLE_BRANCH_USER |
> > +
> 	PERF_SAMPLE_BRANCH_CALL_STACK;
> > +			if (attr->exclude_user) {
> > +				attr->exclude_user = 0;
> > +
> > +				pr_warning("LBR callstack option is only
> available"
> > +					   " to get user callchain
> information."
> > +					   " Force exclude_user to 0.\n");
> > +			}
> 
> I'm not sure what's in a higher priority - maybe I missed earlier discussion.
> IOW what about this?
> 
> 			if (attr->exclude_user) {
> 				pr_warning("LBR callstack option is only
> available"
> 					   " to get user callchain
> information.\n");

I think either is fine. I'd like to add more info "Falling back to framepointers."
based on your changes.
So the user know what they will get then.

What do you think?

pr_warning("LBR callstack option is only available"
	         " to get user callchain information."
	          " Falling back to framepointers.\n");

pr_ warning ("Cannot use LBR callstack with branch stack"
                " Falling back to framepointers.\n");

Thanks,
Kan
> 			} else {
> 				perf_evsel__set_sample_bit(evsel,
> BRANCH_STACK);
> 				attr->branch_sample_type =
> PERF_SAMPLE_BRANCH_USER |
> 
> 	PERF_SAMPLE_BRANCH_CALL_STACK;
> 			}
> 
> > +		} else
> > +			 pr_info("Cannot use LBR callstack with branch
> stack\n");
> 
> It seems pr_warning is more appropriate here.
> 
> Thanks,
> Namhyung
> 
> 
> > +	}
> > +
> >  	if (callchain_param.record_mode == CALLCHAIN_DWARF) {
> >  		if (!function) {
> >  			perf_evsel__set_sample_bit(evsel, REGS_USER);
> @@ -659,7 +676,7 @@
> > void perf_evsel__config(struct perf_evsel *evsel, struct record_opts
> *opts)
> >  	}
> >
> >  	if (callchain_param.enabled && !evsel->no_aux_samples)
> > -		perf_evsel__config_callgraph(evsel);
> > +		perf_evsel__config_callgraph(evsel, opts);
> >
> >  	if (target__has_cpu(&opts->target))
> >  		perf_evsel__set_sample_bit(evsel, CPU);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/