Re: [PATCH v3] perf test probe_vfs_getname: Skip if no suitable line detected

[Namhyung Kim <namhyung@kernel.org>]

Hello,

On Thu, Apr 10, 2025 at 04:32:11PM +0200, Jakub Brnak wrote:
> On Mon, Mar 24, 2025 at 03:45:23PM +0100, Jakub Brnak wrote:
> > In some cases when calling function add_probe_vfs_getname, line number
> > can't be detected by perf probe -L getname_flags:
> > 
> >   78         atomic_set(&result->refcnt, 1);
> > 
> > 	     // one of the following lines should have line number
> > 	     // but sometimes it does not because of optimization
> > 	     result->uptr = filename;
> >              result->aname = NULL;
> > 
> >   81         audit_getname(result);
> > 
> > To prevent false failures, skip the affected tests
> > if no suitable line numbers can be detected.
> > 
> > Signed-off-by: Jakub Brnak <jbrnak@redhat.com>

Sorry for the long delay.  It looks ok to me.

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung

> > ---
> > v3:
> > - ensure POSIX compliance
> > 
> > v2: 
> > https://lore.kernel.org/linux-perf-users/Z9tKat6vvC1XUj0U@google.com/
> > - check if return from add_vfs_getname equals to 1
> > sice it is only option in case of fail 
> > 
> > v1:
> >  https://lore.kernel.org/linux-perf-users/Z8pAep0GJsMFTyEi@google.com/T/#t
> > ---
> >  tools/perf/tests/shell/lib/probe_vfs_getname.sh          | 8 +++++++-
> >  tools/perf/tests/shell/probe_vfs_getname.sh              | 8 +++++++-
> >  .../perf/tests/shell/record+script_probe_vfs_getname.sh  | 8 +++++++-
> >  tools/perf/tests/shell/trace+probe_vfs_getname.sh        | 9 +++++++--
> >  4 files changed, 28 insertions(+), 5 deletions(-)
> > 
> > diff --git a/tools/perf/tests/shell/lib/probe_vfs_getname.sh b/tools/perf/tests/shell/lib/probe_vfs_getname.sh
> > index 5c33ec7a5a63..89f72a4c818c 100644
> > --- a/tools/perf/tests/shell/lib/probe_vfs_getname.sh
> > +++ b/tools/perf/tests/shell/lib/probe_vfs_getname.sh
> > @@ -19,8 +19,14 @@ add_probe_vfs_getname() {
> >  			result_aname_re="[[:space:]]+([[:digit:]]+)[[:space:]]+result->aname = NULL;"
> >  			line=$(perf probe -L getname_flags 2>&1 | grep -E "$result_aname_re" | sed -r "s/$result_aname_re/\1/")
> >  		fi
> > +
> > +		if [ -z "$line" ] ; then
> > +			echo "Could not find probeable line"
> > +			return 2
> > +		fi
> > +
> >  		perf probe -q       "vfs_getname=getname_flags:${line} pathname=result->name:string" || \
> > -		perf probe $add_probe_verbose "vfs_getname=getname_flags:${line} pathname=filename:ustring"
> > +		perf probe $add_probe_verbose "vfs_getname=getname_flags:${line} pathname=filename:ustring" || return 1
> >  	fi
> >  }
> >  
> > diff --git a/tools/perf/tests/shell/probe_vfs_getname.sh b/tools/perf/tests/shell/probe_vfs_getname.sh
> > index c51a32931af6..0f52654c914a 100755
> > --- a/tools/perf/tests/shell/probe_vfs_getname.sh
> > +++ b/tools/perf/tests/shell/probe_vfs_getname.sh
> > @@ -13,7 +13,13 @@ skip_if_no_perf_probe || exit 2
> >  # shellcheck source=lib/probe_vfs_getname.sh
> >  . "$(dirname $0)"/lib/probe_vfs_getname.sh
> >  
> > -add_probe_vfs_getname || skip_if_no_debuginfo
> > +add_probe_vfs_getname
> >  err=$?
> > +
> > +if [ $err -eq 1 ] ; then
> > +	skip_if_no_debuginfo
> > +	err=$?
> > +fi
> > +
> >  cleanup_probe_vfs_getname
> >  exit $err
> > diff --git a/tools/perf/tests/shell/record+script_probe_vfs_getname.sh b/tools/perf/tests/shell/record+script_probe_vfs_getname.sh
> > index fd5b10d46915..1ad252f0d36e 100755
> > --- a/tools/perf/tests/shell/record+script_probe_vfs_getname.sh
> > +++ b/tools/perf/tests/shell/record+script_probe_vfs_getname.sh
> > @@ -35,8 +35,14 @@ perf_script_filenames() {
> >  	grep -E " +touch +[0-9]+ +\[[0-9]+\] +[0-9]+\.[0-9]+: +probe:vfs_getname[_0-9]*: +\([[:xdigit:]]+\) +pathname=\"${file}\""
> >  }
> >  
> > -add_probe_vfs_getname || skip_if_no_debuginfo
> > +add_probe_vfs_getname
> >  err=$?
> > +
> > +if [ $err -eq 1 ] ; then
> > +        skip_if_no_debuginfo
> > +        err=$?
> > +fi
> > +
> >  if [ $err -ne 0 ] ; then
> >  	exit $err
> >  fi
> > diff --git a/tools/perf/tests/shell/trace+probe_vfs_getname.sh b/tools/perf/tests/shell/trace+probe_vfs_getname.sh
> > index 60fccb62c540..5d5019988d61 100755
> > --- a/tools/perf/tests/shell/trace+probe_vfs_getname.sh
> > +++ b/tools/perf/tests/shell/trace+probe_vfs_getname.sh
> > @@ -25,9 +25,14 @@ trace_open_vfs_getname() {
> >  	grep -E " +[0-9]+\.[0-9]+ +\( +[0-9]+\.[0-9]+ ms\): +touch/[0-9]+ open(at)?\((dfd: +CWD, +)?filename: +\"?${file}\"?, +flags: CREAT\|NOCTTY\|NONBLOCK\|WRONLY, +mode: +IRUGO\|IWUGO\) += +[0-9]+$"
> >  }
> >  
> > -
> > -add_probe_vfs_getname || skip_if_no_debuginfo
> > +add_probe_vfs_getname
> >  err=$?
> > +
> > +if [ $err -eq 1 ] ; then
> > +        skip_if_no_debuginfo
> > +        err=$?
> > +fi
> > +
> >  if [ $err -ne 0 ] ; then
> >  	exit $err
> >  fi
> > -- 
> > 2.48.1
> > 
> 
> Hi, anyone interested in this patch?
> 
> Thanks,
> Jakub
>