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

[Google]

Hi Linyu,

On Tue, 31 May 2022 14:29:12 +0800
Linyu Yuan <quic_linyyuan@xxxxxxxxxxx> wrote:

> add kprobe and eprobe event test for new GRP/ only format.
> 
> Signed-off-by: Linyu Yuan <quic_linyyuan@xxxxxxxxxxx>
> ---
> 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) <mhiramat@xxxxxxxxxx>