| Message ID | 20181128133119.13149-1-tstoyanov@vmware.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3] tools/lib/traceevent: make libtraceevent thread safe | expand |
On Wed, 28 Nov 2018 13:31:31 +0000 Tzvetomir Stoyanov <tstoyanov@vmware.com> wrote: > This patch is a PoC about transforming libtraceevent Hi Tzvetomir, You may want to talk with Yordan about how to send patches with a subject like: [POC][PATCH v3] tools/lib/traceevent: make libtraceevent thread safe As the subject should state that this is a proof of concept patch. > into a thread safe library. It implements per thread local > storage and internal APIs to access it. It covers only > tep->last_event cache, but easily can be extended with all > library's thread sensitive data. > > Signed-off-by: Tzvetomir Stoyanov <tstoyanov@vmware.com> > --- > v2: Added local thread data per tep context > v3: Implemented APIs to access specific tread local data, > instead of the whole struct tep_thread_data. > --- > tools/lib/traceevent/event-parse-local.h | 18 ++- > tools/lib/traceevent/event-parse-thread.c | 131 ++++++++++++++++++++++ > tools/lib/traceevent/event-parse.c | 21 ++-- > 3 files changed, 158 insertions(+), 12 deletions(-) > create mode 100644 tools/lib/traceevent/event-parse-thread.c > > diff --git a/tools/lib/traceevent/event-parse-local.h b/tools/lib/traceevent/event-parse-local.h > index 9a092dd4a86d..aa9418fdefd0 100644 > --- a/tools/lib/traceevent/event-parse-local.h > +++ b/tools/lib/traceevent/event-parse-local.h > @@ -14,6 +14,14 @@ struct func_list; > struct event_handler; > struct func_resolver; > > +/* cache */ > +struct tep_thread_data { > + struct tep_handle *tep; > + struct tep_thread_data *next; > + > + struct tep_event *last_event; > +}; > + > struct tep_handle { > int ref_count; > > @@ -83,9 +91,6 @@ struct tep_handle { > struct event_handler *handlers; > struct tep_function_handler *func_handlers; > > - /* cache */ > - struct tep_event *last_event; > - > char *trace_clock; > }; > > @@ -96,4 +101,11 @@ unsigned short tep_data2host2(struct tep_handle *pevent, unsigned short data); > unsigned int tep_data2host4(struct tep_handle *pevent, unsigned int data); > unsigned long long tep_data2host8(struct tep_handle *pevent, unsigned long long data); > > +/* tep cache per thread */ > +struct tep_event *tep_find_event_by_id_cache(struct tep_handle *tep, int id); > +struct tep_event * > +tep_find_event_by_name_cache(struct tep_handle *tep, const char *name, const char *sys); > +void tep_set_serach_event_cache(struct tep_handle *tep, struct tep_event *event); > +void tep_destroy_thread_local(struct tep_handle *tep); > + > #endif /* _PARSE_EVENTS_INT_H */ > diff --git a/tools/lib/traceevent/event-parse-thread.c b/tools/lib/traceevent/event-parse-thread.c > new file mode 100644 > index 000000000000..84707c24ac6b > --- /dev/null > +++ b/tools/lib/traceevent/event-parse-thread.c > @@ -0,0 +1,131 @@ > +// SPDX-License-Identifier: LGPL-2.1 > +/* > + * Copyright (C) 2009, 2010 Red Hat Inc, Steven Rostedt <srostedt@redhat.com> > + * > + */ > + > +#include "event-parse.h" > +#include "event-parse-local.h" > +#include "event-utils.h" > + > +static __thread struct tep_thread_data *tep_thread_local; > + > +static struct tep_thread_data *tep_alloc_thread_local(struct tep_handle *tep) > +{ > + struct tep_thread_data *local; > + > + local = calloc(1, sizeof(struct tep_thread_data)); > + if (local) { > + local->tep = tep; > + local->next = tep_thread_local; I really don't want a loop. We should have one thread cache. Heck, then we don't even need to allocate it. -- Steve > + tep_thread_local = local; > + } > + return local; > +} > +
Hi Steven On Wed, Nov 28, 2018 at 4:27 PM Steven Rostedt <rostedt@goodmis.org> wrote: > > > On Wed, 28 Nov 2018 13:31:31 +0000 > Tzvetomir Stoyanov <tstoyanov@vmware.com> wrote: > > > This patch is a PoC about transforming libtraceevent > > Hi Tzvetomir, > > You may want to talk with Yordan about how to send patches with a > subject like: > > [POC][PATCH v3] tools/lib/traceevent: make libtraceevent thread safe > > As the subject should state that this is a proof of concept patch. > > > into a thread safe library. It implements per thread local > > storage and internal APIs to access it. It covers only > > tep->last_event cache, but easily can be extended with all > > library's thread sensitive data. > > > > Signed-off-by: Tzvetomir Stoyanov <tstoyanov@vmware.com> > > --- > > v2: Added local thread data per tep context > > v3: Implemented APIs to access specific tread local data, > > instead of the whole struct tep_thread_data. > > --- > > tools/lib/traceevent/event-parse-local.h | 18 ++- > > tools/lib/traceevent/event-parse-thread.c | 131 ++++++++++++++++++++++ > > tools/lib/traceevent/event-parse.c | 21 ++-- > > 3 files changed, 158 insertions(+), 12 deletions(-) > > create mode 100644 tools/lib/traceevent/event-parse-thread.c > > > > diff --git a/tools/lib/traceevent/event-parse-local.h b/tools/lib/traceevent/event-parse-local.h > > index 9a092dd4a86d..aa9418fdefd0 100644 > > --- a/tools/lib/traceevent/event-parse-local.h > > +++ b/tools/lib/traceevent/event-parse-local.h > > @@ -14,6 +14,14 @@ struct func_list; > > struct event_handler; > > struct func_resolver; > > > > +/* cache */ > > +struct tep_thread_data { > > + struct tep_handle *tep; > > + struct tep_thread_data *next; > > + > > + struct tep_event *last_event; > > +}; > > + > > struct tep_handle { > > int ref_count; > > > > @@ -83,9 +91,6 @@ struct tep_handle { > > struct event_handler *handlers; > > struct tep_function_handler *func_handlers; > > > > - /* cache */ > > - struct tep_event *last_event; > > - > > char *trace_clock; > > }; > > > > @@ -96,4 +101,11 @@ unsigned short tep_data2host2(struct tep_handle *pevent, unsigned short data); > > unsigned int tep_data2host4(struct tep_handle *pevent, unsigned int data); > > unsigned long long tep_data2host8(struct tep_handle *pevent, unsigned long long data); > > > > +/* tep cache per thread */ > > +struct tep_event *tep_find_event_by_id_cache(struct tep_handle *tep, int id); > > +struct tep_event * > > +tep_find_event_by_name_cache(struct tep_handle *tep, const char *name, const char *sys); > > +void tep_set_serach_event_cache(struct tep_handle *tep, struct tep_event *event); > > +void tep_destroy_thread_local(struct tep_handle *tep); > > + > > #endif /* _PARSE_EVENTS_INT_H */ > > diff --git a/tools/lib/traceevent/event-parse-thread.c b/tools/lib/traceevent/event-parse-thread.c > > new file mode 100644 > > index 000000000000..84707c24ac6b > > --- /dev/null > > +++ b/tools/lib/traceevent/event-parse-thread.c > > @@ -0,0 +1,131 @@ > > +// SPDX-License-Identifier: LGPL-2.1 > > +/* > > + * Copyright (C) 2009, 2010 Red Hat Inc, Steven Rostedt <srostedt@redhat.com> > > + * > > + */ > > + > > +#include "event-parse.h" > > +#include "event-parse-local.h" > > +#include "event-utils.h" > > + > > +static __thread struct tep_thread_data *tep_thread_local; > > + > > +static struct tep_thread_data *tep_alloc_thread_local(struct tep_handle *tep) > > +{ > > + struct tep_thread_data *local; > > + > > + local = calloc(1, sizeof(struct tep_thread_data)); > > + if (local) { > > + local->tep = tep; > > + local->next = tep_thread_local; > > I really don't want a loop. We should have one thread cache. Heck, then > we don't even need to allocate it. > The reason for dynamic allocation is that we should support the case where we have multiple threads, working with multiple tep handlers. We need each thread to has local data for each tep handler, so we should use list (or other dynamic structure) of all tep handlers per thread. The global variable static __thread struct tep_thread_data *tep_thread_local; has to store locals for multiple tep handlers, in context of current thread. > -- Steve > > > > + tep_thread_local = local; > > + } > > + return local; > > +} > > + > >
diff --git a/tools/lib/traceevent/event-parse-local.h b/tools/lib/traceevent/event-parse-local.h index 9a092dd4a86d..aa9418fdefd0 100644 --- a/tools/lib/traceevent/event-parse-local.h +++ b/tools/lib/traceevent/event-parse-local.h @@ -14,6 +14,14 @@ struct func_list; struct event_handler; struct func_resolver; +/* cache */ +struct tep_thread_data { + struct tep_handle *tep; + struct tep_thread_data *next; + + struct tep_event *last_event; +}; + struct tep_handle { int ref_count; @@ -83,9 +91,6 @@ struct tep_handle { struct event_handler *handlers; struct tep_function_handler *func_handlers; - /* cache */ - struct tep_event *last_event; - char *trace_clock; }; @@ -96,4 +101,11 @@ unsigned short tep_data2host2(struct tep_handle *pevent, unsigned short data); unsigned int tep_data2host4(struct tep_handle *pevent, unsigned int data); unsigned long long tep_data2host8(struct tep_handle *pevent, unsigned long long data); +/* tep cache per thread */ +struct tep_event *tep_find_event_by_id_cache(struct tep_handle *tep, int id); +struct tep_event * +tep_find_event_by_name_cache(struct tep_handle *tep, const char *name, const char *sys); +void tep_set_serach_event_cache(struct tep_handle *tep, struct tep_event *event); +void tep_destroy_thread_local(struct tep_handle *tep); + #endif /* _PARSE_EVENTS_INT_H */ diff --git a/tools/lib/traceevent/event-parse-thread.c b/tools/lib/traceevent/event-parse-thread.c new file mode 100644 index 000000000000..84707c24ac6b --- /dev/null +++ b/tools/lib/traceevent/event-parse-thread.c @@ -0,0 +1,131 @@ +// SPDX-License-Identifier: LGPL-2.1 +/* + * Copyright (C) 2009, 2010 Red Hat Inc, Steven Rostedt <srostedt@redhat.com> + * + */ + +#include "event-parse.h" +#include "event-parse-local.h" +#include "event-utils.h" + +static __thread struct tep_thread_data *tep_thread_local; + +static struct tep_thread_data *tep_alloc_thread_local(struct tep_handle *tep) +{ + struct tep_thread_data *local; + + local = calloc(1, sizeof(struct tep_thread_data)); + if (local) { + local->tep = tep; + local->next = tep_thread_local; + tep_thread_local = local; + } + return local; +} + +static struct tep_thread_data *tep_get_thread_local(struct tep_handle *tep) +{ + struct tep_thread_data *local; + static __thread struct tep_thread_data *last_local; + + if (last_local && last_local->tep == tep) + return last_local; + + local = tep_thread_local; + while (local) { + if (local->tep == tep) { + last_local = local; + return local; + } + local = local->next; + } + + return NULL; +} + +/** + * tep_find_event_by_id_cache - Find event with given id in the cache + * @tep: a handle to the tep context + * @id: event's id + * + * Searches in the cache for event with given id + */ +struct tep_event *tep_find_event_by_id_cache(struct tep_handle *tep, int id) +{ + struct tep_thread_data *local = tep_get_thread_local(tep); + + if (local && local->last_event && local->last_event->id == id) + return local->last_event; + + return NULL; +} + +/** + * tep_find_event_by_name_cache - Find event with given name and sys in the cache + * @tep: a handle to the tep context + * @name: event's name + * @sys: event's system + * + * Searches in the cache for event with given name and system + */ +struct tep_event * +tep_find_event_by_name_cache(struct tep_handle *tep, const char *name, const char *sys) +{ + struct tep_thread_data *local = tep_get_thread_local(tep); + + if (local && local->last_event && + strcmp(local->last_event->name, name) == 0 && + (!sys || strcmp(local->last_event->system, sys) == 0)) + return local->last_event; + + return NULL; +} + +/** + * tep_set_serach_event_cache - set last used event in the cache + * @tep: a handle to the tep context + * @event: pointer to the last used event + * + * Sets last used event in the cache, to speed up the next searches + */ +void tep_set_serach_event_cache(struct tep_handle *tep, struct tep_event *event) +{ + struct tep_thread_data *local = tep_get_thread_local(tep); + + if (!local) + local = tep_alloc_thread_local(tep); + + if (local) + local->last_event = event; +} + +/** + * tep_destroy_thread_local - destroy local data of tep context + * @tep: a handle to the tep context + * + * Destroys local data of tep context for the current thread + */ +void tep_destroy_thread_local(struct tep_handle *tep) +{ + struct tep_thread_data **local, *del; + + if (!tep_thread_local) + return; + local = &tep_thread_local; + + while (*local) { + if ((*local)->tep == tep) { + tep_thread_local = (*local)->next; + free(*local); + return; + } + if ((*local)->next && (*local)->next->tep == tep) { + del = (*local)->next; + (*local)->next = (*local)->next->next; + free(del); + return; + } + local = &((*local)->next); + } +} + diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c index a5048c1b9bec..8fd727023640 100644 --- a/tools/lib/traceevent/event-parse.c +++ b/tools/lib/traceevent/event-parse.c @@ -3485,10 +3485,11 @@ struct tep_event *tep_find_event(struct tep_handle *pevent, int id) struct tep_event **eventptr; struct tep_event key; struct tep_event *pkey = &key; + struct tep_event *event_cache = tep_find_event_by_id_cache(pevent, id); /* Check cache first */ - if (pevent->last_event && pevent->last_event->id == id) - return pevent->last_event; + if (event_cache) + return event_cache; key.id = id; @@ -3496,7 +3497,7 @@ struct tep_event *tep_find_event(struct tep_handle *pevent, int id) sizeof(*pevent->events), events_id_cmp); if (eventptr) { - pevent->last_event = *eventptr; + tep_set_serach_event_cache(pevent, *eventptr); return *eventptr; } @@ -3516,13 +3517,13 @@ struct tep_event * tep_find_event_by_name(struct tep_handle *pevent, const char *sys, const char *name) { - struct tep_event *event = NULL; int i; + struct tep_event *event = NULL; + struct tep_event *event_cache = tep_find_event_by_name_cache(pevent, name, sys); - if (pevent->last_event && - strcmp(pevent->last_event->name, name) == 0 && - (!sys || strcmp(pevent->last_event->system, sys) == 0)) - return pevent->last_event; + /* Check cache first */ + if (event_cache) + return event_cache; for (i = 0; i < pevent->nr_events; i++) { event = pevent->events[i]; @@ -3533,10 +3534,12 @@ tep_find_event_by_name(struct tep_handle *pevent, break; } } + if (i == pevent->nr_events) event = NULL; + if (event) + tep_set_serach_event_cache(pevent, event); - pevent->last_event = event; return event; }
This patch is a PoC about transforming libtraceevent into a thread safe library. It implements per thread local storage and internal APIs to access it. It covers only tep->last_event cache, but easily can be extended with all library's thread sensitive data. Signed-off-by: Tzvetomir Stoyanov <tstoyanov@vmware.com> --- v2: Added local thread data per tep context v3: Implemented APIs to access specific tread local data, instead of the whole struct tep_thread_data. --- tools/lib/traceevent/event-parse-local.h | 18 ++- tools/lib/traceevent/event-parse-thread.c | 131 ++++++++++++++++++++++ tools/lib/traceevent/event-parse.c | 21 ++-- 3 files changed, 158 insertions(+), 12 deletions(-) create mode 100644 tools/lib/traceevent/event-parse-thread.c