From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (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 CC3DE4D9573; Fri, 25 Sep 2026 16:38:38 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1790354322; cv=none; b=hPOF0KvWGLUjevvlG0119FVG7iXkxek4BX/SzEktpFM/v6wTMLT8O0UxMNo8H9xrKiVHKFpxMiJ9CvuH7sa+Lj2SZyhCVKjvGvQHwJ+6Z9bk60HyaJIY4UmR87mtSVdVubkVqkSZ6i1l3WkJKr4RPi51xNjG7tbeFjqBMPYnjUM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1790354322; c=relaxed/simple; bh=7JEZFKX+P/T1np0rBfRn5XbrnUCGgHH+54tJTV1OQXU=; h=Date:From:To:Cc:Subject:Message-Id:In-Reply-To:References: Mime-Version:Content-Type; b=oZL4iBqI4JK15ZEzZ3Z6VlXHtRxk7A29EmrRPKk5colh67jJmkAV8ctJoYD0WX+JJBNcnXL6/NjAXuARe8mAWf14pOEbUUVNrfH7hWCJT64Q+taZjikWWLB4KEIIakU2IHeLDHzP3umkT5fdk7+g0TLpVzorOEpiB+tNdiIuSg0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=ER993GLF; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="ER993GLF" Received: by smtp.kernel.org (Postfix) with ESMTPSA id AA6201F00893; Fri, 25 Sep 2026 16:38:34 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1790354316; bh=Ef5AYQYzA6WRtJxzWsmKVPm+wztV1FQUcjUH5RY8340=; h=Date:From:To:Cc:Subject:In-Reply-To:References; b=ER993GLFdFWegZ1zeJakvYgwmNtNST0z1wbj7yRNKh1rwstT4dVw1BDWN4mx6hRt+ E9q8dgtPMHNudVEujWTF/4jAJ2r+8gLELbFtKKZLSyoOEO6KVpLXgcWItrwUI+oA0Q CeUlFIdkaIdRVgr1ykl5B1fzAa/gk/ZxMB9IhPCEZv3U2PHJ4mYOsCBXQhliOKU1qV gfL43YaEpPISCRDVyy/PqZD5z3inJ/Ffsnp+tCCVUmikrnjjZoYhXBmWb24FST4Df1 qIVuDhsBhvztbE1aem5vP93USxJ1GCUTXL64TcW/hargRO7ULBLZWo7eGBSfw80mP4 CUNqT89AnHBUw== Date: Sat, 26 Sep 2026 01:38:32 +0900 From: Masami Hiramatsu (Google) To: Andrea Parri Cc: Naveen N Rao , "David S. Miller" , Steven Rostedt , Josef Bacik , 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 Message-Id: <20260926013832.abaa63a3903d563655bc60d6@kernel.org> In-Reply-To: <20260924092142.199198-1-parri.andrea@gmail.com> References: <20260924092142.199198-1-parri.andrea@gmail.com> X-Mailer: Sylpheed 3.8.0beta1 (GTK+ 2.24.33; x86_64-pc-linux-gnu) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Thu, 24 Sep 2026 11:21:39 +0200 Andrea Parri 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: > > __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 > > ... > 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 > --- > 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 > #include > #include > +#include > > #include > #include > @@ -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)