* [RFC PATCH for 4.17 18/21] rseq: selftests: Provide basic test
From: Mathieu Desnoyers @ 2018-03-27 16:05 UTC (permalink / raw)
To: Peter Zijlstra, Paul E . McKenney, Boqun Feng, Andy Lutomirski,
Dave Watson
Cc: linux-kernel, linux-api, Paul Turner, Andrew Morton, Russell King,
Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Andrew Hunter,
Andi Kleen, Chris Lameter, Ben Maurer, Steven Rostedt,
Josh Triplett, Linus Torvalds, Catalin Marinas, Will Deacon,
Michael Kerrisk, Mathieu Desnoyers, Shuah Khan, linux-kselftest
In-Reply-To: <20180327160542.28457-1-mathieu.desnoyers@efficios.com>
"basic_test" only asserts that RSEQ works moderately correctly. E.g.
that the CPUID pointer works.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: Shuah Khan <shuahkh@osg.samsung.com>
CC: Russell King <linux@arm.linux.org.uk>
CC: Catalin Marinas <catalin.marinas@arm.com>
CC: Will Deacon <will.deacon@arm.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Paul Turner <pjt@google.com>
CC: Andrew Hunter <ahh@google.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Andi Kleen <andi@firstfloor.org>
CC: Dave Watson <davejwatson@fb.com>
CC: Chris Lameter <cl@linux.com>
CC: Ingo Molnar <mingo@redhat.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Ben Maurer <bmaurer@fb.com>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: Josh Triplett <josh@joshtriplett.org>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Boqun Feng <boqun.feng@gmail.com>
CC: linux-kselftest@vger.kernel.org
CC: linux-api@vger.kernel.org
---
tools/testing/selftests/rseq/basic_test.c | 55 +++++++++++++++++++++++++++++++
1 file changed, 55 insertions(+)
create mode 100644 tools/testing/selftests/rseq/basic_test.c
diff --git a/tools/testing/selftests/rseq/basic_test.c b/tools/testing/selftests/rseq/basic_test.c
new file mode 100644
index 000000000000..e2086b3885d7
--- /dev/null
+++ b/tools/testing/selftests/rseq/basic_test.c
@@ -0,0 +1,55 @@
+/*
+ * Basic test coverage for critical regions and rseq_current_cpu().
+ */
+
+#define _GNU_SOURCE
+#include <assert.h>
+#include <sched.h>
+#include <signal.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/time.h>
+
+#include "rseq.h"
+
+void test_cpu_pointer(void)
+{
+ cpu_set_t affinity, test_affinity;
+ int i;
+
+ sched_getaffinity(0, sizeof(affinity), &affinity);
+ CPU_ZERO(&test_affinity);
+ for (i = 0; i < CPU_SETSIZE; i++) {
+ if (CPU_ISSET(i, &affinity)) {
+ CPU_SET(i, &test_affinity);
+ sched_setaffinity(0, sizeof(test_affinity),
+ &test_affinity);
+ assert(sched_getcpu() == i);
+ assert(rseq_current_cpu() == i);
+ assert(rseq_current_cpu_raw() == i);
+ assert(rseq_cpu_start() == i);
+ CPU_CLR(i, &test_affinity);
+ }
+ }
+ sched_setaffinity(0, sizeof(affinity), &affinity);
+}
+
+int main(int argc, char **argv)
+{
+ if (rseq_register_current_thread()) {
+ fprintf(stderr, "Error: rseq_register_current_thread(...) failed(%d): %s\n",
+ errno, strerror(errno));
+ goto init_thread_error;
+ }
+ printf("testing current cpu\n");
+ test_cpu_pointer();
+ if (rseq_unregister_current_thread()) {
+ fprintf(stderr, "Error: rseq_unregister_current_thread(...) failed(%d): %s\n",
+ errno, strerror(errno));
+ goto init_thread_error;
+ }
+ return 0;
+
+init_thread_error:
+ return -1;
+}
--
2.11.0
^ permalink raw reply related
* [RFC PATCH for 4.17 19/21] rseq: selftests: Provide basic percpu ops test
From: Mathieu Desnoyers @ 2018-03-27 16:05 UTC (permalink / raw)
To: Peter Zijlstra, Paul E . McKenney, Boqun Feng, Andy Lutomirski,
Dave Watson
Cc: linux-kernel, linux-api, Paul Turner, Andrew Morton, Russell King,
Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Andrew Hunter,
Andi Kleen, Chris Lameter, Ben Maurer, Steven Rostedt,
Josh Triplett, Linus Torvalds, Catalin Marinas, Will Deacon,
Michael Kerrisk, Mathieu Desnoyers, Shuah Khan, linux-kselftest
In-Reply-To: <20180327160542.28457-1-mathieu.desnoyers@efficios.com>
"basic_percpu_ops_test" is a slightly more "realistic" variant,
implementing a few simple per-cpu operations and testing their
correctness.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: Shuah Khan <shuahkh@osg.samsung.com>
CC: Russell King <linux@arm.linux.org.uk>
CC: Catalin Marinas <catalin.marinas@arm.com>
CC: Will Deacon <will.deacon@arm.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Paul Turner <pjt@google.com>
CC: Andrew Hunter <ahh@google.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Andi Kleen <andi@firstfloor.org>
CC: Dave Watson <davejwatson@fb.com>
CC: Chris Lameter <cl@linux.com>
CC: Ingo Molnar <mingo@redhat.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Ben Maurer <bmaurer@fb.com>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: Josh Triplett <josh@joshtriplett.org>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Boqun Feng <boqun.feng@gmail.com>
CC: linux-kselftest@vger.kernel.org
CC: linux-api@vger.kernel.org
---
.../testing/selftests/rseq/basic_percpu_ops_test.c | 296 +++++++++++++++++++++
1 file changed, 296 insertions(+)
create mode 100644 tools/testing/selftests/rseq/basic_percpu_ops_test.c
diff --git a/tools/testing/selftests/rseq/basic_percpu_ops_test.c b/tools/testing/selftests/rseq/basic_percpu_ops_test.c
new file mode 100644
index 000000000000..e585bba0bf8d
--- /dev/null
+++ b/tools/testing/selftests/rseq/basic_percpu_ops_test.c
@@ -0,0 +1,296 @@
+#define _GNU_SOURCE
+#include <assert.h>
+#include <pthread.h>
+#include <sched.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <stddef.h>
+
+#include "percpu-op.h"
+
+#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
+
+struct percpu_lock_entry {
+ intptr_t v;
+} __attribute__((aligned(128)));
+
+struct percpu_lock {
+ struct percpu_lock_entry c[CPU_SETSIZE];
+};
+
+struct test_data_entry {
+ intptr_t count;
+} __attribute__((aligned(128)));
+
+struct spinlock_test_data {
+ struct percpu_lock lock;
+ struct test_data_entry c[CPU_SETSIZE];
+ int reps;
+};
+
+struct percpu_list_node {
+ intptr_t data;
+ struct percpu_list_node *next;
+};
+
+struct percpu_list_entry {
+ struct percpu_list_node *head;
+} __attribute__((aligned(128)));
+
+struct percpu_list {
+ struct percpu_list_entry c[CPU_SETSIZE];
+};
+
+/* A simple percpu spinlock. Returns the cpu lock was acquired on. */
+int rseq_percpu_lock(struct percpu_lock *lock)
+{
+ int cpu;
+
+ for (;;) {
+ int ret;
+
+ cpu = rseq_cpu_start();
+ ret = percpu_cmpeqv_storev(&lock->c[cpu].v,
+ 0, 1, cpu);
+ if (rseq_likely(!ret))
+ break;
+ if (rseq_unlikely(ret < 0)) {
+ perror("cpu_opv");
+ abort();
+ }
+ /* Retry if comparison fails. */
+ }
+ /*
+ * Acquire semantic when taking lock after control dependency.
+ * Matches rseq_smp_store_release().
+ */
+ rseq_smp_acquire__after_ctrl_dep();
+ return cpu;
+}
+
+void rseq_percpu_unlock(struct percpu_lock *lock, int cpu)
+{
+ assert(lock->c[cpu].v == 1);
+ /*
+ * Release lock, with release semantic. Matches
+ * rseq_smp_acquire__after_ctrl_dep().
+ */
+ rseq_smp_store_release(&lock->c[cpu].v, 0);
+}
+
+void *test_percpu_spinlock_thread(void *arg)
+{
+ struct spinlock_test_data *data = arg;
+ int i, cpu;
+
+ if (rseq_register_current_thread()) {
+ fprintf(stderr, "Error: rseq_register_current_thread(...) failed(%d): %s\n",
+ errno, strerror(errno));
+ abort();
+ }
+ for (i = 0; i < data->reps; i++) {
+ cpu = rseq_percpu_lock(&data->lock);
+ data->c[cpu].count++;
+ rseq_percpu_unlock(&data->lock, cpu);
+ }
+ if (rseq_unregister_current_thread()) {
+ fprintf(stderr, "Error: rseq_unregister_current_thread(...) failed(%d): %s\n",
+ errno, strerror(errno));
+ abort();
+ }
+
+ return NULL;
+}
+
+/*
+ * A simple test which implements a sharded counter using a per-cpu
+ * lock. Obviously real applications might prefer to simply use a
+ * per-cpu increment; however, this is reasonable for a test and the
+ * lock can be extended to synchronize more complicated operations.
+ */
+void test_percpu_spinlock(void)
+{
+ const int num_threads = 200;
+ int i;
+ uint64_t sum;
+ pthread_t test_threads[num_threads];
+ struct spinlock_test_data data;
+
+ memset(&data, 0, sizeof(data));
+ data.reps = 5000;
+
+ for (i = 0; i < num_threads; i++)
+ pthread_create(&test_threads[i], NULL,
+ test_percpu_spinlock_thread, &data);
+
+ for (i = 0; i < num_threads; i++)
+ pthread_join(test_threads[i], NULL);
+
+ sum = 0;
+ for (i = 0; i < CPU_SETSIZE; i++)
+ sum += data.c[i].count;
+
+ assert(sum == (uint64_t)data.reps * num_threads);
+}
+
+int percpu_list_push(struct percpu_list *list, struct percpu_list_node *node,
+ int cpu)
+{
+ for (;;) {
+ intptr_t *targetptr, newval, expect;
+ int ret;
+
+ /* Load list->c[cpu].head with single-copy atomicity. */
+ expect = (intptr_t)RSEQ_READ_ONCE(list->c[cpu].head);
+ newval = (intptr_t)node;
+ targetptr = (intptr_t *)&list->c[cpu].head;
+ node->next = (struct percpu_list_node *)expect;
+ ret = percpu_cmpeqv_storev(targetptr, expect, newval, cpu);
+ if (rseq_likely(!ret))
+ break;
+ if (rseq_unlikely(ret < 0)) {
+ perror("cpu_opv");
+ abort();
+ }
+ /* Retry if comparison fails. */
+ }
+ return cpu;
+}
+
+/*
+ * Unlike a traditional lock-less linked list; the availability of a
+ * rseq primitive allows us to implement pop without concerns over
+ * ABA-type races.
+ */
+struct percpu_list_node *percpu_list_pop(struct percpu_list *list,
+ int cpu)
+{
+ struct percpu_list_node *head;
+ intptr_t *targetptr, expectnot, *load;
+ off_t offset;
+ int ret;
+
+ targetptr = (intptr_t *)&list->c[cpu].head;
+ expectnot = (intptr_t)NULL;
+ offset = offsetof(struct percpu_list_node, next);
+ load = (intptr_t *)&head;
+ ret = percpu_cmpnev_storeoffp_load(targetptr, expectnot,
+ offset, load, cpu);
+ if (rseq_unlikely(ret < 0)) {
+ perror("cpu_opv");
+ abort();
+ }
+ if (ret > 0)
+ return NULL;
+ return head;
+}
+
+void *test_percpu_list_thread(void *arg)
+{
+ int i;
+ struct percpu_list *list = (struct percpu_list *)arg;
+
+ if (rseq_register_current_thread()) {
+ fprintf(stderr, "Error: rseq_register_current_thread(...) failed(%d): %s\n",
+ errno, strerror(errno));
+ abort();
+ }
+
+ for (i = 0; i < 100000; i++) {
+ struct percpu_list_node *node;
+
+ node = percpu_list_pop(list, rseq_cpu_start());
+ sched_yield(); /* encourage shuffling */
+ if (node)
+ percpu_list_push(list, node, rseq_cpu_start());
+ }
+
+ if (rseq_unregister_current_thread()) {
+ fprintf(stderr, "Error: rseq_unregister_current_thread(...) failed(%d): %s\n",
+ errno, strerror(errno));
+ abort();
+ }
+
+ return NULL;
+}
+
+/* Simultaneous modification to a per-cpu linked list from many threads. */
+void test_percpu_list(void)
+{
+ int i, j;
+ uint64_t sum = 0, expected_sum = 0;
+ struct percpu_list list;
+ pthread_t test_threads[200];
+ cpu_set_t allowed_cpus;
+
+ memset(&list, 0, sizeof(list));
+
+ /* Generate list entries for every usable cpu. */
+ sched_getaffinity(0, sizeof(allowed_cpus), &allowed_cpus);
+ for (i = 0; i < CPU_SETSIZE; i++) {
+ if (!CPU_ISSET(i, &allowed_cpus))
+ continue;
+ for (j = 1; j <= 100; j++) {
+ struct percpu_list_node *node;
+
+ expected_sum += j;
+
+ node = malloc(sizeof(*node));
+ assert(node);
+ node->data = j;
+ node->next = list.c[i].head;
+ list.c[i].head = node;
+ }
+ }
+
+ for (i = 0; i < 200; i++)
+ pthread_create(&test_threads[i], NULL,
+ test_percpu_list_thread, &list);
+
+ for (i = 0; i < 200; i++)
+ pthread_join(test_threads[i], NULL);
+
+ for (i = 0; i < CPU_SETSIZE; i++) {
+ struct percpu_list_node *node;
+
+ if (!CPU_ISSET(i, &allowed_cpus))
+ continue;
+
+ while ((node = percpu_list_pop(&list, i))) {
+ sum += node->data;
+ free(node);
+ }
+ }
+
+ /*
+ * All entries should now be accounted for (unless some external
+ * actor is interfering with our allowed affinity while this
+ * test is running).
+ */
+ assert(sum == expected_sum);
+}
+
+int main(int argc, char **argv)
+{
+ if (rseq_register_current_thread()) {
+ fprintf(stderr, "Error: rseq_register_current_thread(...) failed(%d): %s\n",
+ errno, strerror(errno));
+ goto error;
+ }
+ printf("spinlock\n");
+ test_percpu_spinlock();
+ printf("percpu_list\n");
+ test_percpu_list();
+ if (rseq_unregister_current_thread()) {
+ fprintf(stderr, "Error: rseq_unregister_current_thread(...) failed(%d): %s\n",
+ errno, strerror(errno));
+ goto error;
+ }
+ return 0;
+
+error:
+ return -1;
+}
+
--
2.11.0
^ permalink raw reply related
* [RFC PATCH for 4.17 20/21] rseq: selftests: Provide parametrized tests
From: Mathieu Desnoyers @ 2018-03-27 16:05 UTC (permalink / raw)
To: Peter Zijlstra, Paul E . McKenney, Boqun Feng, Andy Lutomirski,
Dave Watson
Cc: linux-kernel, linux-api, Paul Turner, Andrew Morton, Russell King,
Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Andrew Hunter,
Andi Kleen, Chris Lameter, Ben Maurer, Steven Rostedt,
Josh Triplett, Linus Torvalds, Catalin Marinas, Will Deacon,
Michael Kerrisk, Mathieu Desnoyers, Shuah Khan, linux-kselftest
In-Reply-To: <20180327160542.28457-1-mathieu.desnoyers@efficios.com>
"param_test" is a parametrizable restartable sequences test. See
the "--help" output for usage.
"param_test_benchmark" is the same as "param_test", but it removes
testing book-keeping code to allow accurate benchmarks.
"param_test_skip_fastpath" is the same as "param_test", but it skips
the rseq fast-path, and only calls the cpu_opv slow path.
"param_test_compare_twice" is the same as "param_test", but it performs
each comparison within rseq critical section twice, thus validating
invariants. If any of the second comparisons fails, an error message
is printed and the test aborts.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: Shuah Khan <shuahkh@osg.samsung.com>
CC: Russell King <linux@arm.linux.org.uk>
CC: Catalin Marinas <catalin.marinas@arm.com>
CC: Will Deacon <will.deacon@arm.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Paul Turner <pjt@google.com>
CC: Andrew Hunter <ahh@google.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Andi Kleen <andi@firstfloor.org>
CC: Dave Watson <davejwatson@fb.com>
CC: Chris Lameter <cl@linux.com>
CC: Ingo Molnar <mingo@redhat.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Ben Maurer <bmaurer@fb.com>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: Josh Triplett <josh@joshtriplett.org>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Boqun Feng <boqun.feng@gmail.com>
CC: linux-kselftest@vger.kernel.org
CC: linux-api@vger.kernel.org
---
tools/testing/selftests/rseq/param_test.c | 1163 +++++++++++++++++++++++++++++
1 file changed, 1163 insertions(+)
create mode 100644 tools/testing/selftests/rseq/param_test.c
diff --git a/tools/testing/selftests/rseq/param_test.c b/tools/testing/selftests/rseq/param_test.c
new file mode 100644
index 000000000000..0a7c05f506be
--- /dev/null
+++ b/tools/testing/selftests/rseq/param_test.c
@@ -0,0 +1,1163 @@
+#define _GNU_SOURCE
+#include <assert.h>
+#include <pthread.h>
+#include <sched.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <syscall.h>
+#include <unistd.h>
+#include <poll.h>
+#include <sys/types.h>
+#include <signal.h>
+#include <errno.h>
+#include <stddef.h>
+
+static inline pid_t gettid(void)
+{
+ return syscall(__NR_gettid);
+}
+
+#define NR_INJECT 9
+static int loop_cnt[NR_INJECT + 1];
+
+static int loop_cnt_1 asm("asm_loop_cnt_1") __attribute__((used));
+static int loop_cnt_2 asm("asm_loop_cnt_2") __attribute__((used));
+static int loop_cnt_3 asm("asm_loop_cnt_3") __attribute__((used));
+static int loop_cnt_4 asm("asm_loop_cnt_4") __attribute__((used));
+static int loop_cnt_5 asm("asm_loop_cnt_5") __attribute__((used));
+static int loop_cnt_6 asm("asm_loop_cnt_6") __attribute__((used));
+
+static int opt_modulo, verbose;
+
+static int opt_yield, opt_signal, opt_sleep,
+ opt_disable_rseq, opt_threads = 200,
+ opt_disable_mod = 0, opt_test = 's', opt_mb = 0;
+
+static long long opt_reps = 5000;
+
+static __thread __attribute__((tls_model("initial-exec")))
+unsigned int signals_delivered;
+
+#ifndef BENCHMARK
+
+static __thread __attribute__((tls_model("initial-exec"), unused))
+unsigned int yield_mod_cnt, nr_abort;
+
+#define printf_verbose(fmt, ...) \
+ do { \
+ if (verbose) \
+ printf(fmt, ## __VA_ARGS__); \
+ } while (0)
+
+#if defined(__x86_64__) || defined(__i386__)
+
+#define INJECT_ASM_REG "eax"
+
+#define RSEQ_INJECT_CLOBBER \
+ , INJECT_ASM_REG
+
+#define RSEQ_INJECT_ASM(n) \
+ "mov asm_loop_cnt_" #n ", %%" INJECT_ASM_REG "\n\t" \
+ "test %%" INJECT_ASM_REG ",%%" INJECT_ASM_REG "\n\t" \
+ "jz 333f\n\t" \
+ "222:\n\t" \
+ "dec %%" INJECT_ASM_REG "\n\t" \
+ "jnz 222b\n\t" \
+ "333:\n\t"
+
+#elif defined(__ARMEL__)
+
+#define RSEQ_INJECT_INPUT \
+ , [loop_cnt_1]"m"(loop_cnt[1]) \
+ , [loop_cnt_2]"m"(loop_cnt[2]) \
+ , [loop_cnt_3]"m"(loop_cnt[3]) \
+ , [loop_cnt_4]"m"(loop_cnt[4]) \
+ , [loop_cnt_5]"m"(loop_cnt[5]) \
+ , [loop_cnt_6]"m"(loop_cnt[6])
+
+#define INJECT_ASM_REG "r4"
+
+#define RSEQ_INJECT_CLOBBER \
+ , INJECT_ASM_REG
+
+#define RSEQ_INJECT_ASM(n) \
+ "ldr " INJECT_ASM_REG ", %[loop_cnt_" #n "]\n\t" \
+ "cmp " INJECT_ASM_REG ", #0\n\t" \
+ "beq 333f\n\t" \
+ "222:\n\t" \
+ "subs " INJECT_ASM_REG ", #1\n\t" \
+ "bne 222b\n\t" \
+ "333:\n\t"
+
+#elif __PPC__
+
+#define RSEQ_INJECT_INPUT \
+ , [loop_cnt_1]"m"(loop_cnt[1]) \
+ , [loop_cnt_2]"m"(loop_cnt[2]) \
+ , [loop_cnt_3]"m"(loop_cnt[3]) \
+ , [loop_cnt_4]"m"(loop_cnt[4]) \
+ , [loop_cnt_5]"m"(loop_cnt[5]) \
+ , [loop_cnt_6]"m"(loop_cnt[6])
+
+#define INJECT_ASM_REG "r18"
+
+#define RSEQ_INJECT_CLOBBER \
+ , INJECT_ASM_REG
+
+#define RSEQ_INJECT_ASM(n) \
+ "lwz %%" INJECT_ASM_REG ", %[loop_cnt_" #n "]\n\t" \
+ "cmpwi %%" INJECT_ASM_REG ", 0\n\t" \
+ "beq 333f\n\t" \
+ "222:\n\t" \
+ "subic. %%" INJECT_ASM_REG ", %%" INJECT_ASM_REG ", 1\n\t" \
+ "bne 222b\n\t" \
+ "333:\n\t"
+#else
+#error unsupported target
+#endif
+
+#define RSEQ_INJECT_FAILED \
+ nr_abort++;
+
+#define RSEQ_INJECT_C(n) \
+{ \
+ int loc_i, loc_nr_loops = loop_cnt[n]; \
+ \
+ for (loc_i = 0; loc_i < loc_nr_loops; loc_i++) { \
+ rseq_barrier(); \
+ } \
+ if (loc_nr_loops == -1 && opt_modulo) { \
+ if (yield_mod_cnt == opt_modulo - 1) { \
+ if (opt_sleep > 0) \
+ poll(NULL, 0, opt_sleep); \
+ if (opt_yield) \
+ sched_yield(); \
+ if (opt_signal) \
+ raise(SIGUSR1); \
+ yield_mod_cnt = 0; \
+ } else { \
+ yield_mod_cnt++; \
+ } \
+ } \
+}
+
+#else
+
+#define printf_verbose(fmt, ...)
+
+#endif /* BENCHMARK */
+
+#include "percpu-op.h"
+
+struct percpu_lock_entry {
+ intptr_t v;
+} __attribute__((aligned(128)));
+
+struct percpu_lock {
+ struct percpu_lock_entry c[CPU_SETSIZE];
+};
+
+struct test_data_entry {
+ intptr_t count;
+} __attribute__((aligned(128)));
+
+struct spinlock_test_data {
+ struct percpu_lock lock;
+ struct test_data_entry c[CPU_SETSIZE];
+};
+
+struct spinlock_thread_test_data {
+ struct spinlock_test_data *data;
+ long long reps;
+ int reg;
+};
+
+struct inc_test_data {
+ struct test_data_entry c[CPU_SETSIZE];
+};
+
+struct inc_thread_test_data {
+ struct inc_test_data *data;
+ long long reps;
+ int reg;
+};
+
+struct percpu_list_node {
+ intptr_t data;
+ struct percpu_list_node *next;
+};
+
+struct percpu_list_entry {
+ struct percpu_list_node *head;
+} __attribute__((aligned(128)));
+
+struct percpu_list {
+ struct percpu_list_entry c[CPU_SETSIZE];
+};
+
+#define BUFFER_ITEM_PER_CPU 100
+
+struct percpu_buffer_node {
+ intptr_t data;
+};
+
+struct percpu_buffer_entry {
+ intptr_t offset;
+ intptr_t buflen;
+ struct percpu_buffer_node **array;
+} __attribute__((aligned(128)));
+
+struct percpu_buffer {
+ struct percpu_buffer_entry c[CPU_SETSIZE];
+};
+
+#define MEMCPY_BUFFER_ITEM_PER_CPU 100
+
+struct percpu_memcpy_buffer_node {
+ intptr_t data1;
+ uint64_t data2;
+};
+
+struct percpu_memcpy_buffer_entry {
+ intptr_t offset;
+ intptr_t buflen;
+ struct percpu_memcpy_buffer_node *array;
+} __attribute__((aligned(128)));
+
+struct percpu_memcpy_buffer {
+ struct percpu_memcpy_buffer_entry c[CPU_SETSIZE];
+};
+
+/* A simple percpu spinlock. */
+static void rseq_percpu_lock(struct percpu_lock *lock, int cpu)
+{
+ for (;;) {
+ int ret;
+
+ ret = percpu_cmpeqv_storev(&lock->c[cpu].v,
+ 0, 1, cpu);
+ if (rseq_likely(!ret))
+ break;
+ if (rseq_unlikely(ret < 0)) {
+ perror("cpu_opv");
+ abort();
+ }
+ /* Retry if comparison fails. */
+ }
+ /*
+ * Acquire semantic when taking lock after control dependency.
+ * Matches rseq_smp_store_release().
+ */
+ rseq_smp_acquire__after_ctrl_dep();
+}
+
+static void rseq_percpu_unlock(struct percpu_lock *lock, int cpu)
+{
+ assert(lock->c[cpu].v == 1);
+ /*
+ * Release lock, with release semantic. Matches
+ * rseq_smp_acquire__after_ctrl_dep().
+ */
+ rseq_smp_store_release(&lock->c[cpu].v, 0);
+}
+
+void *test_percpu_spinlock_thread(void *arg)
+{
+ struct spinlock_thread_test_data *thread_data = arg;
+ struct spinlock_test_data *data = thread_data->data;
+ long long i, reps;
+
+ if (!opt_disable_rseq && thread_data->reg &&
+ rseq_register_current_thread())
+ abort();
+ reps = thread_data->reps;
+ for (i = 0; i < reps; i++) {
+ int cpu = rseq_cpu_start();
+
+ rseq_percpu_lock(&data->lock, cpu);
+ data->c[cpu].count++;
+ rseq_percpu_unlock(&data->lock, cpu);
+#ifndef BENCHMARK
+ if (i != 0 && !(i % (reps / 10)))
+ printf_verbose("tid %d: count %lld\n", (int) gettid(), i);
+#endif
+ }
+ printf_verbose("tid %d: number of rseq abort: %d, signals delivered: %u\n",
+ (int) gettid(), nr_abort, signals_delivered);
+ if (!opt_disable_rseq && thread_data->reg &&
+ rseq_unregister_current_thread())
+ abort();
+ return NULL;
+}
+
+/*
+ * A simple test which implements a sharded counter using a per-cpu
+ * lock. Obviously real applications might prefer to simply use a
+ * per-cpu increment; however, this is reasonable for a test and the
+ * lock can be extended to synchronize more complicated operations.
+ */
+void test_percpu_spinlock(void)
+{
+ const int num_threads = opt_threads;
+ int i, ret;
+ uint64_t sum;
+ pthread_t test_threads[num_threads];
+ struct spinlock_test_data data;
+ struct spinlock_thread_test_data thread_data[num_threads];
+
+ memset(&data, 0, sizeof(data));
+ for (i = 0; i < num_threads; i++) {
+ thread_data[i].reps = opt_reps;
+ if (opt_disable_mod <= 0 || (i % opt_disable_mod))
+ thread_data[i].reg = 1;
+ else
+ thread_data[i].reg = 0;
+ thread_data[i].data = &data;
+ ret = pthread_create(&test_threads[i], NULL,
+ test_percpu_spinlock_thread,
+ &thread_data[i]);
+ if (ret) {
+ errno = ret;
+ perror("pthread_create");
+ abort();
+ }
+ }
+
+ for (i = 0; i < num_threads; i++) {
+ ret = pthread_join(test_threads[i], NULL);
+ if (ret) {
+ errno = ret;
+ perror("pthread_join");
+ abort();
+ }
+ }
+
+ sum = 0;
+ for (i = 0; i < CPU_SETSIZE; i++)
+ sum += data.c[i].count;
+
+ assert(sum == (uint64_t)opt_reps * num_threads);
+}
+
+void *test_percpu_inc_thread(void *arg)
+{
+ struct inc_thread_test_data *thread_data = arg;
+ struct inc_test_data *data = thread_data->data;
+ long long i, reps;
+
+ if (!opt_disable_rseq && thread_data->reg &&
+ rseq_register_current_thread())
+ abort();
+ reps = thread_data->reps;
+ for (i = 0; i < reps; i++) {
+ int cpu, ret;
+
+ cpu = rseq_cpu_start();
+ ret = percpu_addv(&data->c[cpu].count, 1, cpu);
+ if (rseq_unlikely(ret)) {
+ perror("cpu_opv");
+ abort();
+ }
+#ifndef BENCHMARK
+ if (i != 0 && !(i % (reps / 10)))
+ printf_verbose("tid %d: count %lld\n", (int) gettid(), i);
+#endif
+ }
+ printf_verbose("tid %d: number of rseq abort: %d, signals delivered: %u\n",
+ (int) gettid(), nr_abort, signals_delivered);
+ if (!opt_disable_rseq && thread_data->reg &&
+ rseq_unregister_current_thread())
+ abort();
+ return NULL;
+}
+
+void test_percpu_inc(void)
+{
+ const int num_threads = opt_threads;
+ int i, ret;
+ uint64_t sum;
+ pthread_t test_threads[num_threads];
+ struct inc_test_data data;
+ struct inc_thread_test_data thread_data[num_threads];
+
+ memset(&data, 0, sizeof(data));
+ for (i = 0; i < num_threads; i++) {
+ thread_data[i].reps = opt_reps;
+ if (opt_disable_mod <= 0 || (i % opt_disable_mod))
+ thread_data[i].reg = 1;
+ else
+ thread_data[i].reg = 0;
+ thread_data[i].data = &data;
+ ret = pthread_create(&test_threads[i], NULL,
+ test_percpu_inc_thread,
+ &thread_data[i]);
+ if (ret) {
+ errno = ret;
+ perror("pthread_create");
+ abort();
+ }
+ }
+
+ for (i = 0; i < num_threads; i++) {
+ ret = pthread_join(test_threads[i], NULL);
+ if (ret) {
+ errno = ret;
+ perror("pthread_join");
+ abort();
+ }
+ }
+
+ sum = 0;
+ for (i = 0; i < CPU_SETSIZE; i++)
+ sum += data.c[i].count;
+
+ assert(sum == (uint64_t)opt_reps * num_threads);
+}
+
+void percpu_list_push(struct percpu_list *list,
+ struct percpu_list_node *node,
+ int cpu)
+{
+ for (;;) {
+ intptr_t *targetptr, newval, expect;
+ int ret;
+
+ /* Load list->c[cpu].head with single-copy atomicity. */
+ expect = (intptr_t)RSEQ_READ_ONCE(list->c[cpu].head);
+ newval = (intptr_t)node;
+ targetptr = (intptr_t *)&list->c[cpu].head;
+ node->next = (struct percpu_list_node *)expect;
+ ret = percpu_cmpeqv_storev(targetptr, expect, newval, cpu);
+ if (rseq_likely(!ret))
+ break;
+ if (rseq_unlikely(ret < 0)) {
+ perror("cpu_opv");
+ abort();
+ }
+ /* Retry if comparison fails. */
+ }
+}
+
+/*
+ * Unlike a traditional lock-less linked list; the availability of a
+ * rseq primitive allows us to implement pop without concerns over
+ * ABA-type races.
+ */
+struct percpu_list_node *percpu_list_pop(struct percpu_list *list,
+ int cpu)
+{
+ struct percpu_list_node *head;
+ intptr_t *targetptr, expectnot, *load;
+ off_t offset;
+ int ret;
+
+ targetptr = (intptr_t *)&list->c[cpu].head;
+ expectnot = (intptr_t)NULL;
+ offset = offsetof(struct percpu_list_node, next);
+ load = (intptr_t *)&head;
+ ret = percpu_cmpnev_storeoffp_load(targetptr, expectnot,
+ offset, load, cpu);
+ if (rseq_unlikely(ret < 0)) {
+ perror("cpu_opv");
+ abort();
+ }
+ if (ret > 0)
+ return NULL;
+ return head;
+}
+
+void *test_percpu_list_thread(void *arg)
+{
+ long long i, reps;
+ struct percpu_list *list = (struct percpu_list *)arg;
+
+ if (!opt_disable_rseq && rseq_register_current_thread())
+ abort();
+
+ reps = opt_reps;
+ for (i = 0; i < reps; i++) {
+ struct percpu_list_node *node;
+
+ node = percpu_list_pop(list, rseq_cpu_start());
+ if (opt_yield)
+ sched_yield(); /* encourage shuffling */
+ if (node)
+ percpu_list_push(list, node, rseq_cpu_start());
+ }
+
+ printf_verbose("tid %d: number of rseq abort: %d, signals delivered: %u\n",
+ (int) gettid(), nr_abort, signals_delivered);
+ if (!opt_disable_rseq && rseq_unregister_current_thread())
+ abort();
+
+ return NULL;
+}
+
+/* Simultaneous modification to a per-cpu linked list from many threads. */
+void test_percpu_list(void)
+{
+ const int num_threads = opt_threads;
+ int i, j, ret;
+ uint64_t sum = 0, expected_sum = 0;
+ struct percpu_list list;
+ pthread_t test_threads[num_threads];
+ cpu_set_t allowed_cpus;
+
+ memset(&list, 0, sizeof(list));
+
+ /* Generate list entries for every usable cpu. */
+ sched_getaffinity(0, sizeof(allowed_cpus), &allowed_cpus);
+ for (i = 0; i < CPU_SETSIZE; i++) {
+ if (!CPU_ISSET(i, &allowed_cpus))
+ continue;
+ for (j = 1; j <= 100; j++) {
+ struct percpu_list_node *node;
+
+ expected_sum += j;
+
+ node = malloc(sizeof(*node));
+ assert(node);
+ node->data = j;
+ node->next = list.c[i].head;
+ list.c[i].head = node;
+ }
+ }
+
+ for (i = 0; i < num_threads; i++) {
+ ret = pthread_create(&test_threads[i], NULL,
+ test_percpu_list_thread, &list);
+ if (ret) {
+ errno = ret;
+ perror("pthread_create");
+ abort();
+ }
+ }
+
+ for (i = 0; i < num_threads; i++) {
+ ret = pthread_join(test_threads[i], NULL);
+ if (ret) {
+ errno = ret;
+ perror("pthread_join");
+ abort();
+ }
+ }
+
+ for (i = 0; i < CPU_SETSIZE; i++) {
+ struct percpu_list_node *node;
+
+ if (!CPU_ISSET(i, &allowed_cpus))
+ continue;
+
+ while ((node = percpu_list_pop(&list, i))) {
+ sum += node->data;
+ free(node);
+ }
+ }
+
+ /*
+ * All entries should now be accounted for (unless some external
+ * actor is interfering with our allowed affinity while this
+ * test is running).
+ */
+ assert(sum == expected_sum);
+}
+
+bool percpu_buffer_push(struct percpu_buffer *buffer,
+ struct percpu_buffer_node *node,
+ int cpu)
+{
+ for (;;) {
+ intptr_t *targetptr_spec, newval_spec;
+ intptr_t *targetptr_final, newval_final;
+ intptr_t offset;
+ int ret;
+
+ offset = RSEQ_READ_ONCE(buffer->c[cpu].offset);
+ if (offset == buffer->c[cpu].buflen)
+ return false;
+ newval_spec = (intptr_t)node;
+ targetptr_spec = (intptr_t *)&buffer->c[cpu].array[offset];
+ newval_final = offset + 1;
+ targetptr_final = &buffer->c[cpu].offset;
+ if (opt_mb)
+ ret = percpu_cmpeqv_storev_storev_release(
+ targetptr_final, offset, targetptr_spec,
+ newval_spec, newval_final, cpu);
+ else
+ ret = percpu_cmpeqv_storev_storev(targetptr_final,
+ offset, targetptr_spec, newval_spec,
+ newval_final, cpu);
+ if (rseq_likely(!ret))
+ break;
+ if (rseq_unlikely(ret < 0)) {
+ perror("cpu_opv");
+ abort();
+ }
+ /* Retry if comparison fails. */
+ }
+ return true;
+}
+
+struct percpu_buffer_node *percpu_buffer_pop(struct percpu_buffer *buffer,
+ int cpu)
+{
+ struct percpu_buffer_node *head;
+
+ for (;;) {
+ intptr_t *targetptr, newval;
+ intptr_t offset;
+ int ret;
+
+ /* Load offset with single-copy atomicity. */
+ offset = RSEQ_READ_ONCE(buffer->c[cpu].offset);
+ if (offset == 0)
+ return NULL;
+ head = RSEQ_READ_ONCE(buffer->c[cpu].array[offset - 1]);
+ newval = offset - 1;
+ targetptr = (intptr_t *)&buffer->c[cpu].offset;
+ ret = percpu_cmpeqv_cmpeqv_storev(targetptr, offset,
+ (intptr_t *)&buffer->c[cpu].array[offset - 1],
+ (intptr_t)head, newval, cpu);
+ if (rseq_likely(!ret))
+ break;
+ if (rseq_unlikely(ret < 0)) {
+ perror("cpu_opv");
+ abort();
+ }
+ /* Retry if comparison fails. */
+ }
+ return head;
+}
+
+void *test_percpu_buffer_thread(void *arg)
+{
+ long long i, reps;
+ struct percpu_buffer *buffer = (struct percpu_buffer *)arg;
+
+ if (!opt_disable_rseq && rseq_register_current_thread())
+ abort();
+
+ reps = opt_reps;
+ for (i = 0; i < reps; i++) {
+ struct percpu_buffer_node *node;
+
+ node = percpu_buffer_pop(buffer, rseq_cpu_start());
+ if (opt_yield)
+ sched_yield(); /* encourage shuffling */
+ if (node) {
+ if (!percpu_buffer_push(buffer, node,
+ rseq_cpu_start())) {
+ /* Should increase buffer size. */
+ abort();
+ }
+ }
+ }
+
+ printf_verbose("tid %d: number of rseq abort: %d, signals delivered: %u\n",
+ (int) gettid(), nr_abort, signals_delivered);
+ if (!opt_disable_rseq && rseq_unregister_current_thread())
+ abort();
+
+ return NULL;
+}
+
+/* Simultaneous modification to a per-cpu buffer from many threads. */
+void test_percpu_buffer(void)
+{
+ const int num_threads = opt_threads;
+ int i, j, ret;
+ uint64_t sum = 0, expected_sum = 0;
+ struct percpu_buffer buffer;
+ pthread_t test_threads[num_threads];
+ cpu_set_t allowed_cpus;
+
+ memset(&buffer, 0, sizeof(buffer));
+
+ /* Generate list entries for every usable cpu. */
+ sched_getaffinity(0, sizeof(allowed_cpus), &allowed_cpus);
+ for (i = 0; i < CPU_SETSIZE; i++) {
+ if (!CPU_ISSET(i, &allowed_cpus))
+ continue;
+ /* Worse-case is every item in same CPU. */
+ buffer.c[i].array =
+ malloc(sizeof(*buffer.c[i].array) * CPU_SETSIZE *
+ BUFFER_ITEM_PER_CPU);
+ assert(buffer.c[i].array);
+ buffer.c[i].buflen = CPU_SETSIZE * BUFFER_ITEM_PER_CPU;
+ for (j = 1; j <= BUFFER_ITEM_PER_CPU; j++) {
+ struct percpu_buffer_node *node;
+
+ expected_sum += j;
+
+ /*
+ * We could theoretically put the word-sized
+ * "data" directly in the buffer. However, we
+ * want to model objects that would not fit
+ * within a single word, so allocate an object
+ * for each node.
+ */
+ node = malloc(sizeof(*node));
+ assert(node);
+ node->data = j;
+ buffer.c[i].array[j - 1] = node;
+ buffer.c[i].offset++;
+ }
+ }
+
+ for (i = 0; i < num_threads; i++) {
+ ret = pthread_create(&test_threads[i], NULL,
+ test_percpu_buffer_thread, &buffer);
+ if (ret) {
+ errno = ret;
+ perror("pthread_create");
+ abort();
+ }
+ }
+
+ for (i = 0; i < num_threads; i++) {
+ ret = pthread_join(test_threads[i], NULL);
+ if (ret) {
+ errno = ret;
+ perror("pthread_join");
+ abort();
+ }
+ }
+
+ for (i = 0; i < CPU_SETSIZE; i++) {
+ struct percpu_buffer_node *node;
+
+ if (!CPU_ISSET(i, &allowed_cpus))
+ continue;
+
+ while ((node = percpu_buffer_pop(&buffer, i))) {
+ sum += node->data;
+ free(node);
+ }
+ free(buffer.c[i].array);
+ }
+
+ /*
+ * All entries should now be accounted for (unless some external
+ * actor is interfering with our allowed affinity while this
+ * test is running).
+ */
+ assert(sum == expected_sum);
+}
+
+bool percpu_memcpy_buffer_push(struct percpu_memcpy_buffer *buffer,
+ struct percpu_memcpy_buffer_node item, int cpu)
+{
+ for (;;) {
+ intptr_t *targetptr_final, newval_final, offset;
+ char *destptr, *srcptr;
+ size_t copylen;
+ int ret;
+
+ /* Load offset with single-copy atomicity. */
+ offset = RSEQ_READ_ONCE(buffer->c[cpu].offset);
+ if (offset == buffer->c[cpu].buflen)
+ return false;
+ destptr = (char *)&buffer->c[cpu].array[offset];
+ srcptr = (char *)&item;
+ /* copylen must be <= 4kB. */
+ copylen = sizeof(item);
+ newval_final = offset + 1;
+ targetptr_final = &buffer->c[cpu].offset;
+ if (opt_mb)
+ ret = percpu_cmpeqv_memcpy_storev_release(
+ targetptr_final, offset,
+ destptr, srcptr, copylen,
+ newval_final, cpu);
+ else
+ ret = percpu_cmpeqv_memcpy_storev(targetptr_final,
+ offset, destptr, srcptr, copylen,
+ newval_final, cpu);
+ if (rseq_likely(!ret))
+ break;
+ if (rseq_unlikely(ret < 0)) {
+ perror("cpu_opv");
+ abort();
+ }
+ /* Retry if comparison fails. */
+ }
+ return true;
+}
+
+bool percpu_memcpy_buffer_pop(struct percpu_memcpy_buffer *buffer,
+ struct percpu_memcpy_buffer_node *item, int cpu)
+{
+ for (;;) {
+ intptr_t *targetptr_final, newval_final, offset;
+ char *destptr, *srcptr;
+ size_t copylen;
+ int ret;
+
+ /* Load offset with single-copy atomicity. */
+ offset = RSEQ_READ_ONCE(buffer->c[cpu].offset);
+ if (offset == 0)
+ return false;
+ destptr = (char *)item;
+ srcptr = (char *)&buffer->c[cpu].array[offset - 1];
+ /* copylen must be <= 4kB. */
+ copylen = sizeof(*item);
+ newval_final = offset - 1;
+ targetptr_final = &buffer->c[cpu].offset;
+ ret = percpu_cmpeqv_memcpy_storev(targetptr_final,
+ offset, destptr, srcptr, copylen,
+ newval_final, cpu);
+ if (rseq_likely(!ret))
+ break;
+ if (rseq_unlikely(ret < 0)) {
+ perror("cpu_opv");
+ abort();
+ }
+ /* Retry if comparison fails. */
+ }
+ return true;
+}
+
+void *test_percpu_memcpy_buffer_thread(void *arg)
+{
+ long long i, reps;
+ struct percpu_memcpy_buffer *buffer = (struct percpu_memcpy_buffer *)arg;
+
+ if (!opt_disable_rseq && rseq_register_current_thread())
+ abort();
+
+ reps = opt_reps;
+ for (i = 0; i < reps; i++) {
+ struct percpu_memcpy_buffer_node item;
+ bool result;
+
+ result = percpu_memcpy_buffer_pop(buffer, &item,
+ rseq_cpu_start());
+ if (opt_yield)
+ sched_yield(); /* encourage shuffling */
+ if (result) {
+ if (!percpu_memcpy_buffer_push(buffer, item,
+ rseq_cpu_start())) {
+ /* Should increase buffer size. */
+ abort();
+ }
+ }
+ }
+
+ printf_verbose("tid %d: number of rseq abort: %d, signals delivered: %u\n",
+ (int) gettid(), nr_abort, signals_delivered);
+ if (!opt_disable_rseq && rseq_unregister_current_thread())
+ abort();
+
+ return NULL;
+}
+
+/* Simultaneous modification to a per-cpu buffer from many threads. */
+void test_percpu_memcpy_buffer(void)
+{
+ const int num_threads = opt_threads;
+ int i, j, ret;
+ uint64_t sum = 0, expected_sum = 0;
+ struct percpu_memcpy_buffer buffer;
+ pthread_t test_threads[num_threads];
+ cpu_set_t allowed_cpus;
+
+ memset(&buffer, 0, sizeof(buffer));
+
+ /* Generate list entries for every usable cpu. */
+ sched_getaffinity(0, sizeof(allowed_cpus), &allowed_cpus);
+ for (i = 0; i < CPU_SETSIZE; i++) {
+ if (!CPU_ISSET(i, &allowed_cpus))
+ continue;
+ /* Worse-case is every item in same CPU. */
+ buffer.c[i].array =
+ malloc(sizeof(*buffer.c[i].array) * CPU_SETSIZE *
+ MEMCPY_BUFFER_ITEM_PER_CPU);
+ assert(buffer.c[i].array);
+ buffer.c[i].buflen = CPU_SETSIZE * MEMCPY_BUFFER_ITEM_PER_CPU;
+ for (j = 1; j <= MEMCPY_BUFFER_ITEM_PER_CPU; j++) {
+ expected_sum += 2 * j + 1;
+
+ /*
+ * We could theoretically put the word-sized
+ * "data" directly in the buffer. However, we
+ * want to model objects that would not fit
+ * within a single word, so allocate an object
+ * for each node.
+ */
+ buffer.c[i].array[j - 1].data1 = j;
+ buffer.c[i].array[j - 1].data2 = j + 1;
+ buffer.c[i].offset++;
+ }
+ }
+
+ for (i = 0; i < num_threads; i++) {
+ ret = pthread_create(&test_threads[i], NULL,
+ test_percpu_memcpy_buffer_thread,
+ &buffer);
+ if (ret) {
+ errno = ret;
+ perror("pthread_create");
+ abort();
+ }
+ }
+
+ for (i = 0; i < num_threads; i++) {
+ ret = pthread_join(test_threads[i], NULL);
+ if (ret) {
+ errno = ret;
+ perror("pthread_join");
+ abort();
+ }
+ }
+
+ for (i = 0; i < CPU_SETSIZE; i++) {
+ struct percpu_memcpy_buffer_node item;
+
+ if (!CPU_ISSET(i, &allowed_cpus))
+ continue;
+
+ while (percpu_memcpy_buffer_pop(&buffer, &item, i)) {
+ sum += item.data1;
+ sum += item.data2;
+ }
+ free(buffer.c[i].array);
+ }
+
+ /*
+ * All entries should now be accounted for (unless some external
+ * actor is interfering with our allowed affinity while this
+ * test is running).
+ */
+ assert(sum == expected_sum);
+}
+
+static void test_signal_interrupt_handler(int signo)
+{
+ signals_delivered++;
+}
+
+static int set_signal_handler(void)
+{
+ int ret = 0;
+ struct sigaction sa;
+ sigset_t sigset;
+
+ ret = sigemptyset(&sigset);
+ if (ret < 0) {
+ perror("sigemptyset");
+ return ret;
+ }
+
+ sa.sa_handler = test_signal_interrupt_handler;
+ sa.sa_mask = sigset;
+ sa.sa_flags = 0;
+ ret = sigaction(SIGUSR1, &sa, NULL);
+ if (ret < 0) {
+ perror("sigaction");
+ return ret;
+ }
+
+ printf_verbose("Signal handler set for SIGUSR1\n");
+
+ return ret;
+}
+
+static void show_usage(int argc, char **argv)
+{
+ printf("Usage : %s <OPTIONS>\n",
+ argv[0]);
+ printf("OPTIONS:\n");
+ printf(" [-1 loops] Number of loops for delay injection 1\n");
+ printf(" [-2 loops] Number of loops for delay injection 2\n");
+ printf(" [-3 loops] Number of loops for delay injection 3\n");
+ printf(" [-4 loops] Number of loops for delay injection 4\n");
+ printf(" [-5 loops] Number of loops for delay injection 5\n");
+ printf(" [-6 loops] Number of loops for delay injection 6\n");
+ printf(" [-7 loops] Number of loops for delay injection 7 (-1 to enable -m)\n");
+ printf(" [-8 loops] Number of loops for delay injection 8 (-1 to enable -m)\n");
+ printf(" [-9 loops] Number of loops for delay injection 9 (-1 to enable -m)\n");
+ printf(" [-m N] Yield/sleep/kill every modulo N (default 0: disabled) (>= 0)\n");
+ printf(" [-y] Yield\n");
+ printf(" [-k] Kill thread with signal\n");
+ printf(" [-s S] S: =0: disabled (default), >0: sleep time (ms)\n");
+ printf(" [-t N] Number of threads (default 200)\n");
+ printf(" [-r N] Number of repetitions per thread (default 5000)\n");
+ printf(" [-d] Disable rseq system call (no initialization)\n");
+ printf(" [-D M] Disable rseq for each M threads\n");
+ printf(" [-T test] Choose test: (s)pinlock, (l)ist, (b)uffer, (m)emcpy, (i)ncrement\n");
+ printf(" [-M] Push into buffer and memcpy buffer with memory barriers.\n");
+ printf(" [-v] Verbose output.\n");
+ printf(" [-h] Show this help.\n");
+ printf("\n");
+}
+
+int main(int argc, char **argv)
+{
+ int i;
+
+ for (i = 1; i < argc; i++) {
+ if (argv[i][0] != '-')
+ continue;
+ switch (argv[i][1]) {
+ case '1':
+ case '2':
+ case '3':
+ case '4':
+ case '5':
+ case '6':
+ case '7':
+ case '8':
+ case '9':
+ if (argc < i + 2) {
+ show_usage(argc, argv);
+ goto error;
+ }
+ loop_cnt[argv[i][1] - '0'] = atol(argv[i + 1]);
+ i++;
+ break;
+ case 'm':
+ if (argc < i + 2) {
+ show_usage(argc, argv);
+ goto error;
+ }
+ opt_modulo = atol(argv[i + 1]);
+ if (opt_modulo < 0) {
+ show_usage(argc, argv);
+ goto error;
+ }
+ i++;
+ break;
+ case 's':
+ if (argc < i + 2) {
+ show_usage(argc, argv);
+ goto error;
+ }
+ opt_sleep = atol(argv[i + 1]);
+ if (opt_sleep < 0) {
+ show_usage(argc, argv);
+ goto error;
+ }
+ i++;
+ break;
+ case 'y':
+ opt_yield = 1;
+ break;
+ case 'k':
+ opt_signal = 1;
+ break;
+ case 'd':
+ opt_disable_rseq = 1;
+ break;
+ case 'D':
+ if (argc < i + 2) {
+ show_usage(argc, argv);
+ goto error;
+ }
+ opt_disable_mod = atol(argv[i + 1]);
+ if (opt_disable_mod < 0) {
+ show_usage(argc, argv);
+ goto error;
+ }
+ i++;
+ break;
+ case 't':
+ if (argc < i + 2) {
+ show_usage(argc, argv);
+ goto error;
+ }
+ opt_threads = atol(argv[i + 1]);
+ if (opt_threads < 0) {
+ show_usage(argc, argv);
+ goto error;
+ }
+ i++;
+ break;
+ case 'r':
+ if (argc < i + 2) {
+ show_usage(argc, argv);
+ goto error;
+ }
+ opt_reps = atoll(argv[i + 1]);
+ if (opt_reps < 0) {
+ show_usage(argc, argv);
+ goto error;
+ }
+ i++;
+ break;
+ case 'h':
+ show_usage(argc, argv);
+ goto end;
+ case 'T':
+ if (argc < i + 2) {
+ show_usage(argc, argv);
+ goto error;
+ }
+ opt_test = *argv[i + 1];
+ switch (opt_test) {
+ case 's':
+ case 'l':
+ case 'i':
+ case 'b':
+ case 'm':
+ break;
+ default:
+ show_usage(argc, argv);
+ goto error;
+ }
+ i++;
+ break;
+ case 'v':
+ verbose = 1;
+ break;
+ case 'M':
+ opt_mb = 1;
+ break;
+ default:
+ show_usage(argc, argv);
+ goto error;
+ }
+ }
+
+ loop_cnt_1 = loop_cnt[1];
+ loop_cnt_2 = loop_cnt[2];
+ loop_cnt_3 = loop_cnt[3];
+ loop_cnt_4 = loop_cnt[4];
+ loop_cnt_5 = loop_cnt[5];
+ loop_cnt_6 = loop_cnt[6];
+
+ if (set_signal_handler())
+ goto error;
+
+ if (!opt_disable_rseq && rseq_register_current_thread())
+ goto error;
+ switch (opt_test) {
+ case 's':
+ printf_verbose("spinlock\n");
+ test_percpu_spinlock();
+ break;
+ case 'l':
+ printf_verbose("linked list\n");
+ test_percpu_list();
+ break;
+ case 'b':
+ printf_verbose("buffer\n");
+ test_percpu_buffer();
+ break;
+ case 'm':
+ printf_verbose("memcpy buffer\n");
+ test_percpu_memcpy_buffer();
+ break;
+ case 'i':
+ printf_verbose("counter increment\n");
+ test_percpu_inc();
+ break;
+ }
+ if (!opt_disable_rseq && rseq_unregister_current_thread())
+ abort();
+end:
+ return 0;
+
+error:
+ return -1;
+}
--
2.11.0
^ permalink raw reply related
* [RFC PATCH for 4.17 21/21] rseq: selftests: Provide Makefile, scripts, gitignore
From: Mathieu Desnoyers @ 2018-03-27 16:05 UTC (permalink / raw)
To: Peter Zijlstra, Paul E . McKenney, Boqun Feng, Andy Lutomirski,
Dave Watson
Cc: linux-kernel, linux-api, Paul Turner, Andrew Morton, Russell King,
Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Andrew Hunter,
Andi Kleen, Chris Lameter, Ben Maurer, Steven Rostedt,
Josh Triplett, Linus Torvalds, Catalin Marinas, Will Deacon,
Michael Kerrisk, Mathieu Desnoyers, Shuah Khan, linux-kselftest
In-Reply-To: <20180327160542.28457-1-mathieu.desnoyers@efficios.com>
A run_param_test.sh script runs many variants of the parametrizable
tests.
Wire up the rseq Makefile, add directory entry into MAINTAINERS file.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: Shuah Khan <shuahkh@osg.samsung.com>
CC: Russell King <linux@arm.linux.org.uk>
CC: Catalin Marinas <catalin.marinas@arm.com>
CC: Will Deacon <will.deacon@arm.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Paul Turner <pjt@google.com>
CC: Andrew Hunter <ahh@google.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Andi Kleen <andi@firstfloor.org>
CC: Dave Watson <davejwatson@fb.com>
CC: Chris Lameter <cl@linux.com>
CC: Ingo Molnar <mingo@redhat.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Ben Maurer <bmaurer@fb.com>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: Josh Triplett <josh@joshtriplett.org>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Boqun Feng <boqun.feng@gmail.com>
CC: linux-kselftest@vger.kernel.org
CC: linux-api@vger.kernel.org
---
MAINTAINERS | 1 +
tools/testing/selftests/Makefile | 1 +
tools/testing/selftests/rseq/.gitignore | 7 ++
tools/testing/selftests/rseq/Makefile | 37 +++++++
tools/testing/selftests/rseq/run_param_test.sh | 130 +++++++++++++++++++++++++
5 files changed, 176 insertions(+)
create mode 100644 tools/testing/selftests/rseq/.gitignore
create mode 100644 tools/testing/selftests/rseq/Makefile
create mode 100755 tools/testing/selftests/rseq/run_param_test.sh
diff --git a/MAINTAINERS b/MAINTAINERS
index 936ff672d5fb..ba630a386a88 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11830,6 +11830,7 @@ S: Supported
F: kernel/rseq.c
F: include/uapi/linux/rseq.h
F: include/trace/events/rseq.h
+F: tools/testing/selftests/rseq/
RFKILL
M: Johannes Berg <johannes@sipsolutions.net>
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 1322e63f5963..c1c9323d4e50 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -26,6 +26,7 @@ TARGETS += nsfs
TARGETS += powerpc
TARGETS += pstore
TARGETS += ptrace
+TARGETS += rseq
TARGETS += seccomp
TARGETS += sigaltstack
TARGETS += size
diff --git a/tools/testing/selftests/rseq/.gitignore b/tools/testing/selftests/rseq/.gitignore
new file mode 100644
index 000000000000..73dcf05b69fd
--- /dev/null
+++ b/tools/testing/selftests/rseq/.gitignore
@@ -0,0 +1,7 @@
+basic_percpu_ops_test
+basic_test
+basic_rseq_op_test
+param_test
+param_test_benchmark
+param_test_compare_twice
+param_test_skip_fastpath
diff --git a/tools/testing/selftests/rseq/Makefile b/tools/testing/selftests/rseq/Makefile
new file mode 100644
index 000000000000..69f90d718774
--- /dev/null
+++ b/tools/testing/selftests/rseq/Makefile
@@ -0,0 +1,37 @@
+CFLAGS += -O2 -Wall -g -I./ -I../cpu-opv/ -I../../../../usr/include/ -L./ -Wl,-rpath=./
+LDLIBS += -lpthread
+
+# Own dependencies because we only want to build against 1st prerequisite, but
+# still track changes to header files and depend on shared object.
+OVERRIDE_TARGETS = 1
+
+TEST_GEN_PROGS = basic_test basic_percpu_ops_test \
+ param_test param_test_skip_fastpath \
+ param_test_benchmark param_test_compare_twice
+
+TEST_GEN_PROGS_EXTENDED = librseq.so libcpu-op.so
+
+TEST_PROGS = run_param_test.sh
+
+include ../lib.mk
+
+$(OUTPUT)/librseq.so: rseq.c rseq.h rseq-*.h
+ $(CC) $(CFLAGS) -shared -fPIC $< $(LDLIBS) -o $@
+
+$(OUTPUT)/libcpu-op.so: ../cpu-opv/cpu-op.c ../cpu-opv/cpu-op.h
+ $(CC) $(CFLAGS) -shared -fPIC $< $(LDLIBS) -o $@
+
+$(OUTPUT)/%: %.c $(TEST_GEN_PROGS_EXTENDED) rseq.h rseq-*.h ../cpu-opv/cpu-op.h percpu-op.h
+ $(CC) $(CFLAGS) $< $(LDLIBS) -lrseq -lcpu-op -o $@
+
+$(OUTPUT)/param_test_skip_fastpath: param_test.c $(TEST_GEN_PROGS_EXTENDED) \
+ rseq.h rseq-*.h ../cpu-opv/cpu-op.h percpu-op.h
+ $(CC) $(CFLAGS) -DRSEQ_SKIP_FASTPATH $< $(LDLIBS) -lrseq -lcpu-op -o $@
+
+$(OUTPUT)/param_test_benchmark: param_test.c $(TEST_GEN_PROGS_EXTENDED) \
+ rseq.h rseq-*.h ../cpu-opv/cpu-op.h percpu-op.h
+ $(CC) $(CFLAGS) -DBENCHMARK $< $(LDLIBS) -lrseq -lcpu-op -o $@
+
+$(OUTPUT)/param_test_compare_twice: param_test.c $(TEST_GEN_PROGS_EXTENDED) \
+ rseq.h rseq-*.h ../cpu-opv/cpu-op.h percpu-op.h
+ $(CC) $(CFLAGS) -DRSEQ_COMPARE_TWICE $< $(LDLIBS) -lrseq -lcpu-op -o $@
diff --git a/tools/testing/selftests/rseq/run_param_test.sh b/tools/testing/selftests/rseq/run_param_test.sh
new file mode 100755
index 000000000000..64d4c7d722c9
--- /dev/null
+++ b/tools/testing/selftests/rseq/run_param_test.sh
@@ -0,0 +1,130 @@
+#!/bin/bash
+
+EXTRA_ARGS=${@}
+
+OLDIFS="$IFS"
+IFS=$'\n'
+TEST_LIST=(
+ "-T s"
+ "-T l"
+ "-T b"
+ "-T b -M"
+ "-T m"
+ "-T m -M"
+ "-T i"
+)
+
+TEST_NAME=(
+ "spinlock"
+ "list"
+ "buffer"
+ "buffer with barrier"
+ "memcpy"
+ "memcpy with barrier"
+ "increment"
+)
+IFS="$OLDIFS"
+
+REPS=1000
+
+function do_tests()
+{
+ local i=0
+ while [ "$i" -lt "${#TEST_LIST[@]}" ]; do
+ echo "Running test ${TEST_NAME[$i]}"
+ ./param_test ${TEST_LIST[$i]} -r ${REPS} ${@} ${EXTRA_ARGS} || exit 1
+ echo "Running skip fast-path test ${TEST_NAME[$i]}"
+ ./param_test_skip_fastpath ${TEST_LIST[$i]} -r ${REPS} ${@} ${EXTRA_ARGS} || exit 1
+ echo "Running compare-twice test ${TEST_NAME[$i]}"
+ ./param_test_compare_twice ${TEST_LIST[$i]} -r ${REPS} ${@} ${EXTRA_ARGS} || exit 1
+ let "i++"
+ done
+}
+
+echo "Default parameters"
+do_tests
+
+echo "Loop injection: 10000 loops"
+
+OLDIFS="$IFS"
+IFS=$'\n'
+INJECT_LIST=(
+ "1"
+ "2"
+ "3"
+ "4"
+ "5"
+ "6"
+ "7"
+ "8"
+ "9"
+)
+IFS="$OLDIFS"
+
+NR_LOOPS=10000
+
+i=0
+while [ "$i" -lt "${#INJECT_LIST[@]}" ]; do
+ echo "Injecting at <${INJECT_LIST[$i]}>"
+ do_tests -${INJECT_LIST[i]} ${NR_LOOPS}
+ let "i++"
+done
+NR_LOOPS=
+
+function inject_blocking()
+{
+ OLDIFS="$IFS"
+ IFS=$'\n'
+ INJECT_LIST=(
+ "7"
+ "8"
+ "9"
+ )
+ IFS="$OLDIFS"
+
+ NR_LOOPS=-1
+
+ i=0
+ while [ "$i" -lt "${#INJECT_LIST[@]}" ]; do
+ echo "Injecting at <${INJECT_LIST[$i]}>"
+ do_tests -${INJECT_LIST[i]} -1 ${@}
+ let "i++"
+ done
+ NR_LOOPS=
+}
+
+echo "Yield injection (25%)"
+inject_blocking -m 4 -y
+
+echo "Yield injection (50%)"
+inject_blocking -m 2 -y
+
+echo "Yield injection (100%)"
+inject_blocking -m 1 -y
+
+echo "Kill injection (25%)"
+inject_blocking -m 4 -k
+
+echo "Kill injection (50%)"
+inject_blocking -m 2 -k
+
+echo "Kill injection (100%)"
+inject_blocking -m 1 -k
+
+echo "Sleep injection (1ms, 25%)"
+inject_blocking -m 4 -s 1
+
+echo "Sleep injection (1ms, 50%)"
+inject_blocking -m 2 -s 1
+
+echo "Sleep injection (1ms, 100%)"
+inject_blocking -m 1 -s 1
+
+echo "Disable rseq for 25% threads"
+do_tests -D 4
+
+echo "Disable rseq for 50% threads"
+do_tests -D 2
+
+echo "Disable rseq"
+do_tests -d
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v6 bpf-next 07/11] tracepoint: introduce kernel_tracepoint_find_by_name
From: Mathieu Desnoyers @ 2018-03-27 16:09 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: rostedt, David S. Miller, Daniel Borkmann, Linus Torvalds,
Peter Zijlstra, netdev, kernel-team, linux-api
In-Reply-To: <8600dbae-389d-dd01-f003-44f1af7dcf0a@fb.com>
----- On Mar 27, 2018, at 11:53 AM, Alexei Starovoitov ast@fb.com wrote:
> On 3/27/18 7:42 AM, Steven Rostedt wrote:
>> On Tue, 27 Mar 2018 10:18:24 -0400 (EDT)
>> Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
>>
>>> ----- On Mar 27, 2018, at 10:07 AM, rostedt rostedt@goodmis.org wrote:
>>>
>>>> On Mon, 26 Mar 2018 19:47:02 -0700
>>>> Alexei Starovoitov <ast@fb.com> wrote:
>>>>
>>>>> From: Alexei Starovoitov <ast@kernel.org>
>>>>>
>>>>> introduce kernel_tracepoint_find_by_name() helper to let bpf core
>>>>> find tracepoint by name and later attach bpf probe to a tracepoint
>>>>>
>>>>> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
>>>>
>>>> Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
>>>
>>> Steven showed preference for tracepoint_kernel_find_by_name() at some
>>> point (starting with a tracepoint_ prefix). I'm find with either of
>>> the names.
>>
>> Yeah, I do prefer tracepoint_kernel_find_by_name() to stay consistent
>> with the other tracepoint functions. But we have
>> "for_each_kernel_tracepoint()" and not "for_each_tracepoint_kernel()",
>> thus we need to pick being consistent with one or the other. One answer
>> is to use tracpoint_kernel_find_by_name() and rename the for_each to
>> for_each_tracpoint_kernel().
>
> yep. that's exactly the reason I picked kernel_tracepoint_find_by_name()
> to match for_each_kernel_tracepoint() naming.
>
> I can certainly send a follow up patch to rename both to
> *tracepoint_kernel* and then you can nack it because it breaks lttng :)
If Steven prefers changing the name of for_each_kernel_tracepoint() to
for_each_tracepoint_kernel(), I'll adapt LTTng accordingly. I don't
mind either way, as long as the change is justified.
Thanks,
Mathieu
> but let's do it in a separate thread.
>
> Daniel,
> do you mind adding { } as Steven requested while applying or
> you want me to resubmit the whole thing?
>
> Thanks!
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [PATCH v6 bpf-next 07/11] tracepoint: introduce kernel_tracepoint_find_by_name
From: Daniel Borkmann @ 2018-03-27 16:36 UTC (permalink / raw)
To: Alexei Starovoitov, Steven Rostedt, Mathieu Desnoyers
Cc: David S. Miller, Linus Torvalds, Peter Zijlstra, netdev,
kernel-team, linux-api
In-Reply-To: <8600dbae-389d-dd01-f003-44f1af7dcf0a@fb.com>
On 03/27/2018 05:53 PM, Alexei Starovoitov wrote:
> On 3/27/18 7:42 AM, Steven Rostedt wrote:
>> On Tue, 27 Mar 2018 10:18:24 -0400 (EDT)
>> Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
>>> ----- On Mar 27, 2018, at 10:07 AM, rostedt rostedt@goodmis.org wrote:
>>>> On Mon, 26 Mar 2018 19:47:02 -0700
>>>> Alexei Starovoitov <ast@fb.com> wrote:
>>>>> From: Alexei Starovoitov <ast@kernel.org>
>>>>>
>>>>> introduce kernel_tracepoint_find_by_name() helper to let bpf core
>>>>> find tracepoint by name and later attach bpf probe to a tracepoint
>>>>>
>>>>> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
>>>>
>>>> Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
>>>
>>> Steven showed preference for tracepoint_kernel_find_by_name() at some
>>> point (starting with a tracepoint_ prefix). I'm find with either of
>>> the names.
>>
>> Yeah, I do prefer tracepoint_kernel_find_by_name() to stay consistent
>> with the other tracepoint functions. But we have
>> "for_each_kernel_tracepoint()" and not "for_each_tracepoint_kernel()",
>> thus we need to pick being consistent with one or the other. One answer
>> is to use tracpoint_kernel_find_by_name() and rename the for_each to
>> for_each_tracpoint_kernel().
>
> yep. that's exactly the reason I picked kernel_tracepoint_find_by_name()
> to match for_each_kernel_tracepoint() naming.
>
> I can certainly send a follow up patch to rename both to
> *tracepoint_kernel* and then you can nack it because it breaks lttng :)
> but let's do it in a separate thread.
>
> Daniel,
> do you mind adding { } as Steven requested while applying or
> you want me to resubmit the whole thing?
Yeah, I can fix it up while applying.
^ permalink raw reply
* Re: [PATCH v6 bpf-next 08/11] bpf: introduce BPF_RAW_TRACEPOINT
From: Steven Rostedt @ 2018-03-27 17:02 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: davem, daniel, torvalds, peterz, netdev, kernel-team, linux-api,
Mathieu Desnoyers, Kees Cook
In-Reply-To: <20180327024706.2064725-9-ast@fb.com>
On Mon, 26 Mar 2018 19:47:03 -0700
Alexei Starovoitov <ast@fb.com> wrote:
> Ctrl-C of tracing daemon or cmdline tool that uses this feature
> will automatically detach bpf program, unload it and
> unregister tracepoint probe.
>
> On the kernel side for_each_kernel_tracepoint() is used
You need to update the change log to state
kernel_tracepoint_find_by_name().
But looking at the code, I really think you should do it properly and
not rely on a hack that finds your function via kallsyms lookup and
then executing the address it returns.
> to find a tracepoint with "xdp_exception" name
> (that would be __tracepoint_xdp_exception record)
>
> Then kallsyms_lookup_name() is used to find the addr
> of __bpf_trace_xdp_exception() probe function.
>
> And finally tracepoint_probe_register() is used to connect probe
> with tracepoint.
>
> Addition of bpf_raw_tracepoint doesn't interfere with ftrace and perf
> tracepoint mechanisms. perf_event_open() can be used in parallel
> on the same tracepoint.
> Multiple bpf_raw_tracepoint_open("xdp_exception", prog_fd) are permitted.
> Each with its own bpf program. The kernel will execute
> all tracepoint probes and all attached bpf programs.
>
> In the future bpf_raw_tracepoints can be extended with
> query/introspection logic.
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
> include/linux/bpf_types.h | 1 +
> include/linux/trace_events.h | 37 +++++++++
> include/trace/bpf_probe.h | 87 ++++++++++++++++++++
> include/trace/define_trace.h | 1 +
> include/uapi/linux/bpf.h | 11 +++
> kernel/bpf/syscall.c | 78 ++++++++++++++++++
> kernel/trace/bpf_trace.c | 188 +++++++++++++++++++++++++++++++++++++++++++
> 7 files changed, 403 insertions(+)
> create mode 100644 include/trace/bpf_probe.h
>
>
> --- a/include/linux/trace_events.h
> +++ b/include/linux/trace_events.h
> @@ -468,6 +468,8 @@ unsigned int trace_call_bpf(struct trace_event_call *call, void *ctx);
> int perf_event_attach_bpf_prog(struct perf_event *event, struct bpf_prog *prog);
> void perf_event_detach_bpf_prog(struct perf_event *event);
> int perf_event_query_prog_array(struct perf_event *event, void __user *info);
> +int bpf_probe_register(struct tracepoint *tp, struct bpf_prog *prog);
> +int bpf_probe_unregister(struct tracepoint *tp, struct bpf_prog *prog);
> #else
> static inline unsigned int trace_call_bpf(struct trace_event_call *call, void *ctx)
> {
> @@ -487,6 +489,14 @@ perf_event_query_prog_array(struct perf_event *event, void __user *info)
> {
> return -EOPNOTSUPP;
> }
> +static inline int bpf_probe_register(struct tracepoint *tp, struct bpf_prog *p)
> +{
> + return -EOPNOTSUPP;
> +}
> +static inline int bpf_probe_unregister(struct tracepoint *tp, struct bpf_prog *p)
> +{
> + return -EOPNOTSUPP;
> +}
> #endif
>
> enum {
> @@ -546,6 +556,33 @@ extern void ftrace_profile_free_filter(struct perf_event *event);
> void perf_trace_buf_update(void *record, u16 type);
> void *perf_trace_buf_alloc(int size, struct pt_regs **regs, int *rctxp);
>
> +void bpf_trace_run1(struct bpf_prog *prog, u64 arg1);
> +void bpf_trace_run2(struct bpf_prog *prog, u64 arg1, u64 arg2);
> +void bpf_trace_run3(struct bpf_prog *prog, u64 arg1, u64 arg2,
> + u64 arg3);
> +void bpf_trace_run4(struct bpf_prog *prog, u64 arg1, u64 arg2,
> + u64 arg3, u64 arg4);
> +void bpf_trace_run5(struct bpf_prog *prog, u64 arg1, u64 arg2,
> + u64 arg3, u64 arg4, u64 arg5);
> +void bpf_trace_run6(struct bpf_prog *prog, u64 arg1, u64 arg2,
> + u64 arg3, u64 arg4, u64 arg5, u64 arg6);
> +void bpf_trace_run7(struct bpf_prog *prog, u64 arg1, u64 arg2,
> + u64 arg3, u64 arg4, u64 arg5, u64 arg6, u64 arg7);
> +void bpf_trace_run8(struct bpf_prog *prog, u64 arg1, u64 arg2,
> + u64 arg3, u64 arg4, u64 arg5, u64 arg6, u64 arg7,
> + u64 arg8);
> +void bpf_trace_run9(struct bpf_prog *prog, u64 arg1, u64 arg2,
> + u64 arg3, u64 arg4, u64 arg5, u64 arg6, u64 arg7,
> + u64 arg8, u64 arg9);
> +void bpf_trace_run10(struct bpf_prog *prog, u64 arg1, u64 arg2,
> + u64 arg3, u64 arg4, u64 arg5, u64 arg6, u64 arg7,
> + u64 arg8, u64 arg9, u64 arg10);
> +void bpf_trace_run11(struct bpf_prog *prog, u64 arg1, u64 arg2,
> + u64 arg3, u64 arg4, u64 arg5, u64 arg6, u64 arg7,
> + u64 arg8, u64 arg9, u64 arg10, u64 arg11);
> +void bpf_trace_run12(struct bpf_prog *prog, u64 arg1, u64 arg2,
> + u64 arg3, u64 arg4, u64 arg5, u64 arg6, u64 arg7,
> + u64 arg8, u64 arg9, u64 arg10, u64 arg11, u64 arg12);
> void perf_trace_run_bpf_submit(void *raw_data, int size, int rctx,
> struct trace_event_call *call, u64 count,
> struct pt_regs *regs, struct hlist_head *head,
> diff --git a/include/trace/bpf_probe.h b/include/trace/bpf_probe.h
> new file mode 100644
> index 000000000000..d2cc0663e618
> --- /dev/null
> +++ b/include/trace/bpf_probe.h
> @@ -0,0 +1,87 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#undef TRACE_SYSTEM_VAR
> +
> +#ifdef CONFIG_BPF_EVENTS
> +
> +#undef __entry
> +#define __entry entry
> +
> +#undef __get_dynamic_array
> +#define __get_dynamic_array(field) \
> + ((void *)__entry + (__entry->__data_loc_##field & 0xffff))
> +
> +#undef __get_dynamic_array_len
> +#define __get_dynamic_array_len(field) \
> + ((__entry->__data_loc_##field >> 16) & 0xffff)
> +
> +#undef __get_str
> +#define __get_str(field) ((char *)__get_dynamic_array(field))
> +
> +#undef __get_bitmask
> +#define __get_bitmask(field) (char *)__get_dynamic_array(field)
> +
> +#undef __perf_count
> +#define __perf_count(c) (c)
> +
> +#undef __perf_task
> +#define __perf_task(t) (t)
> +
> +/* cast any integer, pointer, or small struct to u64 */
> +#define UINTTYPE(size) \
> + __typeof__(__builtin_choose_expr(size == 1, (u8)1, \
> + __builtin_choose_expr(size == 2, (u16)2, \
> + __builtin_choose_expr(size == 4, (u32)3, \
> + __builtin_choose_expr(size == 8, (u64)4, \
> + (void)5)))))
> +#define __CAST_TO_U64(x) ({ \
> + typeof(x) __src = (x); \
> + UINTTYPE(sizeof(x)) __dst; \
> + memcpy(&__dst, &__src, sizeof(__dst)); \
> + (u64)__dst; })
> +
> +#define __CAST1(a,...) __CAST_TO_U64(a)
> +#define __CAST2(a,...) __CAST_TO_U64(a), __CAST1(__VA_ARGS__)
> +#define __CAST3(a,...) __CAST_TO_U64(a), __CAST2(__VA_ARGS__)
> +#define __CAST4(a,...) __CAST_TO_U64(a), __CAST3(__VA_ARGS__)
> +#define __CAST5(a,...) __CAST_TO_U64(a), __CAST4(__VA_ARGS__)
> +#define __CAST6(a,...) __CAST_TO_U64(a), __CAST5(__VA_ARGS__)
> +#define __CAST7(a,...) __CAST_TO_U64(a), __CAST6(__VA_ARGS__)
> +#define __CAST8(a,...) __CAST_TO_U64(a), __CAST7(__VA_ARGS__)
> +#define __CAST9(a,...) __CAST_TO_U64(a), __CAST8(__VA_ARGS__)
> +#define __CAST10(a,...) __CAST_TO_U64(a), __CAST9(__VA_ARGS__)
> +#define __CAST11(a,...) __CAST_TO_U64(a), __CAST10(__VA_ARGS__)
> +#define __CAST12(a,...) __CAST_TO_U64(a), __CAST11(__VA_ARGS__)
> +/* tracepoints with more than 12 arguments will hit build error */
> +#define CAST_TO_U64(...) CONCATENATE(__CAST, COUNT_ARGS(__VA_ARGS__))(__VA_ARGS__)
> +
> +#undef DECLARE_EVENT_CLASS
> +#define DECLARE_EVENT_CLASS(call, proto, args, tstruct, assign, print) \
> +/* no 'static' here. The bpf probe functions are global */ \
> +notrace void \
I'm curious to why you have notrace here? Since it is separate from
perf and ftrace, for debugging purposes, it could be useful to allow
function tracing to this function.
> +__bpf_trace_##call(void *__data, proto) \
> +{ \
> + struct bpf_prog *prog = __data; \
> + \
> + CONCATENATE(bpf_trace_run, COUNT_ARGS(args))(prog, CAST_TO_U64(args)); \
> +}
> +
> +/*
> + * This part is compiled out, it is only here as a build time check
> + * to make sure that if the tracepoint handling changes, the
> + * bpf probe will fail to compile unless it too is updated.
> + */
> +#undef DEFINE_EVENT
> +#define DEFINE_EVENT(template, call, proto, args) \
> +static inline void bpf_test_probe_##call(void) \
> +{ \
> + check_trace_callback_type_##call(__bpf_trace_##template); \
> +}
> +
> +
> +#undef DEFINE_EVENT_PRINT
> +#define DEFINE_EVENT_PRINT(template, name, proto, args, print) \
> + DEFINE_EVENT(template, name, PARAMS(proto), PARAMS(args))
> +
> +#include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
> +#endif /* CONFIG_BPF_EVENTS */
> diff --git a/include/trace/define_trace.h b/include/trace/define_trace.h
> index 96b22ace9ae7..5f8216bc261f 100644
> --- a/include/trace/define_trace.h
> +++ b/include/trace/define_trace.h
> @@ -95,6 +95,7 @@
> #ifdef TRACEPOINTS_ENABLED
> #include <trace/trace_events.h>
> #include <trace/perf.h>
> +#include <trace/bpf_probe.h>
> #endif
>
> #undef TRACE_EVENT
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 18b7c510c511..1878201c2d77 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -94,6 +94,7 @@ enum bpf_cmd {
> BPF_MAP_GET_FD_BY_ID,
> BPF_OBJ_GET_INFO_BY_FD,
> BPF_PROG_QUERY,
> + BPF_RAW_TRACEPOINT_OPEN,
> };
>
> enum bpf_map_type {
> @@ -134,6 +135,7 @@ enum bpf_prog_type {
> BPF_PROG_TYPE_SK_SKB,
> BPF_PROG_TYPE_CGROUP_DEVICE,
> BPF_PROG_TYPE_SK_MSG,
> + BPF_PROG_TYPE_RAW_TRACEPOINT,
> };
>
> enum bpf_attach_type {
> @@ -344,6 +346,11 @@ union bpf_attr {
> __aligned_u64 prog_ids;
> __u32 prog_cnt;
> } query;
> +
> + struct {
> + __u64 name;
> + __u32 prog_fd;
> + } raw_tracepoint;
> } __attribute__((aligned(8)));
>
> /* BPF helper function descriptions:
> @@ -1152,4 +1159,8 @@ struct bpf_cgroup_dev_ctx {
> __u32 minor;
> };
>
> +struct bpf_raw_tracepoint_args {
> + __u64 args[0];
> +};
> +
> #endif /* _UAPI__LINUX_BPF_H__ */
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 3aeb4ea2a93a..7486b450672e 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -1311,6 +1311,81 @@ static int bpf_obj_get(const union bpf_attr *attr)
> attr->file_flags);
> }
>
> +struct bpf_raw_tracepoint {
> + struct tracepoint *tp;
> + struct bpf_prog *prog;
> +};
> +
> +static int bpf_raw_tracepoint_release(struct inode *inode, struct file *filp)
> +{
> + struct bpf_raw_tracepoint *raw_tp = filp->private_data;
> +
> + if (raw_tp->prog) {
> + bpf_probe_unregister(raw_tp->tp, raw_tp->prog);
> + bpf_prog_put(raw_tp->prog);
> + }
> + kfree(raw_tp);
> + return 0;
> +}
> +
> +static const struct file_operations bpf_raw_tp_fops = {
> + .release = bpf_raw_tracepoint_release,
> + .read = bpf_dummy_read,
> + .write = bpf_dummy_write,
> +};
> +
> +#define BPF_RAW_TRACEPOINT_OPEN_LAST_FIELD raw_tracepoint.prog_fd
> +
> +static int bpf_raw_tracepoint_open(const union bpf_attr *attr)
> +{
> + struct bpf_raw_tracepoint *raw_tp;
> + struct tracepoint *tp;
> + struct bpf_prog *prog;
> + char tp_name[128];
> + int tp_fd, err;
> +
> + if (strncpy_from_user(tp_name, u64_to_user_ptr(attr->raw_tracepoint.name),
> + sizeof(tp_name) - 1) < 0)
> + return -EFAULT;
> + tp_name[sizeof(tp_name) - 1] = 0;
> +
> + tp = kernel_tracepoint_find_by_name(tp_name);
> + if (!tp)
> + return -ENOENT;
> +
> + raw_tp = kmalloc(sizeof(*raw_tp), GFP_USER | __GFP_ZERO);
Please use kzalloc(), instead of open coding the "__GPF_ZERO"
> + if (!raw_tp)
> + return -ENOMEM;
> + raw_tp->tp = tp;
> +
> + prog = bpf_prog_get_type(attr->raw_tracepoint.prog_fd,
> + BPF_PROG_TYPE_RAW_TRACEPOINT);
> + if (IS_ERR(prog)) {
> + err = PTR_ERR(prog);
> + goto out_free_tp;
> + }
> +
> + err = bpf_probe_register(raw_tp->tp, prog);
> + if (err)
> + goto out_put_prog;
> +
> + raw_tp->prog = prog;
> + tp_fd = anon_inode_getfd("bpf-raw-tracepoint", &bpf_raw_tp_fops, raw_tp,
> + O_CLOEXEC);
> + if (tp_fd < 0) {
> + bpf_probe_unregister(raw_tp->tp, prog);
> + err = tp_fd;
> + goto out_put_prog;
> + }
> + return tp_fd;
> +
> +out_put_prog:
> + bpf_prog_put(prog);
> +out_free_tp:
> + kfree(raw_tp);
> + return err;
> +}
> +
> #ifdef CONFIG_CGROUP_BPF
>
> #define BPF_PROG_ATTACH_LAST_FIELD attach_flags
> @@ -1921,6 +1996,9 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz
> case BPF_OBJ_GET_INFO_BY_FD:
> err = bpf_obj_get_info_by_fd(&attr, uattr);
> break;
> + case BPF_RAW_TRACEPOINT_OPEN:
> + err = bpf_raw_tracepoint_open(&attr);
> + break;
> default:
> err = -EINVAL;
> break;
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index c634e093951f..00e86aa11360 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -723,6 +723,86 @@ const struct bpf_verifier_ops tracepoint_verifier_ops = {
> const struct bpf_prog_ops tracepoint_prog_ops = {
> };
>
> +/*
> + * bpf_raw_tp_regs are separate from bpf_pt_regs used from skb/xdp
> + * to avoid potential recursive reuse issue when/if tracepoints are added
> + * inside bpf_*_event_output and/or bpf_get_stack_id
> + */
> +static DEFINE_PER_CPU(struct pt_regs, bpf_raw_tp_regs);
> +BPF_CALL_5(bpf_perf_event_output_raw_tp, struct bpf_raw_tracepoint_args *, args,
> + struct bpf_map *, map, u64, flags, void *, data, u64, size)
> +{
> + struct pt_regs *regs = this_cpu_ptr(&bpf_raw_tp_regs);
> +
> + perf_fetch_caller_regs(regs);
> + return ____bpf_perf_event_output(regs, map, flags, data, size);
> +}
> +
> +static const struct bpf_func_proto bpf_perf_event_output_proto_raw_tp = {
> + .func = bpf_perf_event_output_raw_tp,
> + .gpl_only = true,
> + .ret_type = RET_INTEGER,
> + .arg1_type = ARG_PTR_TO_CTX,
> + .arg2_type = ARG_CONST_MAP_PTR,
> + .arg3_type = ARG_ANYTHING,
> + .arg4_type = ARG_PTR_TO_MEM,
> + .arg5_type = ARG_CONST_SIZE_OR_ZERO,
> +};
> +
> +BPF_CALL_3(bpf_get_stackid_raw_tp, struct bpf_raw_tracepoint_args *, args,
> + struct bpf_map *, map, u64, flags)
> +{
> + struct pt_regs *regs = this_cpu_ptr(&bpf_raw_tp_regs);
> +
> + perf_fetch_caller_regs(regs);
> + /* similar to bpf_perf_event_output_tp, but pt_regs fetched differently */
> + return bpf_get_stackid((unsigned long) regs, (unsigned long) map,
> + flags, 0, 0);
> +}
> +
> +static const struct bpf_func_proto bpf_get_stackid_proto_raw_tp = {
> + .func = bpf_get_stackid_raw_tp,
> + .gpl_only = true,
> + .ret_type = RET_INTEGER,
> + .arg1_type = ARG_PTR_TO_CTX,
> + .arg2_type = ARG_CONST_MAP_PTR,
> + .arg3_type = ARG_ANYTHING,
> +};
> +
> +static const struct bpf_func_proto *raw_tp_prog_func_proto(enum bpf_func_id func_id)
> +{
> + switch (func_id) {
> + case BPF_FUNC_perf_event_output:
> + return &bpf_perf_event_output_proto_raw_tp;
> + case BPF_FUNC_get_stackid:
> + return &bpf_get_stackid_proto_raw_tp;
> + default:
> + return tracing_func_proto(func_id);
> + }
> +}
> +
> +static bool raw_tp_prog_is_valid_access(int off, int size,
> + enum bpf_access_type type,
> + struct bpf_insn_access_aux *info)
> +{
> + /* largest tracepoint in the kernel has 12 args */
> + if (off < 0 || off >= sizeof(__u64) * 12)
> + return false;
> + if (type != BPF_READ)
> + return false;
> + if (off % size != 0)
> + return false;
> + return true;
> +}
> +
> +const struct bpf_verifier_ops raw_tracepoint_verifier_ops = {
> + .get_func_proto = raw_tp_prog_func_proto,
> + .is_valid_access = raw_tp_prog_is_valid_access,
> +};
> +
> +const struct bpf_prog_ops raw_tracepoint_prog_ops = {
> +};
> +
> static bool pe_prog_is_valid_access(int off, int size, enum bpf_access_type type,
> struct bpf_insn_access_aux *info)
> {
> @@ -896,3 +976,111 @@ int perf_event_query_prog_array(struct perf_event *event, void __user *info)
>
> return ret;
> }
> +
> +static __always_inline
> +void __bpf_trace_run(struct bpf_prog *prog, u64 *args)
> +{
> + rcu_read_lock();
> + preempt_disable();
> + (void) BPF_PROG_RUN(prog, args);
> + preempt_enable();
> + rcu_read_unlock();
> +}
> +
Could you add some comments here to explain what the below is doing.
> +#define UNPACK(...) __VA_ARGS__
> +#define REPEAT_1(FN, DL, X, ...) FN(X)
> +#define REPEAT_2(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_1(FN, DL, __VA_ARGS__)
> +#define REPEAT_3(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_2(FN, DL, __VA_ARGS__)
> +#define REPEAT_4(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_3(FN, DL, __VA_ARGS__)
> +#define REPEAT_5(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_4(FN, DL, __VA_ARGS__)
> +#define REPEAT_6(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_5(FN, DL, __VA_ARGS__)
> +#define REPEAT_7(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_6(FN, DL, __VA_ARGS__)
> +#define REPEAT_8(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_7(FN, DL, __VA_ARGS__)
> +#define REPEAT_9(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_8(FN, DL, __VA_ARGS__)
> +#define REPEAT_10(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_9(FN, DL, __VA_ARGS__)
> +#define REPEAT_11(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_10(FN, DL, __VA_ARGS__)
> +#define REPEAT_12(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_11(FN, DL, __VA_ARGS__)
> +#define REPEAT(X, FN, DL, ...) REPEAT_##X(FN, DL, __VA_ARGS__)
> +
> +#define SARG(X) u64 arg##X
> +#define COPY(X) args[X] = arg##X
> +
> +#define __DL_COM (,)
> +#define __DL_SEM (;)
> +
> +#define __SEQ_0_11 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11
> +
> +#define BPF_TRACE_DEFN_x(x) \
> + void bpf_trace_run##x(struct bpf_prog *prog, \
> + REPEAT(x, SARG, __DL_COM, __SEQ_0_11)) \
> + { \
> + u64 args[x]; \
> + REPEAT(x, COPY, __DL_SEM, __SEQ_0_11); \
> + __bpf_trace_run(prog, args); \
> + } \
> + EXPORT_SYMBOL_GPL(bpf_trace_run##x)
> +BPF_TRACE_DEFN_x(1);
> +BPF_TRACE_DEFN_x(2);
> +BPF_TRACE_DEFN_x(3);
> +BPF_TRACE_DEFN_x(4);
> +BPF_TRACE_DEFN_x(5);
> +BPF_TRACE_DEFN_x(6);
> +BPF_TRACE_DEFN_x(7);
> +BPF_TRACE_DEFN_x(8);
> +BPF_TRACE_DEFN_x(9);
> +BPF_TRACE_DEFN_x(10);
> +BPF_TRACE_DEFN_x(11);
> +BPF_TRACE_DEFN_x(12);
> +
> +static int __bpf_probe_register(struct tracepoint *tp, struct bpf_prog *prog)
> +{
> + unsigned long addr;
> + char buf[128];
> +
> + /*
> + * check that program doesn't access arguments beyond what's
> + * available in this tracepoint
> + */
> + if (prog->aux->max_ctx_offset > tp->num_args * sizeof(u64))
> + return -EINVAL;
> +
> + snprintf(buf, sizeof(buf), "__bpf_trace_%s", tp->name);
> + addr = kallsyms_lookup_name(buf);
> + if (!addr)
> + return -ENOENT;
> +
> + return tracepoint_probe_register(tp, (void *)addr, prog);
You are putting in a hell of a lot of trust with kallsyms returning
properly. I can see this being very fragile. This is calling a function
based on the result of kallsyms. I'm sure the security folks would love
this.
There's a few things to make this a bit more robust. One is to add a
table that points to all __bpf_trace_* functions, and verify that the
result from kallsyms is in that table.
Honestly, I think this is too much of a short cut and a hack. I know
you want to keep it "simple" and save space, but you really should do
it the same way ftrace and perf do it. That is, create a section and
have all tracepoints create a structure that holds a pointer to the
tracepoint and to the bpf probe function. Then you don't even need the
kernel_tracepoint_find_by_name(), you just iterate over your table and
you get the tracepoint and the bpf function associated to it.
Relying on kallsyms to return an address to execute is just way too
extreme and fragile for my liking.
-- Steve
> +}
> +
> +int bpf_probe_register(struct tracepoint *tp, struct bpf_prog *prog)
> +{
> + int err;
> +
> + mutex_lock(&bpf_event_mutex);
> + err = __bpf_probe_register(tp, prog);
> + mutex_unlock(&bpf_event_mutex);
> + return err;
> +}
> +
> +static int __bpf_probe_unregister(struct tracepoint *tp, struct bpf_prog *prog)
> +{
> + unsigned long addr;
> + char buf[128];
> +
> + snprintf(buf, sizeof(buf), "__bpf_trace_%s", tp->name);
> + addr = kallsyms_lookup_name(buf);
> + if (!addr)
> + return -ENOENT;
> +
> + return tracepoint_probe_unregister(tp, (void *)addr, prog);
> +}
> +
> +int bpf_probe_unregister(struct tracepoint *tp, struct bpf_prog *prog)
> +{
> + int err;
> +
> + mutex_lock(&bpf_event_mutex);
> + err = __bpf_probe_unregister(tp, prog);
> + mutex_unlock(&bpf_event_mutex);
> + return err;
> +}
^ permalink raw reply
* Re: [PATCH v6 bpf-next 08/11] bpf: introduce BPF_RAW_TRACEPOINT
From: Steven Rostedt @ 2018-03-27 17:11 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: davem, daniel, torvalds, peterz, netdev, kernel-team, linux-api,
Mathieu Desnoyers, Kees Cook
In-Reply-To: <20180327130211.284c8924@gandalf.local.home>
On Tue, 27 Mar 2018 13:02:11 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> Honestly, I think this is too much of a short cut and a hack. I know
> you want to keep it "simple" and save space, but you really should do
> it the same way ftrace and perf do it. That is, create a section and
> have all tracepoints create a structure that holds a pointer to the
> tracepoint and to the bpf probe function. Then you don't even need the
> kernel_tracepoint_find_by_name(), you just iterate over your table and
> you get the tracepoint and the bpf function associated to it.
Also, if you do it the perf/ftrace way, you get support for module
tracepoints pretty much for free. Which would include tracepoints in
networking code that is loaded by a module.
-- Steve
^ permalink raw reply
* Re: [RFC PATCH for 4.15 00/24] Restartable sequences and CPU op vector v11
From: Mathieu Desnoyers @ 2018-03-27 18:15 UTC (permalink / raw)
To: Paul Turner
Cc: Andy Lutomirski, Linus Torvalds, Peter Zijlstra, Paul E. McKenney,
Boqun Feng, Dave Watson, linux-kernel, linux-api, Andrew Morton,
Russell King, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
Andrew Hunter, Andi Kleen, Chris Lameter, Ben Maurer, rostedt,
Josh Triplett, Catalin Marinas, Will
In-Reply-To: <CAPM31RJJhtF3RR_Xf+kim-aas59eQ75AUJuu1qzBU_v0mE6sqQ@mail.gmail.com>
Hi Paul!
I guess stuff like Spectre/Meltdown can turn 1-2 days into months. ;-)
I did not want to distract you too much from that work, but you'll notice
I've sent an updated patch series against 4.16-rc7, aiming at 4.17 [1]
(it should be in your inbox).
I would really appreciate if you can find time to provide feedback on
that version.
Congratulations on the wedding!
Thanks,
Mathieu
[1] https://lkml.kernel.org/r/20180327160542.28457-1-mathieu.desnoyers@efficios.com
----- On Nov 14, 2017, at 4:32 PM, Paul Turner pjt@google.com wrote:
> I have some comments that apply to many of the threads.
> I've been fully occupied with a wedding and a security issue; but I'm
> about to be free to spend the majority of my time on RSEQ things.
> I was sorely hoping that day would be today. But it's looking like
> I'm still a day or two from being free for this.
> Thank you for the extensive clean-ups and user-side development. I
> have some updates on these topics also.
>
> - Paul
>
> On Tue, Nov 14, 2017 at 1:15 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> On Tue, Nov 14, 2017 at 1:08 PM, Linus Torvalds
>> <torvalds@linux-foundation.org> wrote:
>>> On Tue, Nov 14, 2017 at 12:03 PM, Mathieu Desnoyers
>>> <mathieu.desnoyers@efficios.com> wrote:
>>>> Here is the last RFC round of the updated rseq patchset containing:
>>>
>>> Andy? You were the one with concerns here and said you'd have
>>> something else ready for comparison.
>>>
>>
>> I had a long discussion with Mathieu and KS and I think that this is a
>> good compromise. I haven't reviewed the series all that carefully,
>> but I think the idea is sound.
>>
>> Basically, event_counter is gone (to be re-added in a later kernel if
>> it really ends up being necessary, but it looks like it may primarily
>> be a temptation to write subtly incorrect user code and to see
>> scheduling details that shouldn't be readily exposed rather than a
>> genuinely useful feature) and the versioning mechanism for the asm
>> critical section bit is improved. My crazy proposal should be doable
>> on top of this if there's demand and if anyone wants to write the
>> gnarly code involved.
>>
>> IOW no objection from me as long as those changes were made, which I
> > *think* they were. Mathieu?
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [PATCH v6 bpf-next 08/11] bpf: introduce BPF_RAW_TRACEPOINT
From: Alexei Starovoitov @ 2018-03-27 18:45 UTC (permalink / raw)
To: Steven Rostedt
Cc: davem, daniel, torvalds, peterz, netdev, kernel-team, linux-api,
Mathieu Desnoyers, Kees Cook
In-Reply-To: <20180327130211.284c8924@gandalf.local.home>
On 3/27/18 10:02 AM, Steven Rostedt wrote:
> On Mon, 26 Mar 2018 19:47:03 -0700
> Alexei Starovoitov <ast@fb.com> wrote:
>
>
>> Ctrl-C of tracing daemon or cmdline tool that uses this feature
>> will automatically detach bpf program, unload it and
>> unregister tracepoint probe.
>>
>> On the kernel side for_each_kernel_tracepoint() is used
>
> You need to update the change log to state
> kernel_tracepoint_find_by_name().
ahh. right. will do.
>> +#undef DECLARE_EVENT_CLASS
>> +#define DECLARE_EVENT_CLASS(call, proto, args, tstruct, assign, print) \
>> +/* no 'static' here. The bpf probe functions are global */ \
>> +notrace void \
>
> I'm curious to why you have notrace here? Since it is separate from
> perf and ftrace, for debugging purposes, it could be useful to allow
> function tracing to this function.
To avoid unnecessary overhead. And I don't think it's useful to trace
them. They're tiny jump functions of one or two instructions.
Really no point wasting mentry on them.
>> +static int bpf_raw_tracepoint_open(const union bpf_attr *attr)
>> +{
>> + struct bpf_raw_tracepoint *raw_tp;
>> + struct tracepoint *tp;
>> + struct bpf_prog *prog;
>> + char tp_name[128];
>> + int tp_fd, err;
>> +
>> + if (strncpy_from_user(tp_name, u64_to_user_ptr(attr->raw_tracepoint.name),
>> + sizeof(tp_name) - 1) < 0)
>> + return -EFAULT;
>> + tp_name[sizeof(tp_name) - 1] = 0;
>> +
>> + tp = kernel_tracepoint_find_by_name(tp_name);
>> + if (!tp)
>> + return -ENOENT;
>> +
>> + raw_tp = kmalloc(sizeof(*raw_tp), GFP_USER | __GFP_ZERO);
>
> Please use kzalloc(), instead of open coding the "__GPF_ZERO"
right. will do
>
> Could you add some comments here to explain what the below is doing.
To write a proper comment I need to understand it and I don't.
That's the reason why I didn't put in in common header,
because it would require proper comment on what it is and
how one can use it.
I'm expecting Daniel to follow up on this.
>> +#define UNPACK(...) __VA_ARGS__
>> +#define REPEAT_1(FN, DL, X, ...) FN(X)
>> +#define REPEAT_2(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_1(FN, DL, __VA_ARGS__)
>> +#define REPEAT_3(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_2(FN, DL, __VA_ARGS__)
>> +#define REPEAT_4(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_3(FN, DL, __VA_ARGS__)
>> +#define REPEAT_5(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_4(FN, DL, __VA_ARGS__)
>> +#define REPEAT_6(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_5(FN, DL, __VA_ARGS__)
>> +#define REPEAT_7(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_6(FN, DL, __VA_ARGS__)
>> +#define REPEAT_8(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_7(FN, DL, __VA_ARGS__)
>> +#define REPEAT_9(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_8(FN, DL, __VA_ARGS__)
>> +#define REPEAT_10(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_9(FN, DL, __VA_ARGS__)
>> +#define REPEAT_11(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_10(FN, DL, __VA_ARGS__)
>> +#define REPEAT_12(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_11(FN, DL, __VA_ARGS__)
>> +#define REPEAT(X, FN, DL, ...) REPEAT_##X(FN, DL, __VA_ARGS__)
>> +
>> +
>> + snprintf(buf, sizeof(buf), "__bpf_trace_%s", tp->name);
>> + addr = kallsyms_lookup_name(buf);
>> + if (!addr)
>> + return -ENOENT;
>> +
>> + return tracepoint_probe_register(tp, (void *)addr, prog);
>
> You are putting in a hell of a lot of trust with kallsyms returning
> properly. I can see this being very fragile. This is calling a function
> based on the result of kallsyms. I'm sure the security folks would love
> this.
>
> There's a few things to make this a bit more robust. One is to add a
> table that points to all __bpf_trace_* functions, and verify that the
> result from kallsyms is in that table.
>
> Honestly, I think this is too much of a short cut and a hack. I know
> you want to keep it "simple" and save space, but you really should do
> it the same way ftrace and perf do it. That is, create a section and
> have all tracepoints create a structure that holds a pointer to the
> tracepoint and to the bpf probe function. Then you don't even need the
> kernel_tracepoint_find_by_name(), you just iterate over your table and
> you get the tracepoint and the bpf function associated to it.
>
> Relying on kallsyms to return an address to execute is just way too
> extreme and fragile for my liking.
Wasting extra 8bytes * number_of_tracepoints just for lack of trust
in kallsyms doesn't sound like good trade off to me.
If kallsyms are inaccurate all sorts of things will break:
kprobes, livepatch, etc.
I'd rather suggest for ftrace to use kallsyms approach as well
and reduce memory footprint.
^ permalink raw reply
* Re: [PATCH v6 bpf-next 08/11] bpf: introduce BPF_RAW_TRACEPOINT
From: Steven Rostedt @ 2018-03-27 18:58 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: davem, daniel, torvalds, peterz, netdev, kernel-team, linux-api,
Mathieu Desnoyers, Kees Cook
In-Reply-To: <20180327131143.4b83534c@gandalf.local.home>
On Tue, 27 Mar 2018 13:11:43 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> On Tue, 27 Mar 2018 13:02:11 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
>
> > Honestly, I think this is too much of a short cut and a hack. I know
> > you want to keep it "simple" and save space, but you really should do
> > it the same way ftrace and perf do it. That is, create a section and
> > have all tracepoints create a structure that holds a pointer to the
> > tracepoint and to the bpf probe function. Then you don't even need the
> > kernel_tracepoint_find_by_name(), you just iterate over your table and
> > you get the tracepoint and the bpf function associated to it.
>
> Also, if you do it the perf/ftrace way, you get support for module
> tracepoints pretty much for free. Which would include tracepoints in
> networking code that is loaded by a module.
This doesn't include module code (but that wouldn't be too hard to set
up), but I compiled and booted this. I didn't test if it works (I
didn't have the way to test bpf here). But this patch applies on top of
this patch (patch 8). You can remove patch 7 and fold this into this
patch. And then you can also make the __bpf_trace_* function static.
This would be much more robust and less error prone.
-- Steve
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 1ab0e520d6fc..4fab7392e237 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -178,6 +178,15 @@
#define TRACE_SYSCALLS()
#endif
+#ifdef CONFIG_BPF_EVENTS
+#define BPF_RAW_TP() . = ALIGN(8); \
+ VMLINUX_SYMBOL(__start__bpf_raw_tp) = .; \
+ KEEP(*(__bpf_raw_tp_map)) \
+ VMLINUX_SYMBOL(__stop__bpf_raw_tp) = .;
+#else
+#define BPF_RAW_TP()
+#endif
+
#ifdef CONFIG_SERIAL_EARLYCON
#define EARLYCON_TABLE() STRUCT_ALIGN(); \
VMLINUX_SYMBOL(__earlycon_table) = .; \
@@ -576,6 +585,7 @@
*(.init.rodata) \
FTRACE_EVENTS() \
TRACE_SYSCALLS() \
+ BPF_RAW_TP() \
KPROBE_BLACKLIST() \
ERROR_INJECT_WHITELIST() \
MEM_DISCARD(init.rodata) \
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 399ebe6f90cf..fb4778c0a248 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -470,8 +470,9 @@ unsigned int trace_call_bpf(struct trace_event_call *call, void *ctx);
int perf_event_attach_bpf_prog(struct perf_event *event, struct bpf_prog *prog);
void perf_event_detach_bpf_prog(struct perf_event *event);
int perf_event_query_prog_array(struct perf_event *event, void __user *info);
-int bpf_probe_register(struct tracepoint *tp, struct bpf_prog *prog);
-int bpf_probe_unregister(struct tracepoint *tp, struct bpf_prog *prog);
+int bpf_probe_register(struct bpf_raw_event_map *btp, struct bpf_prog *prog);
+int bpf_probe_unregister(struct bpf_raw_event_map *btp, struct bpf_prog *prog);
+struct bpf_raw_event_map *bpf_find_raw_tracepoint(const char *name);
#else
static inline unsigned int trace_call_bpf(struct trace_event_call *call, void *ctx)
{
@@ -491,14 +492,18 @@ perf_event_query_prog_array(struct perf_event *event, void __user *info)
{
return -EOPNOTSUPP;
}
-static inline int bpf_probe_register(struct tracepoint *tp, struct bpf_prog *p)
+static inline int bpf_probe_register(struct bpf_raw_event_map *btp, struct bpf_prog *p)
{
return -EOPNOTSUPP;
}
-static inline int bpf_probe_unregister(struct tracepoint *tp, struct bpf_prog *p)
+static inline int bpf_probe_unregister(struct bpf_raw_event_map *btp, struct bpf_prog *p)
{
return -EOPNOTSUPP;
}
+static inline struct bpf_raw_event_map *bpf_find_raw_tracepoint(const char *name)
+{
+ return NULL;
+}
#endif
enum {
diff --git a/include/linux/tracepoint-defs.h b/include/linux/tracepoint-defs.h
index 39a283c61c51..35db8dd48c4c 100644
--- a/include/linux/tracepoint-defs.h
+++ b/include/linux/tracepoint-defs.h
@@ -36,4 +36,9 @@ struct tracepoint {
u32 num_args;
};
+struct bpf_raw_event_map {
+ struct tracepoint *tp;
+ void *bpf_func;
+};
+
#endif
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index f100c63ff19e..6037a2f0108a 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1312,7 +1312,7 @@ static int bpf_obj_get(const union bpf_attr *attr)
}
struct bpf_raw_tracepoint {
- struct tracepoint *tp;
+ struct bpf_raw_event_map *btp;
struct bpf_prog *prog;
};
@@ -1321,7 +1321,7 @@ static int bpf_raw_tracepoint_release(struct inode *inode, struct file *filp)
struct bpf_raw_tracepoint *raw_tp = filp->private_data;
if (raw_tp->prog) {
- bpf_probe_unregister(raw_tp->tp, raw_tp->prog);
+ bpf_probe_unregister(raw_tp->btp, raw_tp->prog);
bpf_prog_put(raw_tp->prog);
}
kfree(raw_tp);
@@ -1339,7 +1339,7 @@ static const struct file_operations bpf_raw_tp_fops = {
static int bpf_raw_tracepoint_open(const union bpf_attr *attr)
{
struct bpf_raw_tracepoint *raw_tp;
- struct tracepoint *tp;
+ struct bpf_raw_event_map *btp;
struct bpf_prog *prog;
char tp_name[128];
int tp_fd, err;
@@ -1349,14 +1349,14 @@ static int bpf_raw_tracepoint_open(const union bpf_attr *attr)
return -EFAULT;
tp_name[sizeof(tp_name) - 1] = 0;
- tp = kernel_tracepoint_find_by_name(tp_name);
- if (!tp)
+ btp = bpf_find_raw_tracepoint(tp_name);
+ if (!btp)
return -ENOENT;
raw_tp = kmalloc(sizeof(*raw_tp), GFP_USER | __GFP_ZERO);
if (!raw_tp)
return -ENOMEM;
- raw_tp->tp = tp;
+ raw_tp->btp = btp;
prog = bpf_prog_get_type(attr->raw_tracepoint.prog_fd,
BPF_PROG_TYPE_RAW_TRACEPOINT);
@@ -1365,7 +1365,7 @@ static int bpf_raw_tracepoint_open(const union bpf_attr *attr)
goto out_free_tp;
}
- err = bpf_probe_register(raw_tp->tp, prog);
+ err = bpf_probe_register(raw_tp->btp, prog);
if (err)
goto out_put_prog;
@@ -1373,7 +1373,7 @@ static int bpf_raw_tracepoint_open(const union bpf_attr *attr)
tp_fd = anon_inode_getfd("bpf-raw-tracepoint", &bpf_raw_tp_fops, raw_tp,
O_CLOEXEC);
if (tp_fd < 0) {
- bpf_probe_unregister(raw_tp->tp, prog);
+ bpf_probe_unregister(raw_tp->btp, prog);
err = tp_fd;
goto out_put_prog;
}
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index eb58ef156d36..e578b173fe1d 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -965,6 +965,19 @@ int perf_event_query_prog_array(struct perf_event *event, void __user *info)
return ret;
}
+extern struct bpf_raw_event_map *__start__bpf_raw_tp[];
+extern struct bpf_raw_event_map *__stop__bpf_raw_tp[];
+
+struct bpf_raw_event_map *bpf_find_raw_tracepoint(const char *name)
+{
+ struct bpf_raw_event_map* const *btp = __start__bpf_raw_tp;
+
+ for (; btp < __stop__bpf_raw_tp; btp++)
+ if (!strcmp((*btp)->tp->name, name))
+ return *btp;
+ return NULL;
+}
+
static __always_inline
void __bpf_trace_run(struct bpf_prog *prog, u64 *args)
{
@@ -1020,10 +1033,9 @@ BPF_TRACE_DEFN_x(10);
BPF_TRACE_DEFN_x(11);
BPF_TRACE_DEFN_x(12);
-static int __bpf_probe_register(struct tracepoint *tp, struct bpf_prog *prog)
+static int __bpf_probe_register(struct bpf_raw_event_map *btp, struct bpf_prog *prog)
{
- unsigned long addr;
- char buf[128];
+ struct tracepoint *tp = btp->tp;
/*
* check that program doesn't access arguments beyond what's
@@ -1032,43 +1044,25 @@ static int __bpf_probe_register(struct tracepoint *tp, struct bpf_prog *prog)
if (prog->aux->max_ctx_offset > tp->num_args * sizeof(u64))
return -EINVAL;
- snprintf(buf, sizeof(buf), "__bpf_trace_%s", tp->name);
- addr = kallsyms_lookup_name(buf);
- if (!addr)
- return -ENOENT;
-
- return tracepoint_probe_register(tp, (void *)addr, prog);
+ return tracepoint_probe_register(tp, (void *)btp->bpf_func, prog);
}
-int bpf_probe_register(struct tracepoint *tp, struct bpf_prog *prog)
+int bpf_probe_register(struct bpf_raw_event_map *btp, struct bpf_prog *prog)
{
int err;
mutex_lock(&bpf_event_mutex);
- err = __bpf_probe_register(tp, prog);
+ err = __bpf_probe_register(btp, prog);
mutex_unlock(&bpf_event_mutex);
return err;
}
-static int __bpf_probe_unregister(struct tracepoint *tp, struct bpf_prog *prog)
-{
- unsigned long addr;
- char buf[128];
-
- snprintf(buf, sizeof(buf), "__bpf_trace_%s", tp->name);
- addr = kallsyms_lookup_name(buf);
- if (!addr)
- return -ENOENT;
-
- return tracepoint_probe_unregister(tp, (void *)addr, prog);
-}
-
-int bpf_probe_unregister(struct tracepoint *tp, struct bpf_prog *prog)
+int bpf_probe_unregister(struct bpf_raw_event_map *btp, struct bpf_prog *prog)
{
int err;
mutex_lock(&bpf_event_mutex);
- err = __bpf_probe_unregister(tp, prog);
+ err = tracepoint_probe_unregister(btp->tp, (void *)btp->bpf_func, prog);
mutex_unlock(&bpf_event_mutex);
return err;
}
^ permalink raw reply related
* Re: [PATCH v6 bpf-next 08/11] bpf: introduce BPF_RAW_TRACEPOINT
From: Steven Rostedt @ 2018-03-27 19:00 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: davem, daniel, torvalds, peterz, netdev, kernel-team, linux-api,
Mathieu Desnoyers, Kees Cook, Thomas Gleixner, Ingo Molnar
In-Reply-To: <f071b59d-5d67-0eb6-c5b1-68094b0a5118@fb.com>
On Tue, 27 Mar 2018 11:45:34 -0700
Alexei Starovoitov <ast@fb.com> wrote:
> >> +
> >> + snprintf(buf, sizeof(buf), "__bpf_trace_%s", tp->name);
> >> + addr = kallsyms_lookup_name(buf);
> >> + if (!addr)
> >> + return -ENOENT;
> >> +
> >> + return tracepoint_probe_register(tp, (void *)addr, prog);
> >
> > You are putting in a hell of a lot of trust with kallsyms returning
> > properly. I can see this being very fragile. This is calling a function
> > based on the result of kallsyms. I'm sure the security folks would love
> > this.
> >
> > There's a few things to make this a bit more robust. One is to add a
> > table that points to all __bpf_trace_* functions, and verify that the
> > result from kallsyms is in that table.
> >
> > Honestly, I think this is too much of a short cut and a hack. I know
> > you want to keep it "simple" and save space, but you really should do
> > it the same way ftrace and perf do it. That is, create a section and
> > have all tracepoints create a structure that holds a pointer to the
> > tracepoint and to the bpf probe function. Then you don't even need the
> > kernel_tracepoint_find_by_name(), you just iterate over your table and
> > you get the tracepoint and the bpf function associated to it.
> >
> > Relying on kallsyms to return an address to execute is just way too
> > extreme and fragile for my liking.
>
> Wasting extra 8bytes * number_of_tracepoints just for lack of trust
> in kallsyms doesn't sound like good trade off to me.
> If kallsyms are inaccurate all sorts of things will break:
> kprobes, livepatch, etc.
> I'd rather suggest for ftrace to use kallsyms approach as well
> and reduce memory footprint.
If Linus, Thomas, Peter, Ingo, and the security folks trust kallsyms to
return a valid function pointer from a name, then sure, we can try
going that way.
-- Steve
^ permalink raw reply
* Re: [PATCH v6 bpf-next 08/11] bpf: introduce BPF_RAW_TRACEPOINT
From: Steven Rostedt @ 2018-03-27 19:07 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: davem, daniel, torvalds, peterz, netdev, kernel-team, linux-api,
Mathieu Desnoyers, Kees Cook, Thomas Gleixner, Ingo Molnar
In-Reply-To: <20180327150041.3d86e16e@gandalf.local.home>
On Tue, 27 Mar 2018 15:00:41 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> > Wasting extra 8bytes * number_of_tracepoints just for lack of trust
> > in kallsyms doesn't sound like good trade off to me.
> > If kallsyms are inaccurate all sorts of things will break:
> > kprobes, livepatch, etc.
And if kallsyms breaks, these will break by failing to attach, or some
other benign error. Ftrace uses kallsyms to find functions too, but it
only enables functions based on the result, it doesn't use the result
for anything but to compare it to what it already knows.
This is a first that I know of to trust that kallsyms returns something
that you expect to execute with no other validation. It may be valid,
but it also makes me very nervous too. If others are fine with such an
approach, then OK, we can enter a new chapter of development where we
can use kallsyms to find the functions we want to call and use it.
-- Steve
^ permalink raw reply
* Re: [RFC PATCH for 4.17 00/21] Restartable sequences and CPU op vector
From: Peter Zijlstra @ 2018-03-27 19:09 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Paul E . McKenney, Boqun Feng, Andy Lutomirski, Dave Watson,
linux-kernel, linux-api, Paul Turner, Andrew Morton, Russell King,
Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Andrew Hunter,
Andi Kleen, Chris Lameter, Ben Maurer, Steven Rostedt,
Josh Triplett, Linus Torvalds, Catalin Marinas, Will Deacon
In-Reply-To: <20180327160542.28457-1-mathieu.desnoyers@efficios.com>
On Tue, Mar 27, 2018 at 12:05:21PM -0400, Mathieu Desnoyers wrote:
> Hi,
>
> I'm respinning this series for another RFC round. It is based on the
> v4.16-rc7 tag. I am now targeting the 4.17 merge window.
I'll go over the thing again in more detail, but I'm basically ok with
rseq, but I hate that cpu_opv thing with a passion (as you well know).
^ permalink raw reply
* Re: [PATCH v6 bpf-next 08/11] bpf: introduce BPF_RAW_TRACEPOINT
From: Steven Rostedt @ 2018-03-27 19:10 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: davem, daniel, torvalds, peterz, netdev, kernel-team, linux-api,
Mathieu Desnoyers, Kees Cook, Thomas Gleixner, Ingo Molnar,
Andrew Morton
In-Reply-To: <20180327150041.3d86e16e@gandalf.local.home>
[ Added Andrew Morton too ]
On Tue, 27 Mar 2018 15:00:41 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> On Tue, 27 Mar 2018 11:45:34 -0700
> Alexei Starovoitov <ast@fb.com> wrote:
>
> > >> +
> > >> + snprintf(buf, sizeof(buf), "__bpf_trace_%s", tp->name);
> > >> + addr = kallsyms_lookup_name(buf);
> > >> + if (!addr)
> > >> + return -ENOENT;
> > >> +
> > >> + return tracepoint_probe_register(tp, (void *)addr, prog);
> > >
> > > You are putting in a hell of a lot of trust with kallsyms returning
> > > properly. I can see this being very fragile. This is calling a function
> > > based on the result of kallsyms. I'm sure the security folks would love
> > > this.
> > >
> > > There's a few things to make this a bit more robust. One is to add a
> > > table that points to all __bpf_trace_* functions, and verify that the
> > > result from kallsyms is in that table.
> > >
> > > Honestly, I think this is too much of a short cut and a hack. I know
> > > you want to keep it "simple" and save space, but you really should do
> > > it the same way ftrace and perf do it. That is, create a section and
> > > have all tracepoints create a structure that holds a pointer to the
> > > tracepoint and to the bpf probe function. Then you don't even need the
> > > kernel_tracepoint_find_by_name(), you just iterate over your table and
> > > you get the tracepoint and the bpf function associated to it.
> > >
> > > Relying on kallsyms to return an address to execute is just way too
> > > extreme and fragile for my liking.
> >
> > Wasting extra 8bytes * number_of_tracepoints just for lack of trust
> > in kallsyms doesn't sound like good trade off to me.
> > If kallsyms are inaccurate all sorts of things will break:
> > kprobes, livepatch, etc.
> > I'd rather suggest for ftrace to use kallsyms approach as well
> > and reduce memory footprint.
>
> If Linus, Thomas, Peter, Ingo, and the security folks trust kallsyms to
> return a valid function pointer from a name, then sure, we can try
> going that way.
I would like an ack from Linus and/or Andrew before we go further down
this road.
-- Steve
^ permalink raw reply
* Re: [PATCH v6 bpf-next 08/11] bpf: introduce BPF_RAW_TRACEPOINT
From: Mathieu Desnoyers @ 2018-03-27 19:10 UTC (permalink / raw)
To: rostedt
Cc: Alexei Starovoitov, David S. Miller, Daniel Borkmann,
Linus Torvalds, Peter Zijlstra, netdev, kernel-team, linux-api,
Kees Cook, Thomas Gleixner, Ingo Molnar
In-Reply-To: <20180327150041.3d86e16e@gandalf.local.home>
----- On Mar 27, 2018, at 3:00 PM, rostedt rostedt@goodmis.org wrote:
> On Tue, 27 Mar 2018 11:45:34 -0700
> Alexei Starovoitov <ast@fb.com> wrote:
>
>> >> +
>> >> + snprintf(buf, sizeof(buf), "__bpf_trace_%s", tp->name);
>> >> + addr = kallsyms_lookup_name(buf);
>> >> + if (!addr)
>> >> + return -ENOENT;
>> >> +
>> >> + return tracepoint_probe_register(tp, (void *)addr, prog);
>> >
>> > You are putting in a hell of a lot of trust with kallsyms returning
>> > properly. I can see this being very fragile. This is calling a function
>> > based on the result of kallsyms. I'm sure the security folks would love
>> > this.
>> >
>> > There's a few things to make this a bit more robust. One is to add a
>> > table that points to all __bpf_trace_* functions, and verify that the
>> > result from kallsyms is in that table.
>> >
>> > Honestly, I think this is too much of a short cut and a hack. I know
>> > you want to keep it "simple" and save space, but you really should do
>> > it the same way ftrace and perf do it. That is, create a section and
>> > have all tracepoints create a structure that holds a pointer to the
>> > tracepoint and to the bpf probe function. Then you don't even need the
>> > kernel_tracepoint_find_by_name(), you just iterate over your table and
>> > you get the tracepoint and the bpf function associated to it.
>> >
>> > Relying on kallsyms to return an address to execute is just way too
>> > extreme and fragile for my liking.
>>
>> Wasting extra 8bytes * number_of_tracepoints just for lack of trust
>> in kallsyms doesn't sound like good trade off to me.
>> If kallsyms are inaccurate all sorts of things will break:
>> kprobes, livepatch, etc.
>> I'd rather suggest for ftrace to use kallsyms approach as well
>> and reduce memory footprint.
>
> If Linus, Thomas, Peter, Ingo, and the security folks trust kallsyms to
> return a valid function pointer from a name, then sure, we can try
> going that way.
This will crash on ARM Thumb2 kernels. Also, how is this expected to
work on PowerPC ABIv1 without KALLSYMS_ALL ?
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [PATCH v2 bpf-next] bpf, tracing: unbreak lttng
From: Steven Rostedt @ 2018-03-27 19:35 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Alexei Starovoitov, davem, torvalds, peterz, mathieu.desnoyers,
netdev, kernel-team, linux-api
In-Reply-To: <1260578f-4a29-2c72-d81c-3f6e0588bbee@iogearbox.net>
On Tue, 27 Mar 2018 11:39:19 +0200
Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 03/27/2018 02:07 AM, Steven Rostedt wrote:
> > On Mon, 26 Mar 2018 16:02:20 -0700
> > Alexei Starovoitov <ast@kernel.org> wrote:
> >
> >> for_each_kernel_tracepoint() is used by out-of-tree lttng module
> >> and therefore cannot be changed.
> >
> > This is false and misleading. NACK.
>
> Steven, while I really don't care about this particular function, you wrote
> a few emails ago, quote:
>
> Look, the tracepoint code was written by Mathieu for LTTng, and perf
> and ftrace were able to benefit because of it, as well as your bpf
> code. For this, we agreed to keep this function around for his use,
> as its the only thing he requires. Everyone has been fine with that.
> [...] Having that function for LTTng does not hurt us. And I will NACK
> removing it.
>
> So later saying that "this is false and misleading" is kind of misleading
> by itself. ;-) Anyway, it would probably make sense to add a comment to
> for_each_kernel_tracepoint() that this is used by LTTng so that people
> don't accidentally remove it due to missing in-tree user. I would think
> some sort of clarification/background in a comment or such would have
> avoided the whole confusion and resulting discussion around this in the
> first place.
I agree, a comment should be added. But the function can still be
modified, which is what I meant here by being misleading.
>
> Btw, in networking land, as soon as there is no in-tree user for a particular
> kernel function, it will get ripped out, no matter what. Given this is also
> the typical convention in the kernel, it may have caused some confusion with
> above preference.
This is usually the case with me too. This came from Mathieu doing a
lot of work to help perf and ftrace, but keep this for him to maintain
LTTng. This was the solution to a long drawn out flame war. Yes,
there's a lot of history behind that function, and I agree that we
should comment the history behind it.
>
> Anyway, given v6 is out now, I've tossed the old series from bpf-next tree.
> So I hope we can all move on with some more constructive discussion. :-)
Yes, which we are doing around the kallsyms part. ;-)
-- Steve
^ permalink raw reply
* Re: [PATCH v6 bpf-next 08/11] bpf: introduce BPF_RAW_TRACEPOINT
From: Steven Rostedt @ 2018-03-27 21:04 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: davem, daniel, torvalds, peterz, netdev, kernel-team, linux-api,
Mathieu Desnoyers, Kees Cook
In-Reply-To: <20180327145824.602dfdec@gandalf.local.home>
On Tue, 27 Mar 2018 14:58:24 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> +extern struct bpf_raw_event_map *__start__bpf_raw_tp[];
> +extern struct bpf_raw_event_map *__stop__bpf_raw_tp[];
> +
> +struct bpf_raw_event_map *bpf_find_raw_tracepoint(const char *name)
> +{
> + struct bpf_raw_event_map* const *btp = __start__bpf_raw_tp;
> +
> + for (; btp < __stop__bpf_raw_tp; btp++)
> + if (!strcmp((*btp)->tp->name, name))
> + return *btp;
> + return NULL;
> +}
> +
OK, this part is broken, and for some reason it didn't include my
changes to bpf_probe.h. I also tested this without setting BPF_EVENTS,
so I wasn't actually testing it.
I added a test in event_trace_init() to make sure that it worked:
(Not included in the patch below)
{
struct bpf_raw_event_map *btp;
btp = bpf_find_raw_tracepoint("sched_switch");
if (btp)
printk("found BPF_RAW_TRACEPOINT: %s %pS\n",
btp->tp->name, btp->bpf_func);
else
printk("COULD NOT FIND BPF_RAW_TRACEPOINT\n");
}
And it found the tracepoint.
Here's take two....
You can add my: Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
-- Steve
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 1ab0e520d6fc..4fab7392e237 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -178,6 +178,15 @@
#define TRACE_SYSCALLS()
#endif
+#ifdef CONFIG_BPF_EVENTS
+#define BPF_RAW_TP() . = ALIGN(8); \
+ VMLINUX_SYMBOL(__start__bpf_raw_tp) = .; \
+ KEEP(*(__bpf_raw_tp_map)) \
+ VMLINUX_SYMBOL(__stop__bpf_raw_tp) = .;
+#else
+#define BPF_RAW_TP()
+#endif
+
#ifdef CONFIG_SERIAL_EARLYCON
#define EARLYCON_TABLE() STRUCT_ALIGN(); \
VMLINUX_SYMBOL(__earlycon_table) = .; \
@@ -576,6 +585,7 @@
*(.init.rodata) \
FTRACE_EVENTS() \
TRACE_SYSCALLS() \
+ BPF_RAW_TP() \
KPROBE_BLACKLIST() \
ERROR_INJECT_WHITELIST() \
MEM_DISCARD(init.rodata) \
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 399ebe6f90cf..fb4778c0a248 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -470,8 +470,9 @@ unsigned int trace_call_bpf(struct trace_event_call *call, void *ctx);
int perf_event_attach_bpf_prog(struct perf_event *event, struct bpf_prog *prog);
void perf_event_detach_bpf_prog(struct perf_event *event);
int perf_event_query_prog_array(struct perf_event *event, void __user *info);
-int bpf_probe_register(struct tracepoint *tp, struct bpf_prog *prog);
-int bpf_probe_unregister(struct tracepoint *tp, struct bpf_prog *prog);
+int bpf_probe_register(struct bpf_raw_event_map *btp, struct bpf_prog *prog);
+int bpf_probe_unregister(struct bpf_raw_event_map *btp, struct bpf_prog *prog);
+struct bpf_raw_event_map *bpf_find_raw_tracepoint(const char *name);
#else
static inline unsigned int trace_call_bpf(struct trace_event_call *call, void *ctx)
{
@@ -491,14 +492,18 @@ perf_event_query_prog_array(struct perf_event *event, void __user *info)
{
return -EOPNOTSUPP;
}
-static inline int bpf_probe_register(struct tracepoint *tp, struct bpf_prog *p)
+static inline int bpf_probe_register(struct bpf_raw_event_map *btp, struct bpf_prog *p)
{
return -EOPNOTSUPP;
}
-static inline int bpf_probe_unregister(struct tracepoint *tp, struct bpf_prog *p)
+static inline int bpf_probe_unregister(struct bpf_raw_event_map *btp, struct bpf_prog *p)
{
return -EOPNOTSUPP;
}
+static inline struct bpf_raw_event_map *bpf_find_raw_tracepoint(const char *name)
+{
+ return NULL;
+}
#endif
enum {
diff --git a/include/linux/tracepoint-defs.h b/include/linux/tracepoint-defs.h
index 39a283c61c51..35db8dd48c4c 100644
--- a/include/linux/tracepoint-defs.h
+++ b/include/linux/tracepoint-defs.h
@@ -36,4 +36,9 @@ struct tracepoint {
u32 num_args;
};
+struct bpf_raw_event_map {
+ struct tracepoint *tp;
+ void *bpf_func;
+};
+
#endif
diff --git a/include/trace/bpf_probe.h b/include/trace/bpf_probe.h
index d2cc0663e618..bb8ed2f530ad 100644
--- a/include/trace/bpf_probe.h
+++ b/include/trace/bpf_probe.h
@@ -76,7 +76,13 @@ __bpf_trace_##call(void *__data, proto) \
static inline void bpf_test_probe_##call(void) \
{ \
check_trace_callback_type_##call(__bpf_trace_##template); \
-}
+} \
+static struct bpf_raw_event_map __used \
+ __attribute__((section("__bpf_raw_tp_map"))) \
+__bpf_trace_tp_map_##call= { \
+ .tp = &__tracepoint_##call, \
+ .bpf_func = (void *)__bpf_trace_##template, \
+};
#undef DEFINE_EVENT_PRINT
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index f100c63ff19e..6037a2f0108a 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1312,7 +1312,7 @@ static int bpf_obj_get(const union bpf_attr *attr)
}
struct bpf_raw_tracepoint {
- struct tracepoint *tp;
+ struct bpf_raw_event_map *btp;
struct bpf_prog *prog;
};
@@ -1321,7 +1321,7 @@ static int bpf_raw_tracepoint_release(struct inode *inode, struct file *filp)
struct bpf_raw_tracepoint *raw_tp = filp->private_data;
if (raw_tp->prog) {
- bpf_probe_unregister(raw_tp->tp, raw_tp->prog);
+ bpf_probe_unregister(raw_tp->btp, raw_tp->prog);
bpf_prog_put(raw_tp->prog);
}
kfree(raw_tp);
@@ -1339,7 +1339,7 @@ static const struct file_operations bpf_raw_tp_fops = {
static int bpf_raw_tracepoint_open(const union bpf_attr *attr)
{
struct bpf_raw_tracepoint *raw_tp;
- struct tracepoint *tp;
+ struct bpf_raw_event_map *btp;
struct bpf_prog *prog;
char tp_name[128];
int tp_fd, err;
@@ -1349,14 +1349,14 @@ static int bpf_raw_tracepoint_open(const union bpf_attr *attr)
return -EFAULT;
tp_name[sizeof(tp_name) - 1] = 0;
- tp = kernel_tracepoint_find_by_name(tp_name);
- if (!tp)
+ btp = bpf_find_raw_tracepoint(tp_name);
+ if (!btp)
return -ENOENT;
raw_tp = kmalloc(sizeof(*raw_tp), GFP_USER | __GFP_ZERO);
if (!raw_tp)
return -ENOMEM;
- raw_tp->tp = tp;
+ raw_tp->btp = btp;
prog = bpf_prog_get_type(attr->raw_tracepoint.prog_fd,
BPF_PROG_TYPE_RAW_TRACEPOINT);
@@ -1365,7 +1365,7 @@ static int bpf_raw_tracepoint_open(const union bpf_attr *attr)
goto out_free_tp;
}
- err = bpf_probe_register(raw_tp->tp, prog);
+ err = bpf_probe_register(raw_tp->btp, prog);
if (err)
goto out_put_prog;
@@ -1373,7 +1373,7 @@ static int bpf_raw_tracepoint_open(const union bpf_attr *attr)
tp_fd = anon_inode_getfd("bpf-raw-tracepoint", &bpf_raw_tp_fops, raw_tp,
O_CLOEXEC);
if (tp_fd < 0) {
- bpf_probe_unregister(raw_tp->tp, prog);
+ bpf_probe_unregister(raw_tp->btp, prog);
err = tp_fd;
goto out_put_prog;
}
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index eb58ef156d36..d0975094cff7 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -965,6 +965,22 @@ int perf_event_query_prog_array(struct perf_event *event, void __user *info)
return ret;
}
+extern struct bpf_raw_event_map __start__bpf_raw_tp;
+extern struct bpf_raw_event_map __stop__bpf_raw_tp;
+
+struct bpf_raw_event_map *bpf_find_raw_tracepoint(const char *name)
+{
+ const struct bpf_raw_event_map *btp = &__start__bpf_raw_tp;
+ int i = 0;
+
+ for (; btp < &__stop__bpf_raw_tp; btp++) {
+ i++;
+ if (!strcmp(btp->tp->name, name))
+ return btp;
+ }
+ return NULL;
+}
+
static __always_inline
void __bpf_trace_run(struct bpf_prog *prog, u64 *args)
{
@@ -1020,10 +1036,9 @@ BPF_TRACE_DEFN_x(10);
BPF_TRACE_DEFN_x(11);
BPF_TRACE_DEFN_x(12);
-static int __bpf_probe_register(struct tracepoint *tp, struct bpf_prog *prog)
+static int __bpf_probe_register(struct bpf_raw_event_map *btp, struct bpf_prog *prog)
{
- unsigned long addr;
- char buf[128];
+ struct tracepoint *tp = btp->tp;
/*
* check that program doesn't access arguments beyond what's
@@ -1032,43 +1047,25 @@ static int __bpf_probe_register(struct tracepoint *tp, struct bpf_prog *prog)
if (prog->aux->max_ctx_offset > tp->num_args * sizeof(u64))
return -EINVAL;
- snprintf(buf, sizeof(buf), "__bpf_trace_%s", tp->name);
- addr = kallsyms_lookup_name(buf);
- if (!addr)
- return -ENOENT;
-
- return tracepoint_probe_register(tp, (void *)addr, prog);
+ return tracepoint_probe_register(tp, (void *)btp->bpf_func, prog);
}
-int bpf_probe_register(struct tracepoint *tp, struct bpf_prog *prog)
+int bpf_probe_register(struct bpf_raw_event_map *btp, struct bpf_prog *prog)
{
int err;
mutex_lock(&bpf_event_mutex);
- err = __bpf_probe_register(tp, prog);
+ err = __bpf_probe_register(btp, prog);
mutex_unlock(&bpf_event_mutex);
return err;
}
-static int __bpf_probe_unregister(struct tracepoint *tp, struct bpf_prog *prog)
-{
- unsigned long addr;
- char buf[128];
-
- snprintf(buf, sizeof(buf), "__bpf_trace_%s", tp->name);
- addr = kallsyms_lookup_name(buf);
- if (!addr)
- return -ENOENT;
-
- return tracepoint_probe_unregister(tp, (void *)addr, prog);
-}
-
-int bpf_probe_unregister(struct tracepoint *tp, struct bpf_prog *prog)
+int bpf_probe_unregister(struct bpf_raw_event_map *btp, struct bpf_prog *prog)
{
int err;
mutex_lock(&bpf_event_mutex);
- err = __bpf_probe_unregister(tp, prog);
+ err = tracepoint_probe_unregister(btp->tp, (void *)btp->bpf_func, prog);
mutex_unlock(&bpf_event_mutex);
return err;
}
^ permalink raw reply related
* Re: [PATCH v6 bpf-next 08/11] bpf: introduce BPF_RAW_TRACEPOINT
From: Alexei Starovoitov @ 2018-03-27 22:48 UTC (permalink / raw)
To: Steven Rostedt
Cc: davem, daniel, torvalds, peterz, netdev, kernel-team, linux-api,
Mathieu Desnoyers, Kees Cook
In-Reply-To: <20180327170438.77c0f8fd@gandalf.local.home>
On 3/27/18 2:04 PM, Steven Rostedt wrote:
>
> +#ifdef CONFIG_BPF_EVENTS
> +#define BPF_RAW_TP() . = ALIGN(8); \
> + VMLINUX_SYMBOL(__start__bpf_raw_tp) = .; \
> + KEEP(*(__bpf_raw_tp_map)) \
> + VMLINUX_SYMBOL(__stop__bpf_raw_tp) = .;
that looks to be correct, but something wrong with it.
Can you try your mini test with kasan on ?
I'm seeing this crash:
test_stacktrace_[ 18.760662] start ffffffff84642438 stop ffffffff84644f60
map_raw_tp:PASS:[ 18.761467] i 1 btp->tp cccccccccccccccc
prog_load raw tp[ 18.762064] kasan: CONFIG_KASAN_INLINE enabled
0 nsec
[ 18.762704] kasan: GPF could be caused by NULL-ptr deref or user
memory access
[ 18.765125] general protection fault: 0000 [#1] SMP KASAN PTI
[ 18.765830] Modules linked in:
[ 18.778358] Call Trace:
[ 18.778674] bpf_raw_tracepoint_open.isra.27+0x92/0x380
for some reason the start_bpf_raw_tp is off by 8.
Not sure how it works for you.
(gdb) p &__bpf_trace_tp_map_sys_exit
$10 = (struct bpf_raw_event_map *) 0xffffffff84642440
<__bpf_trace_tp_map_sys_exit>
(gdb) p &__start__bpf_raw_tp
$7 = (<data variable, no debug info> *) 0xffffffff84642438
(gdb) p (void*)(&__start__bpf_raw_tp)+8
$11 = (void *) 0xffffffff84642440 <__bpf_trace_tp_map_sys_exit>
^ permalink raw reply
* Re: [PATCH v6 bpf-next 08/11] bpf: introduce BPF_RAW_TRACEPOINT
From: Mathieu Desnoyers @ 2018-03-27 23:13 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: rostedt, David S. Miller, Daniel Borkmann, Linus Torvalds,
Peter Zijlstra, netdev, kernel-team, linux-api, Kees Cook
In-Reply-To: <563f7fa0-5fea-00d3-1eb3-fa00d8cf7e29@fb.com>
----- On Mar 27, 2018, at 6:48 PM, Alexei Starovoitov ast@fb.com wrote:
> On 3/27/18 2:04 PM, Steven Rostedt wrote:
>>
>> +#ifdef CONFIG_BPF_EVENTS
>> +#define BPF_RAW_TP() . = ALIGN(8); \
Given that the section consists of a 16-bytes structure elements
on architectures with 8 bytes pointers, this ". = ALIGN(8)" should
be turned into a STRUCT_ALIGN(), especially given that the compiler
is free to up-align the structure on 32 bytes.
This could explain the kasan splat you are experiencing.
Thanks,
Mathieu
>> + VMLINUX_SYMBOL(__start__bpf_raw_tp) = .; \
>> + KEEP(*(__bpf_raw_tp_map)) \
>> + VMLINUX_SYMBOL(__stop__bpf_raw_tp) = .;
>
> that looks to be correct, but something wrong with it.
>
> Can you try your mini test with kasan on ?
>
> I'm seeing this crash:
> test_stacktrace_[ 18.760662] start ffffffff84642438 stop ffffffff84644f60
> map_raw_tp:PASS:[ 18.761467] i 1 btp->tp cccccccccccccccc
> prog_load raw tp[ 18.762064] kasan: CONFIG_KASAN_INLINE enabled
> 0 nsec
> [ 18.762704] kasan: GPF could be caused by NULL-ptr deref or user
> memory access
> [ 18.765125] general protection fault: 0000 [#1] SMP KASAN PTI
> [ 18.765830] Modules linked in:
> [ 18.778358] Call Trace:
> [ 18.778674] bpf_raw_tracepoint_open.isra.27+0x92/0x380
>
> for some reason the start_bpf_raw_tp is off by 8.
> Not sure how it works for you.
>
> (gdb) p &__bpf_trace_tp_map_sys_exit
> $10 = (struct bpf_raw_event_map *) 0xffffffff84642440
> <__bpf_trace_tp_map_sys_exit>
>
> (gdb) p &__start__bpf_raw_tp
> $7 = (<data variable, no debug info> *) 0xffffffff84642438
>
> (gdb) p (void*)(&__start__bpf_raw_tp)+8
> $11 = (void *) 0xffffffff84642440 <__bpf_trace_tp_map_sys_exit>
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [PATCH v6 bpf-next 08/11] bpf: introduce BPF_RAW_TRACEPOINT
From: Alexei Starovoitov @ 2018-03-28 0:00 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: rostedt, David S. Miller, Daniel Borkmann, Linus Torvalds,
Peter Zijlstra, netdev, kernel-team, linux-api, Kees Cook
In-Reply-To: <430531879.1792.1522192417816.JavaMail.zimbra@efficios.com>
On 3/27/18 4:13 PM, Mathieu Desnoyers wrote:
> ----- On Mar 27, 2018, at 6:48 PM, Alexei Starovoitov ast@fb.com wrote:
>
>> On 3/27/18 2:04 PM, Steven Rostedt wrote:
>>>
>>> +#ifdef CONFIG_BPF_EVENTS
>>> +#define BPF_RAW_TP() . = ALIGN(8); \
>
> Given that the section consists of a 16-bytes structure elements
> on architectures with 8 bytes pointers, this ". = ALIGN(8)" should
> be turned into a STRUCT_ALIGN(), especially given that the compiler
> is free to up-align the structure on 32 bytes.
STRUCT_ALIGN fixed the 'off by 8' issue with kasan,
but it fails without kasan too.
For some reason the whole region __start__bpf_raw_tp - __stop__bpf_raw_tp
comes inited with cccc:
[ 22.703562] i 1 btp ffffffff8288e530 btp->tp cccccccccccccccc func
cccccccccccccccc
[ 22.704638] i 2 btp ffffffff8288e540 btp->tp cccccccccccccccc func
cccccccccccccccc
[ 22.705599] i 3 btp ffffffff8288e550 btp->tp cccccccccccccccc func
cccccccccccccccc
[ 22.706551] i 4 btp ffffffff8288e560 btp->tp cccccccccccccccc func
cccccccccccccccc
[ 22.707503] i 5 btp ffffffff8288e570 btp->tp cccccccccccccccc func
cccccccccccccccc
[ 22.708452] i 6 btp ffffffff8288e580 btp->tp cccccccccccccccc func
cccccccccccccccc
[ 22.709406] i 7 btp ffffffff8288e590 btp->tp cccccccccccccccc func
cccccccccccccccc
[ 22.710368] i 8 btp ffffffff8288e5a0 btp->tp cccccccccccccccc func
cccccccccccccccc
while gdb shows that everything is good inside vmlinux
for exactly these addresses.
Some other linker magic missing?
^ permalink raw reply
* Re: [PATCH v6 bpf-next 08/11] bpf: introduce BPF_RAW_TRACEPOINT
From: Mathieu Desnoyers @ 2018-03-28 0:44 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: rostedt, David S. Miller, Daniel Borkmann, Linus Torvalds,
Peter Zijlstra, netdev, kernel-team, linux-api, Kees Cook
In-Reply-To: <b0763759-3f8b-317b-5fb4-600648495462@fb.com>
----- On Mar 27, 2018, at 8:00 PM, Alexei Starovoitov ast@fb.com wrote:
> On 3/27/18 4:13 PM, Mathieu Desnoyers wrote:
>> ----- On Mar 27, 2018, at 6:48 PM, Alexei Starovoitov ast@fb.com wrote:
>>
>>> On 3/27/18 2:04 PM, Steven Rostedt wrote:
>>>>
>>>> +#ifdef CONFIG_BPF_EVENTS
>>>> +#define BPF_RAW_TP() . = ALIGN(8); \
>>
>> Given that the section consists of a 16-bytes structure elements
>> on architectures with 8 bytes pointers, this ". = ALIGN(8)" should
>> be turned into a STRUCT_ALIGN(), especially given that the compiler
>> is free to up-align the structure on 32 bytes.
>
> STRUCT_ALIGN fixed the 'off by 8' issue with kasan,
> but it fails without kasan too.
> For some reason the whole region __start__bpf_raw_tp - __stop__bpf_raw_tp
> comes inited with cccc:
> [ 22.703562] i 1 btp ffffffff8288e530 btp->tp cccccccccccccccc func
> cccccccccccccccc
> [ 22.704638] i 2 btp ffffffff8288e540 btp->tp cccccccccccccccc func
> cccccccccccccccc
> [ 22.705599] i 3 btp ffffffff8288e550 btp->tp cccccccccccccccc func
> cccccccccccccccc
> [ 22.706551] i 4 btp ffffffff8288e560 btp->tp cccccccccccccccc func
> cccccccccccccccc
> [ 22.707503] i 5 btp ffffffff8288e570 btp->tp cccccccccccccccc func
> cccccccccccccccc
> [ 22.708452] i 6 btp ffffffff8288e580 btp->tp cccccccccccccccc func
> cccccccccccccccc
> [ 22.709406] i 7 btp ffffffff8288e590 btp->tp cccccccccccccccc func
> cccccccccccccccc
> [ 22.710368] i 8 btp ffffffff8288e5a0 btp->tp cccccccccccccccc func
> cccccccccccccccc
>
> while gdb shows that everything is good inside vmlinux
> for exactly these addresses.
> Some other linker magic missing?
No, Steven's iteration code is incorrect.
+extern struct bpf_raw_event_map __start__bpf_raw_tp;
+extern struct bpf_raw_event_map __stop__bpf_raw_tp;
That should be:
extern struct bpf_raw_event_map __start__bpf_raw_tp[];
extern struct bpf_raw_event_map __stop__bpf_raw_tp[];
+
+struct bpf_raw_event_map *bpf_find_raw_tracepoint(const char *name)
+{
+ const struct bpf_raw_event_map *btp = &__start__bpf_raw_tp;
const struct bpf_raw_event_map *btp = __start__bpf_raw_tp;
+ int i = 0;
+
+ for (; btp < &__stop__bpf_raw_tp; btp++) {
for (; btp < __stop__bpf_raw_tp; btp++) {
Those start/stop symbols are given their address by the linker
automatically (this is a GNU linker extension). We don't want
pointers to the symbols, but rather the symbols per se to act
as start/stop addresses.
Thanks,
Mathieu
+ i++;
+ if (!strcmp(btp->tp->name, name))
+ return btp;
+ }
+ return NULL;
+}
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [PATCH v6 bpf-next 08/11] bpf: introduce BPF_RAW_TRACEPOINT
From: Alexei Starovoitov @ 2018-03-28 0:51 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: rostedt, David S. Miller, Daniel Borkmann, Linus Torvalds,
Peter Zijlstra, netdev, kernel-team, linux-api, Kees Cook
In-Reply-To: <1513083686.1804.1522197852512.JavaMail.zimbra@efficios.com>
On 3/27/18 5:44 PM, Mathieu Desnoyers wrote:
> ----- On Mar 27, 2018, at 8:00 PM, Alexei Starovoitov ast@fb.com wrote:
>
>> On 3/27/18 4:13 PM, Mathieu Desnoyers wrote:
>>> ----- On Mar 27, 2018, at 6:48 PM, Alexei Starovoitov ast@fb.com wrote:
>>>
>>>> On 3/27/18 2:04 PM, Steven Rostedt wrote:
>>>>>
>>>>> +#ifdef CONFIG_BPF_EVENTS
>>>>> +#define BPF_RAW_TP() . = ALIGN(8); \
>>>
>>> Given that the section consists of a 16-bytes structure elements
>>> on architectures with 8 bytes pointers, this ". = ALIGN(8)" should
>>> be turned into a STRUCT_ALIGN(), especially given that the compiler
>>> is free to up-align the structure on 32 bytes.
>>
>> STRUCT_ALIGN fixed the 'off by 8' issue with kasan,
>> but it fails without kasan too.
>> For some reason the whole region __start__bpf_raw_tp - __stop__bpf_raw_tp
>> comes inited with cccc:
>> [ 22.703562] i 1 btp ffffffff8288e530 btp->tp cccccccccccccccc func
>> cccccccccccccccc
>> [ 22.704638] i 2 btp ffffffff8288e540 btp->tp cccccccccccccccc func
>> cccccccccccccccc
>> [ 22.705599] i 3 btp ffffffff8288e550 btp->tp cccccccccccccccc func
>> cccccccccccccccc
>> [ 22.706551] i 4 btp ffffffff8288e560 btp->tp cccccccccccccccc func
>> cccccccccccccccc
>> [ 22.707503] i 5 btp ffffffff8288e570 btp->tp cccccccccccccccc func
>> cccccccccccccccc
>> [ 22.708452] i 6 btp ffffffff8288e580 btp->tp cccccccccccccccc func
>> cccccccccccccccc
>> [ 22.709406] i 7 btp ffffffff8288e590 btp->tp cccccccccccccccc func
>> cccccccccccccccc
>> [ 22.710368] i 8 btp ffffffff8288e5a0 btp->tp cccccccccccccccc func
>> cccccccccccccccc
>>
>> while gdb shows that everything is good inside vmlinux
>> for exactly these addresses.
>> Some other linker magic missing?
>
> No, Steven's iteration code is incorrect.
>
> +extern struct bpf_raw_event_map __start__bpf_raw_tp;
> +extern struct bpf_raw_event_map __stop__bpf_raw_tp;
>
> That should be:
>
> extern struct bpf_raw_event_map __start__bpf_raw_tp[];
> extern struct bpf_raw_event_map __stop__bpf_raw_tp[];
>
>
> +
> +struct bpf_raw_event_map *bpf_find_raw_tracepoint(const char *name)
> +{
> + const struct bpf_raw_event_map *btp = &__start__bpf_raw_tp;
>
> const struct bpf_raw_event_map *btp = __start__bpf_raw_tp;
>
> + int i = 0;
> +
> + for (; btp < &__stop__bpf_raw_tp; btp++) {
>
> for (; btp < __stop__bpf_raw_tp; btp++) {
>
> Those start/stop symbols are given their address by the linker
> automatically (this is a GNU linker extension). We don't want
> pointers to the symbols, but rather the symbols per se to act
> as start/stop addresses.
right. that part I fixed first.
Turned out it was in init.data section and got poisoned.
this fixes it:
@@ -258,6 +258,7 @@
LIKELY_PROFILE() \
BRANCH_PROFILE() \
TRACE_PRINTKS() \
+ BPF_RAW_TP() \
TRACEPOINT_STR()
/*
@@ -585,7 +586,6 @@
*(.init.rodata) \
FTRACE_EVENTS() \
TRACE_SYSCALLS() \
- BPF_RAW_TP() \
KPROBE_BLACKLIST() \
ERROR_INJECT_WHITELIST() \
MEM_DISCARD(init.rodata) \
and it works :)
I will clean few other nits I found while debugging and respin.
^ permalink raw reply
* [PATCH v7 bpf-next 00/10] bpf, tracing: introduce bpf raw tracepoints
From: Alexei Starovoitov @ 2018-03-28 2:10 UTC (permalink / raw)
To: davem
Cc: daniel, torvalds, peterz, rostedt, mathieu.desnoyers, netdev,
kernel-team, linux-api
v6->v7:
- adopted Steven's bpf_raw_tp_map section approach to find tracepoint
and corresponding bpf probe function instead of kallsyms approach.
dropped kernel_tracepoint_find_by_name() patch
v5->v6:
- avoid changing semantics of for_each_kernel_tracepoint() function, instead
introduce kernel_tracepoint_find_by_name() helper
v4->v5:
- adopted Daniel's fancy REPEAT macro in bpf_trace.c in patch 7
v3->v4:
- adopted Linus's CAST_TO_U64 macro to cast any integer, pointer, or small
struct to u64. That nicely reduced the size of patch 1
v2->v3:
- with Linus's suggestion introduced generic COUNT_ARGS and CONCATENATE macros
(or rather moved them from apparmor)
that cleaned up patches 6 and 7
- added patch 4 to refactor trace_iwlwifi_dev_ucode_error() from 17 args to 4
Now any tracepoint with >12 args will have build error
v1->v2:
- simplified api by combing bpf_raw_tp_open(name) + bpf_attach(prog_fd) into
bpf_raw_tp_open(name, prog_fd) as suggested by Daniel.
That simplifies bpf_detach as well which is now simple close() of fd.
- fixed memory leak in error path which was spotted by Daniel.
- fixed bpf_get_stackid(), bpf_perf_event_output() called from raw tracepoints
- added more tests
- fixed allyesconfig build caught by buildbot
v1:
This patch set is a different way to address the pressing need to access
task_struct pointers in sched tracepoints from bpf programs.
The first approach simply added these pointers to sched tracepoints:
https://lkml.org/lkml/2017/12/14/753
which Peter nacked.
Few options were discussed and eventually the discussion converged on
doing bpf specific tracepoint_probe_register() probe functions.
Details here:
https://lkml.org/lkml/2017/12/20/929
Patch 1 is kernel wide cleanup of pass-struct-by-value into
pass-struct-by-reference into tracepoints.
Patches 2 and 3 are minor cleanups to address allyesconfig build
Patch 4 refactor trace_iwlwifi_dev_ucode_error from 17 to 4 args
Patch 5 introduces COUNT_ARGS macro
Patch 6 minor prep work to expose number of arguments passed
into tracepoints.
Patch 7 introduces BPF_RAW_TRACEPOINT api.
the auto-cleanup and multiple concurrent users are must have
features of tracing api. For bpf raw tracepoints it looks like:
// load bpf prog with BPF_PROG_TYPE_RAW_TRACEPOINT type
prog_fd = bpf_prog_load(...);
// receive anon_inode fd for given bpf_raw_tracepoint
// and attach bpf program to it
raw_tp_fd = bpf_raw_tracepoint_open("xdp_exception", prog_fd);
Ctrl-C of tracing daemon or cmdline tool will automatically
detach bpf program, unload it and unregister tracepoint probe.
More details in patch 7.
Patch 8 - trivial support in libbpf
Patches 9, 10 - user space tests
samples/bpf/test_overhead performance on 1 cpu:
tracepoint base kprobe+bpf tracepoint+bpf raw_tracepoint+bpf
task_rename 1.1M 769K 947K 1.0M
urandom_read 789K 697K 750K 755K
Alexei Starovoitov (10):
treewide: remove large struct-pass-by-value from tracepoint arguments
net/mediatek: disambiguate mt76 vs mt7601u trace events
net/mac802154: disambiguate mac80215 vs mac802154 trace events
net/wireless/iwlwifi: fix iwlwifi_dev_ucode_error tracepoint
macro: introduce COUNT_ARGS() macro
tracepoint: compute num_args at build time
bpf: introduce BPF_RAW_TRACEPOINT
libbpf: add bpf_raw_tracepoint_open helper
samples/bpf: raw tracepoint test
selftests/bpf: test for bpf_get_stackid() from raw tracepoints
drivers/infiniband/hw/hfi1/file_ops.c | 2 +-
drivers/infiniband/hw/hfi1/trace_ctxts.h | 12 +-
drivers/net/wireless/intel/iwlwifi/dvm/main.c | 7 +-
.../wireless/intel/iwlwifi/iwl-devtrace-iwlwifi.h | 39 ++---
drivers/net/wireless/intel/iwlwifi/iwl-devtrace.c | 1 +
drivers/net/wireless/intel/iwlwifi/mvm/utils.c | 7 +-
drivers/net/wireless/mediatek/mt7601u/trace.h | 6 +-
include/asm-generic/vmlinux.lds.h | 10 ++
include/linux/bpf_types.h | 1 +
include/linux/kernel.h | 7 +
include/linux/trace_events.h | 42 +++++
include/linux/tracepoint-defs.h | 6 +
include/linux/tracepoint.h | 12 +-
include/trace/bpf_probe.h | 91 ++++++++++
include/trace/define_trace.h | 15 +-
include/trace/events/f2fs.h | 2 +-
include/uapi/linux/bpf.h | 11 ++
kernel/bpf/syscall.c | 78 +++++++++
kernel/trace/bpf_trace.c | 183 +++++++++++++++++++++
net/mac802154/trace.h | 8 +-
net/wireless/trace.h | 2 +-
samples/bpf/Makefile | 1 +
samples/bpf/bpf_load.c | 14 ++
samples/bpf/test_overhead_raw_tp_kern.c | 17 ++
samples/bpf/test_overhead_user.c | 12 ++
security/apparmor/include/path.h | 7 +-
sound/firewire/amdtp-stream-trace.h | 2 +-
tools/include/uapi/linux/bpf.h | 11 ++
tools/lib/bpf/bpf.c | 11 ++
tools/lib/bpf/bpf.h | 1 +
tools/testing/selftests/bpf/test_progs.c | 91 +++++++---
31 files changed, 619 insertions(+), 90 deletions(-)
create mode 100644 include/trace/bpf_probe.h
create mode 100644 samples/bpf/test_overhead_raw_tp_kern.c
--
2.9.5
^ permalink raw reply
* [PATCH v7 bpf-next 01/10] treewide: remove large struct-pass-by-value from tracepoint arguments
From: Alexei Starovoitov @ 2018-03-28 2:10 UTC (permalink / raw)
To: davem
Cc: daniel, torvalds, peterz, rostedt, mathieu.desnoyers, netdev,
kernel-team, linux-api
In-Reply-To: <20180328021105.4061744-1-ast@fb.com>
From: Alexei Starovoitov <ast@kernel.org>
- fix trace_hfi1_ctxt_info() to pass large struct by reference instead of by value
- convert 'type array[]' tracepoint arguments into 'type *array',
since compiler will warn that sizeof('type array[]') == sizeof('type *array')
and later should be used instead
The CAST_TO_U64 macro in the later patch will enforce that tracepoint
arguments can only be integers, pointers, or less than 8 byte structures.
Larger structures should be passed by reference.
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
drivers/infiniband/hw/hfi1/file_ops.c | 2 +-
drivers/infiniband/hw/hfi1/trace_ctxts.h | 12 ++++++------
include/trace/events/f2fs.h | 2 +-
net/wireless/trace.h | 2 +-
sound/firewire/amdtp-stream-trace.h | 2 +-
5 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/infiniband/hw/hfi1/file_ops.c b/drivers/infiniband/hw/hfi1/file_ops.c
index 41fafebe3b0d..da4aa1a95b11 100644
--- a/drivers/infiniband/hw/hfi1/file_ops.c
+++ b/drivers/infiniband/hw/hfi1/file_ops.c
@@ -1153,7 +1153,7 @@ static int get_ctxt_info(struct hfi1_filedata *fd, unsigned long arg, u32 len)
cinfo.sdma_ring_size = fd->cq->nentries;
cinfo.rcvegr_size = uctxt->egrbufs.rcvtid_size;
- trace_hfi1_ctxt_info(uctxt->dd, uctxt->ctxt, fd->subctxt, cinfo);
+ trace_hfi1_ctxt_info(uctxt->dd, uctxt->ctxt, fd->subctxt, &cinfo);
if (copy_to_user((void __user *)arg, &cinfo, len))
return -EFAULT;
diff --git a/drivers/infiniband/hw/hfi1/trace_ctxts.h b/drivers/infiniband/hw/hfi1/trace_ctxts.h
index 4eb4cc798035..e00c8a7d559c 100644
--- a/drivers/infiniband/hw/hfi1/trace_ctxts.h
+++ b/drivers/infiniband/hw/hfi1/trace_ctxts.h
@@ -106,7 +106,7 @@ TRACE_EVENT(hfi1_uctxtdata,
TRACE_EVENT(hfi1_ctxt_info,
TP_PROTO(struct hfi1_devdata *dd, unsigned int ctxt,
unsigned int subctxt,
- struct hfi1_ctxt_info cinfo),
+ struct hfi1_ctxt_info *cinfo),
TP_ARGS(dd, ctxt, subctxt, cinfo),
TP_STRUCT__entry(DD_DEV_ENTRY(dd)
__field(unsigned int, ctxt)
@@ -120,11 +120,11 @@ TRACE_EVENT(hfi1_ctxt_info,
TP_fast_assign(DD_DEV_ASSIGN(dd);
__entry->ctxt = ctxt;
__entry->subctxt = subctxt;
- __entry->egrtids = cinfo.egrtids;
- __entry->rcvhdrq_cnt = cinfo.rcvhdrq_cnt;
- __entry->rcvhdrq_size = cinfo.rcvhdrq_entsize;
- __entry->sdma_ring_size = cinfo.sdma_ring_size;
- __entry->rcvegr_size = cinfo.rcvegr_size;
+ __entry->egrtids = cinfo->egrtids;
+ __entry->rcvhdrq_cnt = cinfo->rcvhdrq_cnt;
+ __entry->rcvhdrq_size = cinfo->rcvhdrq_entsize;
+ __entry->sdma_ring_size = cinfo->sdma_ring_size;
+ __entry->rcvegr_size = cinfo->rcvegr_size;
),
TP_printk("[%s] ctxt %u:%u " CINFO_FMT,
__get_str(dev),
diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
index 06c87f9f720c..795698925d20 100644
--- a/include/trace/events/f2fs.h
+++ b/include/trace/events/f2fs.h
@@ -491,7 +491,7 @@ DEFINE_EVENT(f2fs__truncate_node, f2fs_truncate_node,
TRACE_EVENT(f2fs_truncate_partial_nodes,
- TP_PROTO(struct inode *inode, nid_t nid[], int depth, int err),
+ TP_PROTO(struct inode *inode, nid_t *nid, int depth, int err),
TP_ARGS(inode, nid, depth, err),
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index 5152938b358d..018c81fa72fb 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -3137,7 +3137,7 @@ TRACE_EVENT(rdev_start_radar_detection,
TRACE_EVENT(rdev_set_mcast_rate,
TP_PROTO(struct wiphy *wiphy, struct net_device *netdev,
- int mcast_rate[NUM_NL80211_BANDS]),
+ int *mcast_rate),
TP_ARGS(wiphy, netdev, mcast_rate),
TP_STRUCT__entry(
WIPHY_ENTRY
diff --git a/sound/firewire/amdtp-stream-trace.h b/sound/firewire/amdtp-stream-trace.h
index ea0d486652c8..54cdd4ffa9ce 100644
--- a/sound/firewire/amdtp-stream-trace.h
+++ b/sound/firewire/amdtp-stream-trace.h
@@ -14,7 +14,7 @@
#include <linux/tracepoint.h>
TRACE_EVENT(in_packet,
- TP_PROTO(const struct amdtp_stream *s, u32 cycles, u32 cip_header[2], unsigned int payload_length, unsigned int index),
+ TP_PROTO(const struct amdtp_stream *s, u32 cycles, u32 *cip_header, unsigned int payload_length, unsigned int index),
TP_ARGS(s, cycles, cip_header, payload_length, index),
TP_STRUCT__entry(
__field(unsigned int, second)
--
2.9.5
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
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).