2023-06-26 11:22:02

by sunliming

Subject: [PATCH v4 0/3] tracing/user_events: Fix incorrect return value for writes when events are disabled and add its tests

Now the writing operation return the count of writes regardless of whether
events are enabled or disabled. Fix this by just return -EBADF when events
are disabled.

v3 -> v4:
- Change the return value from zero to -EBADF

v2 -> v3:
- Change the return value from -ENOENT to zero

v1 -> v2:
- Change the return value from -EFAULT to -ENOENT

sunliming (3):
tracing/user_events: Fix incorrect return value for writing operation
when events are disabled
selftests/user_events: Enable the event before write_fault test in
ftrace self-test
selftests/user_events: Add test cases when event is disabled

kernel/trace/trace_events_user.c | 3 ++-
tools/testing/selftests/user_events/ftrace_test.c | 8 ++++++++
2 files changed, 10 insertions(+), 1 deletion(-)

--
2.25.1

2023-06-26 11:23:05

by sunliming

Subject: [PATCH v4 1/3] tracing/user_events: Fix incorrect return value for writing operation when events are disabled

The writing operation return the count of writes regardless of whether events
are enabled or disabled. Switch it to return -EBADF to indicates that the event
is disabled.

Signed-off-by: sunliming <[email protected]>
---
kernel/trace/trace_events_user.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/trace_events_user.c b/kernel/trace/trace_events_user.c
index 1ac5ba5685ed..146bb2e9758f 100644
--- a/kernel/trace/trace_events_user.c
+++ b/kernel/trace/trace_events_user.c
@@ -1957,7 +1957,8 @@ static ssize_t user_events_write_core(struct file *file, struct iov_iter *i)

if (unlikely(faulted))
return -EFAULT;
- }
+ } else
+ return -EBADF;

return ret;
}
--
2.25.1

2023-06-26 11:24:32

by sunliming

Subject: [PATCH v4 2/3] selftests/user_events: Enable the event before write_fault test in ftrace self-test

The user_event has not be enabled in write_fault test in ftrace
self-test, Just enable it.

Signed-off-by: sunliming <[email protected]>
---
tools/testing/selftests/user_events/ftrace_test.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/tools/testing/selftests/user_events/ftrace_test.c b/tools/testing/selftests/user_events/ftrace_test.c
index abfb49558a26..d33bd31425db 100644
--- a/tools/testing/selftests/user_events/ftrace_test.c
+++ b/tools/testing/selftests/user_events/ftrace_test.c
@@ -373,6 +373,10 @@ TEST_F(user, write_fault) {
ASSERT_EQ(0, ioctl(self->data_fd, DIAG_IOCSREG, &reg));
ASSERT_EQ(0, reg.write_index);

+ /* Enable event */
+ self->enable_fd = open(enable_file, O_RDWR);
+ ASSERT_NE(-1, write(self->enable_fd, "1", sizeof("1")))
+
/* Write should work normally */
ASSERT_NE(-1, writev(self->data_fd, (const struct iovec *)io, 2));

--
2.25.1

2023-06-26 11:24:54

by sunliming

Subject: [PATCH v4 3/3] selftests/user_events: Add test cases when event is disabled

When user_events are disabled, it's write operation should return -EBADF.
Add this test cases.

Signed-off-by: sunliming <[email protected]>
---
tools/testing/selftests/user_events/ftrace_test.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/tools/testing/selftests/user_events/ftrace_test.c b/tools/testing/selftests/user_events/ftrace_test.c
index d33bd31425db..c9dc99bcafec 100644
--- a/tools/testing/selftests/user_events/ftrace_test.c
+++ b/tools/testing/selftests/user_events/ftrace_test.c
@@ -297,6 +297,10 @@ TEST_F(user, write_events) {
io[0].iov_base = &reg.write_index;
io[0].iov_len = sizeof(reg.write_index);

+ /* Write should return -EBADF when event is not enabled */
+ ASSERT_EQ(-1, writev(self->data_fd, (const struct iovec *)io, 3));
+ ASSERT_EQ(EBADF, errno);
+
/* Enable event */
self->enable_fd = open(enable_file, O_RDWR);
ASSERT_NE(-1, write(self->enable_fd, "1", sizeof("1")))
--
2.25.1

2023-06-26 16:49:57

by Beau Belgrave

Subject: Re: [PATCH v4 0/3] tracing/user_events: Fix incorrect return value for writes when events are disabled and add its tests

On Mon, Jun 26, 2023 at 07:13:41PM +0800, sunliming wrote:
> Now the writing operation return the count of writes regardless of whether
> events are enabled or disabled. Fix this by just return -EBADF when events
> are disabled.
>
> v3 -> v4:
> - Change the return value from zero to -EBADF
>

Applied these locally, ran a few tests. This looks good to me.

Acked-by: Beau Belgrave <[email protected]>

Thanks,
-Beau

> v2 -> v3:
> - Change the return value from -ENOENT to zero
>
> v1 -> v2:
> - Change the return value from -EFAULT to -ENOENT
>
> sunliming (3):
> tracing/user_events: Fix incorrect return value for writing operation
> when events are disabled
> selftests/user_events: Enable the event before write_fault test in
> ftrace self-test
> selftests/user_events: Add test cases when event is disabled
>
> kernel/trace/trace_events_user.c | 3 ++-
> tools/testing/selftests/user_events/ftrace_test.c | 8 ++++++++
> 2 files changed, 10 insertions(+), 1 deletion(-)
>
> --
> 2.25.1