+Jiri since he wrote the original code
On 22/04/16 10:55, Adrian Hunter wrote:
> On 19/04/16 11:56, Chris Phlipot wrote:
>> The existing implentation implementation of thread__resolve_callchain,
>
> Remove 'implentation'
>
>> under certain circumstanes, can assemble callchain entries in the
>
> 'circumstanes' -> 'circumstances'
>
>> incorrect order.
>>
>> A the callchain entries are resolved incorrectly for a sample when all
>> of the following conditions are met:
>>
>> 1. callchain_param.order is set to ORDER_CALLER
>>
>> 2. thread__resolve_callchain_sample is able to resolve callchain entries
>> for the sample.
>>
>> 3. unwind__get_entries is also able to resolve callchain entries for the
>> sample.
>>
>> The fix is accomplished by reversing the order in which
>> thread__resolve_callchain_sample and unwind__get_entries are called
>> when callchain_param.order is set to ORDER_CALLER.
>
> Can you give an example of the commands you used and what the call chain
> looked like before and after.
>
> Also please run ./scripts/checkpatch.pl
>
>>
>> Unwind specific code from thread__resolve_callchain is also moved into a
>> new static function to improve readability of the fix.
>>
>> Signed-off-by: Chris Phlipot <cphlip...@gmail.com>
>> ---
>> tools/perf/util/machine.c | 57
>> ++++++++++++++++++++++++++++++++++-------------
>> 1 file changed, 41 insertions(+), 16 deletions(-)
>>
>> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
>> index 0c4dabc..dd086c8 100644
>> --- a/tools/perf/util/machine.c
>> +++ b/tools/perf/util/machine.c
>> @@ -1806,8 +1806,6 @@ static int thread__resolve_callchain_sample(struct
>> thread *thread,
>> int skip_idx = -1;
>> int first_call = 0;
>>
>> - callchain_cursor_reset(cursor);
>> -
>> if (has_branch_callstack(evsel)) {
>> err = resolve_lbr_callchain_sample(thread, cursor, sample,
>> parent,
>> root_al, max_stack);
>> @@ -1918,20 +1916,12 @@ static int unwind_entry(struct unwind_entry *entry,
>> void *arg)
>> entry->map, entry->sym);
>> }
>>
>> -int thread__resolve_callchain(struct thread *thread,
>> - struct callchain_cursor *cursor,
>> - struct perf_evsel *evsel,
>> - struct perf_sample *sample,
>> - struct symbol **parent,
>> - struct addr_location *root_al,
>> - int max_stack)
>> -{
>> - int ret = thread__resolve_callchain_sample(thread, cursor, evsel,
>> - sample, parent,
>> - root_al, max_stack);
>> - if (ret)
>> - return ret;
>> -
>> +static int thread__resolve_callchain_unwind(struct thread *thread,
>> + struct callchain_cursor *cursor,
>> + struct perf_evsel *evsel,
>> + struct perf_sample *sample,
>> + int max_stack)
>> +{
>> /* Can we do dwarf post unwind? */
>> if (!((evsel->attr.sample_type & PERF_SAMPLE_REGS_USER) &&
>> (evsel->attr.sample_type & PERF_SAMPLE_STACK_USER)))
>> @@ -1944,7 +1934,42 @@ int thread__resolve_callchain(struct thread *thread,
>>
>> return unwind__get_entries(unwind_entry, cursor,
>> thread, sample, max_stack);
>> +}
>> +
>> +int thread__resolve_callchain(struct thread *thread,
>> + struct callchain_cursor *cursor,
>> + struct perf_evsel *evsel,
>> + struct perf_sample *sample,
>> + struct symbol **parent,
>> + struct addr_location *root_al,
>> + int max_stack)
>> +{
>> + int ret = 0;
>> + callchain_cursor_reset(&callchain_cursor);
>> +
>> + if (callchain_param.order == ORDER_CALLEE) {
>> + ret = thread__resolve_callchain_sample(thread, cursor,
>> + evsel, sample,
>> + parent, root_al,
>> + max_stack);
>> + if (ret)
>> + return ret;
>> + ret = thread__resolve_callchain_unwind(thread, cursor,
>> + evsel, sample,
>> + max_stack);
>> + } else {
>> + ret = thread__resolve_callchain_unwind(thread, cursor,
>> + evsel, sample,
>> + max_stack);
>> + if (ret)
>> + return ret;
>> + ret = thread__resolve_callchain_sample(thread, cursor,
>> + evsel, sample,
>> + parent, root_al,
>> + max_stack);
>> + }
>>
>> + return ret;
>> }
>>
>> int machine__for_each_thread(struct machine *machine,
>>
>
>
