This series patches fix the incorrect trace record for empty arguments events and add correspondent selftests.
V3: Improve modification descriptions to maintain consistent formatting
v2: Change "i->count" to "i->count !=0 " to prevent compiler optimization Add correspondent selftests
sunliming (4): tracing/user_events: Fix the incorrect trace record for empty arguments events selftests/user_events: Add ftrace self-test for empty arguments events selftests/user_events: Clear the events after perf self-test selftests/user_events: Add perf self-test for empty arguments events
kernel/trace/trace_events_user.c | 4 +- .../selftests/user_events/ftrace_test.c | 33 ++++++++ .../testing/selftests/user_events/perf_test.c | 82 +++++++++++++++++++ 3 files changed, 117 insertions(+), 2 deletions(-)
The user_events support events that has empty arguments. But the trace event is discarded and not really committed when the arguments is empty. Fix this by not attempting to copy in zero-length data.
Signed-off-by: sunliming sunliming@kylinos.cn --- kernel/trace/trace_events_user.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_events_user.c b/kernel/trace/trace_events_user.c index 0d91dac206ff..698703a3d234 100644 --- a/kernel/trace/trace_events_user.c +++ b/kernel/trace/trace_events_user.c @@ -1399,7 +1399,7 @@ static void user_event_ftrace(struct user_event *user, struct iov_iter *i, if (unlikely(!entry)) return;
- if (unlikely(!copy_nofault(entry + 1, i->count, i))) + if (unlikely(i->count != 0 && !copy_nofault(entry + 1, i->count, i))) goto discard;
if (!list_empty(&user->validators) && @@ -1440,7 +1440,7 @@ static void user_event_perf(struct user_event *user, struct iov_iter *i,
perf_fetch_caller_regs(regs);
- if (unlikely(!copy_nofault(perf_entry + 1, i->count, i))) + if (unlikely(i->count != 0 && !copy_nofault(perf_entry + 1, i->count, i))) goto discard;
if (!list_empty(&user->validators) &&
Tests to ensure events that has empty arguments can input trace record correctly when using ftrace.
Signed-off-by: sunliming sunliming@kylinos.cn --- .../selftests/user_events/ftrace_test.c | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+)
diff --git a/tools/testing/selftests/user_events/ftrace_test.c b/tools/testing/selftests/user_events/ftrace_test.c index 6e8c4b47281c..abfb49558a26 100644 --- a/tools/testing/selftests/user_events/ftrace_test.c +++ b/tools/testing/selftests/user_events/ftrace_test.c @@ -316,6 +316,39 @@ TEST_F(user, write_events) { ASSERT_EQ(EINVAL, errno); }
+TEST_F(user, write_empty_events) { + struct user_reg reg = {0}; + struct iovec io[1]; + int before = 0, after = 0; + + reg.size = sizeof(reg); + reg.name_args = (__u64)"__test_event"; + reg.enable_bit = 31; + reg.enable_addr = (__u64)&self->check; + reg.enable_size = sizeof(self->check); + + io[0].iov_base = ®.write_index; + io[0].iov_len = sizeof(reg.write_index); + + /* Register should work */ + ASSERT_EQ(0, ioctl(self->data_fd, DIAG_IOCSREG, ®)); + ASSERT_EQ(0, reg.write_index); + ASSERT_EQ(0, self->check); + + /* Enable event */ + self->enable_fd = open(enable_file, O_RDWR); + ASSERT_NE(-1, write(self->enable_fd, "1", sizeof("1"))) + + /* Event should now be enabled */ + ASSERT_EQ(1 << reg.enable_bit, self->check); + + /* Write should make it out to ftrace buffers */ + before = trace_bytes(); + ASSERT_NE(-1, writev(self->data_fd, (const struct iovec *)io, 1)); + after = trace_bytes(); + ASSERT_GT(after, before); +} + TEST_F(user, write_fault) { struct user_reg reg = {0}; struct iovec io[2];
When the self test is completed, perf self-test left the user events not to be cleared. Clear the events by unregister and delete the event.
Signed-off-by: sunliming sunliming@kylinos.cn --- .../testing/selftests/user_events/perf_test.c | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+)
diff --git a/tools/testing/selftests/user_events/perf_test.c b/tools/testing/selftests/user_events/perf_test.c index a070258d4449..e97f24ab6e2f 100644 --- a/tools/testing/selftests/user_events/perf_test.c +++ b/tools/testing/selftests/user_events/perf_test.c @@ -81,6 +81,32 @@ static int get_offset(void) return offset; }
+static int clear(int *check) +{ + struct user_unreg unreg = {0}; + + unreg.size = sizeof(unreg); + unreg.disable_bit = 31; + unreg.disable_addr = (__u64)check; + + int fd = open(data_file, O_RDWR); + + if (fd == -1) + return -1; + + if (ioctl(fd, DIAG_IOCSUNREG, &unreg) == -1) + if (errno != ENOENT) + return -1; + + if (ioctl(fd, DIAG_IOCSDEL, "__test_event") == -1) + if (errno != ENOENT) + return -1; + + close(fd); + + return 0; +} + FIXTURE(user) { int data_fd; int check; @@ -93,6 +119,9 @@ FIXTURE_SETUP(user) {
FIXTURE_TEARDOWN(user) { close(self->data_fd); + + if (clear(&self->check) != 0) + printf("WARNING: Clear didn't work!\n"); }
TEST_F(user, perf_write) {
Tests to ensure events that has empty arguments can input trace record correctly when using perf.
Signed-off-by: sunliming sunliming@kylinos.cn --- .../testing/selftests/user_events/perf_test.c | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+)
diff --git a/tools/testing/selftests/user_events/perf_test.c b/tools/testing/selftests/user_events/perf_test.c index e97f24ab6e2f..8b09be566fa2 100644 --- a/tools/testing/selftests/user_events/perf_test.c +++ b/tools/testing/selftests/user_events/perf_test.c @@ -189,6 +189,59 @@ TEST_F(user, perf_write) { ASSERT_EQ(0, self->check); }
+TEST_F(user, perf_empty_events) { + struct perf_event_attr pe = {0}; + struct user_reg reg = {0}; + struct perf_event_mmap_page *perf_page; + int page_size = sysconf(_SC_PAGESIZE); + int id, fd; + __u32 *val; + + reg.size = sizeof(reg); + reg.name_args = (__u64)"__test_event"; + reg.enable_bit = 31; + reg.enable_addr = (__u64)&self->check; + reg.enable_size = sizeof(self->check); + + /* Register should work */ + ASSERT_EQ(0, ioctl(self->data_fd, DIAG_IOCSREG, ®)); + ASSERT_EQ(0, reg.write_index); + ASSERT_EQ(0, self->check); + + /* Id should be there */ + id = get_id(); + ASSERT_NE(-1, id); + + pe.type = PERF_TYPE_TRACEPOINT; + pe.size = sizeof(pe); + pe.config = id; + pe.sample_type = PERF_SAMPLE_RAW; + pe.sample_period = 1; + pe.wakeup_events = 1; + + /* Tracepoint attach should work */ + fd = perf_event_open(&pe, 0, -1, -1, 0); + ASSERT_NE(-1, fd); + + perf_page = mmap(NULL, page_size * 2, PROT_READ, MAP_SHARED, fd, 0); + ASSERT_NE(MAP_FAILED, perf_page); + + /* Status should be updated */ + ASSERT_EQ(1 << reg.enable_bit, self->check); + + /* Ensure write shows up at correct offset */ + ASSERT_NE(-1, write(self->data_fd, ®.write_index, + sizeof(reg.write_index))); + val = (void *)(((char *)perf_page) + perf_page->data_offset); + ASSERT_EQ(PERF_RECORD_SAMPLE, *val); + + munmap(perf_page, page_size * 2); + close(fd); + + /* Status should be updated */ + ASSERT_EQ(0, self->check); +} + int main(int argc, char **argv) { return test_harness_run(argc, argv);
On Tue, Jun 06, 2023 at 02:20:23PM +0800, sunliming wrote:
This series patches fix the incorrect trace record for empty arguments events and add correspondent selftests.
V3: Improve modification descriptions to maintain consistent formatting
v2: Change "i->count" to "i->count !=0 " to prevent compiler optimization Add correspondent selftests
sunliming (4): tracing/user_events: Fix the incorrect trace record for empty arguments events selftests/user_events: Add ftrace self-test for empty arguments events selftests/user_events: Clear the events after perf self-test selftests/user_events: Add perf self-test for empty arguments events
kernel/trace/trace_events_user.c | 4 +- .../selftests/user_events/ftrace_test.c | 33 ++++++++ .../testing/selftests/user_events/perf_test.c | 82 +++++++++++++++++++ 3 files changed, 117 insertions(+), 2 deletions(-)
-- 2.25.1
Thank you, this series looks good to me.
Acked-by: Beau Belgrave beaub@linux.microsoft.com
Thanks, -Beau
On Tue, 6 Jun 2023 14:20:23 +0800 sunliming sunliming@kylinos.cn wrote:
This series patches fix the incorrect trace record for empty arguments events and add correspondent selftests.
Looks good for me.
Acked-by: Masami Hiramatsu (Google) mhiramat@kernel.org
for the series.
V3: Improve modification descriptions to maintain consistent formatting
v2: Change "i->count" to "i->count !=0 " to prevent compiler optimization Add correspondent selftests
sunliming (4): tracing/user_events: Fix the incorrect trace record for empty arguments events selftests/user_events: Add ftrace self-test for empty arguments events selftests/user_events: Clear the events after perf self-test selftests/user_events: Add perf self-test for empty arguments events
kernel/trace/trace_events_user.c | 4 +- .../selftests/user_events/ftrace_test.c | 33 ++++++++ .../testing/selftests/user_events/perf_test.c | 82 +++++++++++++++++++ 3 files changed, 117 insertions(+), 2 deletions(-)
-- 2.25.1
linux-kselftest-mirror@lists.linaro.org
-
Beau Belgrave -
Masami Hiramatsu -
sunliming