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

[Athira Rajeev]

> On 14 Nov 2024, at 3:35 PM, Michael Petlan <mpetlan at redhat.com> wrote:
> 
> 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.

Hi,

Thanks for checking Michael.

If the patch looks good, can we please get this pulled in ?

Thanks
Athira
> 
> 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
>> 
>>