lkml.org 
[lkml]   [2010]   [Mar]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[037/156] perf annotate: Defer allocating sym_priv->hist array
    2.6.33-stable review patch.  If anyone has any objections, please let us know.

    ------------------

    From: Arnaldo Carvalho de Melo <acme@infradead.org>

    commit 628ada0cb03666dd463f7c25947eaccdf440c309 upstream

    Because symbol->end is not fixed up at symbol_filter time, only
    after all symbols for a DSO are loaded, and that, for asm
    symbols, may be bogus, causing segfaults when hits happen in
    these symbols.

    Backported-from: 628ada0
    Reported-by: David Miller <davem@davemloft.net>
    Reported-by: Anton Blanchard <anton@samba.org>
    Acked-by: David Miller <davem@davemloft.net>
    Cc: Frédéric Weisbecker <fweisbec@gmail.com>
    Cc: Mike Galbraith <efault@gmx.de>
    Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
    Cc: Paul Mackerras <paulus@samba.org>
    LKML-Reference: <20100225155740.GB8553@ghostprotocols.net>
    Signed-off-by: Ingo Molnar <mingo@elte.hu>
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

    ---
    tools/perf/builtin-annotate.c | 65 +++++++++++++++++++++---------------------
    tools/perf/util/symbol.c | 2 -
    tools/perf/util/symbol.h | 2 +
    3 files changed, 36 insertions(+), 33 deletions(-)

    --- a/tools/perf/builtin-annotate.c
    +++ b/tools/perf/builtin-annotate.c
    @@ -53,32 +53,20 @@ struct sym_priv {

    static const char *sym_hist_filter;

    -static int symbol_filter(struct map *map __used, struct symbol *sym)
    +static int sym__alloc_hist(struct symbol *self)
    {
    - if (sym_hist_filter == NULL ||
    - strcmp(sym->name, sym_hist_filter) == 0) {
    - struct sym_priv *priv = symbol__priv(sym);
    - const int size = (sizeof(*priv->hist) +
    - (sym->end - sym->start) * sizeof(u64));
    -
    - priv->hist = malloc(size);
    - if (priv->hist)
    - memset(priv->hist, 0, size);
    - return 0;
    - }
    - /*
    - * FIXME: We should really filter it out, as we don't want to go thru symbols
    - * we're not interested, and if a DSO ends up with no symbols, delete it too,
    - * but right now the kernel loading routines in symbol.c bail out if no symbols
    - * are found, fix it later.
    - */
    - return 0;
    + struct sym_priv *priv = symbol__priv(self);
    + const int size = (sizeof(*priv->hist) +
    + (self->end - self->start) * sizeof(u64));
    +
    + priv->hist = zalloc(size);
    + return priv->hist == NULL ? -1 : 0;
    }

    /*
    * collect histogram counts
    */
    -static void hist_hit(struct hist_entry *he, u64 ip)
    +static int annotate__hist_hit(struct hist_entry *he, u64 ip)
    {
    unsigned int sym_size, offset;
    struct symbol *sym = he->sym;
    @@ -88,11 +76,11 @@ static void hist_hit(struct hist_entry *
    he->count++;

    if (!sym || !he->map)
    - return;
    + return 0;

    priv = symbol__priv(sym);
    - if (!priv->hist)
    - return;
    + if (priv->hist == NULL && sym__alloc_hist(sym) < 0)
    + return -ENOMEM;

    sym_size = sym->end - sym->start;
    offset = ip - sym->start;
    @@ -102,7 +90,7 @@ static void hist_hit(struct hist_entry *
    he->map->unmap_ip(he->map, ip));

    if (offset >= sym_size)
    - return;
    + return 0;

    h = priv->hist;
    h->sum++;
    @@ -114,18 +102,31 @@ static void hist_hit(struct hist_entry *
    he->sym->name,
    (void *)(unsigned long)ip, ip - he->sym->start,
    h->ip[offset]);
    + return 0;
    }

    static int perf_session__add_hist_entry(struct perf_session *self,
    struct addr_location *al, u64 count)
    {
    - bool hit;
    - struct hist_entry *he = __perf_session__add_hist_entry(self, al, NULL,
    - count, &hit);
    - if (he == NULL)
    - return -ENOMEM;
    - hist_hit(he, al->addr);
    - return 0;
    + bool hit;
    + struct hist_entry *he;
    +
    + if (sym_hist_filter != NULL &&
    + (al->sym == NULL || strcmp(sym_hist_filter, al->sym->name) != 0)) {
    + /* We're only interested in a symbol named sym_hist_filter */
    + if (al->sym != NULL) {
    + rb_erase(&al->sym->rb_node,
    + &al->map->dso->symbols[al->map->type]);
    + symbol__delete(al->sym);
    + }
    + return 0;
    + }
    +
    + he = __perf_session__add_hist_entry(self, al, NULL, count, &hit);
    + if (he == NULL)
    + return -ENOMEM;
    +
    + return annotate__hist_hit(he, al->addr);
    }

    static int process_sample_event(event_t *event, struct perf_session *session)
    @@ -135,7 +136,7 @@ static int process_sample_event(event_t
    dump_printf("(IP, %d): %d: %p\n", event->header.misc,
    event->ip.pid, (void *)(long)event->ip.ip);

    - if (event__preprocess_sample(event, session, &al, symbol_filter) < 0) {
    + if (event__preprocess_sample(event, session, &al, NULL) < 0) {
    fprintf(stderr, "problem processing %d event, skipping it.\n",
    event->header.type);
    return -1;
    --- a/tools/perf/util/symbol.c
    +++ b/tools/perf/util/symbol.c
    @@ -149,7 +149,7 @@ static struct symbol *symbol__new(u64 st
    return self;
    }

    -static void symbol__delete(struct symbol *self)
    +void symbol__delete(struct symbol *self)
    {
    free(((void *)self) - symbol_conf.priv_size);
    }
    --- a/tools/perf/util/symbol.h
    +++ b/tools/perf/util/symbol.h
    @@ -49,6 +49,8 @@ struct symbol {
    char name[0];
    };

    +void symbol__delete(struct symbol *self);
    +
    struct strlist;

    struct symbol_conf {

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2010-03-31 01:23    [W:2.719 / U:2.684 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site