public inbox for linux-trace-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] fprobe: Add unregister_fprobe_sync() for synchronous unregistration
@ 2026-04-27 12:09 Masami Hiramatsu (Google)
  2026-04-28 18:27 ` Steven Rostedt
  2026-05-03  3:25 ` kernel test robot
  0 siblings, 2 replies; 3+ messages in thread
From: Masami Hiramatsu (Google) @ 2026-04-27 12:09 UTC (permalink / raw)
  To: Steven Rostedt, Masami Hiramatsu
  Cc: Mathieu Desnoyers, Jonathan Corbet, linux-kernel,
	linux-trace-kernel, linux-doc

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Currently, unregister_fprobe() removes the ftrace hooks but does not
wait for the RCU grace period to expire. This is efficient for batch
unregistration of multiple fprobes (to avoid multiple RCU grace period
latencies), but it leaves a window where probe handlers might still be
running on other CPUs after the function returns.
If a caller needs to free the fprobe structure or unload the module
immediately after unregistration, they must manually call
synchronize_rcu() to prevent use-after-free issues.

To simplify this use case, introduce unregister_fprobe_sync(). This
function unregisters the fprobe and waits for the RCU grace period to
complete before returning.

Also, update the documentation of unregister_fprobe() to clarify its
non-blocking behavior and suggest using unregister_fprobe_sync() for the
last probe in a batch. Finally, update the fprobe sample module to use
the synchronous version on exit to ensure safe module unloading.
And add a fix to use synchronous version in the sample code and
trace_fprobe (unexpected error case).

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 Documentation/trace/fprobe.rst  |   15 ++++++++++++---
 include/linux/fprobe.h          |    5 +++++
 kernel/trace/fprobe.c           |   30 ++++++++++++++++++++++++++++++
 kernel/trace/trace_fprobe.c     |    9 +++++++--
 samples/fprobe/fprobe_example.c |    2 +-
 5 files changed, 55 insertions(+), 6 deletions(-)

diff --git a/Documentation/trace/fprobe.rst b/Documentation/trace/fprobe.rst
index 95998b189ae3..eee4860ab29a 100644
--- a/Documentation/trace/fprobe.rst
+++ b/Documentation/trace/fprobe.rst
@@ -65,6 +65,12 @@ To disable (remove from functions) this fprobe, call::
 
   unregister_fprobe(&fp);
 
+Or if you need to wait for the RCU grace period to ensure no handlers
+are running on any CPU (e.g., before freeing the `fprobe` structure),
+use::
+
+  unregister_fprobe_sync(&fp);
+
 You can temporally (soft) disable the fprobe by::
 
   disable_fprobe(&fp);
@@ -81,9 +87,12 @@ Same as ftrace, the registered callbacks will start being called some time
 after the register_fprobe() is called and before it returns. See
 Documentation/trace/ftrace.rst.
 
-Also, the unregister_fprobe() will guarantee that both enter and exit
-handlers are no longer being called by functions after unregister_fprobe()
-returns as same as unregister_ftrace_function().
+Also, the `unregister_fprobe_sync()` will guarantee that both enter and exit
+handlers are no longer being called by functions after it returns.
+On the other hand, `unregister_fprobe()` does not wait for the RCU grace period,
+so handlers might still be running on other CPUs for a short time after it returns.
+This is useful when you unregister multiple fprobes in a batch to avoid
+waiting for the RCU grace period for each one.
 
 The fprobe entry/exit handler
 =============================
diff --git a/include/linux/fprobe.h b/include/linux/fprobe.h
index 0a3bcd1718f3..6ae452e250a1 100644
--- a/include/linux/fprobe.h
+++ b/include/linux/fprobe.h
@@ -94,6 +94,7 @@ int register_fprobe(struct fprobe *fp, const char *filter, const char *notfilter
 int register_fprobe_ips(struct fprobe *fp, unsigned long *addrs, int num);
 int register_fprobe_syms(struct fprobe *fp, const char **syms, int num);
 int unregister_fprobe(struct fprobe *fp);
+int unregister_fprobe_sync(struct fprobe *fp);
 bool fprobe_is_registered(struct fprobe *fp);
 int fprobe_count_ips_from_filter(const char *filter, const char *notfilter);
 #else
@@ -113,6 +114,10 @@ static inline int unregister_fprobe(struct fprobe *fp)
 {
 	return -EOPNOTSUPP;
 }
+static inline int unregister_fprobe_sync(struct fprobe *fp)
+{
+	return -EOPNOTSUPP;
+}
 static inline bool fprobe_is_registered(struct fprobe *fp)
 {
 	return false;
diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c
index cc49ebd2a773..5f3e48385a47 100644
--- a/kernel/trace/fprobe.c
+++ b/kernel/trace/fprobe.c
@@ -1097,6 +1097,9 @@ static int unregister_fprobe_nolock(struct fprobe *fp)
  * @fp: A fprobe data structure to be unregistered.
  *
  * Unregister fprobe (and remove ftrace hooks from the function entries).
+ * Note: This function does not wait for RCU grace period, since user
+ * may use several fprobes (and then unregister them one by one). In that
+ * case, it is recommended to use unregister_fprobe_sync() for the last fprobe.
  *
  * Return 0 if @fp is unregistered successfully, -errno if not.
  */
@@ -1110,6 +1113,33 @@ int unregister_fprobe(struct fprobe *fp)
 }
 EXPORT_SYMBOL_GPL(unregister_fprobe);
 
+/**
+ * unregister_fprobe_sync() - Unregister fprobe synchronously with RCU grace period.
+ * @fp: A fprobe data structure to be unregistered.
+ *
+ * Unregister fprobe (and remove ftrace hooks from the function entries) and
+ * wait for the RCU grace period to finish. This is useful for preventing
+ * the fprobe from being used after it is unregistered.
+ *
+ * Return 0 if @fp is unregistered successfully, -errno if not.
+ */
+int unregister_fprobe_sync(struct fprobe *fp)
+{
+	int ret;
+
+	guard(mutex)(&fprobe_mutex);
+	if (!fp || !fprobe_registered(fp))
+		return -EINVAL;
+
+	ret = unregister_fprobe_nolock(fp);
+	if (ret)
+		return ret;
+
+	synchronize_rcu();
+	return 0;
+}
+EXPORT_SYMBOL_GPL(unregister_fprobe_sync);
+
 static int __init fprobe_initcall(void)
 {
 	rhltable_init(&fprobe_ip_table, &fprobe_rht_params);
diff --git a/kernel/trace/trace_fprobe.c b/kernel/trace/trace_fprobe.c
index 9f5f08c0e7c2..fa5b41f7f306 100644
--- a/kernel/trace/trace_fprobe.c
+++ b/kernel/trace/trace_fprobe.c
@@ -845,8 +845,13 @@ static int __register_trace_fprobe(struct trace_fprobe *tf)
 /* Internal unregister function - just handle fprobe and flags */
 static void __unregister_trace_fprobe(struct trace_fprobe *tf)
 {
-	if (trace_fprobe_is_registered(tf))
-		unregister_fprobe(&tf->fp);
+	/*
+	 * Here, @tf must NOT be busy, so it MUST be unregistered already.
+	 * But if it is unexpectedly registered, unregister it synchronously.
+	 */
+	if (WARN_ON_ONCE(trace_fprobe_is_registered(tf)))
+		unregister_fprobe_sync(&tf->fp);
+
 	if (tf->tuser) {
 		tracepoint_user_put(tf->tuser);
 		tf->tuser = NULL;
diff --git a/samples/fprobe/fprobe_example.c b/samples/fprobe/fprobe_example.c
index bfe98ce826f3..382d2f67672a 100644
--- a/samples/fprobe/fprobe_example.c
+++ b/samples/fprobe/fprobe_example.c
@@ -142,7 +142,7 @@ static int __init fprobe_init(void)
 
 static void __exit fprobe_exit(void)
 {
-	unregister_fprobe(&sample_probe);
+	unregister_fprobe_sync(&sample_probe);
 
 	pr_info("fprobe at %s unregistered. %ld times hit, %ld times missed\n",
 		symbol, nhit, sample_probe.nmissed);


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] fprobe: Add unregister_fprobe_sync() for synchronous unregistration
  2026-04-27 12:09 [PATCH] fprobe: Add unregister_fprobe_sync() for synchronous unregistration Masami Hiramatsu (Google)
@ 2026-04-28 18:27 ` Steven Rostedt
  2026-05-03  3:25 ` kernel test robot
  1 sibling, 0 replies; 3+ messages in thread
From: Steven Rostedt @ 2026-04-28 18:27 UTC (permalink / raw)
  To: Masami Hiramatsu (Google)
  Cc: Mathieu Desnoyers, Jonathan Corbet, linux-kernel,
	linux-trace-kernel, linux-doc

On Mon, 27 Apr 2026 21:09:58 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:

> +/**
> + * unregister_fprobe_sync() - Unregister fprobe synchronously with RCU grace period.
> + * @fp: A fprobe data structure to be unregistered.
> + *
> + * Unregister fprobe (and remove ftrace hooks from the function entries) and
> + * wait for the RCU grace period to finish. This is useful for preventing
> + * the fprobe from being used after it is unregistered.
> + *
> + * Return 0 if @fp is unregistered successfully, -errno if not.
> + */
> +int unregister_fprobe_sync(struct fprobe *fp)
> +{
> +	int ret;
> +
> +	guard(mutex)(&fprobe_mutex);
> +	if (!fp || !fprobe_registered(fp))
> +		return -EINVAL;
> +
> +	ret = unregister_fprobe_nolock(fp);
> +	if (ret)
> +		return ret;
> +
> +	synchronize_rcu();

Hmm, do we really need to hold the fprobe_mutex when doing the
synchronize_rcu()? This could cause other updates to have to wait longer
too.

-- Steve


> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(unregister_fprobe_sync);

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] fprobe: Add unregister_fprobe_sync() for synchronous unregistration
  2026-04-27 12:09 [PATCH] fprobe: Add unregister_fprobe_sync() for synchronous unregistration Masami Hiramatsu (Google)
  2026-04-28 18:27 ` Steven Rostedt
@ 2026-05-03  3:25 ` kernel test robot
  1 sibling, 0 replies; 3+ messages in thread
From: kernel test robot @ 2026-05-03  3:25 UTC (permalink / raw)
  To: Masami Hiramatsu (Google), Steven Rostedt
  Cc: llvm, oe-kbuild-all, Mathieu Desnoyers, Jonathan Corbet,
	linux-kernel, linux-trace-kernel, linux-doc

Hi Masami,

kernel test robot noticed the following build errors:

[auto build test ERROR on trace/for-next]
[cannot apply to linus/master v7.1-rc1 next-20260430]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Masami-Hiramatsu-Google/fprobe-Add-unregister_fprobe_sync-for-synchronous-unregistration/20260427-214258
base:   https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace for-next
patch link:    https://lore.kernel.org/r/177729179863.401400.6063130067239479972.stgit%40mhiramat.tok.corp.google.com
patch subject: [PATCH] fprobe: Add unregister_fprobe_sync() for synchronous unregistration
config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20260503/202605031133.LJkoT4xo-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260503/202605031133.LJkoT4xo-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202605031133.LJkoT4xo-lkp@intel.com/

All errors (new ones prefixed by >>):

>> kernel/trace/fprobe.c:983:14: error: call to undeclared function 'fprobe_registered'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     983 |         if (!fp || !fprobe_registered(fp))
         |                     ^
>> kernel/trace/fprobe.c:986:8: error: call to undeclared function 'unregister_fprobe_nolock'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     986 |         ret = unregister_fprobe_nolock(fp);
         |               ^
   kernel/trace/fprobe.c:986:8: note: did you mean 'unregister_fprobe_sync'?
   kernel/trace/fprobe.c:978:5: note: 'unregister_fprobe_sync' declared here
     978 | int unregister_fprobe_sync(struct fprobe *fp)
         |     ^
     979 | {
     980 |         int ret;
     981 | 
     982 |         guard(mutex)(&fprobe_mutex);
     983 |         if (!fp || !fprobe_registered(fp))
     984 |                 return -EINVAL;
     985 | 
     986 |         ret = unregister_fprobe_nolock(fp);
         |               ~~~~~~~~~~~~~~~~~~~~~~~~
         |               unregister_fprobe_sync
   2 errors generated.


vim +/fprobe_registered +983 kernel/trace/fprobe.c

   967	
   968	/**
   969	 * unregister_fprobe_sync() - Unregister fprobe synchronously with RCU grace period.
   970	 * @fp: A fprobe data structure to be unregistered.
   971	 *
   972	 * Unregister fprobe (and remove ftrace hooks from the function entries) and
   973	 * wait for the RCU grace period to finish. This is useful for preventing
   974	 * the fprobe from being used after it is unregistered.
   975	 *
   976	 * Return 0 if @fp is unregistered successfully, -errno if not.
   977	 */
   978	int unregister_fprobe_sync(struct fprobe *fp)
   979	{
   980		int ret;
   981	
   982		guard(mutex)(&fprobe_mutex);
 > 983		if (!fp || !fprobe_registered(fp))
   984			return -EINVAL;
   985	
 > 986		ret = unregister_fprobe_nolock(fp);
   987		if (ret)
   988			return ret;
   989	
   990		synchronize_rcu();
   991		return 0;
   992	}
   993	EXPORT_SYMBOL_GPL(unregister_fprobe_sync);
   994	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-05-03  3:25 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-27 12:09 [PATCH] fprobe: Add unregister_fprobe_sync() for synchronous unregistration Masami Hiramatsu (Google)
2026-04-28 18:27 ` Steven Rostedt
2026-05-03  3:25 ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox