Re: [PATCH bpf-next] selftests/bpf: umount children of TDIR in test_bpffs

[Manu Bretelle <chantr4@gmail.com>]

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.