From: Yonghong Song <yonghong.song@linux.dev>
To: Manu Bretelle <chantr4@gmail.com>,
Kui-Feng Lee <sinquersw@gmail.com>,
ast@kernel.org
Cc: bpf@vger.kernel.org, andrii@kernel.org, daniel@iogearbox.net,
martin.lau@linux.dev, song@kernel.org, john.fastabend@gmail.com,
kpsingh@kernel.org, sdf@google.com, haoluo@google.com,
jolsa@kernel.org, mykolal@fb.com, shuah@kernel.org
Subject: Re: [PATCH bpf-next] selftests/bpf: umount children of TDIR in test_bpffs
Date: Thu, 26 Oct 2023 23:08:35 -0700 [thread overview]
Message-ID: <6e4e46a2-77de-45d0-a1ec-b5622e1d75e0@linux.dev> (raw)
In-Reply-To: <ZTiqp7URqNjqrSEk@surya>
On 10/24/23 10:41 PM, Manu Bretelle wrote:
> On Tue, Oct 24, 2023 at 02:29:19PM -0700, Kui-Feng Lee wrote:
>>
>> On 10/24/23 13:18, Manu Bretelle wrote:
>>> Currently this tests tries to umount /sys/kernel/debug (TDIR) but the
>>> system it is running on may have mounts below.
>>>
>>> For example, danobi/vmtest [0] VMs have
>>> mount -t tracefs tracefs /sys/kernel/debug/tracing
>>> as part of their init.
>>>
>>> This change list mounts and will umount any mounts below TDIR before
>>> umounting TDIR itself.
>>>
>>> Note that it is not umounting recursively, so in the case of a sub-mount
>>> of TDIR having another sub-mount, this will fail as mtab is ordered.
>> Should we move TID to a random path likes "/sys/kernel/debug-<pid>/"?
>>
> Fair point, I suppose we would want to keep TDIR a defined string as it does
> simplify the gymnastic involved through the rest of the script, but yeah
> looking at the original commit:
> edb65ee5aa25 (selftests/bpf: Add bpffs preload test)
>
> I don't see any reason to use an alternate directory and rather mkdir it vs
> umounting the original one.
> so something like
>
> #define TDIR "/sys/kernel/test_bpffs"
>
> Would probably do.
>
> Alexei could confirm his original intent probably.
Maybe/sys/kernel/tracing should work too? Not sure whether it is universally
available or not.
>
>>> Test:
>>>
>>> Originally:
>>>
>>> $ vmtest -k $KERNEL_REPO/arch/x86_64/boot/bzImage "./test_progs -vv -a test_bpffs"
>>> => bzImage
>>> ===> Booting
>>> ===> Setting up VM
>>> ===> Running command
>>> [ 2.138818] bpf_testmod: loading out-of-tree module taints kernel.
>>> [ 2.140913] bpf_testmod: module verification failed: signature and/or required key missing - tainting kernel
>>> bpf_testmod.ko is already unloaded.
>>> Loading bpf_testmod.ko...
>>> Successfully loaded bpf_testmod.ko.
>>> test_test_bpffs:PASS:clone 0 nsec
>>> fn:PASS:unshare 0 nsec
>>> fn:PASS:mount / 0 nsec
>>> fn:FAIL:umount /sys/kernel/debug unexpected error: -1 (errno 16)
>>> bpf_testmod.ko is already unloaded.
>>> Loading bpf_testmod.ko...
>>> Successfully loaded bpf_testmod.ko.
>>> test_test_bpffs:PASS:clone 0 nsec
>>> test_test_bpffs:PASS:waitpid 0 nsec
>>> test_test_bpffs:FAIL:bpffs test failed 255#282 test_bpffs:FAIL
>>> Summary: 0/0 PASSED, 0 SKIPPED, 1 FAILED
>>> Successfully unloaded bpf_testmod.ko.
>>> Command failed with exit code: 1
>>>
>>> After this change:
>>>
>>> $ vmtest -k $KERNEL_REPO/arch/x86_64/boot/bzImage "./test_progs -vv -a test_bpffs"
>>> => bzImage
>>> ===> Booting
>>> ===> Setting up VM
>>> ===> Running command
>>> [ 2.035210] bpf_testmod: loading out-of-tree module taints kernel.
>>> [ 2.036510] bpf_testmod: module verification failed: signature and/or required key missing - tainting kernel
>>> bpf_testmod.ko is already unloaded.
>>> Loading bpf_testmod.ko...
>>> Successfully loaded bpf_testmod.ko.
>>> test_test_bpffs:PASS:clone 0 nsec
>>> fn:PASS:unshare 0 nsec
>>> fn:PASS:mount / 0 nsec
>>> fn:PASS:accessing /etc/mtab 0 nsec
>>> fn:PASS:umount /sys/kernel/debug/tracing 0 nsec
>>> fn:PASS:umount /sys/kernel/debug 0 nsec
>>> fn:PASS:mount tmpfs 0 nsec
>>> fn:PASS:mkdir /sys/kernel/debug/fs1 0 nsec
>>> fn:PASS:mkdir /sys/kernel/debug/fs2 0 nsec
>>> fn:PASS:mount bpffs /sys/kernel/debug/fs1 0 nsec
>>> fn:PASS:mount bpffs /sys/kernel/debug/fs2 0 nsec
>>> fn:PASS:reading /sys/kernel/debug/fs1/maps.debug 0 nsec
>>> fn:PASS:reading /sys/kernel/debug/fs2/progs.debug 0 nsec
>>> fn:PASS:creating /sys/kernel/debug/fs1/a 0 nsec
>>> fn:PASS:creating /sys/kernel/debug/fs1/a/1 0 nsec
>>> fn:PASS:creating /sys/kernel/debug/fs1/b 0 nsec
>>> fn:PASS:create_map(ARRAY) 0 nsec
>>> fn:PASS:pin map 0 nsec
>>> fn:PASS:stat(/sys/kernel/debug/fs1/a) 0 nsec
>>> fn:PASS:renameat2(/fs1/a, /fs1/b, RENAME_EXCHANGE) 0 nsec
>>> fn:PASS:stat(/sys/kernel/debug/fs1/b) 0 nsec
>>> fn:PASS:b should have a's inode 0 nsec
>>> fn:PASS:access(/sys/kernel/debug/fs1/b/1) 0 nsec
>>> fn:PASS:stat(/sys/kernel/debug/fs1/map) 0 nsec
>>> fn:PASS:renameat2(/fs1/c, /fs1/b, RENAME_EXCHANGE) 0 nsec
>>> fn:PASS:stat(/sys/kernel/debug/fs1/b) 0 nsec
>>> fn:PASS:b should have c's inode 0 nsec
>>> fn:PASS:access(/sys/kernel/debug/fs1/c/1) 0 nsec
>>> fn:PASS:renameat2(RENAME_NOREPLACE) 0 nsec
>>> fn:PASS:access(/sys/kernel/debug/fs1/b) 0 nsec
>>> bpf_testmod.ko is already unloaded.
>>> Loading bpf_testmod.ko...
>>> Successfully loaded bpf_testmod.ko.
>>> test_test_bpffs:PASS:clone 0 nsec
>>> test_test_bpffs:PASS:waitpid 0 nsec
>>> test_test_bpffs:PASS:bpffs test 0 nsec
>>> #282 test_bpffs:OK
>>> Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED
>>> Successfully unloaded bpf_testmod.ko.
>>>
>>> [0] https://github.com/danobi/vmtest
>>>
>>> Signed-off-by: Manu Bretelle <chantr4@gmail.com>
>>> ---
>>> .../selftests/bpf/prog_tests/test_bpffs.c | 28 +++++++++++++++++++
>>> 1 file changed, 28 insertions(+)
>>>
>>> diff --git a/tools/testing/selftests/bpf/prog_tests/test_bpffs.c b/tools/testing/selftests/bpf/prog_tests/test_bpffs.c
>>> index 214d9f4a94a5..001bf694c269 100644
>>> --- a/tools/testing/selftests/bpf/prog_tests/test_bpffs.c
>>> +++ b/tools/testing/selftests/bpf/prog_tests/test_bpffs.c
>>> @@ -3,12 +3,14 @@
>>> #define _GNU_SOURCE
>>> #include <stdio.h>
>>> #include <sched.h>
>>> +#include <mntent.h>
>>> #include <sys/mount.h>
>>> #include <sys/stat.h>
>>> #include <sys/types.h>
>>> #include <test_progs.h>
>>> #define TDIR "/sys/kernel/debug"
>>> +#define MTAB "/etc/mtab"
>>> static int read_iter(char *file)
>>> {
>>> @@ -32,6 +34,8 @@ static int read_iter(char *file)
>>> static int fn(void)
>>> {
>>> + /* A buffer to store logging messages */
>>> + char buf[1024];
>>> struct stat a, b, c;
>>> int err, map;
>>> @@ -43,6 +47,30 @@ static int fn(void)
>>> if (!ASSERT_OK(err, "mount /"))
>>> goto out;
>>> + /* TDIR may have mounts below. unount them first */
>>> + FILE *mtab = setmntent(MTAB, "r");
>>> +
>>> + if (!ASSERT_TRUE(mtab != NULL, "accessing " MTAB)) {
>>> + err = errno;
>>> + goto out;
>>> + }
>>> +
>>> + struct mntent *mnt = NULL;
>>> +
>>> + while ((mnt = getmntent(mtab)) != NULL) {
>>> + if (strlen(mnt->mnt_dir) > strlen(TDIR) &&
>>> + strncmp(TDIR, mnt->mnt_dir, strlen(TDIR)) == 0) {
>>> + snprintf(buf, sizeof(buf) - 1, "umount %s", mnt->mnt_dir);
>>> + err = umount(mnt->mnt_dir);
>>> + if (!ASSERT_OK(err, buf)) {
>>> + endmntent(mtab);
>>> + goto out;
>>> + }
>>> + }
>>> + }
>>> + // Ignore any error here
>>> + endmntent(mtab);
>>> +
>>> err = umount(TDIR);
>>> if (!ASSERT_OK(err, "umount " TDIR))
>>> goto out;
next prev parent reply other threads:[~2023-10-27 6:08 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-24 20:18 [PATCH bpf-next] selftests/bpf: umount children of TDIR in test_bpffs Manu Bretelle
2023-10-24 21:29 ` Kui-Feng Lee
2023-10-25 5:41 ` Manu Bretelle
2023-10-26 21:35 ` Alexei Starovoitov
2023-10-28 0:56 ` Manu Bretelle
2023-10-27 6:08 ` Yonghong Song [this message]
2023-10-27 21:09 ` Manu Bretelle
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=6e4e46a2-77de-45d0-a1ec-b5622e1d75e0@linux.dev \
--to=yonghong.song@linux.dev \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=chantr4@gmail.com \
--cc=daniel@iogearbox.net \
--cc=haoluo@google.com \
--cc=john.fastabend@gmail.com \
--cc=jolsa@kernel.org \
--cc=kpsingh@kernel.org \
--cc=martin.lau@linux.dev \
--cc=mykolal@fb.com \
--cc=sdf@google.com \
--cc=shuah@kernel.org \
--cc=sinquersw@gmail.com \
--cc=song@kernel.org \
/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.