Re: [RFC PATCH bpf-next v3 09/12] selftests/bpf: Add tests for memcg_bpf_ops

[hui.zhu@linux.dev]

2026年1月24日 04:47, "JP Kobryn" <inwardvessel@gmail.com mailto:inwardvessel@gmail.com?to=%22JP%20Kobryn%22%20%3Cinwardvessel%40gmail.com%3E > 写到:


> 
> Hi Hui,
> 
> On 1/23/26 1:00 AM, Hui Zhu wrote:
> 
> > 
> > From: Hui Zhu <zhuhui@kylinos.cn>
> >  Add a comprehensive selftest suite for the `memcg_bpf_ops`
> >  functionality. These tests validate that BPF programs can correctly
> >  influence memory cgroup throttling behavior by implementing the new
> >  hooks.
> >  The test suite is added in `prog_tests/memcg_ops.c` and covers
> >  several key scenarios:
> >  1. `test_memcg_ops_over_high`:
> >  Verifies that a BPF program can trigger throttling on a low-priority
> >  cgroup by returning a delay from the `get_high_delay_ms` hook when a
> >  high-priority cgroup is under pressure.
> >  2. `test_memcg_ops_below_low_over_high`:
> >  Tests the combination of the `below_low` and `get_high_delay_ms`
> >  hooks, ensuring they work together as expected.
> >  3. `test_memcg_ops_below_min_over_high`:
> >  Validates the interaction between the `below_min` and
> >  `get_high_delay_ms` hooks.
> >  The test framework sets up a cgroup hierarchy with high and low
> >  priority groups, attaches BPF programs, runs memory-intensive
> >  workloads, and asserts that the observed throttling (measured by
> >  workload execution time) matches expectations.
> >  The BPF program (`progs/memcg_ops.c`) uses a tracepoint on
> >  `memcg:count_memcg_events` (specifically PGFAULT) to detect memory
> >  pressure and trigger the appropriate hooks in response. This test
> >  suite provides essential validation for the new memory control
> >  mechanisms.
> >  Signed-off-by: Geliang Tang <geliang@kernel.org>
> >  Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
> >  ---
> > 
> [..]
> 
> > 
> > diff --git a/tools/testing/selftests/bpf/prog_tests/memcg_ops.c b/tools/testing/selftests/bpf/prog_tests/memcg_ops.c
> >  new file mode 100644
> >  index 000000000000..9a8d16296f2d
> >  --- /dev/null
> >  +++ b/tools/testing/selftests/bpf/prog_tests/memcg_ops.c
> >  @@ -0,0 +1,537 @@
> > 
> [..]
> 
> > 
> > +
> >  +static void
> >  +real_test_memcg_ops_child_work(const char *cgroup_path,
> >  + char *data_filename,
> >  + char *time_filename,
> >  + int read_times)
> >  +{
> >  + struct timeval start, end;
> >  + double elapsed;
> >  + FILE *fp;
> >  +
> >  + if (!ASSERT_OK(join_parent_cgroup(cgroup_path), "join_parent_cgroup"))
> >  + goto out;
> >  +
> >  + if (env.verbosity >= VERBOSE_NORMAL)
> >  + printf("%s %d begin\n", __func__, getpid());
> >  +
> >  + gettimeofday(&start, NULL);
> >  +
> >  + if (!ASSERT_OK(write_file(data_filename), "write_file"))
> >  + goto out;
> >  +
> >  + if (env.verbosity >= VERBOSE_NORMAL)
> >  + printf("%s %d write_file done\n", __func__, getpid());
> >  +
> >  + if (!ASSERT_OK(read_file(data_filename, read_times), "read_file"))
> >  + goto out;
> >  +
> >  + gettimeofday(&end, NULL);
> >  +
> >  + elapsed = (end.tv_sec - start.tv_sec) +
> >  + (end.tv_usec - start.tv_usec) / 1000000.0;
> >  +
> >  + if (env.verbosity >= VERBOSE_NORMAL)
> >  + printf("%s %d end %.6f\n", __func__, getpid(), elapsed);
> >  +
> >  + fp = fopen(time_filename, "w");
> >  + if (!ASSERT_OK_PTR(fp, "fopen"))
> >  + goto out;
> >  + fprintf(fp, "%.6f", elapsed);
> >  + fclose(fp);
> >  +
> >  +out:
> >  + exit(0);
> >  +}
> >  +
> > 
> [..]
> 
> > 
> > +static void real_test_memcg_ops(int read_times)
> >  +{
> >  + int ret;
> >  + char data_file1[] = "/tmp/test_data_XXXXXX";
> >  + char data_file2[] = "/tmp/test_data_XXXXXX";
> >  + char time_file1[] = "/tmp/test_time_XXXXXX";
> >  + char time_file2[] = "/tmp/test_time_XXXXXX";
> >  + pid_t pid1, pid2;
> >  + double time1, time2;
> >  +
> >  + ret = mkstemp(data_file1);
> >  + if (!ASSERT_GT(ret, 0, "mkstemp"))
> >  + return;
> >  + close(ret);
> >  + ret = mkstemp(data_file2);
> >  + if (!ASSERT_GT(ret, 0, "mkstemp"))
> >  + goto cleanup_data_file1;
> >  + close(ret);
> >  + ret = mkstemp(time_file1);
> >  + if (!ASSERT_GT(ret, 0, "mkstemp"))
> >  + goto cleanup_data_file2;
> >  + close(ret);
> >  + ret = mkstemp(time_file2);
> >  + if (!ASSERT_GT(ret, 0, "mkstemp"))
> >  + goto cleanup_time_file1;
> >  + close(ret);
> >  +
> >  + pid1 = fork();
> >  + if (!ASSERT_GE(pid1, 0, "fork"))
> >  + goto cleanup;
> >  + if (pid1 == 0)
> >  + real_test_memcg_ops_child_work(CG_LOW_DIR,
> >  + data_file1,
> >  + time_file1,
> >  + read_times);
> > 
> Would it be better to call exit() after real_test_memcg_ops_child_work()
> instead of within it? This way the fork/exit/wait logic is contained in
> the same scope making the lifetimes easier to track. I had to go back
> and search for the call to exit() since at a glance this function
> appears to proceed to call fork() and waitpid() from within both parent
> and child procs (though it really does not).
>

I will fix it.

Best,
Hui