* [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).