[PATCH bpf-next 0/2] selftests/bpf: Fix the bpffs handling of cgroup_hierarchical_stats

[PATCH bpf-next 0/2] selftests/bpf: Fix the bpffs handling of cgroup_hierarchical_stats

[Qiliang Yuan]

cgroup_hierarchical_stats mounts a bpffs on /sys/fs/bpf to pin its
iterators, and unmounts it in the end if the test mounted it itself.

It mounts a bpffs even when one is already there, and its cleanup checks
the other way around whether it did, so it never unmounts it. Each run
leaves another bpffs stacked on /sys/fs/bpf, and while the test runs,
tests running in parallel lose what they pinned, which makes
bpftool_maps_access and bpftool_metadata fail now and then.

Patch 1 makes the cleanup unmount the bpffs that the test mounted,
patch 2 stops mounting one over an existing bpffs.

Signed-off-by: Qiliang Yuan <odys.yuan@gmail.com>
---
Qiliang Yuan (2):
      selftests/bpf: Unmount the bpffs that cgroup_hierarchical_stats mounted
      selftests/bpf: Don't stack a bpffs in cgroup_hierarchical_stats

 .../bpf/prog_tests/cgroup_hierarchical_stats.c      | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)
---
base-commit: e1d84a37cba984388988d2f1ddc84561413f0db2
change-id: 20261008-selftests-bpf-cgroup-stats-bpffs-4534db24d15c

Best regards,
-- 
Qiliang Yuan <odys.yuan@gmail.com>

[PATCH bpf-next 1/2] selftests/bpf: Unmount the bpffs that cgroup_hierarchical_stats mounted

[Qiliang Yuan]

setup_bpffs() mounts a bpffs on /sys/fs/bpf and records in mounted_bpffs
whether it did, so that cleanup_bpffs() unmounts only a bpffs that the
test mounted itself.

cleanup_bpffs() checks the flag the other way around. As mounting on
/sys/fs/bpf succeeds even when a bpffs is already there, the test never
unmounts the bpffs it mounted, and every run leaves another one stacked
on /sys/fs/bpf.

Unmount the bpffs only when setup_bpffs() mounted it.

Fixes: 88886309d2e8 ("selftests/bpf: add a selftest for cgroup hierarchical stats collection")
Signed-off-by: Qiliang Yuan <odys.yuan@gmail.com>
---
 tools/testing/selftests/bpf/prog_tests/cgroup_hierarchical_stats.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_hierarchical_stats.c b/tools/testing/selftests/bpf/prog_tests/cgroup_hierarchical_stats.c
index 3bd27d2ea668c..a93b16ed629e9 100644
--- a/tools/testing/selftests/bpf/prog_tests/cgroup_hierarchical_stats.c
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_hierarchical_stats.c
@@ -110,7 +110,7 @@ static void cleanup_bpffs(void)
 	ASSERT_OK(rmdir(BPFFS_ATTACH_COUNTERS), "rmdir "BPFFS_ATTACH_COUNTERS);
 
 	/* Unmount bpffs, if it wasn't already mounted when we started */
-	if (mounted_bpffs)
+	if (!mounted_bpffs)
 		return;
 
 	ASSERT_OK(umount(BPFFS_ROOT), "unmount bpffs");

-- 
2.43.0

[PATCH bpf-next 2/2] selftests/bpf: Don't stack a bpffs in cgroup_hierarchical_stats

[Qiliang Yuan]

setup_bpffs() mounts a bpffs on /sys/fs/bpf even when one is already
there, while test_progs runs other tests that pin objects in /sys/fs/bpf
in parallel.

The new, empty bpffs hides the old one until the test ends, so those
tests lose what they pinned or the directories they pin into.
bpftool_maps_access then fails to pin an iterator because
/sys/fs/bpf/test_bpftool_map doesn't exist anymore, and bpftool_metadata
can't find the program it pinned in /sys/fs/bpf/test_metadata.

Mount a bpffs only if /sys/fs/bpf isn't one already.

Fixes: 88886309d2e8 ("selftests/bpf: add a selftest for cgroup hierarchical stats collection")
Signed-off-by: Qiliang Yuan <odys.yuan@gmail.com>
---
 .../bpf/prog_tests/cgroup_hierarchical_stats.c        | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_hierarchical_stats.c b/tools/testing/selftests/bpf/prog_tests/cgroup_hierarchical_stats.c
index a93b16ed629e9..8e0b67df700bf 100644
--- a/tools/testing/selftests/bpf/prog_tests/cgroup_hierarchical_stats.c
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_hierarchical_stats.c
@@ -23,8 +23,10 @@
 #include <asm-generic/errno.h>
 #include <errno.h>
 #include <sys/types.h>
+#include <linux/magic.h>
 #include <sys/mount.h>
 #include <sys/stat.h>
+#include <sys/vfs.h>
 #include <unistd.h>
 
 #include <test_progs.h>
@@ -88,13 +90,22 @@ static int read_from_file(const char *path, char *buf, size_t size)
 /* mounts bpffs and mkdir for reading stats, returns 0 on success. */
 static int setup_bpffs(void)
 {
+	struct statfs st;
 	int err;
 
-	/* Mount bpffs */
-	err = mount("bpf", BPFFS_ROOT, "bpf", 0, NULL);
-	mounted_bpffs = !err;
-	if (ASSERT_FALSE(err && errno != EBUSY, "mount"))
+	/*
+	 * Mount bpffs unless it is already there. Mounting another one on top
+	 * would hide what other tests running in parallel pin in it.
+	 */
+	err = statfs(BPFFS_ROOT, &st);
+	if (!ASSERT_OK(err, "statfs " BPFFS_ROOT))
 		return err;
+	if (st.f_type != BPF_FS_MAGIC) {
+		err = mount("bpf", BPFFS_ROOT, "bpf", 0, NULL);
+		if (!ASSERT_OK(err, "mount"))
+			return err;
+		mounted_bpffs = true;
+	}
 
 	/* Create a directory to contain stat files in bpffs */
 	err = mkdir(BPFFS_ATTACH_COUNTERS, 0755);

-- 
2.43.0

Re: [PATCH bpf-next 1/2] selftests/bpf: Unmount the bpffs that cgroup_hierarchical_stats mounted

[Yosry Ahmed]

On Thu, Oct 8, 2026 at 7:03 AM Qiliang Yuan <odys.yuan@gmail.com> wrote:
>
> setup_bpffs() mounts a bpffs on /sys/fs/bpf and records in mounted_bpffs
> whether it did, so that cleanup_bpffs() unmounts only a bpffs that the
> test mounted itself.
>
> cleanup_bpffs() checks the flag the other way around. As mounting on
> /sys/fs/bpf succeeds even when a bpffs is already there, the test never
> unmounts the bpffs it mounted, and every run leaves another one stacked
> on /sys/fs/bpf.
>
> Unmount the bpffs only when setup_bpffs() mounted it.
>
> Fixes: 88886309d2e8 ("selftests/bpf: add a selftest for cgroup hierarchical stats collection")
> Signed-off-by: Qiliang Yuan <odys.yuan@gmail.com>

Reviewed-by: Yosry Ahmed <yosry@kernel.org>

> ---
>  tools/testing/selftests/bpf/prog_tests/cgroup_hierarchical_stats.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_hierarchical_stats.c b/tools/testing/selftests/bpf/prog_tests/cgroup_hierarchical_stats.c
> index 3bd27d2ea668c..a93b16ed629e9 100644
> --- a/tools/testing/selftests/bpf/prog_tests/cgroup_hierarchical_stats.c
> +++ b/tools/testing/selftests/bpf/prog_tests/cgroup_hierarchical_stats.c
> @@ -110,7 +110,7 @@ static void cleanup_bpffs(void)
>         ASSERT_OK(rmdir(BPFFS_ATTACH_COUNTERS), "rmdir "BPFFS_ATTACH_COUNTERS);
>
>         /* Unmount bpffs, if it wasn't already mounted when we started */
> -       if (mounted_bpffs)
> +       if (!mounted_bpffs)
>                 return;
>
>         ASSERT_OK(umount(BPFFS_ROOT), "unmount bpffs");
>
> --
> 2.43.0
>

Re: [PATCH bpf-next 2/2] selftests/bpf: Don't stack a bpffs in cgroup_hierarchical_stats

[Yosry Ahmed]

On Thu, Oct 8, 2026 at 7:03 AM Qiliang Yuan <odys.yuan@gmail.com> wrote:
>
> setup_bpffs() mounts a bpffs on /sys/fs/bpf even when one is already
> there, while test_progs runs other tests that pin objects in /sys/fs/bpf
> in parallel.
>
> The new, empty bpffs hides the old one until the test ends, so those
> tests lose what they pinned or the directories they pin into.
> bpftool_maps_access then fails to pin an iterator because
> /sys/fs/bpf/test_bpftool_map doesn't exist anymore, and bpftool_metadata
> can't find the program it pinned in /sys/fs/bpf/test_metadata.

Seems like both of these tests do not mount bpffs, but rely on its
existence. If it's generally expected that bpffs is mounted when the
tests are run, should we drop the mounting code here instead?

Alternatively, should we have a common helper for creating directories
in bpffs that mounts it if it isn't already there?

>
> Mount a bpffs only if /sys/fs/bpf isn't one already.
>
> Fixes: 88886309d2e8 ("selftests/bpf: add a selftest for cgroup hierarchical stats collection")
> Signed-off-by: Qiliang Yuan <odys.yuan@gmail.com>
> ---
>  .../bpf/prog_tests/cgroup_hierarchical_stats.c        | 19 +++++++++++++++----
>  1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_hierarchical_stats.c b/tools/testing/selftests/bpf/prog_tests/cgroup_hierarchical_stats.c
> index a93b16ed629e9..8e0b67df700bf 100644
> --- a/tools/testing/selftests/bpf/prog_tests/cgroup_hierarchical_stats.c
> +++ b/tools/testing/selftests/bpf/prog_tests/cgroup_hierarchical_stats.c
> @@ -23,8 +23,10 @@
>  #include <asm-generic/errno.h>
>  #include <errno.h>
>  #include <sys/types.h>
> +#include <linux/magic.h>
>  #include <sys/mount.h>
>  #include <sys/stat.h>
> +#include <sys/vfs.h>
>  #include <unistd.h>
>
>  #include <test_progs.h>
> @@ -88,13 +90,22 @@ static int read_from_file(const char *path, char *buf, size_t size)
>  /* mounts bpffs and mkdir for reading stats, returns 0 on success. */
>  static int setup_bpffs(void)
>  {
> +       struct statfs st;
>         int err;
>
> -       /* Mount bpffs */
> -       err = mount("bpf", BPFFS_ROOT, "bpf", 0, NULL);
> -       mounted_bpffs = !err;
> -       if (ASSERT_FALSE(err && errno != EBUSY, "mount"))
> +       /*
> +        * Mount bpffs unless it is already there. Mounting another one on top
> +        * would hide what other tests running in parallel pin in it.
> +        */
> +       err = statfs(BPFFS_ROOT, &st);
> +       if (!ASSERT_OK(err, "statfs " BPFFS_ROOT))
>                 return err;
> +       if (st.f_type != BPF_FS_MAGIC) {
> +               err = mount("bpf", BPFFS_ROOT, "bpf", 0, NULL);
> +               if (!ASSERT_OK(err, "mount"))
> +                       return err;
> +               mounted_bpffs = true;
> +       }
>
>         /* Create a directory to contain stat files in bpffs */
>         err = mkdir(BPFFS_ATTACH_COUNTERS, 0755);
>
> --
> 2.43.0
>

Re: [PATCH bpf-next 2/2] selftests/bpf: Don't stack a bpffs in cgroup_hierarchical_stats

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] selftests/bpf: stale mounted_bpffs flag causes unintended unmount of shared bpffs
- [Medium] selftests/bpf: uninitialized fds close stdin on setup error path
- [Low] bpf: selftests: spurious test failure due to unhandled EEXIST on mkdir
--

commit 73ba1f06cd40bfa82a4b5df33520980276eaa88f
Author: Qiliang Yuan <odys.yuan@gmail.com>

selftests/bpf: Don't stack a bpffs in cgroup_hierarchical_stats

This commit changes the test to only mount a new BPF filesystem if one is
not already present at /sys/fs/bpf. It avoids hiding an existing mount,
which would cause parallel tests to lose access to their pinned objects.

> diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_hierarchical_stats.c b/tools/testing/selftests/bpf/prog_tests/cgroup_hierarchical_stats.c
> index a93b16ed629e9..8e0b67df700bf 100644
> --- a/tools/testing/selftests/bpf/prog_tests/cgroup_hierarchical_stats.c
> +++ b/tools/testing/selftests/bpf/prog_tests/cgroup_hierarchical_stats.c

[ ... ]

> @@ -88,13 +90,22 @@ static int read_from_file(const char *path, char *buf, size_t size)
>  /* mounts bpffs and mkdir for reading stats, returns 0 on success. */
>  static int setup_bpffs(void)
>  {
> +	struct statfs st;
>  	int err;
>  
> -	/* Mount bpffs */
> -	err = mount("bpf", BPFFS_ROOT, "bpf", 0, NULL);
> -	mounted_bpffs = !err;
> -	if (ASSERT_FALSE(err && errno != EBUSY, "mount"))
> +	/*
> +	 * Mount bpffs unless it is already there. Mounting another one on top
> +	 * would hide what other tests running in parallel pin in it.
> +	 */
> +	err = statfs(BPFFS_ROOT, &st);
> +	if (!ASSERT_OK(err, "statfs " BPFFS_ROOT))
>  		return err;

[Severity: Medium]
If this statfs() check fails, does the subsequent error handling close stdin?

Looking at the error path, if setup_bpffs() returns an error here,
destroy_hierarchy() calls cleanup_cgroups(), which iterates over the
cgroups array and calls close(cgroups[i].fd) and close(root_cgroup_fd):

cleanup_cgroups() {
	close(root_cgroup_fd);
	for (int i = 0; i < N_CGROUPS; i++)
		close(cgroups[i].fd);
	...
}

Since these are static variables without explicit initialization, they
are implicitly initialized to 0. If setup_cgroups() hasn't been called
yet, wouldn't this call close(0) (stdin) multiple times?

In a parallel test runner environment, closing file descriptor 0 can lead
to subsequent opens unexpectedly getting file descriptor 0, which would then
be inadvertently closed by the remaining loop iterations, potentially
corrupting concurrent test operations.

> +	if (st.f_type != BPF_FS_MAGIC) {
> +		err = mount("bpf", BPFFS_ROOT, "bpf", 0, NULL);
> +		if (!ASSERT_OK(err, "mount"))
> +			return err;
> +		mounted_bpffs = true;
> +	}

[Severity: Medium]
Is it possible for the mounted_bpffs flag to remain true from a previous
test run and cause an unintended unmount?

The test runner executes tests sequentially in a single process, so BSS
variables like mounted_bpffs persist across iterations. On the first run,
if the BPFFS is not already mounted, setup_bpffs() mounts it and sets
mounted_bpffs = true.

If cleanup_bpffs() unmounts it but fails to reset mounted_bpffs to false,
on a subsequent run where BPFFS is already mounted, st.f_type ==
BPF_FS_MAGIC is true, and the block above is skipped.

Since mounted_bpffs is still true from the previous run, will
cleanup_bpffs() erroneously unmount the shared BPFFS that it did not mount,
destroying objects pinned by parallel tests?

>  
>  	/* Create a directory to contain stat files in bpffs */
>  	err = mkdir(BPFFS_ATTACH_COUNTERS, 0755);
>  	if (!ASSERT_OK(err, "mkdir"))
>  		return err;

[Severity: Low]
Does this strict ASSERT_OK() check cause spurious test failures if the
directory already exists?

Since the test now reuses an existing BPFFS instead of shadowing it with
a new empty mount, if a previous test run crashes or is aborted, it leaves
behind the BPFFS_ATTACH_COUNTERS directory.

On the next run, mkdir() would return -1 with errno == EEXIST, causing
this check to fail and abort the test. Any pinned files left inside the
directory would also cause subsequent bpf_link__pin() calls to fail with
EEXIST.

Should this gracefully handle the EEXIST case and cleanup leftover state
when reusing a shared filesystem?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008-selftests-bpf-cgroup-stats-bpffs-v1-0-76b9a6d1be34@gmail.com?part=2

Re: [PATCH bpf-next 2/2] selftests/bpf: Don't stack a bpffs in cgroup_hierarchical_stats

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 4726 bytes --]

> selftests/bpf: Don't stack a bpffs in cgroup_hierarchical_stats

TL;DR:
- Commit message may be inaccurate: bpffs mounted before the cgroup
  mount-namespace unshare seems never unmounted afterwards
- Unanswered v1 question: drop the mounting code or add a common helper?

> selftests/bpf: Don't stack a bpffs in cgroup_hierarchical_stats
>
> setup_bpffs() mounts a bpffs on /sys/fs/bpf even when one is already
> there, while test_progs runs other tests that pin objects in /sys/fs/bpf
> in parallel.
>
> The new, empty bpffs hides the old one until the test ends, so those
> tests lose what they pinned or the directories they pin into.
> bpftool_maps_access then fails to pin an iterator because
> /sys/fs/bpf/test_bpftool_map doesn't exist anymore, and bpftool_metadata
> can't find the program it pinned in /sys/fs/bpf/test_metadata.
>
> Mount a bpffs only if /sys/fs/bpf isn't one already.
>
> Fixes: 88886309d2e8 ("selftests/bpf: add a selftest for cgroup hierarchical stats collection")
> Signed-off-by: Qiliang Yuan <odys.yuan@gmail.com>

The changelog says the new bpffs hides the old one "until the test ends".
Is that accurate?

setup_hierarchy() calls setup_bpffs() first and setup_cgroups() second:

    static int setup_hierarchy(void)
    {
        return setup_bpffs() || setup_cgroups();
    }

setup_cgroups() goes through setup_cgroup_environment(), which moves the
process into a private mount namespace:

    tools/testing/selftests/bpf/cgroup_helpers.c:setup_cgroup_environment() {
        ...
        if (unshare(CLONE_NEWNS)) {
        ...
        if (mount("none", "/", NULL, MS_REC | MS_PRIVATE, NULL)) {
        ...
    }

So a bpffs mounted by setup_bpffs() lands in the original mount namespace,
and the test then continues in a private copy and never switches back
(cgroup_helpers.c has no setns()).

cleanup_bpffs() runs after cleanup_cgroups() from destroy_hierarchy(), so
its umount(BPFFS_ROOT) only removes the private copy.  The empty bpffs in
the original namespace seems to stay on top of the real one after the test
ends, and other processes in that namespace would keep losing their pins.

The same applies to the mounted_bpffs = true path that this patch keeps:
the umount in cleanup_bpffs() does not undo the mount done in
setup_bpffs().

The check for an existing bpffs avoids the stacking, but should the
changelog be reworded to describe this accurately?  Alternatively, could the
bpffs setup be done after setup_cgroup_environment(), or the unmount be done
in the original namespace?

> diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_hierarchical_stats.c b/tools/testing/selftests/bpf/prog_tests/cgroup_hierarchical_stats.c
> index a93b16ed629e..8e0b67df700b 100644
> --- a/tools/testing/selftests/bpf/prog_tests/cgroup_hierarchical_stats.c
> +++ b/tools/testing/selftests/bpf/prog_tests/cgroup_hierarchical_stats.c

[ ... ]

> @@ -88,13 +90,22 @@ static int read_from_file(const char *path, char *buf, size_t size)
>  /* mounts bpffs and mkdir for reading stats, returns 0 on success. */
>  static int setup_bpffs(void)
>  {
> +	struct statfs st;
>  	int err;
>
> -	/* Mount bpffs */
> -	err = mount("bpf", BPFFS_ROOT, "bpf", 0, NULL);
> -	mounted_bpffs = !err;
> -	if (ASSERT_FALSE(err && errno != EBUSY, "mount"))
> +	/*
> +	 * Mount bpffs unless it is already there. Mounting another one on top
> +	 * would hide what other tests running in parallel pin in it.
> +	 */
> +	err = statfs(BPFFS_ROOT, &st);
> +	if (!ASSERT_OK(err, "statfs " BPFFS_ROOT))
>  		return err;
> +	if (st.f_type != BPF_FS_MAGIC) {
> +		err = mount("bpf", BPFFS_ROOT, "bpf", 0, NULL);
> +		if (!ASSERT_OK(err, "mount"))
> +			return err;
> +		mounted_bpffs = true;
> +	}

Yosry Ahmed raised a design question on v1 that does not appear to have
been answered.  The other tests that break (bpftool_maps_access and
bpftool_metadata) do not mount bpffs themselves, they rely on it already
existing:

> Seems like both of these tests do not mount bpffs, but rely on its
> existence. If it's generally expected that bpffs is mounted when the tests
> are run, should we drop the mounting code here instead? Alternatively,
> should we have a common helper for creating directories in bpffs that
> mounts it if it isn't already there?

Since this patch keeps the conditional mount in setup_bpffs(), should the
mounting code be dropped here, or should a common helper that creates
directories in bpffs and mounts it only when missing be introduced?

[ ... ]


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/37792241939

Re: [PATCH bpf-next 2/2] selftests/bpf: Don't stack a bpffs in cgroup_hierarchical_stats

[Qiliang Yuan]

On Thu, Oct 8, 2026 at 7:11 AM Yosry Ahmed <yosry@kernel.org> wrote:
> Seems like both of these tests do not mount bpffs, but rely on its
> existence. If it's generally expected that bpffs is mounted when the
> tests are run, should we drop the mounting code here instead?
>
> Alternatively, should we have a common helper for creating directories
> in bpffs that mounts it if it isn't already there?

Thanks for the review. Looking further, the bpffs is mounted in the
wrong place: setup_bpffs() runs before setup_cgroups() moves the test
into a private mount namespace, so the bpffs lands in the namespace
shared with the other tests, and cleanup_bpffs() can't unmount it from
the private one. The BPF CI bot pointed that out as well.

v2 sets up the cgroups first, so that the bpffs is mounted in the
private mount namespace. The test keeps working where no bpffs is
mounted, and it no longer affects any other test, so neither dropping
the mount nor a common helper is needed for this. I kept your
Reviewed-by on the unmount fix, whose code didn't change.

v2: https://lore.kernel.org/r/20261009-selftests-bpf-cgroup-stats-bpffs-v2-0-f93a111da992@gmail.com

Thanks,
Qiliang