Re: [PATCH] fstests: fix _require_debugfs and call it properly

["Darrick J. Wong" <djwong@kernel.org>]

On Fri, May 10, 2024 at 09:37:26AM +0800, Zorro Lang wrote:
> On Tue, Apr 30, 2024 at 02:58:57AM +0800, Zorro Lang wrote:
> > On Mon, Apr 29, 2024 at 09:03:56AM -0700, Darrick J. Wong wrote:
> > > On Mon, Apr 29, 2024 at 02:09:06PM +0800, Zorro Lang wrote:
> > > > The old _require_debugfs helper doesn't work now, fix it to check
> > > > a system supports debugfs. And then call this helper in cases which
> > > > need $DEBUGFS_MNT.
> > > > 
> > > > Signed-off-by: Zorro Lang <zlang@kernel.org>
> > > > ---
> > > >  common/rc       | 10 ++++++++--
> > > >  tests/btrfs/150 |  1 +
> > > >  tests/ceph/001  |  2 +-
> > > >  tests/xfs/499   |  1 +
> > > >  4 files changed, 11 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/common/rc b/common/rc
> > > > index 56f1afb6..6f0bdf60 100644
> > > > --- a/common/rc
> > > > +++ b/common/rc
> > > > @@ -2978,8 +2978,14 @@ _require_sparse_files()
> > > >  
> > > >  _require_debugfs()
> > > >  {
> > > > -    #boot_params always present in debugfs
> > > > -    [ -d "$DEBUGFS_MNT/boot_params" ] || _notrun "Debugfs not mounted"
> > > > +	local type
> > > > +
> > > > +	if [ -d "$DEBUGFS_MNT" ];then
> > > > +		type=$(findmnt -rncv -T $DEBUGFS_MNT -S debugfs -o FSTYPE)
> > > > +		[ "$type" = "debugfs" ] && return 0
> > > > +	fi
> > > > +
> > > > +	_notrun "Cannot find debugfs on $DEBUGFS_MNT"
> > > >  }
> > > >  
> > > >  #
> > > > diff --git a/tests/btrfs/150 b/tests/btrfs/150
> > > > index a02bdafc..fd386d92 100755
> > > > --- a/tests/btrfs/150
> > > > +++ b/tests/btrfs/150
> > > > @@ -17,6 +17,7 @@ _begin_fstest auto quick dangerous read_repair compress
> > > >  . ./common/fail_make_request
> > > >  
> > > >  _supported_fs btrfs
> > > > +_require_debugfs
> > > >  _require_scratch
> > > >  _require_fail_make_request
> > > >  _require_scratch_dev_pool 2
> > > > diff --git a/tests/ceph/001 b/tests/ceph/001
> > > > index 060c4c45..79a5f58a 100755
> > > > --- a/tests/ceph/001
> > > > +++ b/tests/ceph/001
> > > > @@ -20,7 +20,7 @@ _begin_fstest auto quick copy_range
> > > >  
> > > >  # real QA test starts here
> > > >  _supported_fs ceph
> > > > -
> > > > +_require_debugfs
> > > >  _require_xfs_io_command "copy_range"
> > > >  _exclude_test_mount_option "test_dummy_encryption"
> > > >  _require_attrs
> > > > diff --git a/tests/xfs/499 b/tests/xfs/499
> > > > index 9672f95d..6421cc4b 100755
> > > > --- a/tests/xfs/499
> > > > +++ b/tests/xfs/499
> > > > @@ -18,6 +18,7 @@ _register_cleanup "_cleanup" BUS
> > > >  # real QA test starts here
> > > >  _supported_fs xfs
> > > >  _require_command "$CC_PROG" "cc"
> > > > +_require_debugfs
> > > 
> > > This test doesn't mess around with debugfs per se; it actually wants
> > > tracefs.  Perhaps this should be _require_ftrace instead?
> 
> Oh, I just found you've created a _require_ftrace helper in common/tracing:
> 
>   FTRACE_INSTANCES_DIR="/sys/kernel/debug/tracing/instances/"
> 
>   _require_ftrace() {
>           test -d "$FTRACE_INSTANCES_DIR" || \
>                   _notrun "kernel does not support ftrace"
>   }
> 
> Newer kernel has /sys/kernel/tracing/, to avoid the dependence of debugfs.

Ah, I didn't realize that!  I wonder how long that's been in place...

> But there's still /sys/kernel/debug/tracing/, so I'm wondering
> if we should try to use /sys/kernel/debug/tracing at first?
> 
> For example:
> 
> _require_ftrace() {
> 	if [ -d /sys/kernel/tracing/instances/ ];then
> 		FTRACE_DIR="/sys/kernel/tracing"
> 		FTRACE_INSTANCES_DIR="/sys/kernel/tracing/instances"
> 	elif [ -d /sys/kernel/debug/tracing/instances/ ];then
> 		FTRACE_DIR="/sys/kernel/debug/tracing"
> 		FTRACE_INSTANCES_DIR="/sys/kernel/debug/tracing/instances"
> 	else
> 		_notrun "The ftrace is not supported, or tracefs is not mounted"
> 	fi
> }

Yes.

--D

> Thanks,
> Zorro
> 
> > 
> > Sure, but if it depends tracefs only, should we use /sys/kernel/debug/tracing or
> > /sys/kernel/tracing ? If we use /sys/kernel/debug/tracing, is the debugfs needed
> > at first?
> > 
> > Thanks,
> > Zorro
> > 
> > > 
> > > --D
> > > 
> > > >  cprog=$tmp.ftrace.c
> > > >  oprog=$tmp.ftrace
> > > > -- 
> > > > 2.44.0
> > > > 
> > > > 
> > > 
>