Em Wed, Nov 12, 2014 at 06:05:20PM -0800, Andi Kleen escreveu:
> +static int remove_loops(struct branch_entry *l, int nr)
> +{
> +     int i, j, off;
> +     unsigned char chash[CHASHSZ];
> +
> +     memset(chash, NO_ENTRY, sizeof(chash));
> +
> +     BUG_ON(nr >= 256);

What is wrong with return -1 and propagating the error, so that the user
is informed if the data being processed is bogus, stop processing with a
warning or continue processing if finding the next valid record is
possible?

In general just aborting (and this only does that when NDEBUG is set)
should be avoided...

I can go on and help you and do that for you, but why introduce it in
the first place?

- Arnaldo

> +     for (i = 0; i < nr; i++) {
> +             int h = hash_64(l[i].from, CHASHBITS) % CHASHSZ;
> +
> +             /* no collision handling for now */
> +             if (chash[h] == NO_ENTRY) {
> +                     chash[h] = i;
> +             } else if (l[chash[h]].from == l[i].from) {
> +                     bool is_loop = true;
> +                     /* check if it is a real loop */
> +                     off = 0;
> +                     for (j = chash[h]; j < i && i + off < nr; j++, off++)
> +                             if (l[j].from != l[i + off].from) {
> +                                     is_loop = false;
> +                                     break;
> +                             }
> +                     if (is_loop) {
> +                             memmove(l + i, l + i + off,
> +                                     (nr - (i + off)) * sizeof(*l));
> +                             nr -= off;
> +                     }
> +             }
> +     }
> +     return nr;
> +}
> +
>  static int thread__resolve_callchain_sample(struct thread *thread,
>                                            struct ip_callchain *chain,
> +                                          struct branch_stack *branch,
>                                            struct symbol **parent,
>                                            struct addr_location *root_al,
>                                            int max_stack)
> @@ -1438,22 +1484,82 @@ static int thread__resolve_callchain_sample(struct 
> thread *thread,
>       int i;
>       int j;
>       int err;
> -     int skip_idx __maybe_unused;
> +     int skip_idx = -1;
> +     int first_call = 0;
> +
> +     /*
> +      * Based on DWARF debug information, some architectures skip
> +      * a callchain entry saved by the kernel.
> +      */
> +     if (chain->nr < PERF_MAX_STACK_DEPTH)
> +             skip_idx = arch_skip_callchain_idx(thread, chain);
>  
>       callchain_cursor_reset(&callchain_cursor);
>  
> +     /*
> +      * Add branches to call stack for easier browsing. This gives
> +      * more context for a sample than just the callers.
> +      *
> +      * This uses individual histograms of paths compared to the
> +      * aggregated histograms the normal LBR mode uses.
> +      *
> +      * Limitations for now:
> +      * - No extra filters
> +      * - No annotations (should annotate somehow)
> +      */
> +
> +     if (branch && callchain_param.branch_callstack) {
> +             int nr = min(max_stack, (int)branch->nr);
> +             struct branch_entry be[nr];
> +
> +             if (branch->nr > PERF_MAX_BRANCH_DEPTH) {
> +                     pr_warning("corrupted branch chain. skipping...\n");
> +                     goto check_calls;
> +             }
> +
> +             for (i = 0; i < nr; i++) {
> +                     if (callchain_param.order == ORDER_CALLEE) {
> +                             be[i] = branch->entries[i];
> +                             /*
> +                              * Check for overlap into the callchain.
> +                              * The return address is one off compared to
> +                              * the branch entry. To adjust for this
> +                              * assume the calling instruction is not longer
> +                              * than 8 bytes.
> +                              */
> +                             if (i == skip_idx ||
> +                                 chain->ips[first_call] >= PERF_CONTEXT_MAX)
> +                                     first_call++;
> +                             else if (be[i].from < chain->ips[first_call] &&
> +                                 be[i].from >= chain->ips[first_call] - 8)
> +                                     first_call++;
> +                     } else
> +                             be[i] = branch->entries[branch->nr - i - 1];
> +             }
> +
> +             nr = remove_loops(be, nr);
> +
> +             for (i = 0; i < nr; i++) {
> +                     err = add_callchain_ip(thread, parent, root_al,
> +                                            -1, be[i].to);
> +                     if (!err)
> +                             err = add_callchain_ip(thread, parent, root_al,
> +                                                    -1, be[i].from);
> +                     if (err == -EINVAL)
> +                             break;
> +                     if (err)
> +                             return err;
> +             }
> +             chain_nr -= nr;
> +     }
> +
> +check_calls:
>       if (chain->nr > PERF_MAX_STACK_DEPTH) {
>               pr_warning("corrupted callchain. skipping...\n");
>               return 0;
>       }
>  
> -     /*
> -      * Based on DWARF debug information, some architectures skip
> -      * a callchain entry saved by the kernel.
> -      */
> -     skip_idx = arch_skip_callchain_idx(thread, chain);
> -
> -     for (i = 0; i < chain_nr; i++) {
> +     for (i = first_call; i < chain_nr; i++) {
>               u64 ip;
>  
>               if (callchain_param.order == ORDER_CALLEE)
> @@ -1517,6 +1623,7 @@ int thread__resolve_callchain(struct thread *thread,
>                             int max_stack)
>  {
>       int ret = thread__resolve_callchain_sample(thread, sample->callchain,
> +                                                sample->branch_stack,
>                                                  parent, root_al, max_stack);
>       if (ret)
>               return ret;
> diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> index ded3ca7..b364d96 100644
> --- a/tools/perf/util/symbol.h
> +++ b/tools/perf/util/symbol.h
> @@ -123,7 +123,8 @@ struct symbol_conf {
>                       demangle,
>                       demangle_kernel,
>                       filter_relative,
> -                     show_hist_headers;
> +                     show_hist_headers,
> +                     branch_callstack;
>       const char      *vmlinux_name,
>                       *kallsyms_name,
>                       *source_prefix,
> -- 
> 1.9.3
--
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/

Reply via email to