| Message ID | 20200702185704.248123446@goodmis.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tools lib traceevent: Patches from the trace-cmd repo | expand |
On Fri, Jul 3, 2020 at 3:57 AM Steven Rostedt <rostedt@goodmis.org> wrote: > > From: "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> > > Implement new traceevent plugin API, which can be used to add new plugins > directories: > enum tep_plugin_load_priority { > TEP_PLUGIN_FIRST, > TEP_PLUGIN_LAST, > }; > int tep_add_plugin_path(struct tep_handle *tep, char *path, > enum tep_plugin_load_priority prio); > > It adds the "path" as new plugin directory, in the context of > the handler "tep". The tep_load_plugins() API searches for plugins > in this new location. Depending of the priority "prio", the plugins > from this directory are loaded before (TEP_PLUGIN_FIRST) or after > (TEP_PLUGIN_LAST) the ordinary libtraceevent plugin locations. > > Link: http://lore.kernel.org/linux-trace-devel/20191007114947.17104-2-tz.stoyanov@gmail.com > Link: http://lore.kernel.org/linux-trace-devel/20200625100516.365338-6-tz.stoyanov@gmail.com > > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> > Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org> > --- > tools/lib/traceevent/event-parse-local.h | 5 +- > tools/lib/traceevent/event-parse.c | 1 + > tools/lib/traceevent/event-parse.h | 7 +++ > tools/lib/traceevent/event-plugin.c | 70 ++++++++++++++++++++++++ > 4 files changed, 82 insertions(+), 1 deletion(-) > > diff --git a/tools/lib/traceevent/event-parse-local.h b/tools/lib/traceevent/event-parse-local.h > index cee469803a34..96a0b0ca0675 100644 > --- a/tools/lib/traceevent/event-parse-local.h > +++ b/tools/lib/traceevent/event-parse-local.h > @@ -13,6 +13,7 @@ struct func_map; > struct func_list; > struct event_handler; > struct func_resolver; > +struct tep_plugins_dir; > > struct tep_handle { > int ref_count; > @@ -47,7 +48,6 @@ struct tep_handle { > struct printk_list *printklist; > unsigned int printk_count; > > - > struct tep_event **events; > int nr_events; > struct tep_event **sort_events; > @@ -81,10 +81,13 @@ struct tep_handle { > > /* cache */ > struct tep_event *last_event; > + > + struct tep_plugins_dir *plugins_dir; > }; > > void tep_free_event(struct tep_event *event); > void tep_free_format_field(struct tep_format_field *field); > +void tep_free_plugin_paths(struct tep_handle *tep); > > unsigned short tep_data2host2(struct tep_handle *tep, unsigned short data); > unsigned int tep_data2host4(struct tep_handle *tep, unsigned int data); > diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c > index e1bd2a93c6db..064c100d2d5a 100644 > --- a/tools/lib/traceevent/event-parse.c > +++ b/tools/lib/traceevent/event-parse.c > @@ -7065,6 +7065,7 @@ void tep_free(struct tep_handle *tep) > free(tep->events); > free(tep->sort_events); > free(tep->func_resolver); > + tep_free_plugin_paths(tep); > > free(tep); > } > diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h > index 02c0438527de..91f462f5a606 100644 > --- a/tools/lib/traceevent/event-parse.h > +++ b/tools/lib/traceevent/event-parse.h > @@ -393,6 +393,13 @@ struct tep_plugin_list; > > #define INVALID_PLUGIN_LIST_OPTION ((char **)((unsigned long)-1)) > > +enum tep_plugin_load_priority { > + TEP_PLUGIN_FIRST, > + TEP_PLUGIN_LAST, > +}; > + > +int tep_add_plugin_path(struct tep_handle *tep, char *path, > + enum tep_plugin_load_priority prio); > struct tep_plugin_list *tep_load_plugins(struct tep_handle *tep); > void tep_unload_plugins(struct tep_plugin_list *plugin_list, > struct tep_handle *tep); > diff --git a/tools/lib/traceevent/event-plugin.c b/tools/lib/traceevent/event-plugin.c > index e8f4329ba8e0..1d4f1809cf17 100644 > --- a/tools/lib/traceevent/event-plugin.c > +++ b/tools/lib/traceevent/event-plugin.c > @@ -39,6 +39,12 @@ struct tep_plugin_list { > void *handle; > }; > > +struct tep_plugins_dir { > + struct tep_plugins_dir *next; > + char *path; > + enum tep_plugin_load_priority prio; > +}; > + > static void lower_case(char *str) > { > if (!str) > @@ -544,6 +550,7 @@ void tep_load_plugins_hook(struct tep_handle *tep, const char *suffix, > void *data), > void *data) > { > + struct tep_plugins_dir *dir = NULL; > char *home; > char *path; > char *envdir; > @@ -552,6 +559,15 @@ void tep_load_plugins_hook(struct tep_handle *tep, const char *suffix, > if (tep && tep->flags & TEP_DISABLE_PLUGINS) > return; > > + if (tep) > + dir = tep->plugins_dir; > + while (dir) { > + if (dir->prio == TEP_PLUGIN_FIRST) > + load_plugins_dir(tep, suffix, dir->path, > + load_plugin, data); > + dir = dir->next; > + } > + > /* > * If a system plugin directory was defined, > * check that first. > @@ -586,6 +602,15 @@ void tep_load_plugins_hook(struct tep_handle *tep, const char *suffix, > > load_plugins_dir(tep, suffix, path, load_plugin, data); > > + if (tep) > + dir = tep->plugins_dir; > + while (dir) { > + if (dir->prio == TEP_PLUGIN_LAST) > + load_plugins_dir(tep, suffix, dir->path, > + load_plugin, data); > + dir = dir->next; > + } > + > free(path); > } > > @@ -598,6 +623,51 @@ tep_load_plugins(struct tep_handle *tep) > return list; > } > > +/** > + * tep_add_plugin_path - Add a new plugin directory. > + * @tep: Trace event handler. > + * @path: Path to a directory. All files with extension .so in that Is the extension (".so") fixed? I think a new API has the suffix argument which may change it... ? > + * directory will be loaded as plugins. > + *@prio: Load priority of the plugins in that directory. > + * > + * Returns -1 in case of an error, 0 otherwise. > + */ > +int tep_add_plugin_path(struct tep_handle *tep, char *path, > + enum tep_plugin_load_priority prio) > +{ > + struct tep_plugins_dir *dir; > + > + if (!tep || !path) > + return -1; > + > + dir = calloc(1, sizeof(*dir)); > + if (!dir) > + return -1; > + > + dir->path = strdup(path); It needs to check the return value.. Thanks Namhyung > + dir->prio = prio; > + dir->next = tep->plugins_dir; > + tep->plugins_dir = dir; > + > + return 0; > +} > + > +void tep_free_plugin_paths(struct tep_handle *tep) > +{ > + struct tep_plugins_dir *dir; > + > + if (!tep) > + return; > + > + dir = tep->plugins_dir; > + while (dir) { > + tep->plugins_dir = tep->plugins_dir->next; > + free(dir->path); > + free(dir); > + dir = tep->plugins_dir; > + } > +} > + > void > tep_unload_plugins(struct tep_plugin_list *plugin_list, struct tep_handle *tep) > { > -- > 2.26.2 > >
On Wed, 8 Jul 2020 00:06:38 +0900 Namhyung Kim <namhyung@kernel.org> wrote: > > > > +/** > > + * tep_add_plugin_path - Add a new plugin directory. > > + * @tep: Trace event handler. > > + * @path: Path to a directory. All files with extension .so in that > > Is the extension (".so") fixed? I think a new API has the suffix argument > which may change it... ? So this should add a "suffix" argument? NULL for ".so"? > > > > + * directory will be loaded as plugins. > > + *@prio: Load priority of the plugins in that directory. > > + * > > + * Returns -1 in case of an error, 0 otherwise. > > + */ > > +int tep_add_plugin_path(struct tep_handle *tep, char *path, > > + enum tep_plugin_load_priority prio) > > +{ > > + struct tep_plugins_dir *dir; > > + > > + if (!tep || !path) > > + return -1; > > + > > + dir = calloc(1, sizeof(*dir)); > > + if (!dir) > > + return -1; > > + > > + dir->path = strdup(path); > > It needs to check the return value.. Yes it does indeed. BTW, since these patches are already in trace-cmd.git, would be OK if we just write patches on top of this series to address your concerns. This way, we would be also adding them to trace-cmd.git as well. I eventually want a separate libraries repo on kernel.org that this lives in and remove it from the tools/lib directory of the kernel. -- Steve > > > + dir->prio = prio; > > + dir->next = tep->plugins_dir; > > + tep->plugins_dir = dir; > > + > > + return 0; > > +} > > + > > +void tep_free_plugin_paths(struct tep_handle *tep) > > +{ > > + struct tep_plugins_dir *dir; > > + > > + if (!tep) > > + return; > > + > > + dir = tep->plugins_dir; > > + while (dir) { > > + tep->plugins_dir = tep->plugins_dir->next; > > + free(dir->path); > > + free(dir); > > + dir = tep->plugins_dir; > > + } > > +} > > + > > void > > tep_unload_plugins(struct tep_plugin_list *plugin_list, struct tep_handle *tep) > > { > > -- > > 2.26.2 > > > >
On Wed, Jul 8, 2020 at 12:24 AM Steven Rostedt <rostedt@goodmis.org> wrote: > > On Wed, 8 Jul 2020 00:06:38 +0900 > Namhyung Kim <namhyung@kernel.org> wrote: > > > > > > +/** > > > + * tep_add_plugin_path - Add a new plugin directory. > > > + * @tep: Trace event handler. > > > + * @path: Path to a directory. All files with extension .so in that > > > > Is the extension (".so") fixed? I think a new API has the suffix argument > > which may change it... ? > > So this should add a "suffix" argument? NULL for ".so"? I think it's just fine to change the comment. The file extension handling belongs to the plugin load API and we are adding the directory path here IMHO. > > > > > > > > + * directory will be loaded as plugins. > > > + *@prio: Load priority of the plugins in that directory. > > > + * > > > + * Returns -1 in case of an error, 0 otherwise. > > > + */ > > > +int tep_add_plugin_path(struct tep_handle *tep, char *path, > > > + enum tep_plugin_load_priority prio) > > > +{ > > > + struct tep_plugins_dir *dir; > > > + > > > + if (!tep || !path) > > > + return -1; > > > + > > > + dir = calloc(1, sizeof(*dir)); > > > + if (!dir) > > > + return -1; > > > + > > > + dir->path = strdup(path); > > > > It needs to check the return value.. > > Yes it does indeed. > > BTW, since these patches are already in trace-cmd.git, would be OK if > we just write patches on top of this series to address your concerns. > This way, we would be also adding them to trace-cmd.git as well. I'm ok with it. I'll review more patches soon.. Thanks Namhyung > > I eventually want a separate libraries repo on kernel.org that this > lives in and remove it from the tools/lib directory of the kernel. > > -- Steve > > > > > > > + dir->prio = prio; > > > + dir->next = tep->plugins_dir; > > > + tep->plugins_dir = dir; > > > + > > > + return 0; > > > +} > > > + > > > +void tep_free_plugin_paths(struct tep_handle *tep) > > > +{ > > > + struct tep_plugins_dir *dir; > > > + > > > + if (!tep) > > > + return; > > > + > > > + dir = tep->plugins_dir; > > > + while (dir) { > > > + tep->plugins_dir = tep->plugins_dir->next; > > > + free(dir->path); > > > + free(dir); > > > + dir = tep->plugins_dir; > > > + } > > > +} > > > + > > > void > > > tep_unload_plugins(struct tep_plugin_list *plugin_list, struct tep_handle *tep) > > > { > > > -- > > > 2.26.2 > > > > > > >
diff --git a/tools/lib/traceevent/event-parse-local.h b/tools/lib/traceevent/event-parse-local.h index cee469803a34..96a0b0ca0675 100644 --- a/tools/lib/traceevent/event-parse-local.h +++ b/tools/lib/traceevent/event-parse-local.h @@ -13,6 +13,7 @@ struct func_map; struct func_list; struct event_handler; struct func_resolver; +struct tep_plugins_dir; struct tep_handle { int ref_count; @@ -47,7 +48,6 @@ struct tep_handle { struct printk_list *printklist; unsigned int printk_count; - struct tep_event **events; int nr_events; struct tep_event **sort_events; @@ -81,10 +81,13 @@ struct tep_handle { /* cache */ struct tep_event *last_event; + + struct tep_plugins_dir *plugins_dir; }; void tep_free_event(struct tep_event *event); void tep_free_format_field(struct tep_format_field *field); +void tep_free_plugin_paths(struct tep_handle *tep); unsigned short tep_data2host2(struct tep_handle *tep, unsigned short data); unsigned int tep_data2host4(struct tep_handle *tep, unsigned int data); diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c index e1bd2a93c6db..064c100d2d5a 100644 --- a/tools/lib/traceevent/event-parse.c +++ b/tools/lib/traceevent/event-parse.c @@ -7065,6 +7065,7 @@ void tep_free(struct tep_handle *tep) free(tep->events); free(tep->sort_events); free(tep->func_resolver); + tep_free_plugin_paths(tep); free(tep); } diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h index 02c0438527de..91f462f5a606 100644 --- a/tools/lib/traceevent/event-parse.h +++ b/tools/lib/traceevent/event-parse.h @@ -393,6 +393,13 @@ struct tep_plugin_list; #define INVALID_PLUGIN_LIST_OPTION ((char **)((unsigned long)-1)) +enum tep_plugin_load_priority { + TEP_PLUGIN_FIRST, + TEP_PLUGIN_LAST, +}; + +int tep_add_plugin_path(struct tep_handle *tep, char *path, + enum tep_plugin_load_priority prio); struct tep_plugin_list *tep_load_plugins(struct tep_handle *tep); void tep_unload_plugins(struct tep_plugin_list *plugin_list, struct tep_handle *tep); diff --git a/tools/lib/traceevent/event-plugin.c b/tools/lib/traceevent/event-plugin.c index e8f4329ba8e0..1d4f1809cf17 100644 --- a/tools/lib/traceevent/event-plugin.c +++ b/tools/lib/traceevent/event-plugin.c @@ -39,6 +39,12 @@ struct tep_plugin_list { void *handle; }; +struct tep_plugins_dir { + struct tep_plugins_dir *next; + char *path; + enum tep_plugin_load_priority prio; +}; + static void lower_case(char *str) { if (!str) @@ -544,6 +550,7 @@ void tep_load_plugins_hook(struct tep_handle *tep, const char *suffix, void *data), void *data) { + struct tep_plugins_dir *dir = NULL; char *home; char *path; char *envdir; @@ -552,6 +559,15 @@ void tep_load_plugins_hook(struct tep_handle *tep, const char *suffix, if (tep && tep->flags & TEP_DISABLE_PLUGINS) return; + if (tep) + dir = tep->plugins_dir; + while (dir) { + if (dir->prio == TEP_PLUGIN_FIRST) + load_plugins_dir(tep, suffix, dir->path, + load_plugin, data); + dir = dir->next; + } + /* * If a system plugin directory was defined, * check that first. @@ -586,6 +602,15 @@ void tep_load_plugins_hook(struct tep_handle *tep, const char *suffix, load_plugins_dir(tep, suffix, path, load_plugin, data); + if (tep) + dir = tep->plugins_dir; + while (dir) { + if (dir->prio == TEP_PLUGIN_LAST) + load_plugins_dir(tep, suffix, dir->path, + load_plugin, data); + dir = dir->next; + } + free(path); } @@ -598,6 +623,51 @@ tep_load_plugins(struct tep_handle *tep) return list; } +/** + * tep_add_plugin_path - Add a new plugin directory. + * @tep: Trace event handler. + * @path: Path to a directory. All files with extension .so in that + * directory will be loaded as plugins. + *@prio: Load priority of the plugins in that directory. + * + * Returns -1 in case of an error, 0 otherwise. + */ +int tep_add_plugin_path(struct tep_handle *tep, char *path, + enum tep_plugin_load_priority prio) +{ + struct tep_plugins_dir *dir; + + if (!tep || !path) + return -1; + + dir = calloc(1, sizeof(*dir)); + if (!dir) + return -1; + + dir->path = strdup(path); + dir->prio = prio; + dir->next = tep->plugins_dir; + tep->plugins_dir = dir; + + return 0; +} + +void tep_free_plugin_paths(struct tep_handle *tep) +{ + struct tep_plugins_dir *dir; + + if (!tep) + return; + + dir = tep->plugins_dir; + while (dir) { + tep->plugins_dir = tep->plugins_dir->next; + free(dir->path); + free(dir); + dir = tep->plugins_dir; + } +} + void tep_unload_plugins(struct tep_plugin_list *plugin_list, struct tep_handle *tep) {