Re: [PATCH bpf-next v4 2/5] selftests/bpf: Add tests for open-coded style process file iterator

[Jiri Olsa <olsajiri@gmail.com>]

On Tue, Nov 26, 2024 at 10:24:07PM +0000, Juntong Deng wrote:
> On 2024/11/20 11:27, Jiri Olsa wrote:
> > On Tue, Nov 19, 2024 at 05:53:59PM +0000, Juntong Deng wrote:
> > 
> > SNIP
> > 
> > > +static void subtest_task_file_iters(void)
> > > +{
> > > +	int prog_fd, child_pid, wstatus, err = 0;
> > > +	const int stack_size = 1024 * 1024;
> > > +	struct iters_task_file *skel;
> > > +	struct files_test_args args;
> > > +	struct bpf_program *prog;
> > > +	bool setup_end, test_end;
> > > +	char *stack;
> > > +
> > > +	skel = iters_task_file__open_and_load();
> > > +	if (!ASSERT_OK_PTR(skel, "open_and_load"))
> > > +		return;
> > > +
> > > +	if (!ASSERT_OK(skel->bss->err, "pre_test_err"))
> > > +		goto cleanup_skel;
> > > +
> > > +	prog = bpf_object__find_program_by_name(skel->obj, "test_bpf_iter_task_file");
> > > +	if (!ASSERT_OK_PTR(prog, "find_program_by_name"))
> > > +		goto cleanup_skel;
> > > +
> > > +	prog_fd = bpf_program__fd(prog);
> > > +	if (!ASSERT_GT(prog_fd, -1, "bpf_program__fd"))
> > > +		goto cleanup_skel;
> > 
> > I don't think you need to check on this once we did iters_task_file__open_and_load
> > 
> > > +
> > > +	stack = (char *)malloc(stack_size);
> > > +	if (!ASSERT_OK_PTR(stack, "clone_stack"))
> > > +		goto cleanup_skel;
> > > +
> > > +	setup_end = false;
> > > +	test_end = false;
> > > +
> > > +	args.setup_end = &setup_end;
> > > +	args.test_end = &test_end;
> > > +
> > > +	/* Note that there is no CLONE_FILES */
> > > +	child_pid = clone(task_file_test_process, stack + stack_size, CLONE_VM | SIGCHLD, &args);
> > > +	if (!ASSERT_GT(child_pid, -1, "child_pid"))
> > > +		goto cleanup_stack;
> > > +
> > > +	while (!setup_end)
> > > +		;
> > 
> > I thin kthe preferred way is to synchronize through pipe,
> > you can check prog_tests/uprobe_multi_test.c
> > 
> 
> Thanks for your reply.
> 
> Do we really need to use pipe? Currently this test is very simple.
> 
> In this test, all files opened by the test process will be closed first
> so that there is an empty file description table, and then open the
> test files.
> 
> This way the test process has only 3 newly opened files and the file
> descriptors are always 0, 1, 2.
> 
> Although using pipe is feasible, this test will become more complicated
> than it is now.

I see, I missed the close_range call.. anyway I'd still prefer pipe to busy waiting

perhaps you could use fentry probe triggered by the task_file_test_process
and do the fd/file iteration in there? that way there'be no need for the sync

jirka