* [PATCH bpf-next] selftests/bpf: umount children of TDIR in test_bpffs
@ 2023-10-24 20:18 Manu Bretelle
2023-10-24 21:29 ` Kui-Feng Lee
0 siblings, 1 reply; 7+ messages in thread
From: Manu Bretelle @ 2023-10-24 20:18 UTC (permalink / raw)
To: bpf, andrii, daniel, ast, martin.lau, song, yonghong.song,
john.fastabend, kpsingh, sdf, haoluo, jolsa, mykolal, shuah
Cc: Manu Bretelle
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.
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;
--
2.40.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: umount children of TDIR in test_bpffs
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
0 siblings, 1 reply; 7+ messages in thread
From: Kui-Feng Lee @ 2023-10-24 21:29 UTC (permalink / raw)
To: Manu Bretelle, bpf, andrii, daniel, ast, martin.lau, song,
yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa,
mykolal, shuah
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>/"?
>
> 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;
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: umount children of TDIR in test_bpffs
2023-10-24 21:29 ` Kui-Feng Lee
@ 2023-10-25 5:41 ` Manu Bretelle
2023-10-26 21:35 ` Alexei Starovoitov
2023-10-27 6:08 ` Yonghong Song
0 siblings, 2 replies; 7+ messages in thread
From: Manu Bretelle @ 2023-10-25 5:41 UTC (permalink / raw)
To: Kui-Feng Lee, ast
Cc: bpf, andrii, daniel, martin.lau, song, yonghong.song,
john.fastabend, kpsingh, sdf, haoluo, jolsa, mykolal, shuah
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.
> >
> > 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;
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: umount children of TDIR in test_bpffs
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
1 sibling, 1 reply; 7+ messages in thread
From: Alexei Starovoitov @ 2023-10-26 21:35 UTC (permalink / raw)
To: Manu Bretelle
Cc: Kui-Feng Lee, Alexei Starovoitov, bpf, Andrii Nakryiko,
Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Mykola Lysenko, Shuah Khan
On Tue, Oct 24, 2023 at 10:42 PM Manu Bretelle <chantr4@gmail.com> 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.
I don't remember why I picked /sys/kernel/debug back then.
I suspect TDIR /tmp/foo and mkdir would work the same way.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: umount children of TDIR in test_bpffs
2023-10-25 5:41 ` Manu Bretelle
2023-10-26 21:35 ` Alexei Starovoitov
@ 2023-10-27 6:08 ` Yonghong Song
2023-10-27 21:09 ` Manu Bretelle
1 sibling, 1 reply; 7+ messages in thread
From: Yonghong Song @ 2023-10-27 6:08 UTC (permalink / raw)
To: Manu Bretelle, Kui-Feng Lee, ast
Cc: bpf, andrii, daniel, martin.lau, song, john.fastabend, kpsingh,
sdf, haoluo, jolsa, mykolal, shuah
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;
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: umount children of TDIR in test_bpffs
2023-10-27 6:08 ` Yonghong Song
@ 2023-10-27 21:09 ` Manu Bretelle
0 siblings, 0 replies; 7+ messages in thread
From: Manu Bretelle @ 2023-10-27 21:09 UTC (permalink / raw)
To: Yonghong Song
Cc: Kui-Feng Lee, ast, bpf, andrii, daniel, martin.lau, song,
john.fastabend, kpsingh, sdf, haoluo, jolsa, mykolal, shuah
On Thu, Oct 26, 2023 at 11:08:35PM -0700, Yonghong Song wrote:
>
> 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.
>
>
I think we just need an existing directory (whichever) to mount tmpfs on, and Alexei previous
comment seems to confirm that. But importantly, nothing that has sub-mounts
if we don't want to get into the business of recursively umounting sub-mounts.
I will just pick up a name which is random enough to not realistically exist on
a system, mkdir it instead of the existing umount(TDIR) and we should be good.
> >
> > > > 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;
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: umount children of TDIR in test_bpffs
2023-10-26 21:35 ` Alexei Starovoitov
@ 2023-10-28 0:56 ` Manu Bretelle
0 siblings, 0 replies; 7+ messages in thread
From: Manu Bretelle @ 2023-10-28 0:56 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Kui-Feng Lee, Alexei Starovoitov, bpf, Andrii Nakryiko,
Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Mykola Lysenko, Shuah Khan
On Thu, Oct 26, 2023 at 02:35:11PM -0700, Alexei Starovoitov wrote:
> On Tue, Oct 24, 2023 at 10:42 PM Manu Bretelle <chantr4@gmail.com> 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.
>
> I don't remember why I picked /sys/kernel/debug back then.
> I suspect TDIR /tmp/foo and mkdir would work the same way.
Yeah. I suspect the reason you used an existing directory is that
/sys/kernel is not mutable from userspace.
I ended up picking a random name under /tmp and mkdir.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-10-28 0:56 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
2023-10-27 21:09 ` Manu Bretelle
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox