On Thu, Aug 31, 2023 at 03:37:37PM +0800, Hou Tao wrote: > Hi > > On 8/30/2023 5:35 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. > > To make test_d_path self-test pass, beside attaching to a different > function (e.g., __fput_sync or filp_flush), we could also use > close_range() or even dup2() to close the created fd, because these > syscalls still use filp_close() to close the opened file. nice, I like the close_range solution ;-) patch below fixes the test > > > > > I can't see any other solution than to hook filp_flush function and > > that also means we need to add it to btf_allowlist_d_path list, so > > it can use the d_path helper. > > > > But it's probably not very stable because filp_flush is static so it > > could be potentially inlined. > > > > Also if we'd keep the current filp_close hook and find a way how to 'wait' > > for it to be called so user space can go with checks, then it looks > > like d_path might not work properly when the task is no longer around. > > It seems there is no need to wait for it to be called, because > filp_close() is still called synchronously by some syscall (e.g., > close_range or io_uring). So if the bpf program tries to collect many > close event as possible, it should be attach to both filp_close() and > __fput_sync(), right ? > right, when we hook to close_range it's still synchronous thanks, jirka --- diff --git a/tools/testing/selftests/bpf/prog_tests/d_path.c b/tools/testing/selftests/bpf/prog_tests/d_path.c index 911345c526e6..8a558c980753 100644 --- a/tools/testing/selftests/bpf/prog_tests/d_path.c +++ b/tools/testing/selftests/bpf/prog_tests/d_path.c @@ -91,6 +91,7 @@ static int trigger_fstat_events(pid_t pid) out_close: /* triggers filp_close */ +#define close(fd) close_range(fd, fd, 0) close(pipefd[0]); close(pipefd[1]); close(sockfd); @@ -98,6 +99,7 @@ static int trigger_fstat_events(pid_t pid) close(devfd); close(localfd); close(indicatorfd); +#undef close return ret; }
