From nobody Sun Oct 5 23:54:47 2025 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DB40A289E1D; Tue, 29 Jul 2025 10:29:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1753784963; cv=none; b=UO5sJ7uwMhWFws46j6fhGiujyN8MUHRPElY1/zQQc91iHDglBD39RGH7Tv8etPhkgBxI+xkRdg79iwtHkdXMB2mzjKh2osYwjhmtx2CfaZNooA4/ghV9mD/FVc9fdmgt0Sai4cC5sI96p65JQgAGXzYIU2yDkTV3Jfw6dh+8vtQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1753784963; c=relaxed/simple; bh=LkMiSu9rJHL78CHuuZGPPKsY5tkXMO6EbHSe73wtQp4=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=uCFAKkRjk5sOist45vBcKiIfXIYglFWvog8lGCpMa7/dsGEUxreEnU+Qwgf//tLF06Gb+vemJf9C94SJtTfpd1OnBrtZFoRLGpte81ChV5OViXx16o+MlkbSEHjzSE/6wXX4RFZe6+1Qn7dToTtciJOQ1RwtraKZ7yiZdL3pLjs= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=rVYSeU9r; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="rVYSeU9r" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 9DE94C4CEEF; Tue, 29 Jul 2025 10:29:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1753784962; bh=LkMiSu9rJHL78CHuuZGPPKsY5tkXMO6EbHSe73wtQp4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=rVYSeU9r1+uoZGBIGoBMQ6R+h4oY3hJF2KI+FDxgk5+uR6qnN5Wb9kjh7u6IMih6i O5jfI1GiHFgpxpo+HuYXZuHthjNJ5ms74hedEX7TfJmXzkiqMU4TMDwErUdajRxIwQ O4pt4+0jtJrojvbSrwLxvmoUwewjfLJRbfn7zHT383JT3QbYk6+V18BIHn7B/kD2MP StEoMpXkVCDweHzbG1dt+yA7qia0tfLCmQ0MwB57/y5SoTn99bOjhYcKdX9R27adND zlglEGgArKhemOYfUc9dZf2QNGZkqHcblZvwB6M5cK5f9t/nLeFIkPUSeZ5gLNSfSI TJlplUdrP/sww== From: Jiri Olsa To: Steven Rostedt , Florent Revest , Mark Rutland Cc: bpf@vger.kernel.org, linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Menglong Dong Subject: [RFC 06/10] ftrace: Use direct hash interface in direct functions Date: Tue, 29 Jul 2025 12:28:09 +0200 Message-ID: <20250729102813.1531457-7-jolsa@kernel.org> X-Mailer: git-send-email 2.50.1 In-Reply-To: <20250729102813.1531457-1-jolsa@kernel.org> References: <20250729102813.1531457-1-jolsa@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" Implement current *_ftrace_direct function with their *_hash function counterparts. Signed-off-by: Jiri Olsa --- include/linux/ftrace.h | 17 +-- kernel/bpf/trampoline.c | 10 +- kernel/trace/ftrace.c | 242 +++++----------------------------- kernel/trace/trace_selftest.c | 5 +- 4 files changed, 45 insertions(+), 229 deletions(-) diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h index 9a6fcdafeda2..85f4ab1a1e72 100644 --- a/include/linux/ftrace.h +++ b/include/linux/ftrace.h @@ -536,11 +536,10 @@ struct ftrace_func_entry { =20 #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS unsigned long ftrace_find_rec_direct(unsigned long ip); -int register_ftrace_direct(struct ftrace_ops *ops, unsigned long addr); -int unregister_ftrace_direct(struct ftrace_ops *ops, unsigned long addr, +int register_ftrace_direct(struct ftrace_ops *ops, unsigned long ip, unsig= ned long addr); +int unregister_ftrace_direct(struct ftrace_ops *ops, unsigned long ip, uns= igned long addr, bool free_filters); -int modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr); -int modify_ftrace_direct_nolock(struct ftrace_ops *ops, unsigned long addr= ); +int modify_ftrace_direct(struct ftrace_ops *ops, unsigned long ip, unsigne= d long addr, bool lock_direct_mutex); =20 int register_ftrace_direct_hash(struct ftrace_ops *ops, struct ftrace_hash= *hash); int unregister_ftrace_direct_hash(struct ftrace_ops *ops, struct ftrace_ha= sh *hash); @@ -554,20 +553,16 @@ static inline unsigned long ftrace_find_rec_direct(un= signed long ip) { return 0; } -static inline int register_ftrace_direct(struct ftrace_ops *ops, unsigned = long addr) +static inline int register_ftrace_direct(struct ftrace_ops *ops, unsigned = long ip, unsigned long addr) { return -ENODEV; } -static inline int unregister_ftrace_direct(struct ftrace_ops *ops, unsigne= d long addr, +static inline int unregister_ftrace_direct(struct ftrace_ops *ops, unsigne= d long ip, unsigned long addr, bool free_filters) { return -ENODEV; } -static inline int modify_ftrace_direct(struct ftrace_ops *ops, unsigned lo= ng addr) -{ - return -ENODEV; -} -static inline int modify_ftrace_direct_nolock(struct ftrace_ops *ops, unsi= gned long addr) +static inline int modify_ftrace_direct(struct ftrace_ops *ops, unsigned lo= ng ip, unsigned long addr, bool lock_direct_mutex) { return -ENODEV; } diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c index 0e364614c3a2..6bf272715f0e 100644 --- a/kernel/bpf/trampoline.c +++ b/kernel/bpf/trampoline.c @@ -181,7 +181,7 @@ static int unregister_fentry(struct bpf_trampoline *tr,= void *old_addr) int ret; =20 if (tr->func.ftrace_managed) - ret =3D unregister_ftrace_direct(tr->fops, (long)old_addr, false); + ret =3D unregister_ftrace_direct(tr->fops, (unsigned long) ip, (long)old= _addr, false); else ret =3D bpf_arch_text_poke(ip, BPF_MOD_CALL, old_addr, NULL); =20 @@ -195,10 +195,7 @@ static int modify_fentry(struct bpf_trampoline *tr, vo= id *old_addr, void *new_ad int ret; =20 if (tr->func.ftrace_managed) { - if (lock_direct_mutex) - ret =3D modify_ftrace_direct(tr->fops, (long)new_addr); - else - ret =3D modify_ftrace_direct_nolock(tr->fops, (long)new_addr); + ret =3D modify_ftrace_direct(tr->fops, (unsigned long) ip, (long)new_add= r, lock_direct_mutex); } else { ret =3D bpf_arch_text_poke(ip, BPF_MOD_CALL, old_addr, new_addr); } @@ -220,8 +217,7 @@ static int register_fentry(struct bpf_trampoline *tr, v= oid *new_addr) } =20 if (tr->func.ftrace_managed) { - ftrace_set_filter_ip(tr->fops, (unsigned long)ip, 0, 1); - ret =3D register_ftrace_direct(tr->fops, (long)new_addr); + ret =3D register_ftrace_direct(tr->fops, (unsigned long)ip, (long)new_ad= dr); } else { ret =3D bpf_arch_text_poke(ip, BPF_MOD_CALL, NULL, new_addr); } diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index fcb8f2d3172b..151ca94f496a 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c @@ -2593,16 +2593,6 @@ unsigned long ftrace_find_rec_direct(unsigned long i= p) return entry->direct; } =20 -static void call_direct_funcs(unsigned long ip, unsigned long pip, - struct ftrace_ops *ops, struct ftrace_regs *fregs) -{ - unsigned long addr =3D READ_ONCE(ops->direct_call); - - if (!addr) - return; - - arch_ftrace_set_direct_caller(fregs, addr); -} #endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */ =20 /** @@ -5935,28 +5925,24 @@ static int check_direct_multi(struct ftrace_ops *op= s) return 0; } =20 -static void remove_direct_functions_hash(struct ftrace_hash *hash, unsigne= d long addr) +static void register_ftrace_direct_cb(struct rcu_head *rhp) { - struct ftrace_func_entry *entry, *del; - int size, i; + struct ftrace_hash *fhp =3D container_of(rhp, struct ftrace_hash, rcu); =20 - size =3D 1 << hash->size_bits; - for (i =3D 0; i < size; i++) { - hlist_for_each_entry(entry, &hash->buckets[i], hlist) { - del =3D __ftrace_lookup_ip(direct_functions, entry->ip); - if (del && del->direct =3D=3D addr) { - remove_hash_entry(direct_functions, del); - kfree(del); - } - } - } + free_ftrace_hash(fhp); } =20 -static void register_ftrace_direct_cb(struct rcu_head *rhp) +static struct ftrace_hash *hash_from_ip(unsigned long ip, unsigned long ad= dr) { - struct ftrace_hash *fhp =3D container_of(rhp, struct ftrace_hash, rcu); + struct ftrace_hash *hash; =20 - free_ftrace_hash(fhp); + ip =3D ftrace_location(ip); + if (!ip) + return NULL; + hash =3D alloc_ftrace_hash(FTRACE_HASH_DEFAULT_BITS); + if (!hash || !add_hash_entry_direct(hash, ip, addr)) + return NULL; + return hash; } =20 /** @@ -5981,89 +5967,17 @@ static void register_ftrace_direct_cb(struct rcu_he= ad *rhp) * -ENODEV - @ip does not point to a ftrace nop location (or not support= ed) * -ENOMEM - There was an allocation failure. */ -int register_ftrace_direct(struct ftrace_ops *ops, unsigned long addr) +int register_ftrace_direct(struct ftrace_ops *ops, unsigned long ip, unsig= ned long addr) { - struct ftrace_hash *hash, *new_hash =3D NULL, *free_hash =3D NULL; - struct ftrace_func_entry *entry, *new; - int err =3D -EBUSY, size, i; - - if (ops->func || ops->trampoline) - return -EINVAL; - if (!(ops->flags & FTRACE_OPS_FL_INITIALIZED)) - return -EINVAL; - if (ops->flags & FTRACE_OPS_FL_ENABLED) - return -EINVAL; - - hash =3D ops->func_hash->filter_hash; - if (ftrace_hash_empty(hash)) - return -EINVAL; - - mutex_lock(&direct_mutex); - - /* Make sure requested entries are not already registered.. */ - size =3D 1 << hash->size_bits; - for (i =3D 0; i < size; i++) { - hlist_for_each_entry(entry, &hash->buckets[i], hlist) { - if (ftrace_find_rec_direct(entry->ip)) - goto out_unlock; - } - } - - err =3D -ENOMEM; - - /* Make a copy hash to place the new and the old entries in */ - size =3D hash->count + direct_functions->count; - size =3D fls(size); - if (size > FTRACE_HASH_MAX_BITS) - size =3D FTRACE_HASH_MAX_BITS; - new_hash =3D alloc_ftrace_hash(size); - if (!new_hash) - goto out_unlock; - - /* Now copy over the existing direct entries */ - size =3D 1 << direct_functions->size_bits; - for (i =3D 0; i < size; i++) { - hlist_for_each_entry(entry, &direct_functions->buckets[i], hlist) { - new =3D add_hash_entry(new_hash, entry->ip); - if (!new) - goto out_unlock; - new->direct =3D entry->direct; - } - } - - /* ... and add the new entries */ - size =3D 1 << hash->size_bits; - for (i =3D 0; i < size; i++) { - hlist_for_each_entry(entry, &hash->buckets[i], hlist) { - new =3D add_hash_entry(new_hash, entry->ip); - if (!new) - goto out_unlock; - /* Update both the copy and the hash entry */ - new->direct =3D addr; - entry->direct =3D addr; - } - } - - free_hash =3D direct_functions; - rcu_assign_pointer(direct_functions, new_hash); - new_hash =3D NULL; - - ops->func =3D call_direct_funcs; - ops->flags =3D MULTI_FLAGS; - ops->trampoline =3D FTRACE_REGS_ADDR; - ops->direct_call =3D addr; - - err =3D register_ftrace_function_nolock(ops); - - out_unlock: - mutex_unlock(&direct_mutex); - - if (free_hash && free_hash !=3D EMPTY_HASH) - call_rcu_tasks(&free_hash->rcu, register_ftrace_direct_cb); + struct ftrace_hash *hash; + int err; =20 - if (new_hash) - free_ftrace_hash(new_hash); + hash =3D hash_from_ip(ip, addr); + if (!hash) + return -ENOMEM; =20 + err =3D register_ftrace_direct_hash(ops, hash); + free_ftrace_hash(hash); return err; } EXPORT_SYMBOL_GPL(register_ftrace_direct); @@ -6083,111 +5997,24 @@ EXPORT_SYMBOL_GPL(register_ftrace_direct); * 0 on success * -EINVAL - The @ops object was not properly registered. */ -int unregister_ftrace_direct(struct ftrace_ops *ops, unsigned long addr, +int unregister_ftrace_direct(struct ftrace_ops *ops, unsigned long ip, uns= igned long addr, bool free_filters) { - struct ftrace_hash *hash =3D ops->func_hash->filter_hash; + struct ftrace_hash *hash; int err; =20 - if (check_direct_multi(ops)) - return -EINVAL; - if (!(ops->flags & FTRACE_OPS_FL_ENABLED)) - return -EINVAL; - - mutex_lock(&direct_mutex); - err =3D unregister_ftrace_function(ops); - remove_direct_functions_hash(hash, addr); - mutex_unlock(&direct_mutex); - - /* cleanup for possible another register call */ - ops->func =3D NULL; - ops->trampoline =3D 0; + hash =3D hash_from_ip(ip, addr); + if (!hash) + return -ENOMEM; =20 + err =3D unregister_ftrace_direct_hash(ops, hash); + free_ftrace_hash(hash); if (free_filters) ftrace_free_filter(ops); return err; } EXPORT_SYMBOL_GPL(unregister_ftrace_direct); =20 -static int -__modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr) -{ - struct ftrace_hash *hash; - struct ftrace_func_entry *entry, *iter; - static struct ftrace_ops tmp_ops =3D { - .func =3D ftrace_stub, - .flags =3D FTRACE_OPS_FL_STUB, - }; - int i, size; - int err; - - lockdep_assert_held_once(&direct_mutex); - - /* Enable the tmp_ops to have the same functions as the direct ops */ - ftrace_ops_init(&tmp_ops); - tmp_ops.func_hash =3D ops->func_hash; - tmp_ops.direct_call =3D addr; - - err =3D register_ftrace_function_nolock(&tmp_ops); - if (err) - return err; - - /* - * Now the ftrace_ops_list_func() is called to do the direct callers. - * We can safely change the direct functions attached to each entry. - */ - mutex_lock(&ftrace_lock); - - hash =3D ops->func_hash->filter_hash; - size =3D 1 << hash->size_bits; - for (i =3D 0; i < size; i++) { - hlist_for_each_entry(iter, &hash->buckets[i], hlist) { - entry =3D __ftrace_lookup_ip(direct_functions, iter->ip); - if (!entry) - continue; - entry->direct =3D addr; - } - } - /* Prevent store tearing if a trampoline concurrently accesses the value = */ - WRITE_ONCE(ops->direct_call, addr); - - mutex_unlock(&ftrace_lock); - - /* Removing the tmp_ops will add the updated direct callers to the functi= ons */ - unregister_ftrace_function(&tmp_ops); - - return err; -} - -/** - * modify_ftrace_direct_nolock - Modify an existing direct 'multi' call - * to call something else - * @ops: The address of the struct ftrace_ops object - * @addr: The address of the new trampoline to call at @ops functions - * - * This is used to unregister currently registered direct caller and - * register new one @addr on functions registered in @ops object. - * - * Note there's window between ftrace_shutdown and ftrace_startup calls - * where there will be no callbacks called. - * - * Caller should already have direct_mutex locked, so we don't lock - * direct_mutex here. - * - * Returns: zero on success. Non zero on error, which includes: - * -EINVAL - The @ops object was not properly registered. - */ -int modify_ftrace_direct_nolock(struct ftrace_ops *ops, unsigned long addr) -{ - if (check_direct_multi(ops)) - return -EINVAL; - if (!(ops->flags & FTRACE_OPS_FL_ENABLED)) - return -EINVAL; - - return __modify_ftrace_direct(ops, addr); -} -EXPORT_SYMBOL_GPL(modify_ftrace_direct_nolock); - /** * modify_ftrace_direct - Modify an existing direct 'multi' call * to call something else @@ -6203,18 +6030,17 @@ EXPORT_SYMBOL_GPL(modify_ftrace_direct_nolock); * Returns: zero on success. Non zero on error, which includes: * -EINVAL - The @ops object was not properly registered. */ -int modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr) +int modify_ftrace_direct(struct ftrace_ops *ops, unsigned long ip, unsigne= d long addr, bool lock_direct_mutex) { + struct ftrace_hash *hash; int err; =20 - if (check_direct_multi(ops)) - return -EINVAL; - if (!(ops->flags & FTRACE_OPS_FL_ENABLED)) - return -EINVAL; + hash =3D hash_from_ip(ip, addr); + if (!hash) + return -ENOMEM; =20 - mutex_lock(&direct_mutex); - err =3D __modify_ftrace_direct(ops, addr); - mutex_unlock(&direct_mutex); + err =3D modify_ftrace_direct_hash(ops, hash, lock_direct_mutex); + free_ftrace_hash(hash); return err; } EXPORT_SYMBOL_GPL(modify_ftrace_direct); diff --git a/kernel/trace/trace_selftest.c b/kernel/trace/trace_selftest.c index d88c44f1dfa5..37f5eb1f252b 100644 --- a/kernel/trace/trace_selftest.c +++ b/kernel/trace/trace_selftest.c @@ -1135,8 +1135,7 @@ trace_selftest_startup_function_graph(struct tracer *= trace, * Register direct function together with graph tracer * and make sure we get graph trace. */ - ftrace_set_filter_ip(&direct, (unsigned long)DYN_FTRACE_TEST_NAME, 0, 0); - ret =3D register_ftrace_direct(&direct, + ret =3D register_ftrace_direct(&direct, (unsigned long)DYN_FTRACE_TEST_NA= ME, (unsigned long)ftrace_stub_direct_tramp); if (ret) goto out; @@ -1159,7 +1158,7 @@ trace_selftest_startup_function_graph(struct tracer *= trace, =20 unregister_ftrace_graph(&fgraph_ops); =20 - ret =3D unregister_ftrace_direct(&direct, + ret =3D unregister_ftrace_direct(&direct, (unsigned long)DYN_FTRACE_TEST_= NAME, (unsigned long)ftrace_stub_direct_tramp, true); if (ret) --=20 2.50.1