----- On Mar 26, 2018, at 12:35 PM, rostedt rostedt@goodmis.org wrote:

On Mon, 26 Mar 2018 09:25:07 -0700
Alexei Starovoitov [off-list ref] wrote:

quoted

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