[PATCH] tools/perf/tests/base_probe: Fix check for the count of existing probes in test_adding_kernel

[Michael Petlan]

On Sun, 3 Nov 2024, Athira Rajeev wrote:
> > On 17 Oct 2024, at 3:44 PM, Michael Petlan <mpetlan at redhat.com> wrote:
> > 
> > On Mon, 14 Oct 2024, Athira Rajeev wrote:
[...]
> > 
> > I am wondering how it could happen that there were other
> > probes in the system?
> 
> Hi Michael,
>
Hello Athira.

> Sorry for the late response.
> 
> Yes, there are uprobes listed as part of “perf probe” in the environment where I saw the test needing this change. Sharing the result below from perf probe: 
> 
> # ./perf probe -l
>   uprobes:p_uprobe_dns_events_osquery4026531841 (on getaddrinfo in XX)
>   uprobes:p_uprobe_dns_events_osquery4026532336 (on 0x129a60 in XX)
>   uprobes:p_uprobe_dns_events_osquery4026532344 (on 0x129a60 in XX)
>   uprobes:p_uprobe_ebpf_compat_check_osquery (on __GI___backtrace in XX)
>   uprobes:p_uprobe_sys_hook_osquery (on backtrace_symbols in XX)
> 
> These can’t be removed.
> 
> # ./perf probe -d uprobes:p_uprobe_dns_events_osquery4026531841
> Removed event: uprobes:p_uprobe_dns_events_osquery4026531841
> Failed to delete event: Device or resource busy
>   Error: Failed to delete events.
>

Ah, this is interesting, I have never hit that. However, it makes sense,
if the resource is busy.

However, in that case it comes to my mind that in general, these tests
should not be run in any production environment, where one could rely on
some probes will exist, etc. In case some of the probes above was not
busy, it'd be probably cleaned up by the testcase, which might be unexpected
by the creator/user of the uprobes... Maybe we should get rid of the
probe cleaning for that cases, but I'd prefer to keep it.

> Considering above scenario, patch here takes the probe count using:
> NO_OF_PROBES=`$CMD_PERF probe -l $TEST_PROBE| wc -l`
> 
> Also similarly looks for TEST_PROBE in result log in case of probe —del as well
> 
> Any comments Michael ?
>
Yes, we probably should tweak it as you suggest.

Thanks,
Michael


> Thanks
> Athira
> 
> > 
> > Cheers,
> > 
> > Michael
> > 
> >> 
> >> Example, in the system where this failed, already some
> >> probes were default added. So count became 10
> >>  ./perf probe -l | wc -l
> >>  10
> >> 
> >> So to be specific for "inode_permission", restrict the
> >> probe count check to that probe point alone using :
> >> NO_OF_PROBES=`$CMD_PERF probe -l $TEST_PROBE| wc -l`
> >> 
> >> Similarly while removing the probe using "probe --del *",
> >> ( removing all probes ), check uses:
> >> 
> >> ../common/check_all_lines_matched.pl "Removed event: probe:$TEST_PROBE"
> >> 
> >> But if there are other probes in the system, the log will
> >> contain reference to other existing probe too. Hence change
> >> usage of check_all_lines_matched.pl to check_all_patterns_found.pl
> >> This will make sure expecting string comes in the result
> >> 
> >> Signed-off-by: Athira Rajeev <atrajeev at linux.vnet.ibm.com>
> >> ---
> >> tools/perf/tests/shell/base_probe/test_adding_kernel.sh | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/tools/perf/tests/shell/base_probe/test_adding_kernel.sh b/tools/perf/tests/shell/base_probe/test_adding_kernel.sh
> >> index d541ffd44a93..f8b5f096d0d7 100755
> >> --- a/tools/perf/tests/shell/base_probe/test_adding_kernel.sh
> >> +++ b/tools/perf/tests/shell/base_probe/test_adding_kernel.sh
> >> @@ -169,7 +169,7 @@ print_results $PERF_EXIT_CODE $CHECK_EXIT_CODE "force-adding probes :: second pr
> >> (( TEST_RESULT += $? ))
> >> 
> >> # adding existing probe with '--force' should pass
> >> -NO_OF_PROBES=`$CMD_PERF probe -l | wc -l`
> >> +NO_OF_PROBES=`$CMD_PERF probe -l $TEST_PROBE| wc -l`
> >> $CMD_PERF probe --force --add $TEST_PROBE 2> $LOGS_DIR/adding_kernel_forceadd_03.err
> >> PERF_EXIT_CODE=$?
> >> 
> >> @@ -205,7 +205,7 @@ print_results $PERF_EXIT_CODE $CHECK_EXIT_CODE "using doubled probe"
> >> $CMD_PERF probe --del \* 2> $LOGS_DIR/adding_kernel_removing_wildcard.err
> >> PERF_EXIT_CODE=$?
> >> 
> >> -../common/check_all_lines_matched.pl "Removed event: probe:$TEST_PROBE" "Removed event: probe:${TEST_PROBE}_1" < $LOGS_DIR/adding_kernel_removing_wildcard.err
> >> +../common/check_all_patterns_found.pl "Removed event: probe:$TEST_PROBE" "Removed event: probe:${TEST_PROBE}_1" < $LOGS_DIR/adding_kernel_removing_wildcard.err
> >> CHECK_EXIT_CODE=$?
> >> 
> >> print_results $PERF_EXIT_CODE $CHECK_EXIT_CODE "removing multiple probes"
> >> -- 
> >> 2.43.5
> 
> 
>