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

[Athira Rajeev]

> On 17 Dec 2024, at 12:27 AM, Athira Rajeev <atrajeev at linux.vnet.ibm.com> wrote:
> 
> 
> 
>> 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

Hi,

Looking for review comments on this

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