mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
To: Andrea Parri <parri.andrea@gmail.com>
Cc: Naveen N Rao <naveen@kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	Steven Rostedt <rostedt@goodmis.org>,
	Josef Bacik <josef@toxicpanda.com>,
	linux-trace-kernel@vger.kernel.org, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org
Subject: Re: [PATCH] kprobes: Fix permanent hang when flushing the kprobe optimizer
Date: Sat, 26 Sep 2026 01:38:32 +0900	[thread overview]
Message-ID: <20260926013832.abaa63a3903d563655bc60d6@kernel.org> (raw)
In-Reply-To: <20260924092142.199198-1-parri.andrea@gmail.com>

On Thu, 24 Sep 2026 11:21:39 +0200
Andrea Parri <parri.andrea@gmail.com> wrote:

> Writing 0 to /proc/sys/debug/kprobes-optimization while a kprobe is
> jump-optimized never returns. The writer sleeps in D state forever with
> kprobe_sysctl_mutex held, so any later read or write of that sysctl
> hangs as well. For example, with vfs_read+9 as an optimizable address
> in this build:
> 
>   # cd /sys/kernel/tracing
>   # echo 'p:myprobe vfs_read+9' >> kprobe_events
>   # echo 1 > events/kprobes/myprobe/enable
>   # # wait until /sys/kernel/debug/kprobes/list shows [OPTIMIZED]
>   # echo 0 > /proc/sys/debug/kprobes-optimization
> 
>   INFO: task sh:246 blocked for more than 10 seconds.
>   Call Trace:
>    <TASK>
>    __schedule+0x1176/0x4f70
>    schedule+0xdc/0x2c0
>    schedule_timeout+0x17b/0x260
>    wait_for_completion+0x173/0x3c0
>    wait_for_kprobe_optimizer_locked+0xbc/0x130
>    proc_kprobes_optimization_handler+0x156/0x1b0
>    proc_sys_call_handler+0x324/0x490
>    vfs_write+0x52d/0xfe0
>    ksys_write+0xff/0x200
>    do_syscall_64+0x106/0x630
>    entry_SYSCALL_64_after_hwframe+0x77/0x7f
>    </TASK>
>   ...
>   INFO: task cat:265 is blocked on a mutex likely owned by task sh:246.
> 
> wait_for_kprobe_optimizer_locked() reinitializes optimizer_completion,
> asks the optimizer thread to flush and sleeps in wait_for_completion().
> The thread drains the (un)optimizing lists, but calls complete() only
> if completion_done() is true, i.e. if the completion is already done,
> which never happens while someone waits. disarm_all_kprobes() and
> kprobe_trace_self_tests_init() wait the same way.
> 
> Calling complete() unconditionally would not be enough: the waiter
> drops kprobe_mutex while it sleeps, and nothing else serializes the
> sysctl handler against the debugfs "enabled" file. A second flusher
> that still finds the lists non-empty, e.g. because a disabled probe is
> queued for unoptimizing, reinitializes the completion under the first:
> 
>   sysctl write                      debugfs "enabled" write
>   unoptimize_all_kprobes()
>     wait_for_kprobe_optimizer_locked()
>       init_completion(c)
>       mutex_unlock(&kprobe_mutex)
>       wait_for_completion(c)
>                                     disarm_all_kprobes()
>                                       wait_for_kprobe_optimizer_locked()
>                                         init_completion(c)
>                                           // c->wait is reset, the first
>                                           // waiter is off the queue
>                                         mutex_unlock(&kprobe_mutex)
>                                         wait_for_completion(c)
>   kprobe_optimizer()
>     complete(c)
>       // wakes the debugfs writer only
> 
> where c is &optimizer_completion. Lining up the two writes during an
> optimizer pass loses the sysctl writer this way.
> 
> Replace the completion with a counter of optimizer passes, bumped at the
> end of each pass and signalled with wake_up_var_locked(), both under
> kprobe_mutex. A flusher samples the count and waits with
> wait_var_event_mutex(), which drops kprobe_mutex only while sleeping, so
> a new count means a whole pass ran in the meantime. Nothing is
> reinitialized, so several flushers can sleep in the wait at once.
> 
> Fixes: 73c12f209462 ("kprobes: Use dedicated kthread for kprobe optimizer")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
> ---
> This overlaps with two patches under discussion in the "rcu-tasks: build
> Tasks RCU on Tasks Trace readers in trampolines" thread:
> 
>  - Masami's RFC "kprobes: Make optprobe optimizer multi-generational and
>    asynchronous" [1] rewrites the same code.  It drops the
>    completion_done() check, which fixes the hang reported here, but it
>    keeps init_completion() in wait_for_kprobe_optimizer_locked(), so the
>    concurrent-flusher case described above remains.  The two patches
>    conflict textually.

OK, this fix looks good to me. Let me pick this for probes/fixes and
rebase mine on top of it. Anyway I think we need both.

Thank you,

> 
>  - Josef's "kprobes: Expose the optprobe jump window to Tasks RCU" [2]
>    changes kprobe_optimizer() around synchronize_rcu_tasks(); its
>    kernel/kprobes.c hunks still apply on top of this patch.
> 
> [1] https://lore.kernel.org/all/179017148080.466588.9116221556625712980.stgit@devnote2/
> [2] https://lore.kernel.org/all/20260922-b4-rcu-tasks-preempt-qs-v5-3-410f57770bad@toxicpanda.com/
> 
>  kernel/kprobes.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 6337da5cab9e7..4edd8ca5c6578 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -42,6 +42,7 @@
>  #include <linux/execmem.h>
>  #include <linux/cleanup.h>
>  #include <linux/wait.h>
> +#include <linux/wait_bit.h>
>  
>  #include <asm/sections.h>
>  #include <asm/cacheflush.h>
> @@ -526,7 +527,8 @@ enum {
>  	OPTIMIZER_ST_FLUSHING = 2,
>  };
>  
> -static DECLARE_COMPLETION(optimizer_completion);
> +/* Bumped at the end of each kprobe_optimizer() pass, under 'kprobe_mutex' */
> +static unsigned long optimizer_passes;
>  
>  #define OPTIMIZE_DELAY 5
>  
> @@ -654,9 +656,9 @@ static void kprobe_optimizer(void)
>  		do_free_cleaned_kprobes();
>  	}
>  
> -	/* Step 5: Kick optimizer again if needed. But if there is a flush requested, */
> -	if (completion_done(&optimizer_completion))
> -		complete(&optimizer_completion);
> +	/* Step 5: Wake up flushers, and kick optimizer again if needed. */
> +	optimizer_passes++;
> +	wake_up_var_locked(&optimizer_passes, &kprobe_mutex);
>  
>  	if (!list_empty(&optimizing_list) || !list_empty(&unoptimizing_list))
>  		kick_kprobe_optimizer();	/*normal kick*/
> @@ -708,7 +710,8 @@ static void wait_for_kprobe_optimizer_locked(void)
>  	lockdep_assert_held(&kprobe_mutex);
>  
>  	while (!list_empty(&optimizing_list) || !list_empty(&unoptimizing_list)) {
> -		init_completion(&optimizer_completion);
> +		unsigned long passes = optimizer_passes;
> +
>  		/*
>  		 * Set state to OPTIMIZER_ST_FLUSHING and wake up the thread if it's
>  		 * idle. If it's already kicked, it will see the state change.
> @@ -717,9 +720,12 @@ static void wait_for_kprobe_optimizer_locked(void)
>  			OPTIMIZER_ST_FLUSHING) != OPTIMIZER_ST_FLUSHING)
>  			wake_up(&kprobe_optimizer_wait);
>  
> -		mutex_unlock(&kprobe_mutex);
> -		wait_for_completion(&optimizer_completion);
> -		mutex_lock(&kprobe_mutex);
> +		/*
> +		 * kprobe_optimizer() holds 'kprobe_mutex' for a whole pass, which
> +		 * this drops while sleeping, so a new count means a full pass ran.
> +		 */
> +		wait_var_event_mutex(&optimizer_passes,
> +				     optimizer_passes != passes, &kprobe_mutex);
>  	}
>  }
>  
> -- 
> 2.53.0
> 


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

      parent reply	other threads:[~2026-09-25 16:38 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-24  9:21 Andrea Parri
2026-09-24 15:44 ` Bradley Morgan
2026-09-25 16:38 ` Masami Hiramatsu [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260926013832.abaa63a3903d563655bc60d6@kernel.org \
    --to=mhiramat@kernel.org \
    --cc=davem@davemloft.net \
    --cc=josef@toxicpanda.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=naveen@kernel.org \
    --cc=parri.andrea@gmail.com \
    --cc=rostedt@goodmis.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®