Re: [PATCH bpf-next v2 04/18] ftrace: add reset_ftrace_direct_ips
From: Steven Rostedt <rostedt@goodmis.org>
Date: 2025-07-03 15:30:12
Also in:
bpf, lkml
On Thu, 3 Jul 2025 20:15:07 +0800 Menglong Dong [off-list ref] wrote: Note, the tracing subsystem uses capitalization in the subject: ftrace: Add reset_ftrace_direct_ips
For now, we can change the address of a direct ftrace_ops with modify_ftrace_direct(). However, we can't change the functions to filter for a direct ftrace_ops. Therefore, we introduce the function reset_ftrace_direct_ips() to do such things, and this function will reset the functions to filter for a direct ftrace_ops. This function do such thing in following steps: 1. filter out the new functions from ips that don't exist in the ops->func_hash->filter_hash and add them to the new hash. 2. add all the functions in the new ftrace_hash to direct_functions by ftrace_direct_update(). 3. reset the functions to filter of the ftrace_ops to the ips with ftrace_set_filter_ips(). 4. remove the functions that in the old ftrace_hash, but not in the new ftrace_hash from direct_functions.
Please also include a module that can be loaded for testing. See samples/ftrace/ftrace-direct* But make it a separate patch. And you'll need to add a test in selftests. See tools/testing/selftests/ftrace/test.d/direct
quoted hunk ↗ jump to hunk
Signed-off-by: Menglong Dong <redacted> --- include/linux/ftrace.h | 7 ++++ kernel/trace/ftrace.c | 75 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+)diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h index b672ca15f265..b7c60f5a4120 100644 --- a/include/linux/ftrace.h +++ b/include/linux/ftrace.h@@ -528,6 +528,8 @@ int modify_ftrace_direct_nolock(struct ftrace_ops *ops, unsigned long addr); void ftrace_stub_direct_tramp(void); +int reset_ftrace_direct_ips(struct ftrace_ops *ops, unsigned long *ips, + unsigned int cnt); #else struct ftrace_ops; static inline unsigned long ftrace_find_rec_direct(unsigned long ip)@@ -551,6 +553,11 @@ static inline int modify_ftrace_direct_nolock(struct ftrace_ops *ops, unsigned l { return -ENODEV; } +static inline int reset_ftrace_direct_ips(struct ftrace_ops *ops, unsigned long *ips, + unsigned int cnt) +{ + return -ENODEV; +} /* * This must be implemented by the architecture.diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index f5f6d7bc26f0..db3aa61889d3 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c@@ -6224,6 +6224,81 @@ int modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr) return err; } EXPORT_SYMBOL_GPL(modify_ftrace_direct); + +/* reset the ips for a direct ftrace (add or remove) */
As this function is being used externally, it requires proper KernelDoc headers. What exactly do you mean by "reset"?
+int reset_ftrace_direct_ips(struct ftrace_ops *ops, unsigned long *ips,
+ unsigned int cnt)
+{
+ struct ftrace_hash *hash, *free_hash;
+ struct ftrace_func_entry *entry, *del;
+ unsigned long ip;
+ int err, size;
+
+ if (check_direct_multi(ops))
+ return -EINVAL;
+ if (!(ops->flags & FTRACE_OPS_FL_ENABLED))
+ return -EINVAL;
+
+ mutex_lock(&direct_mutex);
+ hash = alloc_ftrace_hash(FTRACE_HASH_DEFAULT_BITS);
+ if (!hash) {
+ err = -ENOMEM;
+ goto out_unlock;
+ }
+
+ /* find out the new functions from ips and add to hash */Capitalize comment: /* Find out ...
+ for (int i = 0; i < cnt; i++) {
+ ip = ftrace_location(ips[i]);
+ if (!ip) {
+ err = -ENOENT;
+ goto out_unlock;
+ }
+ if (__ftrace_lookup_ip(ops->func_hash->filter_hash, ip))
+ continue;
+ err = __ftrace_match_addr(hash, ip, 0);
+ if (err)
+ goto out_unlock;
+ }
+
+ free_hash = direct_functions;Add newline.
+ /* add the new ips to direct hash. */
Again capitalize.
+ err = ftrace_direct_update(hash, ops->direct_call); + if (err) + goto out_unlock; + + if (free_hash && free_hash != EMPTY_HASH) + call_rcu_tasks(&free_hash->rcu, register_ftrace_direct_cb);
Since the above is now used more than once, let's make it into a helper
function so that if things change, there's only one place to change it:
free_ftrace_direct(free_hash);
static inline void free_ftrace_direct(struct ftrace_hash *hash)
{
if (hash && hash != EMPTY_HASH)
call_rcu_tasks(&free_hash->rcu, register_ftrace_direct_cb);
}
+
+ free_ftrace_hash(hash);
+ hash = alloc_and_copy_ftrace_hash(FTRACE_HASH_DEFAULT_BITS,
+ ops->func_hash->filter_hash);
+ if (!hash) {
+ err = -ENOMEM;
+ goto out_unlock;
+ }
+ err = ftrace_set_filter_ips(ops, ips, cnt, 0, 1);
+
+ /* remove the entries that don't exist in our filter_hash anymore
+ * from the direct_functions.
+ */This isn't the network subsystem, we use the default comment style for multiple lines: /* * line 1 * line 2 * ... */ -- Steve
+ size = 1 << hash->size_bits;
+ for (int i = 0; i < size; i++) {
+ hlist_for_each_entry(entry, &hash->buckets[i], hlist) {
+ if (__ftrace_lookup_ip(ops->func_hash->filter_hash, entry->ip))
+ continue;
+ del = __ftrace_lookup_ip(direct_functions, entry->ip);
+ if (del && del->direct == ops->direct_call) {
+ remove_hash_entry(direct_functions, del);
+ kfree(del);
+ }
+ }
+ }
+out_unlock:
+ mutex_unlock(&direct_mutex);
+ if (hash)
+ free_ftrace_hash(hash);
+ return err;
+}
#endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
/**