* [PATCH lttng-ust v2 2/2] Add perf context support for ARMv7 [not found] <1466722365-2436-1-git-send-email-jdesfossez@efficios.com> @ 2016-06-23 22:52 ` Julien Desfossez 2016-06-27 21:09 ` [PATCH lttng-ust v2 1/2] Keep perf context FD open for other architectures Mathieu Desnoyers [not found] ` <1466722365-2436-2-git-send-email-jdesfossez@efficios.com> 2 siblings, 0 replies; 3+ messages in thread From: Julien Desfossez @ 2016-06-23 22:52 UTC (permalink / raw) To: mathieu.desnoyers; +Cc: lttng-dev, Julien Desfossez Allow to add perf context to UST traces. ARMv7 does not have a reliable way to read perf PMU counters entirely from user-space like we do on x86, so this approach requires a system call every time a counter needs to be read which has a significant performance impact. ARMv7 does not have way to read PMU from userspace because it requires write access to the debug coprocessor to select which PMU counter to read which defeats user-space/kernel protection. For that reason, the bits required to allow user-space access to those registers are not enabled in the kernel and Perf does not expose any information in the shared mmap page, so we do not know what is the counter index. Also, for ARMv7 we cannot the exclude_kernel flag, so the counter stays active even when the process is executing in kernel context (system calls mainly). This generic approach might work on other architecture, but it has not yet been tested so it is not enabled in the code. Signed-off-by: Julien Desfossez <jdesfossez@efficios.com> --- configure.ac | 1 + liblttng-ust-ctl/ustctl.c | 2 +- liblttng-ust/lttng-context-perf-counters.c | 84 +++++++++++++++++++++--------- 3 files changed, 62 insertions(+), 25 deletions(-) diff --git a/configure.ac b/configure.ac index de462ff..5475640 100644 --- a/configure.ac +++ b/configure.ac @@ -219,6 +219,7 @@ AS_CASE([$host_cpu], [i[[3456]]86], [UST_SUPPORT_FOR_ARCH_PERF_EVENT_COUNTERS=yes], [x86_64], [UST_SUPPORT_FOR_ARCH_PERF_EVENT_COUNTERS=yes], [amd64], [UST_SUPPORT_FOR_ARCH_PERF_EVENT_COUNTERS=yes], + [armv7l], [UST_SUPPORT_FOR_ARCH_PERF_EVENT_COUNTERS=yes], [UST_SUPPORT_FOR_ARCH_PERF_EVENT_COUNTERS=no]) AC_MSG_RESULT([$UST_SUPPORT_FOR_ARCH_PERF_EVENT_COUNTERS]) diff --git a/liblttng-ust-ctl/ustctl.c b/liblttng-ust-ctl/ustctl.c index 0165786..97445d5 100644 --- a/liblttng-ust-ctl/ustctl.c +++ b/liblttng-ust-ctl/ustctl.c @@ -1742,7 +1742,7 @@ int ustctl_get_instance_id(struct ustctl_consumer_stream *stream, return client_cb->instance_id(buf, handle, id); } -#if defined(__x86_64__) || defined(__i386__) +#if defined(__x86_64__) || defined(__i386__) || defined(__ARM_ARCH_7A__) int ustctl_has_perf_counters(void) { diff --git a/liblttng-ust/lttng-context-perf-counters.c b/liblttng-ust/lttng-context-perf-counters.c index 725b1b4..a981aa9 100644 --- a/liblttng-ust/lttng-context-perf-counters.c +++ b/liblttng-ust/lttng-context-perf-counters.c @@ -91,17 +91,13 @@ uint64_t rdpmc(unsigned int counter) return low | ((uint64_t) high) << 32; } -#else /* defined(__x86_64__) || defined(__i386__) */ - -#error "Perf event counters are only supported on x86 so far." - -#endif /* #else defined(__x86_64__) || defined(__i386__) */ - static -uint64_t read_perf_counter(struct perf_event_mmap_page *pc) +uint64_t read_perf_counter( + struct lttng_perf_counter_thread_field *thread_field) { uint32_t seq, idx; uint64_t count; + struct perf_event_mmap_page *pc = thread_field->pc; if (caa_unlikely(!pc)) return 0; @@ -122,6 +118,29 @@ uint64_t read_perf_counter(struct perf_event_mmap_page *pc) return count; } +#elif defined (__ARM_ARCH_7A__) + +static +uint64_t read_perf_counter( + struct lttng_perf_counter_thread_field *thread_field) +{ + uint64_t count; + + if (caa_unlikely(thread_field->fd < 0)) + return 0; + + if (read(thread_field->fd, &count, sizeof(count)) < sizeof(count)) + return 0; + + return count; +} + +#else /* defined(__x86_64__) || defined(__i386__) || defined(__ARM_ARCH_7A__) */ + +#error "Perf event counters are only supported on x86 and ARMv7 so far." + +#endif /* #else defined(__x86_64__) || defined(__i386__) || defined(__ARM_ARCH_7A__) */ + static int sys_perf_event_open(struct perf_event_attr *attr, pid_t pid, int cpu, int group_fd, @@ -144,6 +163,20 @@ int open_perf_fd(struct perf_event_attr *attr) } static +void close_perf_fd(int fd) +{ + int ret; + + if (fd < 0) + return; + + ret = close(fd); + if (ret) { + perror("Error closing LTTng-UST perf memory mapping FD"); + } +} + +static struct perf_event_mmap_page *setup_perf( struct lttng_perf_counter_thread_field *thread_field) { @@ -160,25 +193,10 @@ struct perf_event_mmap_page *setup_perf( thread_field->fd = -1; #endif -end: return perf_addr; } static -void close_perf_fd(int fd) -{ - int ret; - - if (fd < 0) - return; - - ret = close(fd); - if (ret) { - perror("Error closing LTTng-UST perf memory mapping FD"); - } -} - -static void unmap_perf_page(struct perf_event_mmap_page *pc) { int ret; @@ -292,7 +310,7 @@ uint64_t wrapper_perf_counter_read(struct lttng_ctx_field *field) perf_field = field->u.perf_counter; perf_thread_field = get_thread_field(perf_field); - return read_perf_counter(perf_thread_field->pc); + return read_perf_counter(perf_thread_field); } static @@ -363,6 +381,24 @@ void lttng_destroy_perf_counter_field(struct lttng_ctx_field *field) free(perf_field); } +#ifdef __ARM_ARCH_7A__ + +static +int perf_get_exclude_kernel(void) +{ + return 0; +} + +#else /* __ARM_ARCH_7A__ */ + +static +int perf_get_exclude_kernel(void) +{ + return 1; +} + +#endif /* __ARM_ARCH_7A__ */ + /* Called with UST lock held */ int lttng_add_perf_counter_to_ctx(uint32_t type, uint64_t config, @@ -413,7 +449,7 @@ int lttng_add_perf_counter_to_ctx(uint32_t type, perf_field->attr.type = type; perf_field->attr.config = config; - perf_field->attr.exclude_kernel = 1; + perf_field->attr.exclude_kernel = perf_get_exclude_kernel(); CDS_INIT_LIST_HEAD(&perf_field->thread_field_list); field->u.perf_counter = perf_field; -- 1.9.1 _______________________________________________ lttng-dev mailing list lttng-dev@lists.lttng.org https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH lttng-ust v2 1/2] Keep perf context FD open for other architectures [not found] <1466722365-2436-1-git-send-email-jdesfossez@efficios.com> 2016-06-23 22:52 ` [PATCH lttng-ust v2 2/2] Add perf context support for ARMv7 Julien Desfossez @ 2016-06-27 21:09 ` Mathieu Desnoyers [not found] ` <1466722365-2436-2-git-send-email-jdesfossez@efficios.com> 2 siblings, 0 replies; 3+ messages in thread From: Mathieu Desnoyers @ 2016-06-27 21:09 UTC (permalink / raw) To: Julien Desfossez; +Cc: lttng-dev ----- On Jun 23, 2016, at 6:52 PM, Julien Desfossez jdesfossez@efficios.com wrote: > Instead of closing the perf context after the page has been mapped, keep > it open so it can be used with the read() system call if the > architecture does not support direct access from user-space. > > Signed-off-by: Julien Desfossez <jdesfossez@efficios.com> > --- > liblttng-ust/lttng-context-perf-counters.c | 53 +++++++++++++++++++++++------- > 1 file changed, 41 insertions(+), 12 deletions(-) > > diff --git a/liblttng-ust/lttng-context-perf-counters.c > b/liblttng-ust/lttng-context-perf-counters.c > index 97ddf97..725b1b4 100644 > --- a/liblttng-ust/lttng-context-perf-counters.c > +++ b/liblttng-ust/lttng-context-perf-counters.c > @@ -55,6 +55,7 @@ struct lttng_perf_counter_thread_field { > struct perf_event_mmap_page *pc; > struct cds_list_head thread_field_node; /* Per-field list of thread fields > (node) */ > struct cds_list_head rcu_field_node; /* RCU per-thread list of fields (node) */ > + int fd; /* Perf FD */ > }; > > struct lttng_perf_counter_thread { > @@ -131,24 +132,50 @@ int sys_perf_event_open(struct perf_event_attr *attr, > } > > static > -struct perf_event_mmap_page *setup_perf(struct perf_event_attr *attr) > +int open_perf_fd(struct perf_event_attr *attr) > { > - void *perf_addr; > - int fd, ret; > + int fd; > > fd = sys_perf_event_open(attr, 0, -1, -1, 0); > if (fd < 0) > - return NULL; > + return -1; > + > + return fd; > +} > + > +static > +struct perf_event_mmap_page *setup_perf( > + struct lttng_perf_counter_thread_field *thread_field) > +{ > + void *perf_addr; > > perf_addr = mmap(NULL, sizeof(struct perf_event_mmap_page), > - PROT_READ, MAP_SHARED, fd, 0); > + PROT_READ, MAP_SHARED, thread_field->fd, 0); > if (perf_addr == MAP_FAILED) > - return NULL; > + perf_addr = NULL; > + > + /* No need to keep the FD open on x86 */ > +#if defined(__x86_64__) || defined(__i386__) > + close_perf_fd(thread_field->fd); > + thread_field->fd = -1; > +#endif No #if/#endif whatsoever in the code please. We don't want lttng to end up looking like glibc. ;) If needed, move that at the beginning of the file with e.g. #if defined(__x86_64__) || defined(__i386__) static bool arch_perf_use_read(void) { return true; } #else static bool arch_perf_use_read(void) { return false; } #endif > + > +end: > + return perf_addr; > +} > + > +static > +void close_perf_fd(int fd) > +{ > + int ret; > + > + if (fd < 0) > + return; > + > ret = close(fd); > if (ret) { You can remove those extra { }. > perror("Error closing LTTng-UST perf memory mapping FD"); > } > - return perf_addr; > } > > static > @@ -221,7 +248,9 @@ struct lttng_perf_counter_thread_field * > if (!thread_field) > abort(); > thread_field->field = perf_field; > - thread_field->pc = setup_perf(&perf_field->attr); > + thread_field->fd = open_perf_fd(&perf_field->attr); > + if (thread_field->fd >= 0) > + thread_field->pc = setup_perf(thread_field); > /* Note: thread_field->pc can be NULL if setup_perf() fails. */ Do we need to update this comment to state that thread_field->fd can also be -1 if open_perf_fd fails ? Thanks, Mathieu > ust_lock_nocheck(); > cds_list_add_rcu(&thread_field->rcu_field_node, > @@ -293,6 +322,7 @@ static > void lttng_destroy_perf_thread_field( > struct lttng_perf_counter_thread_field *thread_field) > { > + close_perf_fd(thread_field->fd); > unmap_perf_page(thread_field->pc); > cds_list_del_rcu(&thread_field->rcu_field_node); > cds_list_del(&thread_field->thread_field_node); > @@ -341,7 +371,6 @@ int lttng_add_perf_counter_to_ctx(uint32_t type, > { > struct lttng_ctx_field *field; > struct lttng_perf_counter_field *perf_field; > - struct perf_event_mmap_page *tmp_pc; > char *name_alloc; > int ret; > > @@ -389,12 +418,12 @@ int lttng_add_perf_counter_to_ctx(uint32_t type, > field->u.perf_counter = perf_field; > > /* Ensure that this perf counter can be used in this process. */ > - tmp_pc = setup_perf(&perf_field->attr); > - if (!tmp_pc) { > + ret = open_perf_fd(&perf_field->attr); > + if (ret < 0) { > ret = -ENODEV; > goto setup_error; > } > - unmap_perf_page(tmp_pc); > + close_perf_fd(ret); > > /* > * Contexts can only be added before tracing is started, so we > -- > 1.9.1 > > _______________________________________________ > lttng-dev mailing list > lttng-dev@lists.lttng.org > https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com _______________________________________________ lttng-dev mailing list lttng-dev@lists.lttng.org https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev ^ permalink raw reply [flat|nested] 3+ messages in thread
[parent not found: <1466722365-2436-2-git-send-email-jdesfossez@efficios.com>]
* Re: [PATCH lttng-ust v2 2/2] Add perf context support for ARMv7 [not found] ` <1466722365-2436-2-git-send-email-jdesfossez@efficios.com> @ 2016-06-27 21:13 ` Mathieu Desnoyers 0 siblings, 0 replies; 3+ messages in thread From: Mathieu Desnoyers @ 2016-06-27 21:13 UTC (permalink / raw) To: Julien Desfossez; +Cc: lttng-dev ----- On Jun 23, 2016, at 6:52 PM, Julien Desfossez jdesfossez@efficios.com wrote: > Allow to add perf context to UST traces. ARMv7 does not have a reliable > way to read perf PMU counters entirely from user-space like we do on > x86, so this approach requires a system call every time a counter needs > to be read which has a significant performance impact. > > ARMv7 does not have way to read PMU from userspace because it requires > write access to the debug coprocessor to select which PMU counter to > read which defeats user-space/kernel protection. For that reason, the > bits required to allow user-space access to those registers are not > enabled in the kernel and Perf does not expose any information in the > shared mmap page, so we do not know what is the counter index. Also, for > ARMv7 we cannot the exclude_kernel flag, so the counter stays active we cannot .. set ? > even when the process is executing in kernel context (system calls > mainly). > > This generic approach might work on other architecture, but it has not > yet been tested so it is not enabled in the code. > > Signed-off-by: Julien Desfossez <jdesfossez@efficios.com> > --- > configure.ac | 1 + > liblttng-ust-ctl/ustctl.c | 2 +- > liblttng-ust/lttng-context-perf-counters.c | 84 +++++++++++++++++++++--------- > 3 files changed, 62 insertions(+), 25 deletions(-) > > diff --git a/configure.ac b/configure.ac > index de462ff..5475640 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -219,6 +219,7 @@ AS_CASE([$host_cpu], > [i[[3456]]86], [UST_SUPPORT_FOR_ARCH_PERF_EVENT_COUNTERS=yes], > [x86_64], [UST_SUPPORT_FOR_ARCH_PERF_EVENT_COUNTERS=yes], > [amd64], [UST_SUPPORT_FOR_ARCH_PERF_EVENT_COUNTERS=yes], > + [armv7l], [UST_SUPPORT_FOR_ARCH_PERF_EVENT_COUNTERS=yes], > [UST_SUPPORT_FOR_ARCH_PERF_EVENT_COUNTERS=no]) > AC_MSG_RESULT([$UST_SUPPORT_FOR_ARCH_PERF_EVENT_COUNTERS]) > > diff --git a/liblttng-ust-ctl/ustctl.c b/liblttng-ust-ctl/ustctl.c > index 0165786..97445d5 100644 > --- a/liblttng-ust-ctl/ustctl.c > +++ b/liblttng-ust-ctl/ustctl.c > @@ -1742,7 +1742,7 @@ int ustctl_get_instance_id(struct ustctl_consumer_stream > *stream, > return client_cb->instance_id(buf, handle, id); > } > > -#if defined(__x86_64__) || defined(__i386__) > +#if defined(__x86_64__) || defined(__i386__) || defined(__ARM_ARCH_7A__) > > int ustctl_has_perf_counters(void) > { > diff --git a/liblttng-ust/lttng-context-perf-counters.c > b/liblttng-ust/lttng-context-perf-counters.c > index 725b1b4..a981aa9 100644 > --- a/liblttng-ust/lttng-context-perf-counters.c > +++ b/liblttng-ust/lttng-context-perf-counters.c > @@ -91,17 +91,13 @@ uint64_t rdpmc(unsigned int counter) > return low | ((uint64_t) high) << 32; > } > > -#else /* defined(__x86_64__) || defined(__i386__) */ > - > -#error "Perf event counters are only supported on x86 so far." > - > -#endif /* #else defined(__x86_64__) || defined(__i386__) */ > - > static > -uint64_t read_perf_counter(struct perf_event_mmap_page *pc) > +uint64_t read_perf_counter( > + struct lttng_perf_counter_thread_field *thread_field) > { > uint32_t seq, idx; > uint64_t count; > + struct perf_event_mmap_page *pc = thread_field->pc; > > if (caa_unlikely(!pc)) > return 0; > @@ -122,6 +118,29 @@ uint64_t read_perf_counter(struct perf_event_mmap_page *pc) > return count; > } > > +#elif defined (__ARM_ARCH_7A__) > + > +static > +uint64_t read_perf_counter( > + struct lttng_perf_counter_thread_field *thread_field) > +{ > + uint64_t count; > + > + if (caa_unlikely(thread_field->fd < 0)) > + return 0; > + > + if (read(thread_field->fd, &count, sizeof(count)) < sizeof(count)) ^ This could also be a caa_unlikely(). Thanks, Mathieu > + return 0; > + > + return count; > +} > + > +#else /* defined(__x86_64__) || defined(__i386__) || defined(__ARM_ARCH_7A__) > */ > + > +#error "Perf event counters are only supported on x86 and ARMv7 so far." > + > +#endif /* #else defined(__x86_64__) || defined(__i386__) || > defined(__ARM_ARCH_7A__) */ > + > static > int sys_perf_event_open(struct perf_event_attr *attr, > pid_t pid, int cpu, int group_fd, > @@ -144,6 +163,20 @@ int open_perf_fd(struct perf_event_attr *attr) > } > > static > +void close_perf_fd(int fd) > +{ > + int ret; > + > + if (fd < 0) > + return; > + > + ret = close(fd); > + if (ret) { > + perror("Error closing LTTng-UST perf memory mapping FD"); > + } > +} > + > +static > struct perf_event_mmap_page *setup_perf( > struct lttng_perf_counter_thread_field *thread_field) > { > @@ -160,25 +193,10 @@ struct perf_event_mmap_page *setup_perf( > thread_field->fd = -1; > #endif > > -end: > return perf_addr; > } > > static > -void close_perf_fd(int fd) > -{ > - int ret; > - > - if (fd < 0) > - return; > - > - ret = close(fd); > - if (ret) { > - perror("Error closing LTTng-UST perf memory mapping FD"); > - } > -} > - > -static > void unmap_perf_page(struct perf_event_mmap_page *pc) > { > int ret; > @@ -292,7 +310,7 @@ uint64_t wrapper_perf_counter_read(struct lttng_ctx_field > *field) > > perf_field = field->u.perf_counter; > perf_thread_field = get_thread_field(perf_field); > - return read_perf_counter(perf_thread_field->pc); > + return read_perf_counter(perf_thread_field); > } > > static > @@ -363,6 +381,24 @@ void lttng_destroy_perf_counter_field(struct > lttng_ctx_field *field) > free(perf_field); > } > > +#ifdef __ARM_ARCH_7A__ > + > +static > +int perf_get_exclude_kernel(void) > +{ > + return 0; > +} > + > +#else /* __ARM_ARCH_7A__ */ > + > +static > +int perf_get_exclude_kernel(void) > +{ > + return 1; > +} > + > +#endif /* __ARM_ARCH_7A__ */ > + > /* Called with UST lock held */ > int lttng_add_perf_counter_to_ctx(uint32_t type, > uint64_t config, > @@ -413,7 +449,7 @@ int lttng_add_perf_counter_to_ctx(uint32_t type, > > perf_field->attr.type = type; > perf_field->attr.config = config; > - perf_field->attr.exclude_kernel = 1; > + perf_field->attr.exclude_kernel = perf_get_exclude_kernel(); > CDS_INIT_LIST_HEAD(&perf_field->thread_field_list); > field->u.perf_counter = perf_field; > > -- > 1.9.1 -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com _______________________________________________ lttng-dev mailing list lttng-dev@lists.lttng.org https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-06-27 21:13 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1466722365-2436-1-git-send-email-jdesfossez@efficios.com>
2016-06-23 22:52 ` [PATCH lttng-ust v2 2/2] Add perf context support for ARMv7 Julien Desfossez
2016-06-27 21:09 ` [PATCH lttng-ust v2 1/2] Keep perf context FD open for other architectures Mathieu Desnoyers
[not found] ` <1466722365-2436-2-git-send-email-jdesfossez@efficios.com>
2016-06-27 21:13 ` [PATCH lttng-ust v2 2/2] Add perf context support for ARMv7 Mathieu Desnoyers
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.