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

[Athira Rajeev]

> On 5 Dec 2024, at 11:16 PM, Athira Rajeev <atrajeev at linux.vnet.ibm.com> wrote:
> 
> 
> 
>> 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 ?
> 

Hi,

Can we please pull in this patch if it looks fine.

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