Adding register_ftrace_direct_hash function that registers
all entries (ip -> direct) provided in hash argument.

The difference to current register_ftrace_direct is
 - hash argument that allows to register multiple ip -> direct
   entries at once
 - we can call register_ftrace_direct_hash multiple times on the
   same ftrace_ops object, becase after first registration with
   register_ftrace_function_nolock, it uses ftrace_update_ops to
   update the ftrace_ops object

This change will allow us to have simple ftrace_ops for all bpf
direct interface users in following changes.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 include/linux/ftrace.h |   7 +++
 kernel/trace/ftrace.c  | 110 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 117 insertions(+)

diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 7ded7df6e9b5..2705c292341a 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -526,6 +526,8 @@ int unregister_ftrace_direct(struct ftrace_ops *ops, unsigned long addr,
 int modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr);
 int modify_ftrace_direct_nolock(struct ftrace_ops *ops, unsigned long addr);
 
+int register_ftrace_direct_hash(struct ftrace_ops *ops, struct ftrace_hash *hash);
+
 void ftrace_stub_direct_tramp(void);
 
 #else
@@ -552,6 +554,11 @@ static inline int modify_ftrace_direct_nolock(struct ftrace_ops *ops, unsigned l
 	return -ENODEV;
 }
 
+int register_ftrace_direct_hash(struct ftrace_ops *ops, struct ftrace_hash *hash)
+{
+	return -ENODEV;
+}
+
 /*
  * This must be implemented by the architecture.
  * It is the way the ftrace direct_ops helper, when called
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index a45556257963..06528af2281f 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -6219,6 +6219,116 @@ int modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
 	return err;
 }
 EXPORT_SYMBOL_GPL(modify_ftrace_direct);
+
+static unsigned long hash_count(struct ftrace_hash *hash)
+{
+	return hash ? hash->count : 0;
+}
+
+/**
+ * hash_add - adds two struct ftrace_hash and returns the result
+ * @a: struct ftrace_hash object
+ * @b: struct ftrace_hash object
+ *
+ * Returns struct ftrace_hash object on success, NULL on error.
+ */
+static struct ftrace_hash *hash_add(struct ftrace_hash *a, struct ftrace_hash *b)
+{
+	struct ftrace_func_entry *entry;
+	struct ftrace_hash *add;
+	int size, i;
+
+	size = hash_count(a) + hash_count(b);
+	if (size > 32)
+		size = 32;
+
+	add = alloc_and_copy_ftrace_hash(fls(size), a);
+	if (!add)
+		goto error;
+
+	size = 1 << b->size_bits;
+	for (i = 0; i < size; i++) {
+		hlist_for_each_entry(entry, &b->buckets[i], hlist) {
+			if (add_hash_entry_direct(add, entry->ip, entry->direct) == NULL)
+				goto error;
+		}
+	}
+	return add;
+
+ error:
+	free_ftrace_hash(add);
+	return NULL;
+}
+
+int register_ftrace_direct_hash(struct ftrace_ops *ops, struct ftrace_hash *hash)
+{
+	struct ftrace_hash *filter_hash = NULL, *new_hash = NULL, *free_hash = NULL;
+	struct ftrace_func_entry *entry;
+	int i, size, err = -EINVAL;
+	bool reg;
+
+	if (!hash_count(hash))
+		return 0;
+
+	mutex_lock(&direct_mutex);
+
+	/* Make sure requested entries are not already registered. */
+	size = 1 << hash->size_bits;
+	for (i = 0; i < size; i++) {
+		hlist_for_each_entry(entry, &hash->buckets[i], hlist) {
+			if (__ftrace_lookup_ip(direct_functions, entry->ip))
+				goto out_unlock;
+		}
+	}
+
+	filter_hash = ops->func_hash ? ops->func_hash->filter_hash : NULL;
+
+	/* If there's nothing in filter_hash we need to register the ops. */
+	reg = hash_count(filter_hash) == 0;
+	if (reg) {
+		if (ops->func || ops->trampoline)
+			goto out_unlock;
+		if (ops->flags & FTRACE_OPS_FL_ENABLED)
+			goto out_unlock;
+	}
+
+	err = -ENOMEM;
+	filter_hash = hash_add(filter_hash, hash);
+	if (!filter_hash)
+		goto out_unlock;
+
+	new_hash = hash_add(direct_functions, hash);
+	if (!new_hash)
+		goto out_unlock;
+
+	free_hash = direct_functions;
+	rcu_assign_pointer(direct_functions, new_hash);
+
+	if (reg) {
+		ops->func = call_direct_funcs;
+		ops->flags = MULTI_FLAGS;
+		ops->trampoline = FTRACE_REGS_ADDR;
+		ops->local_hash.filter_hash = filter_hash;
+
+		err = register_ftrace_function_nolock(ops);
+		if (!err)
+			filter_hash = NULL;
+	} else {
+		err = ftrace_update_ops(ops, filter_hash, EMPTY_HASH);
+	}
+
+ out_unlock:
+	mutex_unlock(&direct_mutex);
+
+	if (free_hash && free_hash != EMPTY_HASH)
+		call_rcu_tasks(&free_hash->rcu, register_ftrace_direct_cb);
+	if (filter_hash)
+		free_ftrace_hash(filter_hash);
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(register_ftrace_direct_hash);
+
 #endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
 
 /**
-- 
2.51.0

On Tue, 23 Sep 2025 23:51:40 +0200
Jiri Olsa <jolsa@kernel.org> wrote:

> Adding register_ftrace_direct_hash function that registers
> all entries (ip -> direct) provided in hash argument.
> 
> The difference to current register_ftrace_direct is
>  - hash argument that allows to register multiple ip -> direct
>    entries at once

I'm a bit confused. How is this different? Doesn't
register_ftrace_direct() register multiple ip -> direct entries at once
too? But instead of using a passed in hash, it uses the hash from
within the ftrace_ops.

>  - we can call register_ftrace_direct_hash multiple times on the
>    same ftrace_ops object, becase after first registration with
>    register_ftrace_function_nolock, it uses ftrace_update_ops to
>    update the ftrace_ops object

OK, I don't like the name "register" here. "register" should be for the
first instance and then it is registered. If you call it multiple times
on the same ops without "unregister" it should give an error.

Perhaps call this "update_ftrace_direct()" where it can update a direct
ftrace_ops from?

> 
> This change will allow us to have simple ftrace_ops for all bpf
> direct interface users in following changes.

After applying all the patches, I have this:

$ git grep register_ftrace_direct_hash
include/linux/ftrace.h:int register_ftrace_direct_hash(struct ftrace_ops *ops, struct ftrace_hash *hash);
include/linux/ftrace.h:int unregister_ftrace_direct_hash(struct ftrace_ops *ops, struct ftrace_hash *hash);
include/linux/ftrace.h:int register_ftrace_direct_hash(struct ftrace_ops *ops, struct ftrace_hash *hash)
include/linux/ftrace.h:int unregister_ftrace_direct_hash(struct ftrace_ops *ops, struct ftrace_hash *hash)
kernel/trace/ftrace.c:  err = register_ftrace_direct_hash(ops, hash);
kernel/trace/ftrace.c:  err = unregister_ftrace_direct_hash(ops, hash);
kernel/trace/ftrace.c:int register_ftrace_direct_hash(struct ftrace_ops *ops, struct ftrace_hash *hash)
kernel/trace/ftrace.c:EXPORT_SYMBOL_GPL(register_ftrace_direct_hash);
kernel/trace/ftrace.c:int unregister_ftrace_direct_hash(struct ftrace_ops *ops, struct ftrace_hash *hash)
kernel/trace/ftrace.c:EXPORT_SYMBOL_GPL(unregister_ftrace_direct_hash);

Where I do not see it is used outside of ftrace.c. Why is it exported?

-- Steve

On Wed, Sep 24, 2025 at 05:04:15AM -0400, Steven Rostedt wrote:
> On Tue, 23 Sep 2025 23:51:40 +0200
> Jiri Olsa <jolsa@kernel.org> wrote:
> 
> > Adding register_ftrace_direct_hash function that registers
> > all entries (ip -> direct) provided in hash argument.
> > 
> > The difference to current register_ftrace_direct is
> >  - hash argument that allows to register multiple ip -> direct
> >    entries at once
> 
> I'm a bit confused. How is this different? Doesn't
> register_ftrace_direct() register multiple ip -> direct entries at once
> too? But instead of using a passed in hash, it uses the hash from
> within the ftrace_ops.

right, but that assumes that we can touch the hash in ftrace_ops directly,
but register_ftrace_direct_hash semantics is bit different, because it allows
to register new (ip,addr) entries on already 'running' ftrace_ops, in which
case you can't change the ftrace_ops hash directly

> 
> >  - we can call register_ftrace_direct_hash multiple times on the
> >    same ftrace_ops object, becase after first registration with
> >    register_ftrace_function_nolock, it uses ftrace_update_ops to
> >    update the ftrace_ops object
> 
> OK, I don't like the name "register" here. "register" should be for the
> first instance and then it is registered. If you call it multiple times
> on the same ops without "unregister" it should give an error.
> 
> Perhaps call this "update_ftrace_direct()" where it can update a direct
> ftrace_ops from?

I agree the 'register' naming is confusing in here.. but we still need to
use 3 functions for register/unregister/modify operations, so perhaps:

   update_ftrace_direct_add(ops, hash)
   update_ftrace_direct_del(ops, hash)
   update_ftrace_direct_mod(ops, hash)

?

> 
> > 
> > This change will allow us to have simple ftrace_ops for all bpf
> > direct interface users in following changes.
> 
> After applying all the patches, I have this:
> 
> $ git grep register_ftrace_direct_hash
> include/linux/ftrace.h:int register_ftrace_direct_hash(struct ftrace_ops *ops, struct ftrace_hash *hash);
> include/linux/ftrace.h:int unregister_ftrace_direct_hash(struct ftrace_ops *ops, struct ftrace_hash *hash);
> include/linux/ftrace.h:int register_ftrace_direct_hash(struct ftrace_ops *ops, struct ftrace_hash *hash)
> include/linux/ftrace.h:int unregister_ftrace_direct_hash(struct ftrace_ops *ops, struct ftrace_hash *hash)
> kernel/trace/ftrace.c:  err = register_ftrace_direct_hash(ops, hash);
> kernel/trace/ftrace.c:  err = unregister_ftrace_direct_hash(ops, hash);
> kernel/trace/ftrace.c:int register_ftrace_direct_hash(struct ftrace_ops *ops, struct ftrace_hash *hash)
> kernel/trace/ftrace.c:EXPORT_SYMBOL_GPL(register_ftrace_direct_hash);
> kernel/trace/ftrace.c:int unregister_ftrace_direct_hash(struct ftrace_ops *ops, struct ftrace_hash *hash)
> kernel/trace/ftrace.c:EXPORT_SYMBOL_GPL(unregister_ftrace_direct_hash);
> 
> Where I do not see it is used outside of ftrace.c. Why is it exported?

I have bpf changes using this that I did not post yet, but even with that
there's probably no reason to export this.. will remove

thanks,
jirka

On Wed, 24 Sep 2025 16:37:03 +0200
Jiri Olsa <olsajiri@gmail.com> wrote:

> I have bpf changes using this that I did not post yet, but even with that
> there's probably no reason to export this.. will remove

I'm interested in seeing these patches, as the ftrace hashes were
supposed to be opaque from other parts of the kernel.

-- Steve

On Wed, Sep 24, 2025 at 11:07:03AM -0400, Steven Rostedt wrote:
> On Wed, 24 Sep 2025 16:37:03 +0200
> Jiri Olsa <olsajiri@gmail.com> wrote:
> 
> > I have bpf changes using this that I did not post yet, but even with that
> > there's probably no reason to export this.. will remove
> 
> I'm interested in seeing these patches, as the ftrace hashes were
> supposed to be opaque from other parts of the kernel.

branch:
  https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git/log/?h=bpf/tracing_multi

used in this commit:
  https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git/commit/?h=bpf/tracing_multi&id=8814108949537edaae84fbeee1cbc28280590b7f

background.. it's poc code for bpf tracing-multi attachment. Most likely we will
go with Menglong change instead [1], but it could use this direct interface in a
same way for speeding up the attachment

jirka


[1] https://lore.kernel.org/bpf/20250528034712.138701-1-dongml2@chinatelecom.cn/