All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Qiliang Yuan" <odys.yuan@gmail.com>
Cc: bpf@vger.kernel.org
Subject: Re: [PATCH bpf-next v2 1/2] selftests/bpf: Keep the bpffs of cgroup_hierarchical_stats private
Date: Fri, 09 Oct 2026 02:12:54 +0000	[thread overview]
Message-ID: <sashiko-outbox-164719@kernel.org> (raw)
In-Reply-To: <20261009-selftests-bpf-cgroup-stats-bpffs-v2-1-f93a111da992@gmail.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] bpf: unbalanced lifecycle in destroy_hierarchy() teardown
--

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

selftests/bpf: Keep the bpffs of cgroup_hierarchical_stats private

setup_hierarchy() mounts a bpffs on /sys/fs/bpf with setup_bpffs(), and
then sets up the cgroups with setup_cgroups(), which moves the test into
a private mount namespace. The patch sets up the cgroups first, so that
the bpffs is mounted in the private mount 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 3bd27d2ea668c..ad19d6d41918a 100644
> --- a/tools/testing/selftests/bpf/prog_tests/cgroup_hierarchical_stats.c
> +++ b/tools/testing/selftests/bpf/prog_tests/cgroup_hierarchical_stats.c
> @@ -151,7 +151,7 @@ static void cleanup_cgroups(void)
>  /* Sets up cgroup hiearchary, returns 0 on success. */
>  static int setup_hierarchy(void)
>  {
> -	return setup_bpffs() || setup_cgroups();
> +	return setup_cgroups() || setup_bpffs();
>  }
>  
>  static void destroy_hierarchy(void)

[Severity: Low]
Does destroy_hierarchy() need to be updated to reverse the teardown order?

Because the setup order has been changed to initialize cgroups before bpffs,
the test is placed in a private mount namespace before mounting the BPF
filesystem. 

However, destroy_hierarchy() continues to execute teardown in FIFO order
(cleanup_cgroups() followed by cleanup_bpffs()) instead of LIFO order.

While cleanup_cgroups() does not fully destroy the namespace until process
exit (preventing immediate crashes during cleanup_bpffs()), should
cleanup_bpffs() be called before cleanup_cgroups() to properly mirror the
new initialization order?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-selftests-bpf-cgroup-stats-bpffs-v2-0-f93a111da992@gmail.com?part=1

  reply	other threads:[~2026-10-09  2:12 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-09  2:04 [PATCH bpf-next v2 0/2] selftests/bpf: Fix the bpffs handling of cgroup_hierarchical_stats Qiliang Yuan
2026-10-09  2:04 ` [PATCH bpf-next v2 1/2] selftests/bpf: Keep the bpffs of cgroup_hierarchical_stats private Qiliang Yuan
2026-10-09  2:12   ` sashiko-bot [this message]
2026-10-09  2:04 ` [PATCH bpf-next v2 2/2] selftests/bpf: Unmount the bpffs that cgroup_hierarchical_stats mounted Qiliang Yuan
2026-10-09  2:14   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=sashiko-outbox-164719@kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=odys.yuan@gmail.com \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.