Hi, Cody
On Thu, 9 Aug 2012 15:18:27 -0700, Cody P. Schafer wrote:
> kallsyms__parse() takes a callback that is called on every discovered
> symbol. As /proc/kallsyms does not supply symbol sizes, the callback was
> simply called with end=start, faking the symbol size to 1.
>
> All of the callbacks (there are 2) used in calls to kallsyms__parse()
> are _only_ used as callbacks for kallsyms__parse().
>
> Given that kallsyms__parse() lacks real information about what
> end/length should be, don't make up a length in kallsyms__parse().
> Instead have the callbacks handle guessing the length.
>
> Also relocate a comment regarding symbol creation to the callback which
> does symbol creation (kallsyms__parse() is not in general used to create
> symbols).
>
> Signed-off-by: Cody P Schafer <c...@linux.vnet.ibm.com>
> ---
> tools/perf/util/event.c | 2 +-
> tools/perf/util/symbol.c | 21 ++++++++++-----------
> tools/perf/util/symbol.h | 2 +-
> 3 files changed, 12 insertions(+), 13 deletions(-)
>
> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> index 2a6f33c..3a0f1a5 100644
> --- a/tools/perf/util/event.c
> +++ b/tools/perf/util/event.c
> @@ -412,7 +412,7 @@ struct process_symbol_args {
> };
>
> static int find_symbol_cb(void *arg, const char *name, char type,
> - u64 start, u64 end __used)
> + u64 start)
> {
> struct process_symbol_args *args = arg;
>
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index df4736d..b2f7597 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -574,7 +574,7 @@ size_t dso__fprintf(struct dso *dso, enum map_type type,
> FILE *fp)
>
> int kallsyms__parse(const char *filename, void *arg,
> int (*process_symbol)(void *arg, const char *name,
> - char type, u64 start, u64 end))
> + char type, u64 start))
> {
> char *line = NULL;
> size_t n;
> @@ -614,13 +614,8 @@ int kallsyms__parse(const char *filename, void *arg,
> break;
> }
>
> - /*
> - * module symbols are not sorted so we add all
> - * symbols, setting length to 1, and rely on
> - * symbols__fixup_end() to fix it up.
> - */
> err = process_symbol(arg, symbol_name,
> - symbol_type, start, start);
> + symbol_type, start);
> if (err)
> break;
> }
> @@ -647,7 +642,7 @@ static u8 kallsyms2elf_type(char type)
> }
>
> static int map__process_kallsym_symbol(void *arg, const char *name,
> - char type, u64 start, u64 end)
> + char type, u64 start)
> {
> struct symbol *sym;
> struct process_kallsyms_args *a = arg;
> @@ -656,8 +651,12 @@ static int map__process_kallsym_symbol(void *arg, const
> char *name,
> if (!symbol_type__is_a(type, a->map->type))
> return 0;
>
> - sym = symbol__new(start, end - start + 1,
> - kallsyms2elf_type(type), name);
> + /*
> + * module symbols are not sorted so we add all
> + * symbols, setting length to 1, and rely on
> + * symbols__fixup_end() to fix it up.
> + */
> + sym = symbol__new(start, 1, kallsyms2elf_type(type), name);
I guess that length of 1 effectively same as zero length in this case
since we end up calling symbols__fixup_end. The 'end - start + 1' part
looks like a leftover from previous change and not needed anymore -
KSYM_NAME_LEN check too, IMHO - so I suggest using 0 length to make it
clear.
And it seems you need to rebase the series onto Arnaldo's current
perf/core branch which separates out ELF bits to symbol-elf.c.
Thanks,
Namhyung
> if (sym == NULL)
> return -ENOMEM;
> /*
> @@ -2528,7 +2527,7 @@ struct process_args {
> };
>
> static int symbol__in_kernel(void *arg, const char *name,
> - char type __used, u64 start, u64 end __used)
> + char type __used, u64 start)
> {
> struct process_args *args = arg;
>
> diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> index 1fe733a..c8ec1d7 100644
> --- a/tools/perf/util/symbol.h
> +++ b/tools/perf/util/symbol.h
> @@ -297,7 +297,7 @@ bool __dsos__read_build_ids(struct list_head *head, bool
> with_hits);
> int build_id__sprintf(const u8 *build_id, int len, char *bf);
> int kallsyms__parse(const char *filename, void *arg,
> int (*process_symbol)(void *arg, const char *name,
> - char type, u64 start, u64 end));
> + char type, u64 start));
>
> void machine__destroy_kernel_maps(struct machine *machine);
> int __machine__create_kernel_maps(struct machine *machine, struct dso
> *kernel);
--
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/
