[lttng-dev] [PATCH lttng-ust 2/2] Add perf context support for ARMv7

[Mathieu Desnoyers]

----- On Jun 23, 2016, at 4:15 PM, Julien Desfossez jdesfossez at 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 everytime a counter needs

everytime -> every time

> to be read which has a significant performance impact.
> 
> This generic approach might work on other architecture, but it has not
> yet been tested so it is not enabled in the code.

We might want to say a few words on why ARMv7 does not have way
to read PMU from userspace (requires write access to the debug
coprocessor to select which PMU counter to read or such, which
defeats userspace/kernel protection).

> 
> Signed-off-by: Julien Desfossez <jdesfossez at efficios.com>
> ---
> configure.ac                               |  1 +
> liblttng-ust-ctl/ustctl.c                  |  2 +-
> liblttng-ust/lttng-context-perf-counters.c | 71 ++++++++++++++++++++----------
> 3 files changed, 50 insertions(+), 24 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..220686b 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,31 @@ 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;
> +	int ret;

remove ret variable.

> +
> +	if (caa_unlikely(thread_field->fd < 0))
> +		return 0;
> +
> +	ret = read(thread_field->fd, &count, sizeof(count));
> +	if (ret < sizeof(count))

if (read(...) < 0)
  return 0;


> +		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 +165,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 +195,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 +312,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
> @@ -413,7 +433,12 @@ int lttng_add_perf_counter_to_ctx(uint32_t type,
> 
> 	perf_field->attr.type = type;
> 	perf_field->attr.config = config;
> +#if defined (__ARM_ARCH_7A__)
> +	/* exclude_kernel does not work on ARM. */
> +	perf_field->attr.exclude_kernel = 0;
> +#else
> 	perf_field->attr.exclude_kernel = 1;
> +#endif

Please move to a separate function e.g.

#ifdef __ARM_ARCH_7A__

static int perf_get_exclude_kernel(void)
{
  return 0;
}

#else

static int perf_get_exclude_kernel(void)
{
  return 1;
}

#endif

Thanks,

Mathieu

> 	CDS_INIT_LIST_HEAD(&perf_field->thread_field_list);
> 	field->u.perf_counter = perf_field;
> 
> --
> 1.9.1
> 
> _______________________________________________
> lttng-dev mailing list
> lttng-dev at lists.lttng.org
> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com