2022-05-31 12:16:11

by Linyu Yuan

[permalink] [raw]
Subject: [PATCH v3 3/3] selftests/ftrace: add test case for GRP/ only input

add kprobe and eprobe event test for new GRP/ only format.

Signed-off-by: Linyu Yuan <[email protected]>
---
v3: first add in this version

.../testing/selftests/ftrace/test.d/dynevent/add_remove_eprobe.tc | 7 ++++++-
.../testing/selftests/ftrace/test.d/dynevent/add_remove_kprobe.tc | 7 ++++++-
2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_eprobe.tc b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_eprobe.tc
index 60c02b4..97856df 100644
--- a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_eprobe.tc
+++ b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_eprobe.tc
@@ -1,7 +1,7 @@
#!/bin/sh
# SPDX-License-Identifier: GPL-2.0
# description: Generic dynamic event - add/remove eprobe events
-# requires: dynamic_events events/syscalls/sys_enter_openat "e[:[<group>/]<event>] <attached-group>.<attached-event> [<args>]":README
+# requires: dynamic_events events/syscalls/sys_enter_openat "e[:[<group>/][<event>]] <attached-group>.<attached-event> [<args>]":README

echo 0 > events/enable

@@ -87,4 +87,9 @@ echo "-:eprobes/$EPROBE $SYSTEM/$EVENT $OPTIONS" >> dynamic_events
! grep -q "$EPROBE" dynamic_events
! test -d events/eprobes/$EPROBE

+echo "e:mygroup/ $SYSTEM/$EVENT $OPTIONS" >> dynamic_events
+test -d events/mygroup
+echo "-:mygroup/" >> dynamic_events
+! test -d events/mygroup
+
clear_trace
diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_kprobe.tc b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_kprobe.tc
index b4da41d..6b102a7 100644
--- a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_kprobe.tc
+++ b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_kprobe.tc
@@ -1,7 +1,7 @@
#!/bin/sh
# SPDX-License-Identifier: GPL-2.0
# description: Generic dynamic event - add/remove kprobe events
-# requires: dynamic_events "place: [<module>:]<symbol>":README "place (kretprobe): [<module>:]<symbol>":README
+# requires: dynamic_events "p[:[<group>/][<event>]] <place> [<args>]":README

echo 0 > events/enable
echo > dynamic_events
@@ -23,4 +23,9 @@ grep -q myevent1 dynamic_events

echo > dynamic_events

+echo "p:mygroup/ $PLACE" >> dynamic_events
+test -d events/mygroup
+echo "-:mygroup/" >> dynamic_events
+! test -d events/mygroup
+
clear_trace
--
2.7.4



2022-06-01 18:54:17

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] selftests/ftrace: add test case for GRP/ only input

Hi Linyu,

On Tue, 31 May 2022 14:29:12 +0800
Linyu Yuan <[email protected]> wrote:

> add kprobe and eprobe event test for new GRP/ only format.
>
> Signed-off-by: Linyu Yuan <[email protected]>
> ---
> v3: first add in this version
>
> .../testing/selftests/ftrace/test.d/dynevent/add_remove_eprobe.tc | 7 ++++++-
> .../testing/selftests/ftrace/test.d/dynevent/add_remove_kprobe.tc | 7 ++++++-
> 2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_eprobe.tc b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_eprobe.tc
> index 60c02b4..97856df 100644
> --- a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_eprobe.tc
> +++ b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_eprobe.tc
> @@ -1,7 +1,7 @@
> #!/bin/sh
> # SPDX-License-Identifier: GPL-2.0
> # description: Generic dynamic event - add/remove eprobe events
> -# requires: dynamic_events events/syscalls/sys_enter_openat "e[:[<group>/]<event>] <attached-group>.<attached-event> [<args>]":README
> +# requires: dynamic_events events/syscalls/sys_enter_openat "e[:[<group>/][<event>]] <attached-group>.<attached-event> [<args>]":README

Hmm, this is too restricted. With this change, this entire test only runs after
patching this series.
Can you change the requires so that skip checking the first half, like
"<attached-group>.<attached-event> [<args>]":README
and check whether "e[:[<group>/][<event>]]" is in the README for the new
test code?

> echo 0 > events/enable
>
> @@ -87,4 +87,9 @@ echo "-:eprobes/$EPROBE $SYSTEM/$EVENT $OPTIONS" >> dynamic_events
> ! grep -q "$EPROBE" dynamic_events
> ! test -d events/eprobes/$EPROBE
>
> +echo "e:mygroup/ $SYSTEM/$EVENT $OPTIONS" >> dynamic_events
> +test -d events/mygroup
> +echo "-:mygroup/" >> dynamic_events
> +! test -d events/mygroup
> +
> clear_trace
> diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_kprobe.tc b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_kprobe.tc
> index b4da41d..6b102a7 100644
> --- a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_kprobe.tc
> +++ b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_kprobe.tc
> @@ -1,7 +1,7 @@
> #!/bin/sh
> # SPDX-License-Identifier: GPL-2.0
> # description: Generic dynamic event - add/remove kprobe events
> -# requires: dynamic_events "place: [<module>:]<symbol>":README "place (kretprobe): [<module>:]<symbol>":README
> +# requires: dynamic_events "p[:[<group>/][<event>]] <place> [<args>]":README

Ditto. In this case, don't touch the requires line (because it is not
updated) and just add a README check for below new code.

Thank you,


>
> echo 0 > events/enable
> echo > dynamic_events
> @@ -23,4 +23,9 @@ grep -q myevent1 dynamic_events
>
> echo > dynamic_events
>
> +echo "p:mygroup/ $PLACE" >> dynamic_events
> +test -d events/mygroup
> +echo "-:mygroup/" >> dynamic_events
> +! test -d events/mygroup
> +
> clear_trace
> --
> 2.7.4
>


--
Masami Hiramatsu (Google) <[email protected]>

2022-06-02 01:17:41

by Linyu Yuan

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] selftests/ftrace: add test case for GRP/ only input

hi Masami,

thanks for review, will update your comment in next V4.

thanks

On 6/1/2022 10:57 PM, Masami Hiramatsu (Google) wrote:
> Hi Linyu,
>
> On Tue, 31 May 2022 14:29:12 +0800
> Linyu Yuan <[email protected]> wrote:
>
>> add kprobe and eprobe event test for new GRP/ only format.
>>
>> Signed-off-by: Linyu Yuan <[email protected]>
>> ---
>> v3: first add in this version
>>
>> .../testing/selftests/ftrace/test.d/dynevent/add_remove_eprobe.tc | 7 ++++++-
>> .../testing/selftests/ftrace/test.d/dynevent/add_remove_kprobe.tc | 7 ++++++-
>> 2 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_eprobe.tc b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_eprobe.tc
>> index 60c02b4..97856df 100644
>> --- a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_eprobe.tc
>> +++ b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_eprobe.tc
>> @@ -1,7 +1,7 @@
>> #!/bin/sh
>> # SPDX-License-Identifier: GPL-2.0
>> # description: Generic dynamic event - add/remove eprobe events
>> -# requires: dynamic_events events/syscalls/sys_enter_openat "e[:[<group>/]<event>] <attached-group>.<attached-event> [<args>]":README
>> +# requires: dynamic_events events/syscalls/sys_enter_openat "e[:[<group>/][<event>]] <attached-group>.<attached-event> [<args>]":README
> Hmm, this is too restricted. With this change, this entire test only runs after
> patching this series.
> Can you change the requires so that skip checking the first half, like
> "<attached-group>.<attached-event> [<args>]":README
> and check whether "e[:[<group>/][<event>]]" is in the README for the new
> test code?
>
>> echo 0 > events/enable
>>
>> @@ -87,4 +87,9 @@ echo "-:eprobes/$EPROBE $SYSTEM/$EVENT $OPTIONS" >> dynamic_events
>> ! grep -q "$EPROBE" dynamic_events
>> ! test -d events/eprobes/$EPROBE
>>
>> +echo "e:mygroup/ $SYSTEM/$EVENT $OPTIONS" >> dynamic_events
>> +test -d events/mygroup
>> +echo "-:mygroup/" >> dynamic_events
>> +! test -d events/mygroup
>> +
>> clear_trace
>> diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_kprobe.tc b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_kprobe.tc
>> index b4da41d..6b102a7 100644
>> --- a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_kprobe.tc
>> +++ b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_kprobe.tc
>> @@ -1,7 +1,7 @@
>> #!/bin/sh
>> # SPDX-License-Identifier: GPL-2.0
>> # description: Generic dynamic event - add/remove kprobe events
>> -# requires: dynamic_events "place: [<module>:]<symbol>":README "place (kretprobe): [<module>:]<symbol>":README
>> +# requires: dynamic_events "p[:[<group>/][<event>]] <place> [<args>]":README
> Ditto. In this case, don't touch the requires line (because it is not
> updated) and just add a README check for below new code.
>
> Thank you,
>
>
>>
>> echo 0 > events/enable
>> echo > dynamic_events
>> @@ -23,4 +23,9 @@ grep -q myevent1 dynamic_events
>>
>> echo > dynamic_events
>>
>> +echo "p:mygroup/ $PLACE" >> dynamic_events
>> +test -d events/mygroup
>> +echo "-:mygroup/" >> dynamic_events
>> +! test -d events/mygroup
>> +
>> clear_trace
>> --
>> 2.7.4
>>
>