[PATCH v3 0/4] tracing/user_events: Add empty arguments events

[PATCH v3 0/4] tracing/user_events: Add empty arguments events

[sunliming]

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

[PATCH v3 1/4] tracing/user_events: Fix the incorrect trace record for empty arguments events

[sunliming]

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) &&
-- 
2.25.1

[PATCH v3 2/4] selftests/user_events: Add ftrace self-test for empty arguments events

[sunliming]

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 = &reg.write_index;
+	io[0].iov_len = sizeof(reg.write_index);
+
+	/* Register should work */
+	ASSERT_EQ(0, ioctl(self->data_fd, DIAG_IOCSREG, &reg));
+	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];
-- 
2.25.1

[PATCH v3 3/4] selftests/user_events: Clear the events after perf self-test

[sunliming]

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) {
-- 
2.25.1

[PATCH v3 4/4] selftests/user_events: Add perf self-test for empty arguments events

[sunliming]

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, &reg));
+	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, &reg.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);
-- 
2.25.1

Re: [PATCH v3 0/4] tracing/user_events: Add empty arguments events

[Beau Belgrave]

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

Re: [PATCH v3 0/4] tracing/user_events: Add empty arguments events

[Masami Hiramatsu]

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
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>