On 2015/07/19 19:16, Namhyung Kim wrote:
> On Wed, Jul 15, 2015 at 06:15:30PM +0900, Masami Hiramatsu wrote:
>> Allow user to set group name for adding new event.
>> Note that this can easily shot yourself in the foot.
>> E.g. Existing group name can conflict with other events.
>> Especially, using the group name reserved for kernel
>> modules can break something when loading/unloading
>> modules.
>
> Yes, I agree that this can be dangerous. How about enforcing
> [ku]probes to make the directory of dynamic events safely?
What the safety issue would you afraid?
> I think
> it'd be better putting all dynamic events in a single directory -
> e.g. $tracefs/events/probe/. Any events lack group name are created
> in the directory. Any events have group name create subdirectories as
> group name under the directory. The perf tools (and others too)
> should be changed to lookup the directory after the usual location.
That will be possible, but includes a big change on event namespace,
e.g. how we'll show the events by perf-list? Even if we can avoid
namespace conflict on tracefs, perf-list event namespace is still
fragile.
> What do you think?
I think there are 2 purposes of probe-event, one is just additional
debug points, another is an extensible event-set. The former will not
any namespace problem, we just add it into new namespace. But latter
requires to be treated as a part of existing (in-kernel) events.
And (userspace)SDT is clearly the latter one.
However, avoiding the conflict of namespace is also important, how
about simply using sdt_<PROVIDER>:<NAME> ?
- Give just a name on a userspace binary
perf probe -x <BIN> --add <NAME>=<PROBEDEF>
-> probe_<BIN>:<NAME>
- Give a pair of group and name on a userspace binary
perf probe -x <BIN> --add <GRP>:<NAME>=<PROBEDEF>
-> probe_<GRP>:<NAME>
- Set an sdt event on a userspace binary
perf probe -x <BIN> --add %<PROV>:<NAME> # or %sdt_<PROV> ?
-> sdt_<PROV>:<NAME>
- Set an cached event on a userspace binary
perf probe -x <BIN> --add %<GRP>:<NAME> # or %probe_<GRP> ?
-> probe_<GRP>:<NAME>
Thank you,
>
>>
>> Signed-off-by: Masami Hiramatsu <masami.hiramatsu...@hitachi.com>
>> ---
>> tools/perf/util/probe-event.c | 23 ++++++++++++++---------
>> 1 file changed, 14 insertions(+), 9 deletions(-)
>>
>> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
>> index 262f9d3..c19a380 100644
>> --- a/tools/perf/util/probe-event.c
>> +++ b/tools/perf/util/probe-event.c
>> @@ -1141,10 +1141,8 @@ static int parse_perf_probe_point(char *arg, struct
>> perf_probe_event *pev)
>> bool file_spec = false;
>> /*
>> * <Syntax>
>> - * perf probe [EVENT=]SRC[:LN|;PTN]
>> - * perf probe [EVENT=]FUNC[@SRC][+OFFS|%return|:LN|;PAT]
>> - *
>> - * TODO:Group name support
>> + * perf probe [GRP:][EVENT=]SRC[:LN|;PTN]
>
> Shouldn't it be
> [[GRP:]EVENT=]
>
> ?
>
>> + * perf probe [GRP:][EVENT=]FUNC[@SRC][+OFFS|%return|:LN|;PAT]
>
> Ditto.
>
> Thanks,
> Namhyung
>
>
>> */
>> if (!arg)
>> return -EINVAL;
>> @@ -1153,11 +1151,19 @@ static int parse_perf_probe_point(char *arg, struct
>> perf_probe_event *pev)
>> if (ptr && *ptr == '=') { /* Event name */
>> *ptr = '\0';
>> tmp = ptr + 1;
>> - if (strchr(arg, ':')) {
>> - semantic_error("Group name is not supported yet.\n");
>> - return -ENOTSUP;
>> - }
>> + ptr = strchr(arg, ':');
>> + if (ptr) {
>> + *ptr = '\0';
>> + if (!is_c_func_name(arg))
>> + goto not_fname;
>> + pev->group = strdup(arg);
>> + if (!pev->group)
>> + return -ENOMEM;
>> + arg = ptr + 1;
>> + } else
>> + pev->group = NULL;
>> if (!is_c_func_name(arg)) {
>> +not_fname:
>> semantic_error("%s is bad for event name -it must "
>> "follow C symbol-naming rule.\n", arg);
>> return -EINVAL;
>> @@ -1165,7 +1171,6 @@ static int parse_perf_probe_point(char *arg, struct
>> perf_probe_event *pev)
>> pev->event = strdup(arg);
>> if (pev->event == NULL)
>> return -ENOMEM;
>> - pev->group = NULL;
>> arg = tmp;
>> }
>>
>>
>>
>
--
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: masami.hiramatsu...@hitachi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
