From: Oleg Nesterov <oleg@redhat.com>
To: Andrii Nakryiko <andrii@kernel.org>
Cc: linux-trace-kernel@vger.kernel.org, peterz@infradead.org,
rostedt@goodmis.org, mhiramat@kernel.org, bpf@vger.kernel.org,
linux-kernel@vger.kernel.org, jolsa@kernel.org,
paulmck@kernel.org
Subject: Re: [PATCH 1/3] uprobes: allow put_uprobe() from non-sleepable softirq context
Date: Sun, 15 Sep 2024 16:49:11 +0200 [thread overview]
Message-ID: <20240915144910.GA27726@redhat.com> (raw)
In-Reply-To: <20240909224903.3498207-2-andrii@kernel.org>
On 09/09, Andrii Nakryiko wrote:
>
> Currently put_uprobe() might trigger mutex_lock()/mutex_unlock(), which
> makes it unsuitable to be called from more restricted context like softirq.
>
> Let's make put_uprobe() agnostic to the context in which it is called,
> and use work queue to defer the mutex-protected clean up steps.
...
> +static void uprobe_free_deferred(struct work_struct *work)
> +{
> + struct uprobe *uprobe = container_of(work, struct uprobe, work);
> +
> + /*
> + * If application munmap(exec_vma) before uprobe_unregister()
> + * gets called, we don't get a chance to remove uprobe from
> + * delayed_uprobe_list from remove_breakpoint(). Do it here.
> + */
> + mutex_lock(&delayed_uprobe_lock);
> + delayed_uprobe_remove(uprobe, NULL);
> + mutex_unlock(&delayed_uprobe_lock);
> +
> + kfree(uprobe);
> +}
> +
> static void uprobe_free_rcu(struct rcu_head *rcu)
> {
> struct uprobe *uprobe = container_of(rcu, struct uprobe, rcu);
>
> - kfree(uprobe);
> + INIT_WORK(&uprobe->work, uprobe_free_deferred);
> + schedule_work(&uprobe->work);
> }
This is still wrong afaics...
If put_uprobe() can be called from softirq (after the next patch), then
put_uprobe() and all other users of uprobes_treelock should use
write_lock_bh/read_lock_bh to avoid the deadlock.
To be honest... I simply can't force myself to even try to read 2/3 ;) I'll
try to do this later, but I am sure I will never like it, sorry.
Oleg.
next prev parent reply other threads:[~2024-09-15 14:49 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-09 22:49 [PATCH 0/3] SRCU-protected uretprobes hot path Andrii Nakryiko
2024-09-09 22:49 ` [PATCH 1/3] uprobes: allow put_uprobe() from non-sleepable softirq context Andrii Nakryiko
2024-09-10 2:51 ` Alexei Starovoitov
2024-09-10 5:13 ` Andrii Nakryiko
2024-09-10 15:56 ` Alexei Starovoitov
2024-09-10 17:46 ` Andrii Nakryiko
2024-09-15 14:49 ` Oleg Nesterov [this message]
2024-09-17 8:19 ` Andrii Nakryiko
2024-10-04 20:18 ` Andrii Nakryiko
2024-09-09 22:49 ` [PATCH 2/3] uprobes: SRCU-protect uretprobe lifetime (with timeout) Andrii Nakryiko
2024-09-09 22:49 ` [PATCH 3/3] uprobes: implement SRCU-protected lifetime for single-stepped uprobe Andrii Nakryiko
2024-09-15 14:51 ` Oleg Nesterov
2024-09-17 8:20 ` Andrii Nakryiko
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=20240915144910.GA27726@redhat.com \
--to=oleg@redhat.com \
--cc=andrii@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-trace-kernel@vger.kernel.org \
--cc=mhiramat@kernel.org \
--cc=paulmck@kernel.org \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.