* [PATCH] tools/lib/traceevent/event-parse: delete pevent_register_function
@ 2018-01-15 4:50 Wang YanQing
2018-01-15 10:08 ` Jiri Olsa
0 siblings, 1 reply; 4+ messages in thread
From: Wang YanQing @ 2018-01-15 4:50 UTC (permalink / raw)
To: acme; +Cc: peterz, mingo, jolsa, alexander.shishkin, linux-kernel
After commit 4263cece22e3da94f16fbbcf71ce3807946d3ef3
("perf tools: Stop reading the kallsyms data from perf.data"),
there is no users of pevent_register_function in tree, so we
could just delete it.
Signed-off-by: Wang YanQing <udknight@gmail.com>
---
tools/lib/traceevent/event-parse.c | 183 +------------------------------------
tools/lib/traceevent/event-parse.h | 3 -
2 files changed, 1 insertion(+), 185 deletions(-)
diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 7ce724f..0d125a3 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -347,96 +347,6 @@ struct func_list {
char *mod;
};
-static int func_cmp(const void *a, const void *b)
-{
- const struct func_map *fa = a;
- const struct func_map *fb = b;
-
- if (fa->addr < fb->addr)
- return -1;
- if (fa->addr > fb->addr)
- return 1;
-
- return 0;
-}
-
-/*
- * We are searching for a record in between, not an exact
- * match.
- */
-static int func_bcmp(const void *a, const void *b)
-{
- const struct func_map *fa = a;
- const struct func_map *fb = b;
-
- if ((fa->addr == fb->addr) ||
-
- (fa->addr > fb->addr &&
- fa->addr < (fb+1)->addr))
- return 0;
-
- if (fa->addr < fb->addr)
- return -1;
-
- return 1;
-}
-
-static int func_map_init(struct pevent *pevent)
-{
- struct func_list *funclist;
- struct func_list *item;
- struct func_map *func_map;
- int i;
-
- func_map = malloc(sizeof(*func_map) * (pevent->func_count + 1));
- if (!func_map)
- return -1;
-
- funclist = pevent->funclist;
-
- i = 0;
- while (funclist) {
- func_map[i].func = funclist->func;
- func_map[i].addr = funclist->addr;
- func_map[i].mod = funclist->mod;
- i++;
- item = funclist;
- funclist = funclist->next;
- free(item);
- }
-
- qsort(func_map, pevent->func_count, sizeof(*func_map), func_cmp);
-
- /*
- * Add a special record at the end.
- */
- func_map[pevent->func_count].func = NULL;
- func_map[pevent->func_count].addr = 0;
- func_map[pevent->func_count].mod = NULL;
-
- pevent->func_map = func_map;
- pevent->funclist = NULL;
-
- return 0;
-}
-
-static struct func_map *
-__find_func(struct pevent *pevent, unsigned long long addr)
-{
- struct func_map *func;
- struct func_map key;
-
- if (!pevent->func_map)
- func_map_init(pevent);
-
- key.addr = addr;
-
- func = bsearch(&key, pevent->func_map, pevent->func_count,
- sizeof(*pevent->func_map), func_bcmp);
-
- return func;
-}
-
struct func_resolver {
pevent_func_resolver_t *func;
void *priv;
@@ -448,10 +358,6 @@ struct func_resolver {
* @pevent: handle for the pevent
* @resolver: function to be used
* @priv: resolver function private state.
- *
- * Some tools may have already a way to resolve kernel functions, allow them to
- * keep using it instead of duplicating all the entries inside
- * pevent->funclist.
*/
int pevent_set_function_resolver(struct pevent *pevent,
pevent_func_resolver_t *func, void *priv)
@@ -489,7 +395,7 @@ void pevent_reset_function_resolver(struct pevent *pevent)
struct func_map *map;
if (!pevent->func_resolver)
- return __find_func(pevent, addr);
+ return NULL;
map = &pevent->func_resolver->map;
map->mod = NULL;
@@ -543,75 +449,6 @@ const char *pevent_find_function(struct pevent *pevent, unsigned long long addr)
return map->addr;
}
-/**
- * pevent_register_function - register a function with a given address
- * @pevent: handle for the pevent
- * @function: the function name to register
- * @addr: the address the function starts at
- * @mod: the kernel module the function may be in (NULL for none)
- *
- * This registers a function name with an address and module.
- * The @func passed in is duplicated.
- */
-int pevent_register_function(struct pevent *pevent, char *func,
- unsigned long long addr, char *mod)
-{
- struct func_list *item = malloc(sizeof(*item));
-
- if (!item)
- return -1;
-
- item->next = pevent->funclist;
- item->func = strdup(func);
- if (!item->func)
- goto out_free;
-
- if (mod) {
- item->mod = strdup(mod);
- if (!item->mod)
- goto out_free_func;
- } else
- item->mod = NULL;
- item->addr = addr;
-
- pevent->funclist = item;
- pevent->func_count++;
-
- return 0;
-
-out_free_func:
- free(item->func);
- item->func = NULL;
-out_free:
- free(item);
- errno = ENOMEM;
- return -1;
-}
-
-/**
- * pevent_print_funcs - print out the stored functions
- * @pevent: handle for the pevent
- *
- * This prints out the stored functions.
- */
-void pevent_print_funcs(struct pevent *pevent)
-{
- int i;
-
- if (!pevent->func_map)
- func_map_init(pevent);
-
- for (i = 0; i < (int)pevent->func_count; i++) {
- printf("%016llx %s",
- pevent->func_map[i].addr,
- pevent->func_map[i].func);
- if (pevent->func_map[i].mod)
- printf(" [%s]\n", pevent->func_map[i].mod);
- else
- printf("\n");
- }
-}
-
struct printk_map {
unsigned long long addr;
char *printk;
@@ -6777,7 +6614,6 @@ void pevent_free_format(struct event_format *event)
void pevent_free(struct pevent *pevent)
{
struct cmdline_list *cmdlist, *cmdnext;
- struct func_list *funclist, *funcnext;
struct printk_list *printklist, *printknext;
struct pevent_function_handler *func_handler;
struct event_handler *handle;
@@ -6787,7 +6623,6 @@ void pevent_free(struct pevent *pevent)
return;
cmdlist = pevent->cmdlist;
- funclist = pevent->funclist;
printklist = pevent->printklist;
pevent->ref_count--;
@@ -6807,22 +6642,6 @@ void pevent_free(struct pevent *pevent)
cmdlist = cmdnext;
}
- if (pevent->func_map) {
- for (i = 0; i < (int)pevent->func_count; i++) {
- free(pevent->func_map[i].func);
- free(pevent->func_map[i].mod);
- }
- free(pevent->func_map);
- }
-
- while (funclist) {
- funcnext = funclist->next;
- free(funclist->func);
- free(funclist->mod);
- free(funclist);
- funclist = funcnext;
- }
-
while (pevent->func_handlers) {
func_handler = pevent->func_handlers;
pevent->func_handlers = func_handler->next;
diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index 0c03538..af7a693 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -483,10 +483,7 @@ struct pevent {
struct cmdline_list *cmdlist;
int cmdline_count;
- struct func_map *func_map;
struct func_resolver *func_resolver;
- struct func_list *funclist;
- unsigned int func_count;
struct printk_map *printk_map;
struct printk_list *printklist;
--
1.8.5.6.2.g3d8a54e.dirty
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] tools/lib/traceevent/event-parse: delete pevent_register_function
2018-01-15 4:50 [PATCH] tools/lib/traceevent/event-parse: delete pevent_register_function Wang YanQing
@ 2018-01-15 10:08 ` Jiri Olsa
2018-01-15 11:54 ` Steven Rostedt
0 siblings, 1 reply; 4+ messages in thread
From: Jiri Olsa @ 2018-01-15 10:08 UTC (permalink / raw)
To: Wang YanQing, acme, peterz, mingo, alexander.shishkin,
linux-kernel, Steven Rostedt
On Mon, Jan 15, 2018 at 12:50:14PM +0800, Wang YanQing wrote:
> After commit 4263cece22e3da94f16fbbcf71ce3807946d3ef3
> ("perf tools: Stop reading the kallsyms data from perf.data"),
> there is no users of pevent_register_function in tree, so we
> could just delete it.
>
I don't think you can remove this function,
perf is not the only user there
adding Steven to the loop
jirka
> Signed-off-by: Wang YanQing <udknight@gmail.com>
> ---
> tools/lib/traceevent/event-parse.c | 183 +------------------------------------
> tools/lib/traceevent/event-parse.h | 3 -
> 2 files changed, 1 insertion(+), 185 deletions(-)
>
> diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
> index 7ce724f..0d125a3 100644
> --- a/tools/lib/traceevent/event-parse.c
> +++ b/tools/lib/traceevent/event-parse.c
> @@ -347,96 +347,6 @@ struct func_list {
> char *mod;
> };
>
> -static int func_cmp(const void *a, const void *b)
> -{
> - const struct func_map *fa = a;
> - const struct func_map *fb = b;
> -
> - if (fa->addr < fb->addr)
> - return -1;
> - if (fa->addr > fb->addr)
> - return 1;
> -
> - return 0;
> -}
> -
> -/*
> - * We are searching for a record in between, not an exact
> - * match.
> - */
> -static int func_bcmp(const void *a, const void *b)
> -{
> - const struct func_map *fa = a;
> - const struct func_map *fb = b;
> -
> - if ((fa->addr == fb->addr) ||
> -
> - (fa->addr > fb->addr &&
> - fa->addr < (fb+1)->addr))
> - return 0;
> -
> - if (fa->addr < fb->addr)
> - return -1;
> -
> - return 1;
> -}
> -
> -static int func_map_init(struct pevent *pevent)
> -{
> - struct func_list *funclist;
> - struct func_list *item;
> - struct func_map *func_map;
> - int i;
> -
> - func_map = malloc(sizeof(*func_map) * (pevent->func_count + 1));
> - if (!func_map)
> - return -1;
> -
> - funclist = pevent->funclist;
> -
> - i = 0;
> - while (funclist) {
> - func_map[i].func = funclist->func;
> - func_map[i].addr = funclist->addr;
> - func_map[i].mod = funclist->mod;
> - i++;
> - item = funclist;
> - funclist = funclist->next;
> - free(item);
> - }
> -
> - qsort(func_map, pevent->func_count, sizeof(*func_map), func_cmp);
> -
> - /*
> - * Add a special record at the end.
> - */
> - func_map[pevent->func_count].func = NULL;
> - func_map[pevent->func_count].addr = 0;
> - func_map[pevent->func_count].mod = NULL;
> -
> - pevent->func_map = func_map;
> - pevent->funclist = NULL;
> -
> - return 0;
> -}
> -
> -static struct func_map *
> -__find_func(struct pevent *pevent, unsigned long long addr)
> -{
> - struct func_map *func;
> - struct func_map key;
> -
> - if (!pevent->func_map)
> - func_map_init(pevent);
> -
> - key.addr = addr;
> -
> - func = bsearch(&key, pevent->func_map, pevent->func_count,
> - sizeof(*pevent->func_map), func_bcmp);
> -
> - return func;
> -}
> -
> struct func_resolver {
> pevent_func_resolver_t *func;
> void *priv;
> @@ -448,10 +358,6 @@ struct func_resolver {
> * @pevent: handle for the pevent
> * @resolver: function to be used
> * @priv: resolver function private state.
> - *
> - * Some tools may have already a way to resolve kernel functions, allow them to
> - * keep using it instead of duplicating all the entries inside
> - * pevent->funclist.
> */
> int pevent_set_function_resolver(struct pevent *pevent,
> pevent_func_resolver_t *func, void *priv)
> @@ -489,7 +395,7 @@ void pevent_reset_function_resolver(struct pevent *pevent)
> struct func_map *map;
>
> if (!pevent->func_resolver)
> - return __find_func(pevent, addr);
> + return NULL;
>
> map = &pevent->func_resolver->map;
> map->mod = NULL;
> @@ -543,75 +449,6 @@ const char *pevent_find_function(struct pevent *pevent, unsigned long long addr)
> return map->addr;
> }
>
> -/**
> - * pevent_register_function - register a function with a given address
> - * @pevent: handle for the pevent
> - * @function: the function name to register
> - * @addr: the address the function starts at
> - * @mod: the kernel module the function may be in (NULL for none)
> - *
> - * This registers a function name with an address and module.
> - * The @func passed in is duplicated.
> - */
> -int pevent_register_function(struct pevent *pevent, char *func,
> - unsigned long long addr, char *mod)
> -{
> - struct func_list *item = malloc(sizeof(*item));
> -
> - if (!item)
> - return -1;
> -
> - item->next = pevent->funclist;
> - item->func = strdup(func);
> - if (!item->func)
> - goto out_free;
> -
> - if (mod) {
> - item->mod = strdup(mod);
> - if (!item->mod)
> - goto out_free_func;
> - } else
> - item->mod = NULL;
> - item->addr = addr;
> -
> - pevent->funclist = item;
> - pevent->func_count++;
> -
> - return 0;
> -
> -out_free_func:
> - free(item->func);
> - item->func = NULL;
> -out_free:
> - free(item);
> - errno = ENOMEM;
> - return -1;
> -}
> -
> -/**
> - * pevent_print_funcs - print out the stored functions
> - * @pevent: handle for the pevent
> - *
> - * This prints out the stored functions.
> - */
> -void pevent_print_funcs(struct pevent *pevent)
> -{
> - int i;
> -
> - if (!pevent->func_map)
> - func_map_init(pevent);
> -
> - for (i = 0; i < (int)pevent->func_count; i++) {
> - printf("%016llx %s",
> - pevent->func_map[i].addr,
> - pevent->func_map[i].func);
> - if (pevent->func_map[i].mod)
> - printf(" [%s]\n", pevent->func_map[i].mod);
> - else
> - printf("\n");
> - }
> -}
> -
> struct printk_map {
> unsigned long long addr;
> char *printk;
> @@ -6777,7 +6614,6 @@ void pevent_free_format(struct event_format *event)
> void pevent_free(struct pevent *pevent)
> {
> struct cmdline_list *cmdlist, *cmdnext;
> - struct func_list *funclist, *funcnext;
> struct printk_list *printklist, *printknext;
> struct pevent_function_handler *func_handler;
> struct event_handler *handle;
> @@ -6787,7 +6623,6 @@ void pevent_free(struct pevent *pevent)
> return;
>
> cmdlist = pevent->cmdlist;
> - funclist = pevent->funclist;
> printklist = pevent->printklist;
>
> pevent->ref_count--;
> @@ -6807,22 +6642,6 @@ void pevent_free(struct pevent *pevent)
> cmdlist = cmdnext;
> }
>
> - if (pevent->func_map) {
> - for (i = 0; i < (int)pevent->func_count; i++) {
> - free(pevent->func_map[i].func);
> - free(pevent->func_map[i].mod);
> - }
> - free(pevent->func_map);
> - }
> -
> - while (funclist) {
> - funcnext = funclist->next;
> - free(funclist->func);
> - free(funclist->mod);
> - free(funclist);
> - funclist = funcnext;
> - }
> -
> while (pevent->func_handlers) {
> func_handler = pevent->func_handlers;
> pevent->func_handlers = func_handler->next;
> diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
> index 0c03538..af7a693 100644
> --- a/tools/lib/traceevent/event-parse.h
> +++ b/tools/lib/traceevent/event-parse.h
> @@ -483,10 +483,7 @@ struct pevent {
> struct cmdline_list *cmdlist;
> int cmdline_count;
>
> - struct func_map *func_map;
> struct func_resolver *func_resolver;
> - struct func_list *funclist;
> - unsigned int func_count;
>
> struct printk_map *printk_map;
> struct printk_list *printklist;
> --
> 1.8.5.6.2.g3d8a54e.dirty
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] tools/lib/traceevent/event-parse: delete pevent_register_function
2018-01-15 10:08 ` Jiri Olsa
@ 2018-01-15 11:54 ` Steven Rostedt
2018-01-15 13:13 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 4+ messages in thread
From: Steven Rostedt @ 2018-01-15 11:54 UTC (permalink / raw)
To: Jiri Olsa
Cc: Wang YanQing, acme, peterz, mingo, alexander.shishkin,
linux-kernel
On Mon, 15 Jan 2018 11:08:34 +0100
Jiri Olsa <jolsa@redhat.com> wrote:
> On Mon, Jan 15, 2018 at 12:50:14PM +0800, Wang YanQing wrote:
> > After commit 4263cece22e3da94f16fbbcf71ce3807946d3ef3
> > ("perf tools: Stop reading the kallsyms data from perf.data"),
> > there is no users of pevent_register_function in tree, so we
> > could just delete it.
> >
>
> I don't think you can remove this function,
> perf is not the only user there
Correct. The tools/lib does not have the same requirements as the
kernel code. It's purpose is to be a library for other tools, not just
what is in tree. Otherwise, I would be happy to add trace-cmd to the
kernel proper, as that is where this code originated from.
Don't delete those functions. They are required elsewhere.
-- Steve
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] tools/lib/traceevent/event-parse: delete pevent_register_function
2018-01-15 11:54 ` Steven Rostedt
@ 2018-01-15 13:13 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 4+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-01-15 13:13 UTC (permalink / raw)
To: Steven Rostedt
Cc: Jiri Olsa, Wang YanQing, peterz, mingo, alexander.shishkin,
linux-kernel
Em Mon, Jan 15, 2018 at 06:54:27AM -0500, Steven Rostedt escreveu:
> On Mon, 15 Jan 2018 11:08:34 +0100
> Jiri Olsa <jolsa@redhat.com> wrote:
>
> > On Mon, Jan 15, 2018 at 12:50:14PM +0800, Wang YanQing wrote:
> > > After commit 4263cece22e3da94f16fbbcf71ce3807946d3ef3
> > > ("perf tools: Stop reading the kallsyms data from perf.data"),
> > > there is no users of pevent_register_function in tree, so we
> > > could just delete it.
> > >
> >
> > I don't think you can remove this function,
> > perf is not the only user there
>
> Correct. The tools/lib does not have the same requirements as the
> kernel code. It's purpose is to be a library for other tools, not just
> what is in tree. Otherwise, I would be happy to add trace-cmd to the
> kernel proper, as that is where this code originated from.
>
> Don't delete those functions. They are required elsewhere.
Right, I'll eventually add a 'perf test' entry to use those functions,
testing its functionality so that this doesn't resurfaces in the future
:-)
- Arnaldo
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-01-15 13:13 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-01-15 4:50 [PATCH] tools/lib/traceevent/event-parse: delete pevent_register_function Wang YanQing
2018-01-15 10:08 ` Jiri Olsa
2018-01-15 11:54 ` Steven Rostedt
2018-01-15 13:13 ` Arnaldo Carvalho de Melo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.