take kprobe event as example, when create a group of events,
p[:[GRP/]EVENT] [MOD:]KSYM[+OFFS]|KADDR [FETCHARGS],
according to this format, we must input EVENT name,
this change allow only GRP/ input, EVENT name auto generate from KSYM,
p[:[GRP/][EVENT]] [MOD:]KSYM[+OFFS]|KADDR [FETCHARGS]
similar change apply to eprobe and uprobe.
V2: (v1: https://lore.kernel.org/lkml/[email protected]/)
fix remove comment in V1 patch1,
remove v1 patch2 as it is NACK.
v3: (v2 : https://lore.kernel.org/lkml/[email protected]/)
add selftest cases for kprobe and eprobe event,
remove macro used in v1,v2,
change location to generate eprobe event name.
v4: (v3 : https://lore.kernel.org/lkml/[email protected]/)
fix comment of kprobe/eprobe test case.
v5: (v4: https://lore.kernel.org/lkml/[email protected]/)
for eprobe, when only input a "SYSTEM.", it is invalid.
add Acked-by from Masami Hiramatsu (Google) <[email protected]>
v6: (v5: https://lore.kernel.org/lkml/[email protected]/)
change some code order according review comment from Tom Zanussi,
some minor changes.
Linyu Yuan (3):
tracing: eprobe: remove duplicate is_good_name() operation
tracing: auto generate event name when create a group of events
selftests/ftrace: add test case for GRP/ only input
Documentation/trace/kprobetrace.rst | 8 +++----
Documentation/trace/uprobetracer.rst | 8 +++----
kernel/trace/trace.c | 8 +++----
kernel/trace/trace_dynevent.c | 2 +-
kernel/trace/trace_eprobe.c | 28 +++++++++++-----------
kernel/trace/trace_kprobe.c | 16 ++++++++-----
kernel/trace/trace_probe.c | 4 ++++
kernel/trace/trace_uprobe.c | 12 ++++++----
.../ftrace/test.d/dynevent/add_remove_eprobe.tc | 9 ++++++-
.../ftrace/test.d/dynevent/add_remove_kprobe.tc | 7 ++++++
10 files changed, 64 insertions(+), 38 deletions(-)
--
2.7.4
traceprobe_parse_event_name() already validate SYSTEM and EVENT name,
there is no need to call is_good_name() after it.
Add trace_probe_log_set_index(1) to allow report correct error
if user input wrong SYSTEM.EVENT format.
Acked-by: Masami Hiramatsu (Google) <[email protected]>
Signed-off-by: Linyu Yuan <[email protected]>
---
v2: drop v1 change as it is NACK.
add it to remove duplicate is_good_name().
v3: move it as first patch.
v4: no change
v5: add Acked-by tag
v6: keep is_good_name() check for group and event name
add trace_probe_log_set_index(1) to report correct error message.
kernel/trace/trace_eprobe.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c
index 7d44785..8979cb9e 100644
--- a/kernel/trace/trace_eprobe.c
+++ b/kernel/trace/trace_eprobe.c
@@ -881,13 +881,12 @@ static int __trace_eprobe_create(int argc, const char *argv[])
if (!is_good_name(event) || !is_good_name(group))
goto parse_error;
+ trace_probe_log_set_index(1);
sys_event = argv[1];
ret = traceprobe_parse_event_name(&sys_event, &sys_name, buf2,
sys_event - argv[1]);
if (ret || !sys_name)
goto parse_error;
- if (!is_good_name(sys_event) || !is_good_name(sys_name))
- goto parse_error;
mutex_lock(&event_mutex);
event_call = find_and_get_event(sys_name, sys_event);
--
2.7.4
Hi Linyu,
On Tue, 2022-06-21 at 09:59 +0800, Linyu Yuan wrote:
> traceprobe_parse_event_name() already validate SYSTEM and EVENT name,
> there is no need to call is_good_name() after it.
>
> Add trace_probe_log_set_index(1) to allow report correct error
> if user input wrong SYSTEM.EVENT format.
>
> Acked-by: Masami Hiramatsu (Google) <[email protected]>
> Signed-off-by: Linyu Yuan <[email protected]>
> ---
> v2: drop v1 change as it is NACK.
> add it to remove duplicate is_good_name().
> v3: move it as first patch.
> v4: no change
> v5: add Acked-by tag
> v6: keep is_good_name() check for group and event name
> add trace_probe_log_set_index(1) to report correct error message.
>
> kernel/trace/trace_eprobe.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/kernel/trace/trace_eprobe.c
> b/kernel/trace/trace_eprobe.c
> index 7d44785..8979cb9e 100644
> --- a/kernel/trace/trace_eprobe.c
> +++ b/kernel/trace/trace_eprobe.c
> @@ -881,13 +881,12 @@ static int __trace_eprobe_create(int argc,
> const char *argv[])
> if (!is_good_name(event) || !is_good_name(group))
> goto parse_error;
>
> + trace_probe_log_set_index(1);
Is this something that you noticed missing in the original code and are
adding now? If so, please make this a separate patch. Or is this
something that's needed for the new 'generating event name' code? If
that's the case, please move this to the other patch.
This one should only contain the code related to the duplicate
is_good_name() removal mentioned in the subject. Or if this really
does belong here, please provide more explanation of why it's needed if
you remove the duplicate is_good_name() code.
Thanks,
Tom
> sys_event = argv[1];
> ret = traceprobe_parse_event_name(&sys_event, &sys_name,
> buf2,
> sys_event - argv[1]);
> if (ret || !sys_name)
> goto parse_error;
> - if (!is_good_name(sys_event) || !is_good_name(sys_name))
> - goto parse_error;
>
> mutex_lock(&event_mutex);
> event_call = find_and_get_event(sys_name, sys_event);
hi Tom,
On 6/26/2022 1:25 AM, Tom Zanussi wrote:
> Hi Linyu,
>
> On Tue, 2022-06-21 at 09:59 +0800, Linyu Yuan wrote:
>> traceprobe_parse_event_name() already validate SYSTEM and EVENT name,
>> there is no need to call is_good_name() after it.
>>
>> Add trace_probe_log_set_index(1) to allow report correct error
>> if user input wrong SYSTEM.EVENT format.
>>
>> Acked-by: Masami Hiramatsu (Google) <[email protected]>
>> Signed-off-by: Linyu Yuan <[email protected]>
>> ---
>> v2: drop v1 change as it is NACK.
>> add it to remove duplicate is_good_name().
>> v3: move it as first patch.
>> v4: no change
>> v5: add Acked-by tag
>> v6: keep is_good_name() check for group and event name
>> add trace_probe_log_set_index(1) to report correct error message.
>>
>> kernel/trace/trace_eprobe.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/kernel/trace/trace_eprobe.c
>> b/kernel/trace/trace_eprobe.c
>> index 7d44785..8979cb9e 100644
>> --- a/kernel/trace/trace_eprobe.c
>> +++ b/kernel/trace/trace_eprobe.c
>> @@ -881,13 +881,12 @@ static int __trace_eprobe_create(int argc,
>> const char *argv[])
>> if (!is_good_name(event) || !is_good_name(group))
>> goto parse_error;
>>
>> + trace_probe_log_set_index(1);
> Is this something that you noticed missing in the original code and are
> adding now? If so, please make this a separate patch. Or is this
> something that's needed for the new 'generating event name' code? If
> that's the case, please move this to the other patch.
>
> This one should only contain the code related to the duplicate
> is_good_name() removal mentioned in the subject. Or if this really
> does belong here, please provide more explanation of why it's needed if
> you remove the duplicate is_good_name() code.
thanks, the original code should have this setting, i will create a
separate patch.
>
> Thanks,
>
> Tom
>
>> sys_event = argv[1];
>> ret = traceprobe_parse_event_name(&sys_event, &sys_name,
>> buf2,
>> sys_event - argv[1]);
>> if (ret || !sys_name)
>> goto parse_error;
>> - if (!is_good_name(sys_event) || !is_good_name(sys_name))
>> - goto parse_error;
>>
>> mutex_lock(&event_mutex);
>> event_call = find_and_get_event(sys_name, sys_event);