* [PATCH bpf-next] selftests/bpf: Fix d_path test
@ 2023-08-31 11:00 Jiri Olsa
2023-08-31 11:46 ` Daniel Borkmann
0 siblings, 1 reply; 5+ messages in thread
From: Jiri Olsa @ 2023-08-31 11:00 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: Hou Tao, bpf, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Hou Tao
Recent commit [1] broken d_path test, because now filp_close is not called
directly from sys_close, but eventually later when the file is finally
released.
As suggested by Hou Tao we don't need to re-hook the bpf program, but just
instead we can use sys_close_range to trigger filp_close synchronously.
[1] 021a160abf62 ("fs: use __fput_sync in close(2)")
Suggested-by: Hou Tao <houtao@huaweicloud.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/testing/selftests/bpf/prog_tests/d_path.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/d_path.c b/tools/testing/selftests/bpf/prog_tests/d_path.c
index 911345c526e6..81e34a4a05d1 100644
--- a/tools/testing/selftests/bpf/prog_tests/d_path.c
+++ b/tools/testing/selftests/bpf/prog_tests/d_path.c
@@ -90,7 +90,11 @@ static int trigger_fstat_events(pid_t pid)
fstat(indicatorfd, &fileStat);
out_close:
- /* triggers filp_close */
+ /* sys_close no longer triggers filp_close, but we can
+ * call sys_close_range instead which still does
+ */
+#define close(fd) close_range(fd, fd, 0)
+
close(pipefd[0]);
close(pipefd[1]);
close(sockfd);
@@ -98,6 +102,8 @@ static int trigger_fstat_events(pid_t pid)
close(devfd);
close(localfd);
close(indicatorfd);
+
+#undef close
return ret;
}
--
2.41.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Fix d_path test
2023-08-31 11:00 [PATCH bpf-next] selftests/bpf: Fix d_path test Jiri Olsa
@ 2023-08-31 11:46 ` Daniel Borkmann
2023-08-31 12:25 ` Jiri Olsa
2023-08-31 12:37 ` Hou Tao
0 siblings, 2 replies; 5+ messages in thread
From: Daniel Borkmann @ 2023-08-31 11:46 UTC (permalink / raw)
To: Jiri Olsa, Alexei Starovoitov, Andrii Nakryiko
Cc: Hou Tao, bpf, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Hou Tao
On 8/31/23 1:00 PM, Jiri Olsa wrote:
> Recent commit [1] broken d_path test, because now filp_close is not called
> directly from sys_close, but eventually later when the file is finally
> released.
>
> As suggested by Hou Tao we don't need to re-hook the bpf program, but just
> instead we can use sys_close_range to trigger filp_close synchronously.
>
> [1] 021a160abf62 ("fs: use __fput_sync in close(2)")
> Suggested-by: Hou Tao <houtao@huaweicloud.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> tools/testing/selftests/bpf/prog_tests/d_path.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/d_path.c b/tools/testing/selftests/bpf/prog_tests/d_path.c
> index 911345c526e6..81e34a4a05d1 100644
> --- a/tools/testing/selftests/bpf/prog_tests/d_path.c
> +++ b/tools/testing/selftests/bpf/prog_tests/d_path.c
> @@ -90,7 +90,11 @@ static int trigger_fstat_events(pid_t pid)
> fstat(indicatorfd, &fileStat);
>
> out_close:
> - /* triggers filp_close */
> + /* sys_close no longer triggers filp_close, but we can
> + * call sys_close_range instead which still does
> + */
> +#define close(fd) close_range(fd, fd, 0)
> +
The BPF CI selftest build says:
[...]
TEST-OBJ [test_progs] lookup_key.test.o
TEST-OBJ [test_progs] migrate_reuseport.test.o
TEST-OBJ [test_progs] user_ringbuf.test.o
/tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/d_path.c: In function ‘trigger_fstat_events’:
/tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/d_path.c:96:19: error: implicit declaration of function ‘close_range’ [-Werror=implicit-function-declaration]
96 | #define close(fd) close_range(fd, fd, 0)
| ^~~~~~~~~~~
/tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/d_path.c:98:2: note: in expansion of macro ‘close’
98 | close(pipefd[0]);
| ^~~~~
TEST-OBJ [test_progs] task_pt_regs.test.o
[...]
Perhaps #include <linux/close_range.h> missing ?
> close(pipefd[0]);
> close(pipefd[1]);
> close(sockfd);
> @@ -98,6 +102,8 @@ static int trigger_fstat_events(pid_t pid)
> close(devfd);
> close(localfd);
> close(indicatorfd);
> +
> +#undef close
> return ret;
> }
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Fix d_path test
2023-08-31 11:46 ` Daniel Borkmann
@ 2023-08-31 12:25 ` Jiri Olsa
2023-08-31 12:37 ` Hou Tao
1 sibling, 0 replies; 5+ messages in thread
From: Jiri Olsa @ 2023-08-31 12:25 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Alexei Starovoitov, Andrii Nakryiko, Hou Tao, bpf,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Hou Tao
On Thu, Aug 31, 2023 at 01:46:09PM +0200, Daniel Borkmann wrote:
> On 8/31/23 1:00 PM, Jiri Olsa wrote:
> > Recent commit [1] broken d_path test, because now filp_close is not called
> > directly from sys_close, but eventually later when the file is finally
> > released.
> >
> > As suggested by Hou Tao we don't need to re-hook the bpf program, but just
> > instead we can use sys_close_range to trigger filp_close synchronously.
> >
> > [1] 021a160abf62 ("fs: use __fput_sync in close(2)")
> > Suggested-by: Hou Tao <houtao@huaweicloud.com>
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> > tools/testing/selftests/bpf/prog_tests/d_path.c | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/d_path.c b/tools/testing/selftests/bpf/prog_tests/d_path.c
> > index 911345c526e6..81e34a4a05d1 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/d_path.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/d_path.c
> > @@ -90,7 +90,11 @@ static int trigger_fstat_events(pid_t pid)
> > fstat(indicatorfd, &fileStat);
> > out_close:
> > - /* triggers filp_close */
> > + /* sys_close no longer triggers filp_close, but we can
> > + * call sys_close_range instead which still does
> > + */
> > +#define close(fd) close_range(fd, fd, 0)
> > +
>
> The BPF CI selftest build says:
>
> [...]
> TEST-OBJ [test_progs] lookup_key.test.o
> TEST-OBJ [test_progs] migrate_reuseport.test.o
> TEST-OBJ [test_progs] user_ringbuf.test.o
> /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/d_path.c: In function ‘trigger_fstat_events’:
> /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/d_path.c:96:19: error: implicit declaration of function ‘close_range’ [-Werror=implicit-function-declaration]
> 96 | #define close(fd) close_range(fd, fd, 0)
> | ^~~~~~~~~~~
> /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/d_path.c:98:2: note: in expansion of macro ‘close’
> 98 | close(pipefd[0]);
> | ^~~~~
> TEST-OBJ [test_progs] task_pt_regs.test.o
> [...]
>
> Perhaps #include <linux/close_range.h> missing ?
yes, will send v2
thanks,
jirka
>
> > close(pipefd[0]);
> > close(pipefd[1]);
> > close(sockfd);
> > @@ -98,6 +102,8 @@ static int trigger_fstat_events(pid_t pid)
> > close(devfd);
> > close(localfd);
> > close(indicatorfd);
> > +
> > +#undef close
> > return ret;
> > }
> >
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Fix d_path test
2023-08-31 11:46 ` Daniel Borkmann
2023-08-31 12:25 ` Jiri Olsa
@ 2023-08-31 12:37 ` Hou Tao
2023-08-31 12:52 ` Jiri Olsa
1 sibling, 1 reply; 5+ messages in thread
From: Hou Tao @ 2023-08-31 12:37 UTC (permalink / raw)
To: Daniel Borkmann, Jiri Olsa
Cc: bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Hou Tao,
Alexei Starovoitov, Andrii Nakryiko
Hi,
On 8/31/2023 7:46 PM, Daniel Borkmann wrote:
> On 8/31/23 1:00 PM, Jiri Olsa wrote:
>> Recent commit [1] broken d_path test, because now filp_close is not
>> called
>> directly from sys_close, but eventually later when the file is finally
>> released.
>>
>> As suggested by Hou Tao we don't need to re-hook the bpf program, but
>> just
>> instead we can use sys_close_range to trigger filp_close synchronously.
>>
>> [1] 021a160abf62 ("fs: use __fput_sync in close(2)")
>> Suggested-by: Hou Tao <houtao@huaweicloud.com>
>> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
>> ---
>> tools/testing/selftests/bpf/prog_tests/d_path.c | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/d_path.c
>> b/tools/testing/selftests/bpf/prog_tests/d_path.c
>> index 911345c526e6..81e34a4a05d1 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/d_path.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/d_path.c
>> @@ -90,7 +90,11 @@ static int trigger_fstat_events(pid_t pid)
>> fstat(indicatorfd, &fileStat);
>> out_close:
>> - /* triggers filp_close */
>> + /* sys_close no longer triggers filp_close, but we can
>> + * call sys_close_range instead which still does
>> + */
>> +#define close(fd) close_range(fd, fd, 0)
>> +
>
> The BPF CI selftest build says:
>
> [...]
> TEST-OBJ [test_progs] lookup_key.test.o
> TEST-OBJ [test_progs] migrate_reuseport.test.o
> TEST-OBJ [test_progs] user_ringbuf.test.o
> /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/d_path.c:
> In function ‘trigger_fstat_events’:
>
> /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/d_path.c:96:19:
> error: implicit declaration of function ‘close_range’
> [-Werror=implicit-function-declaration]
> 96 | #define close(fd) close_range(fd, fd, 0)
> | ^~~~~~~~~~~
>
> /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/d_path.c:98:2:
> note: in expansion of macro ‘close’
> 98 | close(pipefd[0]);
> | ^~~~~
> TEST-OBJ [test_progs] task_pt_regs.test.o
> [...]
>
> Perhaps #include <linux/close_range.h> missing ?
Including <linux/close_range.h> doesn't help because it only defines two
macros.
I got the same error when testing locally. It seems close_range() was
introduced by glibc 2.34 [1] and it was defined in <unistd.h>, but the
version of glibc in my local VM is 2.29. I modify d_path locally to call
close_range through syscall():
diff --git a/tools/testing/selftests/bpf/prog_tests/d_path.c
b/tools/testing/selftests/bpf/prog_tests/d_path.c
index 81e34a4a05d1..c5811843ce7e 100644
--- a/tools/testing/selftests/bpf/prog_tests/d_path.c
+++ b/tools/testing/selftests/bpf/prog_tests/d_path.c
@@ -12,6 +12,14 @@
#include "test_d_path_check_rdonly_mem.skel.h"
#include "test_d_path_check_types.skel.h"
+#ifndef __NR_close_range
+#ifdef __alpha__
+#define __NR_close_range 546
+#else
+#define __NR_close_range 436
+#endif
+#endif
+
static int duration;
static struct {
@@ -93,7 +101,7 @@ static int trigger_fstat_events(pid_t pid)
/* sys_close no longer triggers filp_close, but we can
* call sys_close_range instead which still does
*/
-#define close(fd) close_range(fd, fd, 0)
+#define close(fd) syscall(__NR_close_range, fd, fd, 0)
[1]: 9b4feb630e8e arch: wire-up close_range()
>
>> close(pipefd[0]);
>> close(pipefd[1]);
>> close(sockfd);
>> @@ -98,6 +102,8 @@ static int trigger_fstat_events(pid_t pid)
>> close(devfd);
>> close(localfd);
>> close(indicatorfd);
>> +
>> +#undef close
>> return ret;
>> }
>>
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Fix d_path test
2023-08-31 12:37 ` Hou Tao
@ 2023-08-31 12:52 ` Jiri Olsa
0 siblings, 0 replies; 5+ messages in thread
From: Jiri Olsa @ 2023-08-31 12:52 UTC (permalink / raw)
To: Hou Tao
Cc: Daniel Borkmann, bpf, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Hou Tao,
Alexei Starovoitov, Andrii Nakryiko
On Thu, Aug 31, 2023 at 08:37:52PM +0800, Hou Tao wrote:
> Hi,
>
> On 8/31/2023 7:46 PM, Daniel Borkmann wrote:
> > On 8/31/23 1:00 PM, Jiri Olsa wrote:
> >> Recent commit [1] broken d_path test, because now filp_close is not
> >> called
> >> directly from sys_close, but eventually later when the file is finally
> >> released.
> >>
> >> As suggested by Hou Tao we don't need to re-hook the bpf program, but
> >> just
> >> instead we can use sys_close_range to trigger filp_close synchronously.
> >>
> >> [1] 021a160abf62 ("fs: use __fput_sync in close(2)")
> >> Suggested-by: Hou Tao <houtao@huaweicloud.com>
> >> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> >> ---
> >> tools/testing/selftests/bpf/prog_tests/d_path.c | 8 +++++++-
> >> 1 file changed, 7 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/tools/testing/selftests/bpf/prog_tests/d_path.c
> >> b/tools/testing/selftests/bpf/prog_tests/d_path.c
> >> index 911345c526e6..81e34a4a05d1 100644
> >> --- a/tools/testing/selftests/bpf/prog_tests/d_path.c
> >> +++ b/tools/testing/selftests/bpf/prog_tests/d_path.c
> >> @@ -90,7 +90,11 @@ static int trigger_fstat_events(pid_t pid)
> >> fstat(indicatorfd, &fileStat);
> >> out_close:
> >> - /* triggers filp_close */
> >> + /* sys_close no longer triggers filp_close, but we can
> >> + * call sys_close_range instead which still does
> >> + */
> >> +#define close(fd) close_range(fd, fd, 0)
> >> +
> >
> > The BPF CI selftest build says:
> >
> > [...]
> > TEST-OBJ [test_progs] lookup_key.test.o
> > TEST-OBJ [test_progs] migrate_reuseport.test.o
> > TEST-OBJ [test_progs] user_ringbuf.test.o
> > /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/d_path.c:
> > In function ‘trigger_fstat_events’:
> >
> > /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/d_path.c:96:19:
> > error: implicit declaration of function ‘close_range’
> > [-Werror=implicit-function-declaration]
> > 96 | #define close(fd) close_range(fd, fd, 0)
> > | ^~~~~~~~~~~
> >
> > /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/d_path.c:98:2:
> > note: in expansion of macro ‘close’
> > 98 | close(pipefd[0]);
> > | ^~~~~
> > TEST-OBJ [test_progs] task_pt_regs.test.o
> > [...]
> >
> > Perhaps #include <linux/close_range.h> missing ?
>
> Including <linux/close_range.h> doesn't help because it only defines two
> macros.
right, just saw that ;-)
>
> I got the same error when testing locally. It seems close_range() was
> introduced by glibc 2.34 [1] and it was defined in <unistd.h>, but the
> version of glibc in my local VM is 2.29. I modify d_path locally to call
> close_range through syscall():
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/d_path.c
> b/tools/testing/selftests/bpf/prog_tests/d_path.c
> index 81e34a4a05d1..c5811843ce7e 100644
> --- a/tools/testing/selftests/bpf/prog_tests/d_path.c
> +++ b/tools/testing/selftests/bpf/prog_tests/d_path.c
> @@ -12,6 +12,14 @@
> #include "test_d_path_check_rdonly_mem.skel.h"
> #include "test_d_path_check_types.skel.h"
>
> +#ifndef __NR_close_range
> +#ifdef __alpha__
> +#define __NR_close_range 546
> +#else
> +#define __NR_close_range 436
> +#endif
> +#endif
> +
> static int duration;
>
> static struct {
> @@ -93,7 +101,7 @@ static int trigger_fstat_events(pid_t pid)
> /* sys_close no longer triggers filp_close, but we can
> * call sys_close_range instead which still does
> */
> -#define close(fd) close_range(fd, fd, 0)
> +#define close(fd) syscall(__NR_close_range, fd, fd, 0)
>
> [1]: 9b4feb630e8e arch: wire-up close_range()
nice, thanks for all the info
jirka
>
>
> >
> >> close(pipefd[0]);
> >> close(pipefd[1]);
> >> close(sockfd);
> >> @@ -98,6 +102,8 @@ static int trigger_fstat_events(pid_t pid)
> >> close(devfd);
> >> close(localfd);
> >> close(indicatorfd);
> >> +
> >> +#undef close
> >> return ret;
> >> }
> >>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-08-31 12:52 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-31 11:00 [PATCH bpf-next] selftests/bpf: Fix d_path test Jiri Olsa
2023-08-31 11:46 ` Daniel Borkmann
2023-08-31 12:25 ` Jiri Olsa
2023-08-31 12:37 ` Hou Tao
2023-08-31 12:52 ` Jiri Olsa
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.