----- On Mar 26, 2018, at 12:35 PM, rostedt rostedt@xxxxxxxxxxx wrote:
> On Mon, 26 Mar 2018 09:25:07 -0700
> Alexei Starovoitov <ast@xxxxxx> wrote:
>
>> commit log of patch 6 states:
>>
>> "for_each_tracepoint_range() api has no users inside the kernel.
>> Make it more useful with ability to stop for_each() loop depending
>> via callback return value.
>> In such form it's used in subsequent patch."
>>
>> and in patch 7:
>>
>> +static void *__find_tp(struct tracepoint *tp, void *priv)
>> +{
>> + char *name = priv;
>> +
>> + if (!strcmp(tp->name, name))
>> + return tp;
>> + return NULL;
>> +}
>> ...
>> + struct tracepoint *tp;
>> ...
>> + tp = for_each_kernel_tracepoint(__find_tp, tp_name);
>> + if (!tp)
>> + return -ENOENT;
>>
>> still not obvious?
>
> Please just create a new function called tracepoint_find_by_name(), and
> use that. I don't see any benefit in using a for_each* function for
> such a simple routine. Not to mention, you then don't need to know the
> internals of a tracepoint in kernel/bpf/syscall.c.
Steven's approach is fine by me, considering there should never be duplicated
tracepoint definitions (it emits a __tracepoint_##name symbol which would cause
multiple symbols definition errors at link time if there are more than
a single definition per tracepoint name in the core kernel). The exported
API should probably be named "kernel_tracepoint_find_by_name()" or something
similar, thus indicating that it only lookup tracepoints in the core kernel.
Which brings the next question: what are Alexei's plan to handle tracepoints
in modules, considering module load/unload scenarios ? The tracepoint API
has module notifiers for this, but it does not appear to be used in this
patch series.
Thanks,
Mathieu
>
> -- Steve
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
--
To unsubscribe from this list: send the line "unsubscribe linux-api" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
