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

[Athira Rajeev]

> On 17 Oct 2024, at 3:44 PM, Michael Petlan <mpetlan at redhat.com> wrote:
> 
> On Mon, 14 Oct 2024, Athira Rajeev wrote:
>> perftool-testsuite_probe fails in test_adding_kernel as below:
>> Regexp not found: "probe:inode_permission_11"
>> -- [ FAIL ] -- perf_probe :: test_adding_kernel :: force-adding probes ::
>> second probe adding (with force) (output regexp parsing)
>> event syntax error: 'probe:inode_permission_11'
>>   \___ unknown tracepoint
>> 
>> Error:  File /sys/kernel/tracing//events/probe/inode_permission_11
>> not found.
>> Hint:   Perhaps this kernel misses some CONFIG_ setting to
>> enable this feature?.
>> 
>> The test does the following:
>> 1) Adds a probe point first using :
>>    $CMD_PERF probe --add $TEST_PROBE
>> 2) Then tries to add same probe again without —force
>> and expects it to fail. Next tries to add same probe again
>> with —force. In this case, perf probe succeeds and adds
>> the probe with a suffix number. Example:
>> 
>> ./perf probe --add inode_permission
>> Added new event:
>>  probe:inode_permission (on inode_permission)
>> 
>> ./perf probe --add inode_permission --force
>> Added new event:
>>  probe:inode_permission_1 (on inode_permission)
>> 
>>  ./perf probe --add inode_permission --force
>> Added new event:
>>  probe:inode_permission_2 (on inode_permission)
>> 
>> Each time, suffix is added to existing probe name.
>> To get the suffix number, test cases uses :
>> NO_OF_PROBES=`$CMD_PERF probe -l | wc -l`
>> 
>> This will work if there is no other probe existing
>> in the system. If there are any other probes other than
>> kernel probes or inode_permission, ( example: any probe),
>> "perf probe -l" will include count for other probes too.
> 
> Hello.
> 
> When designing this test, I was relying on the fact that
> there are no existing probes, because all should have been
> removed at line 43 of the same test:
> 
> 40 ### basic probe adding
> 41
> 42 for opt in "" "-a" "--add"; do
> --> 43     clear_all_probes
> 44     $CMD_PERF probe $opt $TEST_PROBE 2> $LOGS_DIR/adding_kernel_add$opt.err
> 45     PERF_EXIT_CODE=$?
> 46
> 47 ../common/check_all_patterns_found.pl "Added new events?:" "probe:$TEST_PROBE" "on $TEST_PROBE" < $LOGS_DIR/adding_kernel_add$opt.err
> 48  CHECK_EXIT_CODE=$?
> 49
> 50 print_results $PERF_EXIT_CODE $CHECK_EXIT_CODE "adding probe $TEST_PROBE :: $opt"
> 51  (( TEST_RESULT += $? ))
> 52 done
> 53
> 
> I am wondering how it could happen that there were other
> probes in the system?

Hi Michael,

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.


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 ?

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