[lttng-dev] [PATCH] lttng-modules: Fixes for ARM and 32-bit systems
Mathieu Desnoyers
mathieu.desnoyers at efficios.com
Mon Mar 6 17:12:24 UTC 2017
----- On Mar 3, 2017, at 6:43 PM, Trent Piepho tpiepho at kymetacorp.com wrote:
Hi Trent,
What lttng-modules branch is this for ?
> Patch to fix recv() syscall missing in ARM syscall list.
This was probably missing because the current ARM syscall instrumentation
was generated from a 3.4.25 kernel that was missing SYSCALL macro
description of those system calls. The proper route would be to
regenerate the system call table by running a more recent kernel
and following instrumentation/syscalls/README from lttng-modules.
Those changes will go into master, but those are not fixes per se:
they increase the instrumentation coverage. So those are not strictly
considered as fixes, and should not be backported to stable lttng-modules
versions.
>
> Patch from lttng mailing list that fixes kernel timestamps being off
> by about ~2 seconds on 32-bit systems. Additional patch to fix
> compilation bug on non-X86 introduced by previous patch.
Why are you re-posting an old version of own patch which now merged
into master, stable-2.9 and stable-2.8 branches of lttng-modules ?
I don't understand the purpose of this email.
Thanks,
Mathieu
>
> Signed-off-by: Trent Piepho <tpiepho at kymetacorp.com>
> ---
> ...trumentation-Add-recv-to-ARM-syscall-tabl.patch | 54 ++++++
> ...0002-Fix-nmi-safe-clock-on-32-bit-systems.patch | 188 +++++++++++++++++++++
> ...Fix-trace-clock-wrapper-to-compile-on-ARM.patch | 32 ++++
> 3 files changed, 274 insertions(+)
> create mode 100644
> package/lttng-modules/0001-syscall-instrumentation-Add-recv-to-ARM-syscall-tabl.patch
> create mode 100644
> package/lttng-modules/0002-Fix-nmi-safe-clock-on-32-bit-systems.patch
> create mode 100644
> package/lttng-modules/0003-Fix-trace-clock-wrapper-to-compile-on-ARM.patch
>
> diff --git
> a/package/lttng-modules/0001-syscall-instrumentation-Add-recv-to-ARM-syscall-tabl.patch
> b/package/lttng-modules/0001-syscall-instrumentation-Add-recv-to-ARM-syscall-tabl.patch
> new file mode 100644
> index 0000000..f65a5d4
> --- /dev/null
> +++
> b/package/lttng-modules/0001-syscall-instrumentation-Add-recv-to-ARM-syscall-tabl.patch
> @@ -0,0 +1,54 @@
> +From 318aba6e68e0338656909b207a96566c54becc85 Mon Sep 17 00:00:00 2001
> +From: Trent Piepho <tpiepho at kymetacorp.com>
> +Date: Wed, 22 Feb 2017 12:56:08 -0800
> +Subject: [PATCH] syscall instrumentation: Add recv() to ARM syscall table
> +
> +This was missing for some reason.
> +---
> + instrumentation/syscalls/3.4.25/arm-32-syscalls-3.4.25 | 1 +
> + .../syscalls/headers/arm-32-syscalls-3.4.25_pointers.h | 10 ++++++++++
> + 2 files changed, 11 insertions(+)
> +
> +diff --git a/instrumentation/syscalls/3.4.25/arm-32-syscalls-3.4.25
> b/instrumentation/syscalls/3.4.25/arm-32-syscalls-3.4.25
> +index 65c3973..c2e2429 100644
> +--- a/instrumentation/syscalls/3.4.25/arm-32-syscalls-3.4.25
> ++++ b/instrumentation/syscalls/3.4.25/arm-32-syscalls-3.4.25
> +@@ -221,6 +221,7 @@ syscall sys_getpeername nr 287 nbargs 3 types: (int, struct
> sockaddr *, int *) a
> + syscall sys_socketpair nr 288 nbargs 4 types: (int, int, int, int *) args:
> (family, type, protocol, usockvec)
> + syscall sys_send nr 289 nbargs 4 types: (int, void *, size_t, unsigned) args:
> (fd, buff, len, flags)
> + syscall sys_sendto nr 290 nbargs 6 types: (int, void *, size_t, unsigned,
> struct sockaddr *, int) args: (fd, buff, len, flags, addr, addr_len)
> ++syscall sys_recv nr 291 nbargs 4 types: (int, void *, size_t, unsigned int)
> args: (fd, ubuf, size, flags)
> + syscall sys_recvfrom nr 292 nbargs 6 types: (int, void *, size_t, unsigned,
> struct sockaddr *, int *) args: (fd, ubuf, size, flags, addr, addr_len)
> + syscall sys_shutdown nr 293 nbargs 2 types: (int, int) args: (fd, how)
> + syscall sys_setsockopt nr 294 nbargs 5 types: (int, int, int, char *, int)
> args: (fd, level, optname, optval, optlen)
> +diff --git a/instrumentation/syscalls/headers/arm-32-syscalls-3.4.25_pointers.h
> b/instrumentation/syscalls/headers/arm-32-syscalls-3.4.25_pointers.h
> +index 76aab1b..05aae76 100644
> +--- a/instrumentation/syscalls/headers/arm-32-syscalls-3.4.25_pointers.h
> ++++ b/instrumentation/syscalls/headers/arm-32-syscalls-3.4.25_pointers.h
> +@@ -1024,6 +1024,13 @@ SC_LTTNG_TRACEPOINT_EVENT(send,
> + TP_FIELDS(sc_exit(ctf_integer(long, ret, ret)) sc_inout(ctf_integer(int, fd,
> fd)) sc_inout(ctf_integer(void *, buff, buff)) sc_inout(ctf_integer(size_t,
> len, len)) sc_inout(ctf_integer(unsigned, flags, flags)))
> + )
> + #endif
> ++#ifndef OVERRIDE_32_recv
> ++SC_LTTNG_TRACEPOINT_EVENT(recv,
> ++ TP_PROTO(sc_exit(long ret,) int fd, void * ubuf, size_t size, unsigned int
> flags),
> ++ TP_ARGS(sc_exit(ret,) fd, ubuf, size, flags),
> ++ TP_FIELDS(sc_exit(ctf_integer(long, ret, ret)) sc_inout(ctf_integer(int, fd,
> fd)) sc_inout(ctf_integer(void *, ubuf, ubuf)) sc_inout(ctf_integer(size_t,
> size, size)) sc_inout(ctf_integer(unsigned int, flags, flags)))
> ++)
> ++#endif
> + #ifndef OVERRIDE_32_msgsnd
> + SC_LTTNG_TRACEPOINT_EVENT(msgsnd,
> + TP_PROTO(sc_exit(long ret,) int msqid, struct msgbuf * msgp, size_t msgsz,
> int msgflg),
> +@@ -1762,6 +1769,9 @@ TRACE_SYSCALL_TABLE(send, send, 289, 4)
> + #ifndef OVERRIDE_TABLE_32_sendto
> + TRACE_SYSCALL_TABLE(sendto, sendto, 290, 6)
> + #endif
> ++#ifndef OVERRIDE_TABLE_32_recv
> ++TRACE_SYSCALL_TABLE(recv, recv, 291, 4)
> ++#endif
> + #ifndef OVERRIDE_TABLE_32_recvfrom
> + TRACE_SYSCALL_TABLE(recvfrom, recvfrom, 292, 6)
> + #endif
> +--
> +2.7.0.25.gfc10eb5.dirty
> +
> diff --git
> a/package/lttng-modules/0002-Fix-nmi-safe-clock-on-32-bit-systems.patch
> b/package/lttng-modules/0002-Fix-nmi-safe-clock-on-32-bit-systems.patch
> new file mode 100644
> index 0000000..91296b8
> --- /dev/null
> +++ b/package/lttng-modules/0002-Fix-nmi-safe-clock-on-32-bit-systems.patch
> @@ -0,0 +1,188 @@
> +From 412bf9d51784b49c71b3ddabb3fc0e2b89a59a97 Mon Sep 17 00:00:00 2001
> +From: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
> +Date: Thu, 9 Feb 2017 20:51:29 -0500
> +Subject: [PATCH] Fix: nmi-safe clock on 32-bit systems
> +
> +On 32-bit systems, the current algorithm assume to have one clock read
> +per 32-bit LSB overflow period, which is not guaranteed. It also has an
> +issue on the first clock reads after module load, because the initial
> +value for the last LSB is 0. It can cause the time to stay stuck at the
> +same value for a few seconds at the beginning of the trace.
> +
> +It only affects 32-bit systems with kernels >= 3.17.
> +
> +Fix this by using the non-nmi-safe clock source on 32-bit systems,
> +except for x86 systems that support double-word cmpxchg (cmpxchg8b).
> +
> +Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
> +---
> + wrapper/trace-clock.c | 2 +-
> + wrapper/trace-clock.h | 82 +++++++++++++++++++--------------------------------
> + 2 files changed, 31 insertions(+), 53 deletions(-)
> +
> +diff --git a/wrapper/trace-clock.c b/wrapper/trace-clock.c
> +index d9bc956..8c07942 100644
> +--- a/wrapper/trace-clock.c
> ++++ b/wrapper/trace-clock.c
> +@@ -24,7 +24,7 @@
> + #include <wrapper/trace-clock.h>
> +
> + #if (LINUX_VERSION_CODE >= KERNEL_VERSION(3,17,0))
> +-DEFINE_PER_CPU(local_t, lttng_last_tsc);
> ++DEFINE_PER_CPU(struct lttng_last_timestamp, lttng_last_tsc);
> + EXPORT_PER_CPU_SYMBOL(lttng_last_tsc);
> + #endif /* #else #if (LINUX_VERSION_CODE >= KERNEL_VERSION(3,16,0)) */
> +
> +diff --git a/wrapper/trace-clock.h b/wrapper/trace-clock.h
> +index 496fec4..7f76b1b 100644
> +--- a/wrapper/trace-clock.h
> ++++ b/wrapper/trace-clock.h
> +@@ -59,65 +59,43 @@ extern struct lttng_trace_clock *lttng_trace_clock;
> + #define LTTNG_CLOCK_NMI_SAFE_BROKEN
> + #endif
> +
> ++/*
> ++ * We need clock values to be monotonically increasing per-cpu, which is
> ++ * not strictly guaranteed by ktime_get_mono_fast_ns(). It is
> ++ * straightforward to do on architectures with a 64-bit cmpxchg(), but
> ++ * not so on architectures without 64-bit cmpxchg.
> ++ */
> ++
> + #if (LINUX_VERSION_CODE >= KERNEL_VERSION(3,17,0) \
> ++ && (BITS_PER_LONG == 64 || CONFIG_X86_CMPXCHG64) \
> + && !defined(LTTNG_CLOCK_NMI_SAFE_BROKEN))
> ++#define LTTNG_USE_NMI_SAFE_CLOCK
> ++#endif
> +
> +-DECLARE_PER_CPU(local_t, lttng_last_tsc);
> ++#ifdef LTTNG_USE_NMI_SAFE_CLOCK
> +
> +-#if (BITS_PER_LONG == 32)
> + /*
> +- * Fixup "src_now" using the 32 LSB from "last". We need to handle overflow
> and
> +- * underflow of the 32nd bit. "last" can be above, below or equal to the 32
> LSB
> +- * of "src_now".
> ++ * Ensure the variable is aligned on 64-bit for cmpxchg8b on 32-bit
> ++ * systems.
> + */
> +-static inline u64 trace_clock_fixup(u64 src_now, u32 last)
> +-{
> +- u64 now;
> ++struct lttng_last_timestamp {
> ++ u64 v;
> ++} __attribute__((aligned(sizeof(u64))));
> +
> +- now = src_now & 0xFFFFFFFF00000000ULL;
> +- now |= (u64) last;
> +- /* Detect overflow or underflow between now and last. */
> +- if ((src_now & 0x80000000U) && !(last & 0x80000000U)) {
> +- /*
> +- * If 32nd bit transitions from 1 to 0, and we move forward in
> +- * time from "now" to "last", then we have an overflow.
> +- */
> +- if (((s32) now - (s32) last) < 0)
> +- now += 0x0000000100000000ULL;
> +- } else if (!(src_now & 0x80000000U) && (last & 0x80000000U)) {
> +- /*
> +- * If 32nd bit transitions from 0 to 1, and we move backward in
> +- * time from "now" to "last", then we have an underflow.
> +- */
> +- if (((s32) now - (s32) last) > 0)
> +- now -= 0x0000000100000000ULL;
> +- }
> +- return now;
> +-}
> +-#else /* #if (BITS_PER_LONG == 32) */
> +-/*
> +- * The fixup is pretty easy on 64-bit architectures: "last" is a 64-bit
> +- * value, so we can use last directly as current time.
> +- */
> +-static inline u64 trace_clock_fixup(u64 src_now, u64 last)
> +-{
> +- return last;
> +-}
> +-#endif /* #else #if (BITS_PER_LONG == 32) */
> ++DECLARE_PER_CPU(struct lttng_last_timestamp, lttng_last_tsc);
> +
> + /*
> + * Sometimes called with preemption enabled. Can be interrupted.
> + */
> + static inline u64 trace_clock_monotonic_wrapper(void)
> + {
> +- u64 now;
> +- unsigned long last, result;
> +- local_t *last_tsc;
> ++ u64 now, last, result;
> ++ struct lttng_last_timestamp *last_tsc_ptr;
> +
> + /* Use fast nmi-safe monotonic clock provided by the Linux kernel. */
> + preempt_disable();
> +- last_tsc = lttng_this_cpu_ptr(<tng_last_tsc);
> +- last = local_read(last_tsc);
> ++ last_tsc_ptr = lttng_this_cpu_ptr(<tng_last_tsc);
> ++ last = last_tsc_ptr->v;
> + /*
> + * Read "last" before "now". It is not strictly required, but it ensures
> + * that an interrupt coming in won't artificially trigger a case where
> +@@ -126,9 +104,9 @@ static inline u64 trace_clock_monotonic_wrapper(void)
> + */
> + barrier();
> + now = ktime_get_mono_fast_ns();
> +- if (((long) now - (long) last) < 0)
> +- now = trace_clock_fixup(now, last);
> +- result = local_cmpxchg(last_tsc, last, (unsigned long) now);
> ++ if (U64_MAX / 2 < now - last)
> ++ now = last;
> ++ result = cmpxchg64_local(&last_tsc_ptr->v, last, now);
> + preempt_enable();
> + if (result == last) {
> + /* Update done. */
> +@@ -139,11 +117,11 @@ static inline u64 trace_clock_monotonic_wrapper(void)
> + * "result", since it has been sampled concurrently with our
> + * time read, so it should not be far from "now".
> + */
> +- return trace_clock_fixup(now, result);
> ++ return result;
> + }
> + }
> +
> +-#else /* #if (LINUX_VERSION_CODE >= KERNEL_VERSION(3,17,0)) */
> ++#else /* #ifdef LTTNG_USE_NMI_SAFE_CLOCK */
> + static inline u64 trace_clock_monotonic_wrapper(void)
> + {
> + ktime_t ktime;
> +@@ -158,7 +136,7 @@ static inline u64 trace_clock_monotonic_wrapper(void)
> + ktime = ktime_get();
> + return ktime_to_ns(ktime);
> + }
> +-#endif /* #else #if (LINUX_VERSION_CODE >= KERNEL_VERSION(3,17,0)) */
> ++#endif /* #else #ifdef LTTNG_USE_NMI_SAFE_CLOCK */
> +
> + static inline u64 trace_clock_read64_monotonic(void)
> + {
> +@@ -185,19 +163,19 @@ static inline const char
> *trace_clock_description_monotonic(void)
> + return "Monotonic Clock";
> + }
> +
> +-#if (LINUX_VERSION_CODE >= KERNEL_VERSION(3,17,0))
> ++#ifdef LTTNG_USE_NMI_SAFE_CLOCK
> + static inline int get_trace_clock(void)
> + {
> + printk_once(KERN_WARNING "LTTng: Using mainline kernel monotonic fast clock,
> which is NMI-safe.\n");
> + return 0;
> + }
> +-#else /* #if (LINUX_VERSION_CODE >= KERNEL_VERSION(3,17,0)) */
> ++#else /* #ifdef LTTNG_USE_NMI_SAFE_CLOCK */
> + static inline int get_trace_clock(void)
> + {
> + printk_once(KERN_WARNING "LTTng: Using mainline kernel monotonic clock. NMIs
> will not be traced.\n");
> + return 0;
> + }
> +-#endif /* #else #if (LINUX_VERSION_CODE >= KERNEL_VERSION(3,17,0)) */
> ++#endif /* #else #ifdef LTTNG_USE_NMI_SAFE_CLOCK */
> +
> + static inline void put_trace_clock(void)
> + {
> +--
> +2.7.0.25.gfc10eb5.dirty
> +
> diff --git
> a/package/lttng-modules/0003-Fix-trace-clock-wrapper-to-compile-on-ARM.patch
> b/package/lttng-modules/0003-Fix-trace-clock-wrapper-to-compile-on-ARM.patch
> new file mode 100644
> index 0000000..15708b8
> --- /dev/null
> +++ b/package/lttng-modules/0003-Fix-trace-clock-wrapper-to-compile-on-ARM.patch
> @@ -0,0 +1,32 @@
> +From f69bf94310f0da5ed1791b0da4c1420b235d4b8b Mon Sep 17 00:00:00 2001
> +From: Trent Piepho <tpiepho at kymetacorp.com>
> +Date: Fri, 3 Mar 2017 14:25:16 -0800
> +Subject: [PATCH] Fix trace clock wrapper to compile on ARM
> +
> +The X86 only define isn't present. Just assume 64-bit cmpxcng works
> +on all all the other architectures.
> +---
> + wrapper/trace-clock.h | 6 +++++-
> + 1 file changed, 5 insertions(+), 1 deletion(-)
> +
> +diff --git a/wrapper/trace-clock.h b/wrapper/trace-clock.h
> +index 7f76b1b..066d5e6 100644
> +--- a/wrapper/trace-clock.h
> ++++ b/wrapper/trace-clock.h
> +@@ -66,8 +66,12 @@ extern struct lttng_trace_clock *lttng_trace_clock;
> + * not so on architectures without 64-bit cmpxchg.
> + */
> +
> ++#if defined(ARCH_X86) && !defined(CONFIG_X86_CMPXCHG64)
> ++#define LTTNG_NO_CMPXCHG64
> ++#endif
> ++
> + #if (LINUX_VERSION_CODE >= KERNEL_VERSION(3,17,0) \
> +- && (BITS_PER_LONG == 64 || CONFIG_X86_CMPXCHG64) \
> ++ && !defined(LTTNG_NO_CMPXCHG64) \
> + && !defined(LTTNG_CLOCK_NMI_SAFE_BROKEN))
> + #define LTTNG_USE_NMI_SAFE_CLOCK
> + #endif
> +--
> +2.7.0.25.gfc10eb5.dirty
> +
> --
> 2.7.0.25.gfc10eb5.dirty
>
> _______________________________________________
> 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
More information about the lttng-dev
mailing list