* [PATCH v3] tools/lib/traceevent: make libtraceevent thread safe
@ 2018-11-28 13:31 Tzvetomir Stoyanov
2018-11-28 14:27 ` Steven Rostedt
0 siblings, 1 reply; 3+ messages in thread
From: Tzvetomir Stoyanov @ 2018-11-28 13:31 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel
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
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;
}
--
2.19.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v3] tools/lib/traceevent: make libtraceevent thread safe
2018-11-28 13:31 [PATCH v3] tools/lib/traceevent: make libtraceevent thread safe Tzvetomir Stoyanov
@ 2018-11-28 14:27 ` Steven Rostedt
2018-11-28 14:50 ` Tzvetomir Stoyanov
0 siblings, 1 reply; 3+ messages in thread
From: Steven Rostedt @ 2018-11-28 14:27 UTC (permalink / raw)
To: Tzvetomir Stoyanov; +Cc: linux-trace-devel
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;
> +}
> +
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v3] tools/lib/traceevent: make libtraceevent thread safe
2018-11-28 14:27 ` Steven Rostedt
@ 2018-11-28 14:50 ` Tzvetomir Stoyanov
0 siblings, 0 replies; 3+ messages in thread
From: Tzvetomir Stoyanov @ 2018-11-28 14:50 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel
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;
> > +}
> > +
>
>
--
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-11-29 1:52 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-28 13:31 [PATCH v3] tools/lib/traceevent: make libtraceevent thread safe Tzvetomir Stoyanov
2018-11-28 14:27 ` Steven Rostedt
2018-11-28 14:50 ` Tzvetomir Stoyanov
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).