On 3/26/18 9:35 AM, Steven Rostedt wrote:
On Mon, 26 Mar 2018 09:25:07 -0700 Alexei Starovoitov <a...@fb.com> 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.
It's a standard pattern in the kernel to stop for_each*() iterator when callback function returns non-null. Few examples: idr_for_each cfs_hash_for_each and there are plenty more. I don't mind to _rename_ for_each_kernel_tracepoint() into tracepoint_find_by_name(), but keeping exported function just to be used by out-of-tree modules would be wrong message for the kernel community in general. With my patch the for_each_kernel_tracepoint() will be used by bpf side and out-of-tree can trivially hack their callbacks to keep working. imo that's a better approach then renaming it.
