Re: [PATCH bpf-next 1/2] selftests/bpf: Add bpf_proactive_reclaim test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] bpf: selftest fails instead of skipping on tmpfs-only environments
--

commit 050e765c2f056e2f47574982f5ca0cece0afd7b8
Author: Hui Zhu <zhuhui@kylinos.cn>

selftests/bpf: Add bpf_proactive_reclaim test

This commit adds a selftest for the bpf_proactive_reclaim kfunc. It verifies
the bounded reclaim behavior by charging page cache to a cgroup and requesting
eviction of the allocated pages in a single call.

> diff --git a/tools/testing/selftests/bpf/prog_tests/memcg_proactive_reclaim.c b/tools/testing/selftests/bpf/prog_tests/memcg_proactive_reclaim.c
> new file mode 100644
> index 0000000000000..d642658a21981
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/memcg_proactive_reclaim.c
[ ... ]
> +void test_memcg_proactive_reclaim(void)
> +{
> +	struct memcg_proactive_reclaim *skel = NULL;
> +	struct reclaim_args args = {};
> +
> +	LIBBPF_OPTS(bpf_test_run_opts, opts,
> +		    .ctx_in = &args,
> +		    .ctx_size_in = sizeof(args));
> +
> +	char data_file[PATH_MAX];
> +	static char buf[BUF_SIZE];
> +	__u64 cgroup_id;
> +	const char *dir;
> +	off_t off;
> +	int cg_fd = -1, data_fd = -1, err;
> +
> +	dir = workload_dir();
> +	if (!ASSERT_OK_PTR(dir, "workload dir on a regular filesystem"))
> +		return;

[Severity: Medium]
Will this cause spurious test failures in tmpfs-only environments?

When workload_dir() cannot find a suitable regular filesystem (for instance,
when both '.' and '/tmp' are tmpfs/ramfs), it returns NULL.

Using ASSERT_OK_PTR() here unconditionally fails the test. Could this be
changed to gracefully skip the test instead via test__skip(), so that CI
environments lacking a disk-backed filesystem do not falsely report a failure?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1790222105.git.zhuhui@kylinos.cn?part=1